| Message ID | 20191127094031.140736-1-linus.walleij@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | gpio: Handle counting of Freescale chipselects | expand |
Le 27/11/2019 à 10:40, Linus Walleij a écrit : > We have a special quirk to handle the Freescale > nonstandard SPI chipselect GPIOs in the gpiolib-of.c > file, but it currently only handles the case where > the GPIOs are actually requested (gpiod_*get()). > > We also need to handle that the SPI core attempts > to count the GPIOs before use, and that needs a > similar quirk in the OF part of the library. > > Cc: Christophe Leroy <christophe.leroy@c-s.fr> > Reported-by: Christophe Leroy <christophe.leroy@c-s.fr> > Fixes: 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Still getting: [ 3.374867] fsl_spi: probe of ff000a80.spi failed with error -22 Christophe > --- > Mark: I will merge this through the GPIO tree if > it fixes the problem. > --- > drivers/gpio/gpiolib-of.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c > index 80ea49f570f4..33e16fa17bd8 100644 > --- a/drivers/gpio/gpiolib-of.c > +++ b/drivers/gpio/gpiolib-of.c > @@ -23,6 +23,29 @@ > #include "gpiolib.h" > #include "gpiolib-of.h" > > +/** > + * of_gpio_spi_cs_get_count() - special GPIO counting for SPI > + * Some elder GPIO controllers need special quirks. Currently we handle > + * the Freescale GPIO controller with bindings that doesn't use the > + * established "cs-gpios" for chip selects but instead rely on > + * "gpios" for the chip select lines. If we detect this, we redirect > + * the counting of "cs-gpios" to count "gpios" transparent to the > + * driver. > + */ > +int of_gpio_spi_cs_get_count(struct device *dev, const char *con_id) > +{ > + struct device_node *np = dev->of_node; > + > + if (!IS_ENABLED(CONFIG_SPI_MASTER)) > + return 0; > + if (strcmp(con_id, "cs")) > + return 0; > + if (!of_device_is_compatible(np, "fsl,spi") && > + !of_device_is_compatible(np, "aeroflexgaisler,spictrl")) > + return 0; > + return of_gpio_get_count(dev, NULL); > +} > + > /* > * This is used by external users of of_gpio_count() from <linux/of_gpio.h> > * > @@ -35,6 +58,10 @@ int of_gpio_get_count(struct device *dev, const char *con_id) > char propname[32]; > unsigned int i; > > + ret = of_gpio_spi_cs_get_count(dev, con_id); > + if (ret > 0) > + return ret; > + > for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) { > if (con_id) > snprintf(propname, sizeof(propname), "%s-%s", >
Le 27/11/2019 à 11:07, Christophe Leroy a écrit : > > > Le 27/11/2019 à 10:40, Linus Walleij a écrit : >> We have a special quirk to handle the Freescale >> nonstandard SPI chipselect GPIOs in the gpiolib-of.c >> file, but it currently only handles the case where >> the GPIOs are actually requested (gpiod_*get()). >> >> We also need to handle that the SPI core attempts >> to count the GPIOs before use, and that needs a >> similar quirk in the OF part of the library. >> >> Cc: Christophe Leroy <christophe.leroy@c-s.fr> >> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr> >> Fixes: 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > Still getting: > > [ 3.374867] fsl_spi: probe of ff000a80.spi failed with error -22 Indeed, of_spi_get_gpio_numbers() uses of_gpio_named_count(np, "cs-gpios") which still returns 0; Christophe > > Christophe > > >> --- >> Mark: I will merge this through the GPIO tree if >> it fixes the problem. >> --- >> drivers/gpio/gpiolib-of.c | 27 +++++++++++++++++++++++++++ >> 1 file changed, 27 insertions(+) >> >> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c >> index 80ea49f570f4..33e16fa17bd8 100644 >> --- a/drivers/gpio/gpiolib-of.c >> +++ b/drivers/gpio/gpiolib-of.c >> @@ -23,6 +23,29 @@ >> #include "gpiolib.h" >> #include "gpiolib-of.h" >> +/** >> + * of_gpio_spi_cs_get_count() - special GPIO counting for SPI >> + * Some elder GPIO controllers need special quirks. Currently we handle >> + * the Freescale GPIO controller with bindings that doesn't use the >> + * established "cs-gpios" for chip selects but instead rely on >> + * "gpios" for the chip select lines. If we detect this, we redirect >> + * the counting of "cs-gpios" to count "gpios" transparent to the >> + * driver. >> + */ >> +int of_gpio_spi_cs_get_count(struct device *dev, const char *con_id) >> +{ >> + struct device_node *np = dev->of_node; >> + >> + if (!IS_ENABLED(CONFIG_SPI_MASTER)) >> + return 0; >> + if (strcmp(con_id, "cs")) >> + return 0; >> + if (!of_device_is_compatible(np, "fsl,spi") && >> + !of_device_is_compatible(np, "aeroflexgaisler,spictrl")) >> + return 0; >> + return of_gpio_get_count(dev, NULL); >> +} >> + >> /* >> * This is used by external users of of_gpio_count() from >> <linux/of_gpio.h> >> * >> @@ -35,6 +58,10 @@ int of_gpio_get_count(struct device *dev, const >> char *con_id) >> char propname[32]; >> unsigned int i; >> + ret = of_gpio_spi_cs_get_count(dev, con_id); >> + if (ret > 0) >> + return ret; >> + >> for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) { >> if (con_id) >> snprintf(propname, sizeof(propname), "%s-%s", >>
Le 27/11/2019 à 11:15, Christophe Leroy a écrit : > > > Le 27/11/2019 à 11:07, Christophe Leroy a écrit : >> >> >> Le 27/11/2019 à 10:40, Linus Walleij a écrit : >>> We have a special quirk to handle the Freescale >>> nonstandard SPI chipselect GPIOs in the gpiolib-of.c >>> file, but it currently only handles the case where >>> the GPIOs are actually requested (gpiod_*get()). >>> >>> We also need to handle that the SPI core attempts >>> to count the GPIOs before use, and that needs a >>> similar quirk in the OF part of the library. >>> >>> Cc: Christophe Leroy <christophe.leroy@c-s.fr> >>> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr> >>> Fixes: 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") >>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> >> >> Still getting: >> >> [ 3.374867] fsl_spi: probe of ff000a80.spi failed with error -22 > > Indeed, of_spi_get_gpio_numbers() uses of_gpio_named_count(np, > "cs-gpios") which still returns 0; Replacing by of_gpio_named_count(np, "gpios"); , I get further down to the same spi_fsl_setup() warning as when renaming the property in the DTS. Christophe > > Christophe > >> >> Christophe >> >> >>> --- >>> Mark: I will merge this through the GPIO tree if >>> it fixes the problem. >>> --- >>> drivers/gpio/gpiolib-of.c | 27 +++++++++++++++++++++++++++ >>> 1 file changed, 27 insertions(+) >>> >>> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c >>> index 80ea49f570f4..33e16fa17bd8 100644 >>> --- a/drivers/gpio/gpiolib-of.c >>> +++ b/drivers/gpio/gpiolib-of.c >>> @@ -23,6 +23,29 @@ >>> #include "gpiolib.h" >>> #include "gpiolib-of.h" >>> +/** >>> + * of_gpio_spi_cs_get_count() - special GPIO counting for SPI >>> + * Some elder GPIO controllers need special quirks. Currently we handle >>> + * the Freescale GPIO controller with bindings that doesn't use the >>> + * established "cs-gpios" for chip selects but instead rely on >>> + * "gpios" for the chip select lines. If we detect this, we redirect >>> + * the counting of "cs-gpios" to count "gpios" transparent to the >>> + * driver. >>> + */ >>> +int of_gpio_spi_cs_get_count(struct device *dev, const char *con_id) >>> +{ >>> + struct device_node *np = dev->of_node; >>> + >>> + if (!IS_ENABLED(CONFIG_SPI_MASTER)) >>> + return 0; >>> + if (strcmp(con_id, "cs")) >>> + return 0; >>> + if (!of_device_is_compatible(np, "fsl,spi") && >>> + !of_device_is_compatible(np, "aeroflexgaisler,spictrl")) >>> + return 0; >>> + return of_gpio_get_count(dev, NULL); >>> +} >>> + >>> /* >>> * This is used by external users of of_gpio_count() from >>> <linux/of_gpio.h> >>> * >>> @@ -35,6 +58,10 @@ int of_gpio_get_count(struct device *dev, const >>> char *con_id) >>> char propname[32]; >>> unsigned int i; >>> + ret = of_gpio_spi_cs_get_count(dev, con_id); >>> + if (ret > 0) >>> + return ret; >>> + >>> for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) { >>> if (con_id) >>> snprintf(propname, sizeof(propname), "%s-%s", >>>
On Wed, Nov 27, 2019 at 11:25 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > Le 27/11/2019 à 11:15, Christophe Leroy a écrit : > > Le 27/11/2019 à 11:07, Christophe Leroy a écrit : > >> Le 27/11/2019 à 10:40, Linus Walleij a écrit : > >>> We have a special quirk to handle the Freescale > >>> nonstandard SPI chipselect GPIOs in the gpiolib-of.c > >>> file, but it currently only handles the case where > >>> the GPIOs are actually requested (gpiod_*get()). > >>> > >>> We also need to handle that the SPI core attempts > >>> to count the GPIOs before use, and that needs a > >>> similar quirk in the OF part of the library. > >>> > >>> Cc: Christophe Leroy <christophe.leroy@c-s.fr> > >>> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr> > >>> Fixes: 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") > >>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > >> > >> Still getting: > >> > >> [ 3.374867] fsl_spi: probe of ff000a80.spi failed with error -22 > > > > Indeed, of_spi_get_gpio_numbers() uses of_gpio_named_count(np, > > "cs-gpios") which still returns 0; > > Replacing by of_gpio_named_count(np, "gpios"); , I get further down to > the same spi_fsl_setup() warning as when renaming the property in the DTS. Ah, I got bitten by recursion, sorry. OK I changed to to "gpios" in my patch too, it's the right way. Now we need to find the final culprit that makes it not even work when renaming to "cs-gpios"... Yours, Linus Walleij
Le 27/11/2019 à 11:59, Linus Walleij a écrit : > On Wed, Nov 27, 2019 at 11:25 AM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: >> Le 27/11/2019 à 11:15, Christophe Leroy a écrit : >>> Le 27/11/2019 à 11:07, Christophe Leroy a écrit : >>>> Le 27/11/2019 à 10:40, Linus Walleij a écrit : >>>>> We have a special quirk to handle the Freescale >>>>> nonstandard SPI chipselect GPIOs in the gpiolib-of.c >>>>> file, but it currently only handles the case where >>>>> the GPIOs are actually requested (gpiod_*get()). >>>>> >>>>> We also need to handle that the SPI core attempts >>>>> to count the GPIOs before use, and that needs a >>>>> similar quirk in the OF part of the library. >>>>> >>>>> Cc: Christophe Leroy <christophe.leroy@c-s.fr> >>>>> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr> >>>>> Fixes: 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") >>>>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> >>>> >>>> Still getting: >>>> >>>> [ 3.374867] fsl_spi: probe of ff000a80.spi failed with error -22 >>> >>> Indeed, of_spi_get_gpio_numbers() uses of_gpio_named_count(np, >>> "cs-gpios") which still returns 0; >> >> Replacing by of_gpio_named_count(np, "gpios"); , I get further down to >> the same spi_fsl_setup() warning as when renaming the property in the DTS. > > Ah, I got bitten by recursion, sorry. > > OK I changed to to "gpios" in my patch too, it's the right way. > > Now we need to find the final culprit that makes it not even work when > renaming to "cs-gpios"... > Now that I have added master->use_gpio_descriptors = true; to the fsl driver, this patch crashes: [ 3.156848] BUG: Kernel NULL pointer dereference on read at 0x00000000 [ 3.163062] Faulting instruction address: 0xc058aadc [ 3.167982] Oops: Kernel access of bad area, sig: 11 [#1] [ 3.173322] BE PAGE_SIZE=16K PREEMPT CMPC885 [ 3.177559] CPU: 0 PID: 1 Comm: swapper Not tainted 5.4.0-s3k-dev-00899-g749f15aba2c9-dirty #2515 [ 3.186306] NIP: c058aadc LR: c028973c CTR: 00000000 [ 3.191308] REGS: c60e1b70 TRAP: 0300 Not tainted (5.4.0-s3k-dev-00899-g749f15aba2c9-dirty) [ 3.199801] MSR: 00009032 <EE,ME,IR,DR,RI> CR: 24000224 XER: 20000000 [ 3.206433] DAR: 00000000 DSISR: c0000000 [ 3.206433] GPR00: c028963c c60e1c28 c60d4000 ffffffff c06b512b 00000000 00000020 c0facf33 [ 3.206433] GPR08: c0609438 00000000 00000000 000affff 24000224 00000000 c0003890 00000000 [ 3.206433] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 c0800000 0000009e [ 3.206433] GPR24: c0856778 c06b512c c61f2210 c624fc00 c0857380 00000000 00000000 c624fc00 [ 3.243756] NIP [c058aadc] strcmp+0x10/0x40 [ 3.247867] LR [c028973c] of_gpio_spi_cs_get_count+0x28/0x98 [ 3.253416] Call Trace: [ 3.255881] [c60e1c28] [c034b920] __of_device_is_compatible+0xe4/0x14c (unreliable) [ 3.263437] [c60e1c38] [c028963c] of_gpio_get_count+0x24/0xfc [ 3.269116] [c60e1c88] [c028963c] of_gpio_get_count+0x24/0xfc [ 3.274831] [c60e1cd8] [c0286c1c] gpiod_count+0x34/0x100 [ 3.280089] [c60e1cf8] [c030c5c0] spi_register_controller+0x14c/0xb50 [ 3.286432] [c60e1d48] [c030d004] devm_spi_register_controller+0x40/0x98 [ 3.293047] [c60e1d68] [c030ee60] of_fsl_spi_probe+0x2e8/0x3a8 [ 3.298813] [c60e1db8] [c02c4f3c] platform_drv_probe+0x44/0xa4 [ 3.304598] [c60e1dc8] [c02c30e0] really_probe+0x1ac/0x418 [ 3.310012] [c60e1df8] [c02c3b60] device_driver_attach+0x88/0x90 [ 3.315948] [c60e1e18] [c02c3c08] __driver_attach+0xa0/0x154 [ 3.321540] [c60e1e38] [c02c1140] bus_for_each_dev+0x64/0xb4 [ 3.327134] [c60e1e68] [c02c1b1c] bus_add_driver+0xe0/0x218 [ 3.332646] [c60e1e88] [c02c43c0] driver_register+0x84/0x148 [ 3.338239] [c60e1e98] [c06d8d30] do_one_initcall+0x8c/0x1cc [ 3.343824] [c60e1ef8] [c06d8fac] kernel_init_freeable+0x13c/0x1ec [ 3.349932] [c60e1f28] [c00038a4] kernel_init+0x14/0x110 [ 3.355187] [c60e1f38] [c000e1cc] ret_from_kernel_thread+0x14/0x1c [ 3.361249] Instruction dump: [ 3.364183] 39200000 7d3fe9ae 7f43d378 80010024 bb410008 7c0803a6 38210020 4e800020 [ 3.371841] 3863ffff 3884ffff 48000008 41960024 <8d230001> 8d440001 2e890000 7f895040 [ 3.379710] ---[ end trace 795d948bc094d09f ]--- Christophe
On Wed, Nov 27, 2019 at 1:05 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > Now that I have added master->use_gpio_descriptors = true; to the fsl > driver, this patch crashes: Oh strcpm() doesn't like NULL pointers, OK I toss in a check for that too, thanks. Yours, Linus Walleij
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index 80ea49f570f4..33e16fa17bd8 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -23,6 +23,29 @@ #include "gpiolib.h" #include "gpiolib-of.h" +/** + * of_gpio_spi_cs_get_count() - special GPIO counting for SPI + * Some elder GPIO controllers need special quirks. Currently we handle + * the Freescale GPIO controller with bindings that doesn't use the + * established "cs-gpios" for chip selects but instead rely on + * "gpios" for the chip select lines. If we detect this, we redirect + * the counting of "cs-gpios" to count "gpios" transparent to the + * driver. + */ +int of_gpio_spi_cs_get_count(struct device *dev, const char *con_id) +{ + struct device_node *np = dev->of_node; + + if (!IS_ENABLED(CONFIG_SPI_MASTER)) + return 0; + if (strcmp(con_id, "cs")) + return 0; + if (!of_device_is_compatible(np, "fsl,spi") && + !of_device_is_compatible(np, "aeroflexgaisler,spictrl")) + return 0; + return of_gpio_get_count(dev, NULL); +} + /* * This is used by external users of of_gpio_count() from <linux/of_gpio.h> * @@ -35,6 +58,10 @@ int of_gpio_get_count(struct device *dev, const char *con_id) char propname[32]; unsigned int i; + ret = of_gpio_spi_cs_get_count(dev, con_id); + if (ret > 0) + return ret; + for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) { if (con_id) snprintf(propname, sizeof(propname), "%s-%s",
We have a special quirk to handle the Freescale nonstandard SPI chipselect GPIOs in the gpiolib-of.c file, but it currently only handles the case where the GPIOs are actually requested (gpiod_*get()). We also need to handle that the SPI core attempts to count the GPIOs before use, and that needs a similar quirk in the OF part of the library. Cc: Christophe Leroy <christophe.leroy@c-s.fr> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr> Fixes: 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- Mark: I will merge this through the GPIO tree if it fixes the problem. --- drivers/gpio/gpiolib-of.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) -- 2.23.0