| Message ID | 20211018220504.8301-1-shreeya.patel@collabora.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v2] gpio: Return EPROBE_DEFER if gc->to_irq is NULL | expand |
On Tue, Oct 19, 2021 at 12:05 AM Shreeya Patel <shreeya.patel@collabora.com> wrote: > We are racing the registering of .to_irq when probing the > i2c driver. This results in random failure of touchscreen > devices. > > Following errors could be seen in dmesg logs when gc->to_irq is NULL > > [2.101857] i2c_hid i2c-FTS3528:00: HID over i2c has not been provided an Int IRQ > [2.101953] i2c_hid: probe of i2c-FTS3528:00 failed with error -22 > > To avoid this situation, defer probing until to_irq is registered. > > This issue has been reported many times in past and people have been > using workarounds like changing the pinctrl_amd to built-in instead > of loading it as a module or by adding a softdep for pinctrl_amd into > the config file. > > References :- > https://bugzilla.kernel.org/show_bug.cgi?id=209413 > https://github.com/Syniurge/i2c-amd-mp2/issues/3 > > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> > > --- > Changes in v2 > - Add a condition to check for irq chip to avoid bogus error. This v2 looks acceptable to me. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
Hi Shreeya,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linusw-gpio/for-next]
[also build test ERROR on v5.15 next-20211110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Shreeya-Patel/gpio-Return-EPROBE_DEFER-if-gc-to_irq-is-NULL/20211019-060640
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: sh-rsk7269_defconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/e807153b3cae5acb6d78393087c28e4004cb9856
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Shreeya-Patel/gpio-Return-EPROBE_DEFER-if-gc-to_irq-is-NULL/20211019-060640
git checkout e807153b3cae5acb6d78393087c28e4004cb9856
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sh SHELL=/bin/bash drivers/gpio/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
cc1: warning: arch/sh/include/mach-rsk: No such file or directory [-Wmissing-include-dirs]
cc1: warning: arch/sh/include/mach-rsk: No such file or directory [-Wmissing-include-dirs]
drivers/gpio/gpiolib.c: In function 'gpiod_to_irq':
>> drivers/gpio/gpiolib.c:3068:15: error: 'struct gpio_chip' has no member named 'irq'
3068 | if (gc->irq.chip) {
| ^~
vim +3068 drivers/gpio/gpiolib.c
3036
3037 /**
3038 * gpiod_to_irq() - return the IRQ corresponding to a GPIO
3039 * @desc: gpio whose IRQ will be returned (already requested)
3040 *
3041 * Return the IRQ corresponding to the passed GPIO, or an error code in case of
3042 * error.
3043 */
3044 int gpiod_to_irq(const struct gpio_desc *desc)
3045 {
3046 struct gpio_chip *gc;
3047 int offset;
3048
3049 /*
3050 * Cannot VALIDATE_DESC() here as gpiod_to_irq() consumer semantics
3051 * requires this function to not return zero on an invalid descriptor
3052 * but rather a negative error number.
3053 */
3054 if (!desc || IS_ERR(desc) || !desc->gdev || !desc->gdev->chip)
3055 return -EINVAL;
3056
3057 gc = desc->gdev->chip;
3058 offset = gpio_chip_hwgpio(desc);
3059 if (gc->to_irq) {
3060 int retirq = gc->to_irq(gc, offset);
3061
3062 /* Zero means NO_IRQ */
3063 if (!retirq)
3064 return -ENXIO;
3065
3066 return retirq;
3067 }
> 3068 if (gc->irq.chip) {
3069 /* avoid race condition with other code, which tries to lookup
3070 * an IRQ before the irqchip has been properly registered
3071 * (i.e. while gpiochip is still being brought up).
3072 */
3073 return -EPROBE_DEFER;
3074 }
3075
3076 return -ENXIO;
3077 }
3078 EXPORT_SYMBOL_GPL(gpiod_to_irq);
3079
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Wed, Nov 10, 2021 at 10:16 AM kernel test robot <lkp@intel.com> wrote: > drivers/gpio/gpiolib.c: In function 'gpiod_to_irq': > >> drivers/gpio/gpiolib.c:3068:15: error: 'struct gpio_chip' has no member named 'irq' > 3068 | if (gc->irq.chip) { > | ^~ Right you need an #ifdef CONFIG_GPIOLIB_IRQCHIP in this case I would try: if (IS_ENABLED(CONFIG_GPIOLIB_IRQCHIP) && gc->irq.chip) hoping the compiler is smart enough to deal with that (but I'm not sure it is) Yours, Linus Walleij
On Tue, Oct 19, 2021 at 1:07 AM Shreeya Patel <shreeya.patel@collabora.com> wrote: > > We are racing the registering of .to_irq when probing the > i2c driver. This results in random failure of touchscreen > devices. > > Following errors could be seen in dmesg logs when gc->to_irq is NULL > > [2.101857] i2c_hid i2c-FTS3528:00: HID over i2c has not been provided an Int IRQ > [2.101953] i2c_hid: probe of i2c-FTS3528:00 failed with error -22 > > To avoid this situation, defer probing until to_irq is registered. > > This issue has been reported many times in past and people have been > using workarounds like changing the pinctrl_amd to built-in instead > of loading it as a module or by adding a softdep for pinctrl_amd into > the config file. > References :- > https://bugzilla.kernel.org/show_bug.cgi?id=209413 > https://github.com/Syniurge/i2c-amd-mp2/issues/3 Please, convert them to BugLink: tags. > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> ... > + if (gc->irq.chip) { > + /* avoid race condition with other code, which tries to lookup > + * an IRQ before the irqchip has been properly registered > + * (i.e. while gpiochip is still being brought up). > + */ /* * The style of multi-line comments should be * like this. Pay attention to the grammar. * In your comment the parentheses can be replaced * by a simple ',' (comma without quotes). */ > + return -EPROBE_DEFER; > + } With above (including kbuildbot thingy) addressed Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 27c07108496d..12c088706167 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -3084,6 +3084,14 @@ int gpiod_to_irq(const struct gpio_desc *desc) return retirq; } + if (gc->irq.chip) { + /* avoid race condition with other code, which tries to lookup + * an IRQ before the irqchip has been properly registered + * (i.e. while gpiochip is still being brought up). + */ + return -EPROBE_DEFER; + } + return -ENXIO; } EXPORT_SYMBOL_GPL(gpiod_to_irq);
We are racing the registering of .to_irq when probing the i2c driver. This results in random failure of touchscreen devices. Following errors could be seen in dmesg logs when gc->to_irq is NULL [2.101857] i2c_hid i2c-FTS3528:00: HID over i2c has not been provided an Int IRQ [2.101953] i2c_hid: probe of i2c-FTS3528:00 failed with error -22 To avoid this situation, defer probing until to_irq is registered. This issue has been reported many times in past and people have been using workarounds like changing the pinctrl_amd to built-in instead of loading it as a module or by adding a softdep for pinctrl_amd into the config file. References :- https://bugzilla.kernel.org/show_bug.cgi?id=209413 https://github.com/Syniurge/i2c-amd-mp2/issues/3 Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> --- Changes in v2 - Add a condition to check for irq chip to avoid bogus error. --- drivers/gpio/gpiolib.c | 8 ++++++++ 1 file changed, 8 insertions(+)