[PATCH] pinctrl: mxs: add gpio range support
Dong Aisheng
aisheng.dong at freescale.com
Tue May 15 07:51:14 EDT 2012
On Tue, May 15, 2012 at 01:56:14PM +0800, Shawn Guo wrote:
> The mxs pins are organized in banks and each bank contains 32 pins.
> i.MX23 has 4 banks in total, and every pin in the first 3 banks has
> gpio function available. i.MX28 has 7 banks and the first 5 banks
> are capable of gpio mode.
>
> As the gpio base is runtime determined for each port when booting
> from device tree, the .base of struct pinctrl_gpio_range can only be
> initialized with a offset local to the port, and have to plus gpio base
> of the port at runtime.
>
> Signed-off-by: Shawn Guo <shawn.guo at linaro.org>
> ---
> drivers/pinctrl/pinctrl-imx23.c | 12 ++++++++
> drivers/pinctrl/pinctrl-imx28.c | 19 +++++++++++++
> drivers/pinctrl/pinctrl-mxs.c | 55 +++++++++++++++++++++++++++++++++++++++
> drivers/pinctrl/pinctrl-mxs.h | 2 +
> 4 files changed, 88 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-imx23.c b/drivers/pinctrl/pinctrl-imx23.c
> index 75d3eff..695b64b 100644
> --- a/drivers/pinctrl/pinctrl-imx23.c
> +++ b/drivers/pinctrl/pinctrl-imx23.c
> @@ -261,10 +261,22 @@ static struct mxs_regs imx23_regs = {
> .pull = 0x400,
> };
>
> +/*
> + * The .base is initialized as the gpio offset local to the port, and will
> + * have gpio base of the port added at runtime.
> + */
> +static struct pinctrl_gpio_range imx23_gpio_ranges[] = {
> + { .name = "gpio", .id = 0, .base = 0, .pin_base = 0, .npins = 32, },
> + { .name = "gpio", .id = 1, .base = 0, .pin_base = 32, .npins = 31, },
> + { .name = "gpio", .id = 2, .base = 0, .pin_base = 64, .npins = 32, },
> +};
> +
> static struct mxs_pinctrl_soc_data imx23_pinctrl_data = {
> .regs = &imx23_regs,
> .pins = imx23_pins,
> .npins = ARRAY_SIZE(imx23_pins),
> + .gpio_ranges = imx23_gpio_ranges,
> + .gpio_num_ranges = ARRAY_SIZE(imx23_gpio_ranges),
> };
>
> static int __devinit imx23_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/pinctrl-imx28.c b/drivers/pinctrl/pinctrl-imx28.c
> index b973026..c9a8e67 100644
> --- a/drivers/pinctrl/pinctrl-imx28.c
> +++ b/drivers/pinctrl/pinctrl-imx28.c
> @@ -377,10 +377,29 @@ static struct mxs_regs imx28_regs = {
> .pull = 0x600,
> };
>
> +/*
> + * The .base is initialized as the gpio offset local to the port, and will
> + * have gpio base of the port added at runtime.
> + */
A good idea.
I was also trying to do this for pinctrl gpio dt improvement but i planned to
do such things in core since i thought other SoCs may face the same issue.
Maybe i should send out a RFC patch for open discussion.
> +static struct pinctrl_gpio_range imx28_gpio_ranges[] = {
> + { .name = "gpio", .id = 0, .base = 0, .pin_base = 0, .npins = 8, },
> + { .name = "gpio", .id = 0, .base = 16, .pin_base = 16, .npins = 13, },
> + { .name = "gpio", .id = 1, .base = 0, .pin_base = 32, .npins = 32, },
> + { .name = "gpio", .id = 2, .base = 0, .pin_base = 64, .npins = 11, },
> + { .name = "gpio", .id = 2, .base = 12, .pin_base = 76, .npins = 10, },
> + { .name = "gpio", .id = 2, .base = 24, .pin_base = 88, .npins = 4, },
> + { .name = "gpio", .id = 3, .base = 0, .pin_base = 96, .npins = 19, },
> + { .name = "gpio", .id = 3, .base = 20, .pin_base = 116, .npins = 11, },
> + { .name = "gpio", .id = 4, .base = 0, .pin_base = 128, .npins = 17, },
> + { .name = "gpio", .id = 4, .base = 20, .pin_base = 148, .npins = 1, },
> +};
> +
> static struct mxs_pinctrl_soc_data imx28_pinctrl_data = {
> .regs = &imx28_regs,
> .pins = imx28_pins,
> .npins = ARRAY_SIZE(imx28_pins),
> + .gpio_ranges = imx28_gpio_ranges,
> + .gpio_num_ranges = ARRAY_SIZE(imx28_gpio_ranges),
> };
>
> static int __devinit imx28_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/pinctrl-mxs.c b/drivers/pinctrl/pinctrl-mxs.c
> index ab63d38..ea33ebc 100644
> --- a/drivers/pinctrl/pinctrl-mxs.c
> +++ b/drivers/pinctrl/pinctrl-mxs.c
> @@ -15,6 +15,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/of_gpio.h>
> #include <linux/pinctrl/machine.h>
> #include <linux/pinctrl/pinconf.h>
> #include <linux/pinctrl/pinctrl.h>
> @@ -220,12 +221,41 @@ static void mxs_pinctrl_disable(struct pinctrl_dev *pctldev,
> /* Nothing to do here */
> }
>
> +static int mxs_gpio_request_enable(struct pinctrl_dev *pctldev,
> + struct pinctrl_gpio_range *range,
> + unsigned pinid)
> +{
> + struct mxs_pinctrl_data *d = pinctrl_dev_get_drvdata(pctldev);
> + void __iomem *reg;
> + u8 bank, shift;
> + u16 pin;
> +
> + bank = PINID_TO_BANK(pinid);
> + pin = PINID_TO_PIN(pinid);
> + reg = d->base + d->soc->regs->muxsel;
> + reg += bank * 0x20 + pin / 16 * 0x10;
> + shift = pin % 16 * 2;
> +
> + writel(0x3 << shift, reg + SET);
> +
> + return 0;
> +}
> +
> +static void mxs_gpio_disable_free(struct pinctrl_dev *pctldev,
> + struct pinctrl_gpio_range *range,
> + unsigned pinid)
> +{
> + /* Nothing to do here */
> +}
It seems .gpio_disable_free can be optional, so you can remove this
empty function.
> +
> static struct pinmux_ops mxs_pinmux_ops = {
> .get_functions_count = mxs_pinctrl_get_funcs_count,
> .get_function_name = mxs_pinctrl_get_func_name,
> .get_function_groups = mxs_pinctrl_get_func_groups,
> .enable = mxs_pinctrl_enable,
> .disable = mxs_pinctrl_disable,
> + .gpio_request_enable = mxs_gpio_request_enable,
> + .gpio_disable_free = mxs_gpio_disable_free,
> };
>
> static int mxs_pinconf_get(struct pinctrl_dev *pctldev,
> @@ -482,7 +512,9 @@ int __devinit mxs_pinctrl_probe(struct platform_device *pdev,
> struct mxs_pinctrl_soc_data *soc)
> {
> struct device_node *np = pdev->dev.of_node;
> + struct device_node *child;
> struct mxs_pinctrl_data *d;
> + int id, i, j = 0;
> int ret;
>
> d = devm_kzalloc(&pdev->dev, sizeof(*d), GFP_KERNEL);
> @@ -500,6 +532,26 @@ int __devinit mxs_pinctrl_probe(struct platform_device *pdev,
> mxs_pinctrl_desc.npins = d->soc->npins;
> mxs_pinctrl_desc.name = dev_name(&pdev->dev);
>
> + for_each_child_of_node(np, child) {
> + if (!of_device_is_compatible(child, "fsl,mxs-gpio"))
> + continue;
I planned to use a phandle list to point to gpio nodes rather than forcing
put gpio nodes under pinctrl node.
I will send out the common patch for discussion.
> +
> + id = of_alias_get_id(child, "gpio");
> + if (id < 0 || id > soc->gpio_num_ranges) {
> + dev_err(&pdev->dev, "invalid gpio id: %d\n", id);
> + return id;
> + }
> +
> + for (i = j; i < soc->gpio_num_ranges; i++) {
I'm wondering this may fail if the gpio nodes are not sequentially listed
in dts file.
> + struct pinctrl_gpio_range *range = &soc->gpio_ranges[i];
> + if (range->id == id) {
> + range->gc = of_node_to_gpiochip(child);
shouldn't check return?
> + range->base += range->gc->base;
> + j++;
> + }
> + }
> + }
> +
> platform_set_drvdata(pdev, d);
>
> ret = mxs_pinctrl_probe_dt(pdev, d);
> @@ -515,6 +567,9 @@ int __devinit mxs_pinctrl_probe(struct platform_device *pdev,
> goto err;
> }
>
> + for (i = 0; i < soc->gpio_num_ranges; i++)
> + pinctrl_add_gpio_range(d->pctl, &soc->gpio_ranges[i]);
> +
> return 0;
>
> err:
> diff --git a/drivers/pinctrl/pinctrl-mxs.h b/drivers/pinctrl/pinctrl-mxs.h
> index fdd88d0b..7feef15 100644
> --- a/drivers/pinctrl/pinctrl-mxs.h
> +++ b/drivers/pinctrl/pinctrl-mxs.h
> @@ -82,6 +82,8 @@ struct mxs_pinctrl_soc_data {
> unsigned nfunctions;
> struct mxs_group *groups;
> unsigned ngroups;
> + struct pinctrl_gpio_range *gpio_ranges;
> + unsigned gpio_num_ranges;
> };
>
> int mxs_pinctrl_probe(struct platform_device *pdev,
> --
> 1.7.4.1
>
Regards
Dong Aisheng
More information about the linux-arm-kernel
mailing list