diff mbox series

[RFC,02/13] mmc: meson-gx: fix deferred probing

Message ID 20211223171202.8224-3-s.shtylyov@omp.ru (mailing list archive)
State New, archived
Headers show
Series [RFC,01/13] mmc: bcm2835: fix deferred probing | expand

Commit Message

Sergey Shtylyov Dec. 23, 2021, 5:11 p.m. UTC
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(-)

Comments

Martin Blumenstingl Dec. 24, 2021, 5:35 p.m. UTC | #1
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
Sergey Shtylyov Dec. 24, 2021, 5:37 p.m. UTC | #2
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
Christian Hewitt April 1, 2023, 6:38 a.m. UTC | #3
> 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
Sergey Shtylyov April 2, 2023, 5:58 p.m. UTC | #4
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
Sergey Shtylyov June 8, 2023, 8:07 p.m. UTC | #5
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 mbox series

Patch

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;
 	}