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 <[email protected]>
---
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(+)
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);
--
2.30.2
On Tue, Oct 19, 2021 at 12:05 AM Shreeya Patel
<[email protected]> 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 <[email protected]>
>
> ---
> 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 <[email protected]>
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 <[email protected]>
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/[email protected]
On Wed, Nov 10, 2021 at 10:16 AM kernel test robot <[email protected]> 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
<[email protected]> 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 <[email protected]>
...
> + 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 <[email protected]>
--
With Best Regards,
Andy Shevchenko