Re: [PATCH 3/4] gpiolib: rework quirk handling in of_find_gpio()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

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
>
[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux