Re: [PATCH 3/4] gpiolib: rework quirk handling in of_find_gpio()
From: Conor.Dooley
Date: Fri Sep 16 2022 - 07:09:57 EST
On 08/09/2022 06:39, Dmitry Torokhov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Instead of having a string of "if" statements let's put all quirks into
> an array and iterate over them.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
Hey,
This seems to have landed in -next today as a2b5e207cade which has
unfortunately broken boot for me:
[ 0.765969] Unable to handle kernel paging request at virtual address ffffffad6f697066
[ 0.774948] Oops [#1]
[ 0.777491] Modules linked in:
[ 0.780896] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-rc1-00037-ga2b5e207cade #1
[ 0.789668] Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
[ 0.796512] epc : 0xffffffad6f697066
[ 0.800491] ra : of_find_gpio+0x12c/0x1fa
[ 0.805066] epc : ffffffad6f697066 ra : ffffffff8042032c sp : ffffffc80400b4e0
[ 0.813062] gp : ffffffff810ef490 tp : ffffffe77fe68000 t0 : 0000000400000000
[ 0.821063] t1 : 0000001000000000 t2 : 0000000000000010 s0 : ffffffc80400b560
[ 0.829058] s1 : ffffffff80c52a88 a0 : ffffffe7bfdf37b0 a1 : ffffffff80d5320e
[ 0.837053] a2 : 0000000000000000 a3 : ffffffc80400b4e4 a4 : 6e61722d6f697067
[ 0.845057] a5 : 0000000000000000 a6 : ffffffff80c4e760 a7 : ffffffe77ffb5b73
[ 0.853048] s2 : ffffffc80400b588 s3 : 0000000000000000 s4 : ffffffe77ffad810
[ 0.861052] s5 : ffffffff810f3098 s6 : ffffffff80d5320e s7 : fffffffffffffffe
[ 0.869048] s8 : fffffffffffff000 s9 : 0000000000000003 s10: 0000000000000000
[ 0.877050] s11: ffffffff80d5320e t3 : 0000000200000000 t4 : ffffffff80c4ebe8
[ 0.885045] t5 : ffffffff80d4ce2e t6 : ffffffe77ffb5b72
[ 0.890929] status: 0000000200000120 badaddr: ffffffad6f697066 cause: 000000000000000c
[ 0.900316] ---[ end trace 0000000000000000 ]---
[ 0.905544] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 0.914024] SMP: stopping secondary CPUs
[ 0.918394] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
In case it is useful to you, I have gpio nodes in my devicetree
but I am not building a driver that binds to those nodes. Since I
don't have a driver, that's pretty much all of the relevant info
from the boot log. Anything else you need, lmk and I will try to
provide :)
Thanks,
Conor.
> ---
> drivers/gpio/gpiolib-of.c | 62 ++++++++++++++++-----------------------
> 1 file changed, 26 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 30b89b694530..097e948c1d49 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -372,14 +372,12 @@ EXPORT_SYMBOL_GPL(gpiod_get_from_of_node);
> * properties should be named "foo-gpios" so we have this special kludge for
> * them.
> */
> -static struct gpio_desc *of_find_spi_gpio(struct device *dev,
> +static struct gpio_desc *of_find_spi_gpio(struct device_node *np,
> const char *con_id,
> unsigned int idx,
> enum of_gpio_flags *of_flags)
> {
> char prop_name[32]; /* 32 is max size of property name */
> - const struct device_node *np = dev->of_node;
> - struct gpio_desc *desc;
>
> /*
> * Hopefully the compiler stubs the rest of the function if this
> @@ -395,8 +393,7 @@ static struct gpio_desc *of_find_spi_gpio(struct device *dev,
> /* Will be "gpio-sck", "gpio-mosi" or "gpio-miso" */
> snprintf(prop_name, sizeof(prop_name), "%s-%s", "gpio", con_id);
>
> - desc = of_get_named_gpiod_flags(np, prop_name, idx, of_flags);
> - return desc;
> + return of_get_named_gpiod_flags(np, prop_name, idx, of_flags);
> }
>
> /*
> @@ -404,13 +401,11 @@ static struct gpio_desc *of_find_spi_gpio(struct device *dev,
> * lines rather than "cs-gpios" like all other SPI hardware. Account for this
> * with a special quirk.
> */
> -static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
> +static struct gpio_desc *of_find_spi_cs_gpio(struct device_node *np,
> const char *con_id,
> unsigned int idx,
> enum of_gpio_flags *of_flags)
> {
> - const struct device_node *np = dev->of_node;
> -
> if (!IS_ENABLED(CONFIG_SPI_MASTER))
> return ERR_PTR(-ENOENT);
>
> @@ -428,7 +423,7 @@ static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
> * uses just "gpios" so translate to that when "cs-gpios" is
> * requested.
> */
> - return of_get_named_gpiod_flags(dev->of_node, "gpios", idx, of_flags);
> + return of_get_named_gpiod_flags(np, "gpios", idx, of_flags);
> }
>
> /*
> @@ -436,7 +431,7 @@ static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
> * properties should be named "foo-gpios" so we have this special kludge for
> * them.
> */
> -static struct gpio_desc *of_find_regulator_gpio(struct device *dev,
> +static struct gpio_desc *of_find_regulator_gpio(struct device_node *np,
> const char *con_id,
> unsigned int idx,
> enum of_gpio_flags *of_flags)
> @@ -447,8 +442,6 @@ static struct gpio_desc *of_find_regulator_gpio(struct device *dev,
> "wlf,ldo1ena", /* WM8994 */
> "wlf,ldo2ena", /* WM8994 */
> };
> - const struct device_node *np = dev->of_node;
> - struct gpio_desc *desc;
> int i;
>
> if (!IS_ENABLED(CONFIG_REGULATOR))
> @@ -461,11 +454,10 @@ static struct gpio_desc *of_find_regulator_gpio(struct device *dev,
> if (i < 0)
> return ERR_PTR(-ENOENT);
>
> - desc = of_get_named_gpiod_flags(np, con_id, idx, of_flags);
> - return desc;
> + return of_get_named_gpiod_flags(np, con_id, idx, of_flags);
> }
>
> -static struct gpio_desc *of_find_arizona_gpio(struct device *dev,
> +static struct gpio_desc *of_find_arizona_gpio(struct device_node *np,
> const char *con_id,
> unsigned int idx,
> enum of_gpio_flags *of_flags)
> @@ -476,10 +468,10 @@ static struct gpio_desc *of_find_arizona_gpio(struct device *dev,
> if (!con_id || strcmp(con_id, "wlf,reset"))
> return ERR_PTR(-ENOENT);
>
> - return of_get_named_gpiod_flags(dev->of_node, con_id, idx, of_flags);
> + return of_get_named_gpiod_flags(np, con_id, idx, of_flags);
> }
>
> -static struct gpio_desc *of_find_usb_gpio(struct device *dev,
> +static struct gpio_desc *of_find_usb_gpio(struct device_node *np,
> const char *con_id,
> unsigned int idx,
> enum of_gpio_flags *of_flags)
> @@ -495,14 +487,27 @@ static struct gpio_desc *of_find_usb_gpio(struct device *dev,
> if (!con_id || strcmp(con_id, "fcs,int_n"))
> return ERR_PTR(-ENOENT);
>
> - return of_get_named_gpiod_flags(dev->of_node, con_id, idx, of_flags);
> + return of_get_named_gpiod_flags(np, con_id, idx, of_flags);
> }
>
> +typedef struct gpio_desc *(*of_find_gpio_quirk)(struct device_node *np,
> + const char *con_id,
> + unsigned int idx,
> + enum of_gpio_flags *of_flags);
> +static const of_find_gpio_quirk of_find_gpio_quirks[] = {
> + of_find_spi_gpio,
> + of_find_spi_cs_gpio,
> + of_find_regulator_gpio,
> + of_find_arizona_gpio,
> + of_find_usb_gpio,
> +};
> +
> struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
> unsigned int idx, unsigned long *flags)
> {
> char prop_name[32]; /* 32 is max size of property name */
> enum of_gpio_flags of_flags;
> + const of_find_gpio_quirk *q;
> struct gpio_desc *desc;
> unsigned int i;
>
> @@ -522,24 +527,9 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
> break;
> }
>
> - if (gpiod_not_found(desc)) {
> - /* Special handling for SPI GPIOs if used */
> - desc = of_find_spi_gpio(dev, con_id, idx, &of_flags);
> - }
> -
> - if (gpiod_not_found(desc))
> - desc = of_find_spi_cs_gpio(dev, con_id, idx, &of_flags);
> -
> - if (gpiod_not_found(desc)) {
> - /* Special handling for regulator GPIOs if used */
> - desc = of_find_regulator_gpio(dev, con_id, idx, &of_flags);
> - }
> -
> - if (gpiod_not_found(desc))
> - desc = of_find_arizona_gpio(dev, con_id, idx, &of_flags);
> -
> - if (gpiod_not_found(desc))
> - desc = of_find_usb_gpio(dev, con_id, idx, &of_flags);
> + /* Properly named GPIO was not found, try workarounds */
> + for (q = of_find_gpio_quirks; gpiod_not_found(desc) && *q; q++)
> + desc = (*q)(dev->of_node, con_id, idx, &of_flags);
>
> if (IS_ERR(desc))
> return desc;
> --
> 2.37.2.789.g6183377224-goog
>