Skip to contentSkip to conversation

pinctrl: at91: allow disabled gpio controllers

PATCH Superseded Ludovic Desroches 4 participants 8 messages Jan 14, 2015 – Jan 21, 2015

Subsystems (4)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover pinctrl: at91: allow disabled gpio controllers

Hi Linus,

I have updated the previous version. I have tried to follow your advice but it doesn't seem to be the best solution in my case.

You told me to use gpiochip_add_pingroup_range() instead of pinctrl_add_gpio_range(). I agree that is the proper way to manage a gap but it only solves my issue on gpio side but not on muxing side. I still need some fixes. Moreover it involves to add the gpio-ranges property so to update our dtsi files so it will break backward compatibility.

By the way, I would like to be co-maintainer on this driver as it seems the maintainer is not very active at the moment.

Regards

Ludovic

Ludovic Desroches (3):
pinctrl: at91: allow disabled pio controllers
ARM: at91/dt: sama5d4: add pioD controller
MAINTAINERS: add maintainer for pinctrl-at91

 MAINTAINERS                    |  1 +
 arch/arm/boot/dts/sama5d4.dtsi | 13 +++++++++++++
 drivers/pinctrl/pinctrl-at91.c | 30 ++++++++++++++++++++++--------
 3 files changed, 36 insertions(+), 8 deletions(-)
Signature · 1 line
2.2.0
#1 lore raw

PATCH v2 1/3 pinctrl: at91: allow disabled pio controllers

at91 pinctrl was not designed to support disabled or unused pio controllers. All the pio controllers have to be declared, then set status to disabled if you don't want to use it.

---
 drivers/pinctrl/pinctrl-at91.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)
drivers/pinctrl/pinctrl-at91.c +22 -8
index dfd021e..fc9afb4 100644
@@ -178,6 +178,7 @@ struct at91_pinctrl {
 	struct pinctrl_dev	*pctl;
 
 	int			nbanks;
+	int			nactive_banks;
 
 	uint32_t		*mux_mask;
 	int			nmux;
@@ -982,6 +983,8 @@ static void at91_pinctrl_child_count(struct at91_pinctrl *info,
 	for_each_child_of_node(np, child) {
 		if (of_device_is_compatible(child, gpio_compat)) {
 			info->nbanks++;
+			if (of_device_is_available(child))
+				info->nactive_banks;
 		} else {
 			info->nfunctions++;
 			info->ngroups += of_get_child_count(child);
@@ -1145,8 +1148,12 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
 	dev_dbg(&pdev->dev, "mux-mask\n");
 	tmp = info->mux_mask;
 	for (i = 0; i < info->nbanks; i++) {
+		if (!gpio_chips[i]) {
+			tmp += info->nmux;
+			continue;
+		}
 		for (j = 0; j < info->nmux; j++, tmp++) {
-			dev_dbg(&pdev->dev, "%d:%d\t0x%x\n", i, j, tmp[0]);
+			dev_dbg(&pdev->dev, "pio%c:periphal %c\t0x%x\n", 'A' + i, 'A' + j, tmp[0]);
 		}
 	}
 
@@ -1185,7 +1192,7 @@ static int at91_pinctrl_probe(struct platform_device *pdev)
 {
 	struct at91_pinctrl *info;
 	struct pinctrl_pin_desc *pdesc;
-	int ret, i, j, k;
+	int ret, i, j, k, ngpio_chips_enabled = 0;
 
 	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
@@ -1201,11 +1208,15 @@ static int at91_pinctrl_probe(struct platform_device *pdev)
 	 * need this to proceed.
 	 */
 	for (i = 0; i < info->nbanks; i++) {
-		if (!gpio_chips[i]) {
-			dev_warn(&pdev->dev, "GPIO chip %d not registered yet\n", i);
-			devm_kfree(&pdev->dev, info);
-			return -EPROBE_DEFER;
-		}
+		if (gpio_chips[i])
+			ngpio_chips_enabled++;
+	}
+	if (ngpio_chips_enabled < info->nactive_banks) {
+		dev_warn(&pdev->dev,
+			 "All GPIO chips are not registered yet (%d/%d)\n",
+			 ngpio_chips_enabled, info->nactive_banks);
+		devm_kfree(&pdev->dev, info);
+		return -EPROBE_DEFER;
 	}
 
 	at91_pinctrl_desc.name = dev_name(&pdev->dev);
@@ -1235,7 +1246,8 @@ static int at91_pinctrl_probe(struct platform_device *pdev)
 
 	/* We will handle a range of GPIO pins */
 	for (i = 0; i < info->nbanks; i++)
-		pinctrl_add_gpio_range(info->pctl, &gpio_chips[i]->range);
+		if (gpio_chips[i])
+			pinctrl_add_gpio_range(info->pctl, &gpio_chips[i]->range);
 
 	dev_info(&pdev->dev, "initialized AT91 pinctrl driver\n");
 
@@ -1682,6 +1694,8 @@ static void at91_gpio_probe_fixup(void)
 
 	for (i = 0; i < gpio_banks; i++) {
 		at91_gpio = gpio_chips[i];
+		if (!at91_gpio)
+			continue;
 
 		/*
 		 * GPIO controller are grouped on some SoC:
Signature · 1 line
2.2.0
#2 lore raw

PATCH v2 2/3 ARM: at91/dt: sama5d4: add pioD controller

In reply to Ludovic Desroches

PioD controller was not described in the device tree since we don't use it. As pinctrl-at91 allows disabled gpio controllers in the device tree, we can add it to complete the device description.

---
 arch/arm/boot/dts/sama5d4.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)
arch/arm/boot/dts/sama5d4.dtsi +13 -0
index 1b0f30c..368973e 100644
@@ -62,6 +62,7 @@
 		gpio0 = &pioA;
 		gpio1 = &pioB;
 		gpio2 = &pioC;
+		gpio3 = &pioD;
 		gpio4 = &pioE;
 		tcb0 = &tcb0;
 		tcb1 = &tcb1;
@@ -1111,6 +1112,18 @@
 					clocks = <&pioC_clk>;
 				};
 
+				pioD: gpio at fc068000 {
+					compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
+					reg = <0xfc068000 0x100>;
+					interrupts = <5 IRQ_TYPE_LEVEL_HIGH 1>;
+					#gpio-cells = <2>;
+					gpio-controller;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+					clocks = <&pioD_clk>;
+					status = "disabled";
+				};
+
 				pioE: gpio at fc06d000 {
 					compatible = "atmel,at91sam9x5-gpio", "atmel,at91rm9200-gpio";
 					reg = <0xfc06d000 0x100>;
Signature · 1 line
2.2.0
#3 lore raw

PATCH v2 3/3 MAINTAINERS: add maintainer for pinctrl-at91

In reply to Ludovic Desroches
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)
MAINTAINERS +1 -0
index ddb9ac8..dfa702c 100644
@@ -7405,6 +7405,7 @@ F:	include/linux/pinctrl/
 
 PIN CONTROLLER - ATMEL AT91
 M:	Jean-Christophe Plagniol-Villard <[email protected]>
+M:	Ludovic Desroches <[email protected]>
 L:	linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 F:	drivers/pinctrl/pinctrl-at91.*
Signature · 1 line
2.2.0
#6 lore raw

PATCH 3/3 MAINTAINERS: add maintainer for pinctrl-at91

Quoted 7 lines · Nicolas Ferre

Le 19/01/2015 11:02, Linus Walleij a ?crit :

Quoted 3 lines

On Wed, Jan 14, 2015 at 11:08 AM, Ludovic Desroches <[email protected]> wrote:

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

The more maintainers the better.
Nicolas & J-C: can you ACK this?

Absolutely:

I still maintain it does not see the need for 2 maintainers for a simple drivers

Nack for me

Best Regards,
J.

Quoted 4 lines

Bye,

Signature · 1 line
Nicolas Ferre
#7 lore raw

PATCH 3/3 MAINTAINERS: add maintainer for pinctrl-at91

Quoted 10 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 10 lines · Nicolas Ferre

Le 19/01/2015 11:02, Linus Walleij a ?crit :

Quoted 7 lines
Quoted 1 line · Ludovic Desroches

The more maintainers the better.
Nicolas & J-C: can you ACK this?

Quoted 6 lines
Quoted 2 lines

Absolutely:

I still maintain it does not see the need for 2 maintainers for a simple drivers

Nack for me

Cool, as subsystem maintainer had some delays in getting reviews on patches for the AT91 drivers sometimes, but if you're gonna be as quick in response as on this patch set then I can live without a comaintainer I guess.

Yours,
Linus Walleij