Unable to use gpio-line-names with Atmel pio4 driver
diff mbox series

Message ID CAP6JJ88pMH+KK_D7z+f69cMfwd_0gBX0M1cs+kkHKff4fJzmCw@mail.gmail.com
State New
Headers show
Series
  • Unable to use gpio-line-names with Atmel pio4 driver
Related show

Commit Message

Mark Deneen Feb. 10, 2020, 2:34 p.m. UTC
Hi!

I am working with an in-house board and getting the Kernel and Device
Tree straightened out for our needs.  After seeing the bold statements
in the sysfs documentation regarding gpio access, I figured that you
guys really mean it and decided to do things the right way.  In doing
so, however, I'm finding it to be unexpectedly difficult.

My board is uses the sama5d27 SIP.  There are certain portions of the
board which require GPIO lines to be driven from user-space, and these
aspects do not really fit within any particular kernel driver.  For
example, I need to be able to enable a CAN transceiver whenever the
CAN peripheral is in use, and disable it when it is not in order to
disconnect ourselves from the network.  I want to be able to name this
GPIO in Device Tree so that future revisions of this board have the
flexibility to use a different GPIO for this task and user space does
not need to know that PC31 is connected to the transceiver.

The atmel pio4 driver automatically sets names for the GPIOs [1], from
PA0 to PD31, and the gpiolib ignores the gpio-line-names property if
the chip has already set them. [2]  I worked around this by adding a
"no-autoname" device tree property to the pio4 pinctrl driver.  The
patch is attached, but it is admittedly quick & dirty and not
something I'm ready to put my name on yet.

I'm still unable to use libgpiod effectively, with or without my
changes, and I feel like I am fundamentally missing something.
gpioinfo shows all pins as "input".  Setting a pin's value makes
gpioinfo show that pin as an output until I use gpioget.  At this
point it reverts back to an input:

# gpioinfo | grep PC31
        line  95:       "PC31"       unused   input  active-high
# gpioset 0 95=0
# gpioinfo | grep PC31
        line  95:       "PC31"       unused  output  active-high
# gpioget 0 95
1
# gpioinfo | grep PC31
        line  95:       "PC31"       unused   input  active-high

This was tested with the following dt setup:

&pioA {
        // no-autoname;
        gpio-line-names =
                /* PA */
                "", "", "", "", "", "", "", "",
                "", "", "", "", "GPS_EN", "", "", "",
                "", "", "3V3_EN", "", "", "", "", "",
                "", "", "", "", "", "", "", "",
                /* PB */
                "", "", "", "", "", "", "", "",
                "", "", "", "", "", "", "", "",
                "", "", "", "", "", "", "", "",
                "", "", "", "", "", "", "", "",
                /* PC */
                "GPS_RESET", "", "", "", "", "", "", "",
                "", "", "", "", "", "", "", "",
                "", "", "", "", "", "", "", "",
                "", "", "", "", "", "", "", "CAN0_EN",
                /* PD */
                "", "", "", "", "", "", "", "",
                "", "", "", "", "", "", "", "",
                "", "", "", "", "", "", "", "",
                "", "", "", "", "", "", "", "";
        can0-enable {
                gpios = <PIN_PC31 GPIO_ACTIVE_HIGH>;
                output-low;
                line-name = "CAN0_EN";
        };
};

Any idea where I have gone wrong here?

1. https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpio/gpiolib-of.c?h=linux-5.5.y#n908

2. https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/pinctrl/pinctrl-at91-pio4.c?h=linux-5.5.y#n1089

Patch
diff mbox series

diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
index 6949124..8b76149 100644
--- a/drivers/pinctrl/pinctrl-at91-pio4.c
+++ b/drivers/pinctrl/pinctrl-at91-pio4.c
@@ -996,7 +996,7 @@  static int atmel_pinctrl_probe(struct platform_device *pdev)
 	struct pinctrl_pin_desc	*pin_desc;
 	const char **group_names;
 	const struct of_device_id *match;
-	int i, ret;
+	int i, ret, autoname;
 	struct resource	*res;
 	struct atmel_pioctrl *atmel_pioctrl;
 	const struct atmel_pioctrl_data *atmel_pioctrl_data;
@@ -1056,6 +1056,7 @@  static int atmel_pinctrl_probe(struct platform_device *pdev)
 			GFP_KERNEL);
 	if (!atmel_pioctrl->groups)
 		return -ENOMEM;
+	autoname = !of_property_read_bool(dev->of_node, "no-autoname");
 	for (i = 0 ; i < atmel_pioctrl->npins; i++) {
 		struct atmel_group *group = atmel_pioctrl->groups + i;
 		unsigned bank = ATMEL_PIO_BANK(i);
@@ -1086,8 +1087,9 @@  static int atmel_pinctrl_probe(struct platform_device *pdev)
 	atmel_pioctrl->gpio_chip->ngpio = atmel_pioctrl->npins;
 	atmel_pioctrl->gpio_chip->label = dev_name(dev);
 	atmel_pioctrl->gpio_chip->parent = dev;
-	atmel_pioctrl->gpio_chip->names = atmel_pioctrl->group_names;
-
+	if (autoname) {
+		atmel_pioctrl->gpio_chip->names = atmel_pioctrl->group_names;
+	}
 	atmel_pioctrl->pm_wakeup_sources = devm_kcalloc(dev,
 			atmel_pioctrl->nbanks,
 			sizeof(*atmel_pioctrl->pm_wakeup_sources),