| Message ID | 65f9787ebd6725e90fad9ea7f936ee0d5712da94.1626429286.git.matthias.schiffer@ew.tq-group.com |
|---|---|
| State | Accepted |
| Commit | a946506c48f3bd09363c9d2b0a178e55733bcbb6 |
| Headers | show |
| Series | TQMx86: TQMx110EB and TQMxE40x MFD/GPIO support | expand |
On Fri, 16 Jul 2021, Matthias Schiffer wrote: > The driver was registering IRQ 0 when no IRQ was set. This leads to > warnings with newer kernels. > > Clear the resource flags, so no resource is registered at all in this > case. > > Fixes: 2f17dd34ffed ("mfd: tqmx86: IO controller with I2C, Wachdog and GPIO") > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > --- > > v2: add Fixes line > v3: no changes > > drivers/mfd/tqmx86.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/mfd/tqmx86.c b/drivers/mfd/tqmx86.c > index ddddf08b6a4c..732013f40e4e 100644 > --- a/drivers/mfd/tqmx86.c > +++ b/drivers/mfd/tqmx86.c > @@ -209,6 +209,8 @@ static int tqmx86_probe(struct platform_device *pdev) > > /* Assumes the IRQ resource is first. */ > tqmx_gpio_resources[0].start = gpio_irq; > + } else { > + tqmx_gpio_resources[0].flags = 0; Strange - why is this !0 in the first place? > } > > ocores_platfom_data.clock_khz = tqmx86_board_id_to_clk_rate(board_id); -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
On Thu, 2021-08-05 at 14:30 +0100, Lee Jones wrote: > On Fri, 16 Jul 2021, Matthias Schiffer wrote: > > > The driver was registering IRQ 0 when no IRQ was set. This leads to > > warnings with newer kernels. > > > > Clear the resource flags, so no resource is registered at all in this > > case. > > > > Fixes: 2f17dd34ffed ("mfd: tqmx86: IO controller with I2C, Wachdog and GPIO") > > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > --- > > > > v2: add Fixes line > > v3: no changes > > > > drivers/mfd/tqmx86.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/mfd/tqmx86.c b/drivers/mfd/tqmx86.c > > index ddddf08b6a4c..732013f40e4e 100644 > > --- a/drivers/mfd/tqmx86.c > > +++ b/drivers/mfd/tqmx86.c > > @@ -209,6 +209,8 @@ static int tqmx86_probe(struct platform_device *pdev) > > > > /* Assumes the IRQ resource is first. */ > > tqmx_gpio_resources[0].start = gpio_irq; > > + } else { > > + tqmx_gpio_resources[0].flags = 0; > > Strange - why is this !0 in the first place? I don't see anything strange here. DEFINE_RES_IRQ() sets flags to IORESOURCE_IRQ. We reset it to 0 when there is no IRQ to clear that resource entry. An alternative would be to start with an empty entry and only fill in the fields when an IRQ is used, but that seems more cumbersome than the current code to me. > > > } > > > > ocores_platfom_data.clock_khz = tqmx86_board_id_to_clk_rate(board_id); > >
On Thu, Aug 5, 2021 at 4:40 PM Matthias Schiffer <matthias.schiffer@ew.tq-group.com> wrote: > On Thu, 2021-08-05 at 14:30 +0100, Lee Jones wrote: > > On Fri, 16 Jul 2021, Matthias Schiffer wrote: ... > > Strange - why is this !0 in the first place? > > I don't see anything strange here. DEFINE_RES_IRQ() sets flags to > IORESOURCE_IRQ. We reset it to 0 when there is no IRQ to clear that > resource entry. > > An alternative would be to start with an empty entry and only fill in > the fields when an IRQ is used, but that seems more cumbersome than the > current code to me. Another alternative is to start using the IRQ DISABLED resource flag, but I'm afraid that OF code is not ready for that. https://elixir.bootlin.com/linux/latest/source/include/linux/ioport.h#L331 -- With Best Regards, Andy Shevchenko
On Thu, 2021-08-05 at 16:42 +0300, Andy Shevchenko wrote: > On Thu, Aug 5, 2021 at 4:40 PM Matthias Schiffer > <matthias.schiffer@ew.tq-group.com> wrote: > > On Thu, 2021-08-05 at 14:30 +0100, Lee Jones wrote: > > > On Fri, 16 Jul 2021, Matthias Schiffer wrote: > > ... > > > > Strange - why is this !0 in the first place? > > > > I don't see anything strange here. DEFINE_RES_IRQ() sets flags to > > IORESOURCE_IRQ. We reset it to 0 when there is no IRQ to clear that > > resource entry. > > > > An alternative would be to start with an empty entry and only fill in > > the fields when an IRQ is used, but that seems more cumbersome than the > > current code to me. > > Another alternative is to start using the IRQ DISABLED resource flag, > but I'm afraid that OF code is not ready for that. > https://elixir.bootlin.com/linux/latest/source/include/linux/ioport.h#L331 > As this patch is a fairly simple bugfix, I'd prefer to get it (or a similar fix) applied without having to wait for improvements of the core code - also for the sake of stable backports.
diff --git a/drivers/mfd/tqmx86.c b/drivers/mfd/tqmx86.c index ddddf08b6a4c..732013f40e4e 100644 --- a/drivers/mfd/tqmx86.c +++ b/drivers/mfd/tqmx86.c @@ -209,6 +209,8 @@ static int tqmx86_probe(struct platform_device *pdev) /* Assumes the IRQ resource is first. */ tqmx_gpio_resources[0].start = gpio_irq; + } else { + tqmx_gpio_resources[0].flags = 0; } ocores_platfom_data.clock_khz = tqmx86_board_id_to_clk_rate(board_id);
The driver was registering IRQ 0 when no IRQ was set. This leads to warnings with newer kernels. Clear the resource flags, so no resource is registered at all in this case. Fixes: 2f17dd34ffed ("mfd: tqmx86: IO controller with I2C, Wachdog and GPIO") Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> --- v2: add Fixes line v3: no changes drivers/mfd/tqmx86.c | 2 ++ 1 file changed, 2 insertions(+)