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(+)