| Message ID | 20200912112200.226530-1-megous@megous.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mfd: sun4i-gpadc: Interrupt numbers should start from 1 | expand |
Hi, On Sat, Sep 12, 2020 at 01:22:00PM +0200, Ondrej Jirman wrote: > mfd: sun4i-gpadc: Interrupt numbers should start from 1 Why? An hwirq with 0 is totally fine > This avoids a warning: > > [ 2.891592] ------------[ cut here ]------------ > [ 2.895052] WARNING: CPU: 0 PID: 149 at drivers/base/platform.c:317 __platform_get_irq_byname+0x7c/0x8c > [ 2.903212] usb 1-1: new high-speed USB device number 2 using ehci-platform > [ 2.908930] 0 is an invalid IRQ number > [ 2.911425] Modules linked in: sun4i_gpadc_iio(+) r8188eu(C) lib80211 ohci_platform ohci_hcd ehci_platform ehci_hcd cyttsp4_i2c cyttsp_i2c_common cyttsp4_core g_cdc usb_f_acm u_serial usb_f_ecm u_ether libcomposite sunxi phy_generic musb_hdrc udc_core usbcore sun5ieink > [ 2.934048] CPU: 0 PID: 149 Comm: tablet-init Tainted: G C 5.8.0-rc2-00316-gc6a5213fdeba-dirty #8 > [ 2.943027] Hardware name: Allwinner sun4i/sun5i Families > [ 2.947204] [<c010c080>] (unwind_backtrace) from [<c0109ee4>] (show_stack+0x10/0x14) > [ 2.953714] [<c0109ee4>] (show_stack) from [<c012cd04>] (__warn+0xc0/0xd8) > [ 2.959364] [<c012cd04>] (__warn) from [<c012cda0>] (warn_slowpath_fmt+0x84/0x94) > [ 2.965599] [<c012cda0>] (warn_slowpath_fmt) from [<c0592a60>] (__platform_get_irq_byname+0x7c/0x8c) > [ 2.973480] [<c0592a60>] (__platform_get_irq_byname) from [<c0592a80>] (platform_get_irq_byname+0x10/0x48) > [ 2.981896] [<c0592a80>] (platform_get_irq_byname) from [<bf1142f0>] (sun4i_irq_init+0x38/0xe0 [sun4i_gpadc_iio]) > [ 2.990923] [<bf1142f0>] (sun4i_irq_init [sun4i_gpadc_iio]) from [<bf1145cc>] (sun4i_gpadc_probe+0x234/0x308 [sun4i_gpadc_iio]) > [ 3.001152] [<bf1145cc>] (sun4i_gpadc_probe [sun4i_gpadc_iio]) from [<c0592edc>] (platform_drv_probe+0x48/0x98) > [ 3.010051] [<c0592edc>] (platform_drv_probe) from [<c0591074>] (really_probe+0x1e0/0x348) > [ 3.017152] [<c0591074>] (really_probe) from [<c05912e8>] (driver_probe_device+0x5c/0xb4) > [ 3.024081] [<c05912e8>] (driver_probe_device) from [<c05914e0>] (device_driver_attach+0x58/0x60) > [ 3.031696] [<c05914e0>] (device_driver_attach) from [<c0591540>] (__driver_attach+0x58/0xcc) > [ 3.038966] [<c0591540>] (__driver_attach) from [<c058f418>] (bus_for_each_dev+0x64/0x90) > [ 3.045886] [<c058f418>] (bus_for_each_dev) from [<c0590490>] (bus_add_driver+0x15c/0x1e0) > [ 3.052892] [<c0590490>] (bus_add_driver) from [<c0591dc4>] (driver_register+0x7c/0x114) > [ 3.059731] [<c0591dc4>] (driver_register) from [<c0101674>] (do_one_initcall+0x44/0x194) > [ 3.066696] [<c0101674>] (do_one_initcall) from [<c01946e8>] (do_init_module+0x5c/0x220) > [ 3.073568] [<c01946e8>] (do_init_module) from [<c0196a3c>] (load_module+0x20ec/0x2380) > [ 3.080340] [<c0196a3c>] (load_module) from [<c0196e04>] (sys_init_module+0x134/0x154) > [ 3.087020] [<c0196e04>] (sys_init_module) from [<c0100060>] (ret_fast_syscall+0x0/0x54) > [ 3.093852] Exception stack(0xc8ea7fa8 to 0xc8ea7ff0) > [ 3.097649] 7fa0: 00000000 00000000 b6fe2000 00003b14 00031284 00000005 > [ 3.104596] 7fc0: 00000000 00000000 b6fe2000 00000080 00052220 be9fd940 00052246 0000002e > [ 3.111626] 7fe0: be9fd79c be9fd780 00015bd0 0001dcf4 > [ 3.115468] ---[ end trace df4dd47fb61bf5a4 ]--- In which situation? > Signed-off-by: Ondrej Jirman <megous@megous.com> > --- > include/linux/mfd/sun4i-gpadc.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h > index ea0ccf33a459..021f820f9d52 100644 > --- a/include/linux/mfd/sun4i-gpadc.h > +++ b/include/linux/mfd/sun4i-gpadc.h > @@ -81,8 +81,8 @@ > #define SUN4I_GPADC_TEMP_DATA 0x20 > #define SUN4I_GPADC_DATA 0x24 > > -#define SUN4I_GPADC_IRQ_FIFO_DATA 0 > -#define SUN4I_GPADC_IRQ_TEMP_DATA 1 > +#define SUN4I_GPADC_IRQ_FIFO_DATA 1 > +#define SUN4I_GPADC_IRQ_TEMP_DATA 2 Where is it coming from, and why is it the proper fix? Maxime
Hello Maxime, On Thu, Sep 17, 2020 at 03:19:04PM +0200, Maxime Ripard wrote: > Hi, > > On Sat, Sep 12, 2020 at 01:22:00PM +0200, Ondrej Jirman wrote: > > mfd: sun4i-gpadc: Interrupt numbers should start from 1 > > Why? An hwirq with 0 is totally fine > > > This avoids a warning: > > > > [ 2.891592] ------------[ cut here ]------------ > > [ 2.895052] WARNING: CPU: 0 PID: 149 at drivers/base/platform.c:317 __platform_get_irq_byname+0x7c/0x8c > > [ 2.903212] usb 1-1: new high-speed USB device number 2 using ehci-platform > > [ 2.908930] 0 is an invalid IRQ number > > [ 2.911425] Modules linked in: sun4i_gpadc_iio(+) r8188eu(C) lib80211 ohci_platform ohci_hcd ehci_platform ehci_hcd cyttsp4_i2c cyttsp_i2c_common cyttsp4_core g_cdc usb_f_acm u_serial usb_f_ecm u_ether libcomposite sunxi phy_generic musb_hdrc udc_core usbcore sun5ieink > > [ 2.934048] CPU: 0 PID: 149 Comm: tablet-init Tainted: G C 5.8.0-rc2-00316-gc6a5213fdeba-dirty #8 > > [ 2.943027] Hardware name: Allwinner sun4i/sun5i Families > > [ 2.947204] [<c010c080>] (unwind_backtrace) from [<c0109ee4>] (show_stack+0x10/0x14) > > [ 2.953714] [<c0109ee4>] (show_stack) from [<c012cd04>] (__warn+0xc0/0xd8) > > [ 2.959364] [<c012cd04>] (__warn) from [<c012cda0>] (warn_slowpath_fmt+0x84/0x94) > > [ 2.965599] [<c012cda0>] (warn_slowpath_fmt) from [<c0592a60>] (__platform_get_irq_byname+0x7c/0x8c) > > [ 2.973480] [<c0592a60>] (__platform_get_irq_byname) from [<c0592a80>] (platform_get_irq_byname+0x10/0x48) > > [ 2.981896] [<c0592a80>] (platform_get_irq_byname) from [<bf1142f0>] (sun4i_irq_init+0x38/0xe0 [sun4i_gpadc_iio]) > > [ 2.990923] [<bf1142f0>] (sun4i_irq_init [sun4i_gpadc_iio]) from [<bf1145cc>] (sun4i_gpadc_probe+0x234/0x308 [sun4i_gpadc_iio]) > > [ 3.001152] [<bf1145cc>] (sun4i_gpadc_probe [sun4i_gpadc_iio]) from [<c0592edc>] (platform_drv_probe+0x48/0x98) > > [ 3.010051] [<c0592edc>] (platform_drv_probe) from [<c0591074>] (really_probe+0x1e0/0x348) > > [ 3.017152] [<c0591074>] (really_probe) from [<c05912e8>] (driver_probe_device+0x5c/0xb4) > > [ 3.024081] [<c05912e8>] (driver_probe_device) from [<c05914e0>] (device_driver_attach+0x58/0x60) > > [ 3.031696] [<c05914e0>] (device_driver_attach) from [<c0591540>] (__driver_attach+0x58/0xcc) > > [ 3.038966] [<c0591540>] (__driver_attach) from [<c058f418>] (bus_for_each_dev+0x64/0x90) > > [ 3.045886] [<c058f418>] (bus_for_each_dev) from [<c0590490>] (bus_add_driver+0x15c/0x1e0) > > [ 3.052892] [<c0590490>] (bus_add_driver) from [<c0591dc4>] (driver_register+0x7c/0x114) > > [ 3.059731] [<c0591dc4>] (driver_register) from [<c0101674>] (do_one_initcall+0x44/0x194) > > [ 3.066696] [<c0101674>] (do_one_initcall) from [<c01946e8>] (do_init_module+0x5c/0x220) > > [ 3.073568] [<c01946e8>] (do_init_module) from [<c0196a3c>] (load_module+0x20ec/0x2380) > > [ 3.080340] [<c0196a3c>] (load_module) from [<c0196e04>] (sys_init_module+0x134/0x154) > > [ 3.087020] [<c0196e04>] (sys_init_module) from [<c0100060>] (ret_fast_syscall+0x0/0x54) > > [ 3.093852] Exception stack(0xc8ea7fa8 to 0xc8ea7ff0) > > [ 3.097649] 7fa0: 00000000 00000000 b6fe2000 00003b14 00031284 00000005 > > [ 3.104596] 7fc0: 00000000 00000000 b6fe2000 00000080 00052220 be9fd940 00052246 0000002e > > [ 3.111626] 7fe0: be9fd79c be9fd780 00015bd0 0001dcf4 > > [ 3.115468] ---[ end trace df4dd47fb61bf5a4 ]--- > > In which situation? During boot. It's a new check added in Linux 5.8. https://elixir.bootlin.com/linux/latest/source/drivers/base/platform.c#L317 > > Signed-off-by: Ondrej Jirman <megous@megous.com> > > --- > > include/linux/mfd/sun4i-gpadc.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h > > index ea0ccf33a459..021f820f9d52 100644 > > --- a/include/linux/mfd/sun4i-gpadc.h > > +++ b/include/linux/mfd/sun4i-gpadc.h > > @@ -81,8 +81,8 @@ > > #define SUN4I_GPADC_TEMP_DATA 0x20 > > #define SUN4I_GPADC_DATA 0x24 > > > > -#define SUN4I_GPADC_IRQ_FIFO_DATA 0 > > -#define SUN4I_GPADC_IRQ_TEMP_DATA 1 > > +#define SUN4I_GPADC_IRQ_FIFO_DATA 1 > > +#define SUN4I_GPADC_IRQ_TEMP_DATA 2 > > Where is it coming from, and why is it the proper fix? Actual numbers seem irrelevant. It's just an index into this array: https://elixir.bootlin.com/linux/latest/source/drivers/mfd/sun4i-gpadc.c#L27 The array will get sparse, but that doesn't seem like an issue to me, because the irq code handles the holes in the list if mask is 0. Not sure it's the best fix. regards, o. > Maxime
On Thu, Sep 17, 2020 at 04:01:17PM +0200, Ondřej Jirman wrote: > Hello Maxime, > > On Thu, Sep 17, 2020 at 03:19:04PM +0200, Maxime Ripard wrote: > > Hi, > > > > On Sat, Sep 12, 2020 at 01:22:00PM +0200, Ondrej Jirman wrote: > > > mfd: sun4i-gpadc: Interrupt numbers should start from 1 > > > > Why? An hwirq with 0 is totally fine > > > > > This avoids a warning: > > > > > > [ 2.891592] ------------[ cut here ]------------ > > > [ 2.895052] WARNING: CPU: 0 PID: 149 at drivers/base/platform.c:317 __platform_get_irq_byname+0x7c/0x8c > > > [ 2.903212] usb 1-1: new high-speed USB device number 2 using ehci-platform > > > [ 2.908930] 0 is an invalid IRQ number > > > [ 2.911425] Modules linked in: sun4i_gpadc_iio(+) r8188eu(C) lib80211 ohci_platform ohci_hcd ehci_platform ehci_hcd cyttsp4_i2c cyttsp_i2c_common cyttsp4_core g_cdc usb_f_acm u_serial usb_f_ecm u_ether libcomposite sunxi phy_generic musb_hdrc udc_core usbcore sun5ieink > > > [ 2.934048] CPU: 0 PID: 149 Comm: tablet-init Tainted: G C 5.8.0-rc2-00316-gc6a5213fdeba-dirty #8 > > > [ 2.943027] Hardware name: Allwinner sun4i/sun5i Families > > > [ 2.947204] [<c010c080>] (unwind_backtrace) from [<c0109ee4>] (show_stack+0x10/0x14) > > > [ 2.953714] [<c0109ee4>] (show_stack) from [<c012cd04>] (__warn+0xc0/0xd8) > > > [ 2.959364] [<c012cd04>] (__warn) from [<c012cda0>] (warn_slowpath_fmt+0x84/0x94) > > > [ 2.965599] [<c012cda0>] (warn_slowpath_fmt) from [<c0592a60>] (__platform_get_irq_byname+0x7c/0x8c) > > > [ 2.973480] [<c0592a60>] (__platform_get_irq_byname) from [<c0592a80>] (platform_get_irq_byname+0x10/0x48) > > > [ 2.981896] [<c0592a80>] (platform_get_irq_byname) from [<bf1142f0>] (sun4i_irq_init+0x38/0xe0 [sun4i_gpadc_iio]) > > > [ 2.990923] [<bf1142f0>] (sun4i_irq_init [sun4i_gpadc_iio]) from [<bf1145cc>] (sun4i_gpadc_probe+0x234/0x308 [sun4i_gpadc_iio]) > > > [ 3.001152] [<bf1145cc>] (sun4i_gpadc_probe [sun4i_gpadc_iio]) from [<c0592edc>] (platform_drv_probe+0x48/0x98) > > > [ 3.010051] [<c0592edc>] (platform_drv_probe) from [<c0591074>] (really_probe+0x1e0/0x348) > > > [ 3.017152] [<c0591074>] (really_probe) from [<c05912e8>] (driver_probe_device+0x5c/0xb4) > > > [ 3.024081] [<c05912e8>] (driver_probe_device) from [<c05914e0>] (device_driver_attach+0x58/0x60) > > > [ 3.031696] [<c05914e0>] (device_driver_attach) from [<c0591540>] (__driver_attach+0x58/0xcc) > > > [ 3.038966] [<c0591540>] (__driver_attach) from [<c058f418>] (bus_for_each_dev+0x64/0x90) > > > [ 3.045886] [<c058f418>] (bus_for_each_dev) from [<c0590490>] (bus_add_driver+0x15c/0x1e0) > > > [ 3.052892] [<c0590490>] (bus_add_driver) from [<c0591dc4>] (driver_register+0x7c/0x114) > > > [ 3.059731] [<c0591dc4>] (driver_register) from [<c0101674>] (do_one_initcall+0x44/0x194) > > > [ 3.066696] [<c0101674>] (do_one_initcall) from [<c01946e8>] (do_init_module+0x5c/0x220) > > > [ 3.073568] [<c01946e8>] (do_init_module) from [<c0196a3c>] (load_module+0x20ec/0x2380) > > > [ 3.080340] [<c0196a3c>] (load_module) from [<c0196e04>] (sys_init_module+0x134/0x154) > > > [ 3.087020] [<c0196e04>] (sys_init_module) from [<c0100060>] (ret_fast_syscall+0x0/0x54) > > > [ 3.093852] Exception stack(0xc8ea7fa8 to 0xc8ea7ff0) > > > [ 3.097649] 7fa0: 00000000 00000000 b6fe2000 00003b14 00031284 00000005 > > > [ 3.104596] 7fc0: 00000000 00000000 b6fe2000 00000080 00052220 be9fd940 00052246 0000002e > > > [ 3.111626] 7fe0: be9fd79c be9fd780 00015bd0 0001dcf4 > > > [ 3.115468] ---[ end trace df4dd47fb61bf5a4 ]--- > > > > In which situation? > > During boot. > > It's a new check added in Linux 5.8. https://elixir.bootlin.com/linux/latest/source/drivers/base/platform.c#L317 > > > > Signed-off-by: Ondrej Jirman <megous@megous.com> > > > --- > > > include/linux/mfd/sun4i-gpadc.h | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h > > > index ea0ccf33a459..021f820f9d52 100644 > > > --- a/include/linux/mfd/sun4i-gpadc.h > > > +++ b/include/linux/mfd/sun4i-gpadc.h > > > @@ -81,8 +81,8 @@ > > > #define SUN4I_GPADC_TEMP_DATA 0x20 > > > #define SUN4I_GPADC_DATA 0x24 > > > > > > -#define SUN4I_GPADC_IRQ_FIFO_DATA 0 > > > -#define SUN4I_GPADC_IRQ_TEMP_DATA 1 > > > +#define SUN4I_GPADC_IRQ_FIFO_DATA 1 > > > +#define SUN4I_GPADC_IRQ_TEMP_DATA 2 > > > > Where is it coming from, and why is it the proper fix? > > Actual numbers seem irrelevant. It's just an index into this array: > > https://elixir.bootlin.com/linux/latest/source/drivers/mfd/sun4i-gpadc.c#L27 > > The array will get sparse, but that doesn't seem like an issue to me, > because the irq code handles the holes in the list if mask is 0. > > Not sure it's the best fix. Explaining all that in the commit log would be great, especially if you're not sure. Maxime
diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h index ea0ccf33a459..021f820f9d52 100644 --- a/include/linux/mfd/sun4i-gpadc.h +++ b/include/linux/mfd/sun4i-gpadc.h @@ -81,8 +81,8 @@ #define SUN4I_GPADC_TEMP_DATA 0x20 #define SUN4I_GPADC_DATA 0x24 -#define SUN4I_GPADC_IRQ_FIFO_DATA 0 -#define SUN4I_GPADC_IRQ_TEMP_DATA 1 +#define SUN4I_GPADC_IRQ_FIFO_DATA 1 +#define SUN4I_GPADC_IRQ_TEMP_DATA 2 /* 10s delay before suspending the IP */ #define SUN4I_GPADC_AUTOSUSPEND_DELAY 10000
This avoids a warning: [ 2.891592] ------------[ cut here ]------------ [ 2.895052] WARNING: CPU: 0 PID: 149 at drivers/base/platform.c:317 __platform_get_irq_byname+0x7c/0x8c [ 2.903212] usb 1-1: new high-speed USB device number 2 using ehci-platform [ 2.908930] 0 is an invalid IRQ number [ 2.911425] Modules linked in: sun4i_gpadc_iio(+) r8188eu(C) lib80211 ohci_platform ohci_hcd ehci_platform ehci_hcd cyttsp4_i2c cyttsp_i2c_common cyttsp4_core g_cdc usb_f_acm u_serial usb_f_ecm u_ether libcomposite sunxi phy_generic musb_hdrc udc_core usbcore sun5ieink [ 2.934048] CPU: 0 PID: 149 Comm: tablet-init Tainted: G C 5.8.0-rc2-00316-gc6a5213fdeba-dirty #8 [ 2.943027] Hardware name: Allwinner sun4i/sun5i Families [ 2.947204] [<c010c080>] (unwind_backtrace) from [<c0109ee4>] (show_stack+0x10/0x14) [ 2.953714] [<c0109ee4>] (show_stack) from [<c012cd04>] (__warn+0xc0/0xd8) [ 2.959364] [<c012cd04>] (__warn) from [<c012cda0>] (warn_slowpath_fmt+0x84/0x94) [ 2.965599] [<c012cda0>] (warn_slowpath_fmt) from [<c0592a60>] (__platform_get_irq_byname+0x7c/0x8c) [ 2.973480] [<c0592a60>] (__platform_get_irq_byname) from [<c0592a80>] (platform_get_irq_byname+0x10/0x48) [ 2.981896] [<c0592a80>] (platform_get_irq_byname) from [<bf1142f0>] (sun4i_irq_init+0x38/0xe0 [sun4i_gpadc_iio]) [ 2.990923] [<bf1142f0>] (sun4i_irq_init [sun4i_gpadc_iio]) from [<bf1145cc>] (sun4i_gpadc_probe+0x234/0x308 [sun4i_gpadc_iio]) [ 3.001152] [<bf1145cc>] (sun4i_gpadc_probe [sun4i_gpadc_iio]) from [<c0592edc>] (platform_drv_probe+0x48/0x98) [ 3.010051] [<c0592edc>] (platform_drv_probe) from [<c0591074>] (really_probe+0x1e0/0x348) [ 3.017152] [<c0591074>] (really_probe) from [<c05912e8>] (driver_probe_device+0x5c/0xb4) [ 3.024081] [<c05912e8>] (driver_probe_device) from [<c05914e0>] (device_driver_attach+0x58/0x60) [ 3.031696] [<c05914e0>] (device_driver_attach) from [<c0591540>] (__driver_attach+0x58/0xcc) [ 3.038966] [<c0591540>] (__driver_attach) from [<c058f418>] (bus_for_each_dev+0x64/0x90) [ 3.045886] [<c058f418>] (bus_for_each_dev) from [<c0590490>] (bus_add_driver+0x15c/0x1e0) [ 3.052892] [<c0590490>] (bus_add_driver) from [<c0591dc4>] (driver_register+0x7c/0x114) [ 3.059731] [<c0591dc4>] (driver_register) from [<c0101674>] (do_one_initcall+0x44/0x194) [ 3.066696] [<c0101674>] (do_one_initcall) from [<c01946e8>] (do_init_module+0x5c/0x220) [ 3.073568] [<c01946e8>] (do_init_module) from [<c0196a3c>] (load_module+0x20ec/0x2380) [ 3.080340] [<c0196a3c>] (load_module) from [<c0196e04>] (sys_init_module+0x134/0x154) [ 3.087020] [<c0196e04>] (sys_init_module) from [<c0100060>] (ret_fast_syscall+0x0/0x54) [ 3.093852] Exception stack(0xc8ea7fa8 to 0xc8ea7ff0) [ 3.097649] 7fa0: 00000000 00000000 b6fe2000 00003b14 00031284 00000005 [ 3.104596] 7fc0: 00000000 00000000 b6fe2000 00000080 00052220 be9fd940 00052246 0000002e [ 3.111626] 7fe0: be9fd79c be9fd780 00015bd0 0001dcf4 [ 3.115468] ---[ end trace df4dd47fb61bf5a4 ]--- Signed-off-by: Ondrej Jirman <megous@megous.com> --- include/linux/mfd/sun4i-gpadc.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)