pinctrl: mxs: add gpio range support

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 <[email protected]>

---
 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(-)

drivers/pinctrl/pinctrl-imx23.c +12 -0

index 75d3eff..695b64b 100644
@@ -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)

drivers/pinctrl/pinctrl-imx28.c +0 -0

index b973026..c9a8e67 100644

drivers/pinctrl/pinctrl-imx28.c +15 -0

+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)

drivers/pinctrl/pinctrl-mxs.c +17 -0

index ab63d38..ea33ebc 100644
@@ -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);

drivers/pinctrl/pinctrl-mxs.c +5 -0

+
 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);

drivers/pinctrl/pinctrl-mxs.c +9 -0

+				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:

drivers/pinctrl/pinctrl-mxs.h +2 -0

index fdd88d0b..7feef15 100644
@@ -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,

Signature · 1 line

1.7.4.1

Quoted 4 lines

+/*
+ * 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.

drivers/pinctrl/pinctrl-mxs.c static void mxs_pinctrl_disable(struct pinctrl_dev *pctldev, · line 249

Quoted 6 lines

drivers/pinctrl/pinctrl-mxs.c +6 -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.

Quoted 26 lines

drivers/pinctrl/pinctrl-mxs.c +5 -0

+
 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);
 

drivers/pinctrl/pinctrl-mxs.c int __devinit mxs_pinctrl_probe(struct platform_device *pdev, · line 537

Quoted 3 lines

drivers/pinctrl/pinctrl-mxs.c +3 -0

+	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.

drivers/pinctrl/pinctrl-mxs.c int __devinit mxs_pinctrl_probe(struct platform_device *pdev, · line 545

Quoted 8 lines

drivers/pinctrl/pinctrl-mxs.c +8 -0

+
+		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.

drivers/pinctrl/pinctrl-mxs.c int __devinit mxs_pinctrl_probe(struct platform_device *pdev, · line 548

Quoted 3 lines

drivers/pinctrl/pinctrl-mxs.c +3 -0

+			struct pinctrl_gpio_range *range = &soc->gpio_ranges[i];
+			if (range->id == id) {
+				range->gc = of_node_to_gpiochip(child);

shouldn't check return?

Quoted 5 lines

drivers/pinctrl/pinctrl-mxs.c int __devinit mxs_pinctrl_probe(struct platform_device *pdev, · line 537

Quoted 3 lines

drivers/pinctrl/pinctrl-mxs.c +3 -0

+	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.

On mxs, the gpio nodes are naturally under pinctrl node, because gpio controller is part of pin controller and share the same IO space range with pin controller.

For the phandle approach, there is a patch from Viresh Kumar "gpiolib:
Add of_get_gpio_chip_by_phandle() helper" are currently flowing around on the list. But it hasn't been accepted. The whole pull-request of "SPEAr pinctrl updates for v-3.5" is being held on this.

http://thread.gmane.org/gmane.linux.ports.arm.kernel/166668/focus=167395

Quoted 13 lines

I will send out the common patch for discussion.

drivers/pinctrl/pinctrl-mxs.c int __devinit mxs_pinctrl_probe(struct platform_device *pdev, · line 545

Quoted 8 lines

drivers/pinctrl/pinctrl-mxs.c +8 -0

+
+		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.

This is a mxs specific implementation, and we do have gpio nodes sequentially listed under pinctrl node.

I'm wondering if dt forces user to define the same device sequentially. But if not, the driver may be better not assume how the device node are organized in dts, right?
If we really want do like this, do we need to add some comments in binding doc?

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.