| Message ID | 20211223171202.8224-3-s.shtylyov@omp.ru |
|---|---|
| State | New |
| Headers | show |
| Series | Fix deferred probing in the MMC/SD drivers | expand |
Hi Sergey, thank you for spotting and fixing this! On Thu, Dec 23, 2021 at 6:12 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > > The driver overrides the error codes and IRQ0 returned by platform_get_irq() > to -EINVAL, so if it returns -EPROBE_DEFER, the driver will fail the probe > permanently instead of the deferred probing. Switch to propagating the error > codes upstream. IRQ0 is no longer returned by platform_get_irq(), so we now > can safely ignore it... > > Fixes: cbcaac6d7dd2 ("mmc: meson-gx-mmc: Fix platform_get_irq's error checking > ") I suggest putting the ") on the previous line. Most "Fixes" tag I have seen don't use any line-break at all, even if the line gets long. > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> with above comment addressed you can add my: Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Best regards, Martin
On 12/24/21 8:35 PM, Martin Blumenstingl wrote: >> The driver overrides the error codes and IRQ0 returned by platform_get_irq() >> to -EINVAL, so if it returns -EPROBE_DEFER, the driver will fail the probe >> permanently instead of the deferred probing. Switch to propagating the error >> codes upstream. IRQ0 is no longer returned by platform_get_irq(), so we now >> can safely ignore it... >> >> Fixes: cbcaac6d7dd2 ("mmc: meson-gx-mmc: Fix platform_get_irq's error checking >> ") > I suggest putting the ") on the previous line. Most "Fixes" tag I have > seen don't use any line-break at all, even if the line gets long. Sorry, was a cut & paste artifact that I didn't notice... :-/ >> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> > with above comment addressed you can add my: > Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Thank you! > Best regards, > Martin > MBR, Sergey
> On 23 Dec 2021, at 9:11 pm, Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > > The driver overrides the error codes and IRQ0 returned by platform_get_irq() > to -EINVAL, so if it returns -EPROBE_DEFER, the driver will fail the probe > permanently instead of the deferred probing. Switch to propagating the error > codes upstream. IRQ0 is no longer returned by platform_get_irq(), so we now > can safely ignore it... > > Fixes: cbcaac6d7dd2 ("mmc: meson-gx-mmc: Fix platform_get_irq's error checking > ") > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> > Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > drivers/mmc/host/meson-gx-mmc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c > index 8f36536cb1b6..c765653ee4d0 100644 > --- a/drivers/mmc/host/meson-gx-mmc.c > +++ b/drivers/mmc/host/meson-gx-mmc.c > @@ -1182,8 +1182,8 @@ static int meson_mmc_probe(struct platform_device *pdev) > } > > host->irq = platform_get_irq(pdev, 0); > - if (host->irq <= 0) { > - ret = -EINVAL; > + if (host->irq < 0) { > + ret = host->irq; > goto free_host; > } > Can I ask if this patch/series [0] has been superseded or forgotten or ?? The series it depended upon [1] appears to have been merged a year ago as I can see ce753ad1549c ("platform: finally disallow IRQ0 in platform_get_irq() and its ilk”) in upstream code. I’ve had this patch in my testing kernel for 12+ months now with no observable negative impacts so am wondering if it can be resent and merged or I should drop the patch from my tree? Thx. Christian [0] https://www.spinics.net/lists/linux-mmc/msg68102.html [1] https://marc.info/?l=linux-kernel&m=163623041902285
Hello! On 4/1/23 9:38 AM, Christian Hewitt wrote: [...] >> The driver overrides the error codes and IRQ0 returned by platform_get_irq() >> to -EINVAL, so if it returns -EPROBE_DEFER, the driver will fail the probe >> permanently instead of the deferred probing. Switch to propagating the error >> codes upstream. IRQ0 is no longer returned by platform_get_irq(), so we now >> can safely ignore it... >> >> Fixes: cbcaac6d7dd2 ("mmc: meson-gx-mmc: Fix platform_get_irq's error checking >> ") >> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> >> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >> --- >> drivers/mmc/host/meson-gx-mmc.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c >> index 8f36536cb1b6..c765653ee4d0 100644 >> --- a/drivers/mmc/host/meson-gx-mmc.c >> +++ b/drivers/mmc/host/meson-gx-mmc.c >> @@ -1182,8 +1182,8 @@ static int meson_mmc_probe(struct platform_device *pdev) >> } >> >> host->irq = platform_get_irq(pdev, 0); >> - if (host->irq <= 0) { >> - ret = -EINVAL; >> + if (host->irq < 0) { >> + ret = host->irq; >> goto free_host; >> } > > Can I ask if this patch/series [0] has been superseded or forgotten or ?? I'm sorry, I actually did forget to re-post this series, as asked by Ulf. Partly because the patch that this series depended on took about 4 months to hit the kernel and partly because I suspended my platform_get_irq() related work as I got into PATA development and struggling with a static analyzer's reports... Also, the series doesn't seem superseded as all the patches still apply, sometimes with small offsets... > The series it depended upon [1] That was a single patch. :-) > appears to have been merged a year ago as I > can see ce753ad1549c ("platform: finally disallow IRQ0 in platform_get_irq() > and its ilk”) in upstream code. Not a whole year yet but 11 months surely... :-/ > I’ve had this patch in my testing kernel for > 12+ months now with no observable negative impacts so am wondering if it can > be resent and merged or I should drop the patch from my tree? It? You mean my fix patches, surely? Or you mean you care about patch #2 only? Anyway, I need to find the time to refresh/repost it... > Thx. Christian > > [0] https://www.spinics.net/lists/linux-mmc/msg68102.html > [1] https://marc.info/?l=linux-kernel&m=163623041902285 MBR, Sergey
Hello! On 4/2/23 8:58 PM, Sergey Shtylyov wrote: [...] >>> The driver overrides the error codes and IRQ0 returned by platform_get_irq() >>> to -EINVAL, so if it returns -EPROBE_DEFER, the driver will fail the probe >>> permanently instead of the deferred probing. Switch to propagating the error >>> codes upstream. IRQ0 is no longer returned by platform_get_irq(), so we now >>> can safely ignore it... >>> >>> Fixes: cbcaac6d7dd2 ("mmc: meson-gx-mmc: Fix platform_get_irq's error checking >>> ") >>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> >>> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >>> --- >>> drivers/mmc/host/meson-gx-mmc.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c >>> index 8f36536cb1b6..c765653ee4d0 100644 >>> --- a/drivers/mmc/host/meson-gx-mmc.c >>> +++ b/drivers/mmc/host/meson-gx-mmc.c >>> @@ -1182,8 +1182,8 @@ static int meson_mmc_probe(struct platform_device *pdev) >>> } >>> >>> host->irq = platform_get_irq(pdev, 0); >>> - if (host->irq <= 0) { >>> - ret = -EINVAL; >>> + if (host->irq < 0) { >>> + ret = host->irq; >>> goto free_host; >>> } >> >> Can I ask if this patch/series [0] has been superseded or forgotten or ?? > > I'm sorry, I actually did forget to re-post this series, as asked by Ulf. > Partly because the patch that this series depended on took about 4 months to > hit the kernel and partly because I suspended my platform_get_irq() related > work as I got into PATA development and struggling with a static analyzer's > reports... > Also, the series doesn't seem superseded as all the patches still apply, > sometimes with small offsets... > >> The series it depended upon [1] > > That was a single patch. :-) > >> appears to have been merged a year ago as I >> can see ce753ad1549c ("platform: finally disallow IRQ0 in platform_get_irq() >> and its ilk”) in upstream code. > > Not a whole year yet but 11 months surely... :-/ > >> I’ve had this patch in my testing kernel for >> 12+ months now with no observable negative impacts so am wondering if it can >> be resent and merged or I should drop the patch from my tree? > > It? You mean my fix patches, surely? Or you mean you care about patch #2 > only? Anyway, I need to find the time to refresh/repost it... Now that I'm on vacation, I have found the time -- see the v2 series here: https://lore.kernel.org/all/20230608194519.10665-1-s.shtylyov@omp.ru/ [...] MBR, Sergey
diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c index 8f36536cb1b6..c765653ee4d0 100644 --- a/drivers/mmc/host/meson-gx-mmc.c +++ b/drivers/mmc/host/meson-gx-mmc.c @@ -1182,8 +1182,8 @@ static int meson_mmc_probe(struct platform_device *pdev) } host->irq = platform_get_irq(pdev, 0); - if (host->irq <= 0) { - ret = -EINVAL; + if (host->irq < 0) { + ret = host->irq; goto free_host; }
The driver overrides the error codes and IRQ0 returned by platform_get_irq() to -EINVAL, so if it returns -EPROBE_DEFER, the driver will fail the probe permanently instead of the deferred probing. Switch to propagating the error codes upstream. IRQ0 is no longer returned by platform_get_irq(), so we now can safely ignore it... Fixes: cbcaac6d7dd2 ("mmc: meson-gx-mmc: Fix platform_get_irq's error checking ") Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> --- drivers/mmc/host/meson-gx-mmc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)