Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1678490 > unrolled thread

[PATCH] sata_rcar: fix error return code in sata_rcar_probe()

Back to article view | Back to linux.kernel

Contents

  [PATCH] sata_rcar: fix error return code in sata_rcar_probe() "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-06-30 07:50 +0200
    Re: [PATCH] sata_rcar: fix error return code in sata_rcar_probe() Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-06-30 11:50 +0200
      Re: [PATCH] sata_rcar: fix error return code in sata_rcar_probe() Tejun Heo <tj@kernel.org> - 2017-06-30 14:10 +0200
        Re: [PATCH] sata_rcar: fix error return code in sata_rcar_probe() "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-06-30 21:40 +0200
          Re: [PATCH] sata_rcar: fix error return code in sata_rcar_probe() "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-06-30 21:50 +0200
            Re: [PATCH] sata_rcar: fix error return code in sata_rcar_probe() Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-06-30 22:10 +0200
              Re: [PATCH] sata_rcar: fix error return code in sata_rcar_probe() "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-06-30 22:40 +0200
                [PATCH v2] sata_rcar: fix error return code in sata_rcar_probe() "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-06-30 23:10 +0200
          Re: [PATCH] sata_rcar: fix error return code in sata_rcar_probe() Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-06-30 21:50 +0200
            Re: [PATCH] sata_rcar: fix error return code in sata_rcar_probe() "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-06-30 22:00 +0200
      Re: [PATCH] sata_rcar: fix error return code in sata_rcar_probe() Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-06-30 14:40 +0200

#1678490 — [PATCH] sata_rcar: fix error return code in sata_rcar_probe()

Print error message and propagate the return value of
platform_get_irq on failure.

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/ata/sata_rcar.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index ee98447..c936b2a 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -872,8 +872,10 @@ static int sata_rcar_probe(struct platform_device *pdev)
 	int ret = 0;
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0)
-		return -EINVAL;
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get IRQ\n");
+		return irq;
+	}
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(struct sata_rcar_priv),
 			   GFP_KERNEL);
-- 
2.5.0

[toc] | [next] | [standalone]

#1678636

Hello!

On 6/30/2017 8:22 AM, Gustavo A. R. Silva wrote:

> Print error message and propagate the return value of
> platform_get_irq on failure.

    You should have probably mentioned that this function no longer returns 0 
on error.

> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1678734

On Fri, Jun 30, 2017 at 12:42:38PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 6/30/2017 8:22 AM, Gustavo A. R. Silva wrote:
> 
> > Print error message and propagate the return value of
> > platform_get_irq on failure.
> 
>    You should have probably mentioned that this function no longer returns 0
> on error.

Yeah, the patches looks good to me but I'd really appreciate more
context in the changelogs.  Gustavo, can you please respin the
patches?

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1679083

Hi Tejun, Sergei,

Quoting Tejun Heo <tj@kernel.org>:

> On Fri, Jun 30, 2017 at 12:42:38PM +0300, Sergei Shtylyov wrote:
>> Hello!
>>
>> On 6/30/2017 8:22 AM, Gustavo A. R. Silva wrote:
>>
>> > Print error message and propagate the return value of
>> > platform_get_irq on failure.
>>
>>    You should have probably mentioned that this function no longer returns 0
>> on error.
>
> Yeah, the patches looks good to me but I'd really appreciate more
> context in the changelogs.  Gustavo, can you please respin the
> patches?
>

Absolutely.

What do you think about the following changelog:

platform_get_irq() returns an error code, but the sata_rcar driver
ignores it and always returns -EINVAL. This is not correct, and
prevents -EPROBE_DEFER from being propagated properly.

Print error message and propagate the return value of platform_get_irq
on failure. Also, with this change function sata_rcar_probe() no longer
returns 0 on error.


Thank you!
--
Gustavo A. R. Silva

[toc] | [prev] | [next] | [standalone]

#1679087

Quoting "Gustavo A. R. Silva" <garsilva@embeddedor.com>:

> Hi Tejun, Sergei,
>
> Quoting Tejun Heo <tj@kernel.org>:
>
>> On Fri, Jun 30, 2017 at 12:42:38PM +0300, Sergei Shtylyov wrote:
>>> Hello!
>>>
>>> On 6/30/2017 8:22 AM, Gustavo A. R. Silva wrote:
>>>
>>>> Print error message and propagate the return value of
>>>> platform_get_irq on failure.
>>>
>>>   You should have probably mentioned that this function no longer returns 0
>>> on error.
>>
>> Yeah, the patches looks good to me but I'd really appreciate more
>> context in the changelogs.  Gustavo, can you please respin the
>> patches?
>>
>
> Absolutely.
>
> What do you think about the following changelog:
>
> platform_get_irq() returns an error code, but the sata_rcar driver
> ignores it and always returns -EINVAL. This is not correct, and
> prevents -EPROBE_DEFER from being propagated properly.
>
> Print error message and propagate the return value of platform_get_irq
> on failure. Also, with this change function sata_rcar_probe() no longer
> returns 0 on error.
>

Errata, this would be final the chagelog:

platform_get_irq() returns an error code, but the sata_rcar driver
ignores it and always returns -EINVAL. This is not correct, and
prevents -EPROBE_DEFER from being propagated properly. Also,
notice that platform_get_irq() no longer returns 0 on error.

Print error message and propagate the return value of platform_get_irq
on failure.

Thanks
--
Gustavo A. R. Silva

[toc] | [prev] | [next] | [standalone]

#1679109

On 06/30/2017 10:46 PM, Gustavo A. R. Silva wrote:

>>>>> Print error message and propagate the return value of
>>>>> platform_get_irq on failure.
>>>>
>>>>   You should have probably mentioned that this function no longer returns 0
>>>> on error.
>>>
>>> Yeah, the patches looks good to me but I'd really appreciate more
>>> context in the changelogs.  Gustavo, can you please respin the
>>> patches?
>>>
>>
>> Absolutely.
>>
>> What do you think about the following changelog:
>>
>> platform_get_irq() returns an error code, but the sata_rcar driver
>> ignores it and always returns -EINVAL. This is not correct, and
>> prevents -EPROBE_DEFER from being propagated properly.
>>
>> Print error message and propagate the return value of platform_get_irq
>> on failure. Also, with this change function sata_rcar_probe() no longer
>> returns 0 on error.
>>
>
> Errata, this would be final the chagelog:
>
> platform_get_irq() returns an error code, but the sata_rcar driver
> ignores it and always returns -EINVAL. This is not correct, and
> prevents -EPROBE_DEFER from being propagated properly. Also,
> notice that platform_get_irq() no longer returns 0 on error.
>
> Print error message and propagate the return value of platform_get_irq
> on failure.

    Now I'm OK with that.

> Thanks

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1679118

Quoting Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:

> On 06/30/2017 10:46 PM, Gustavo A. R. Silva wrote:
>
>>>>>> Print error message and propagate the return value of
>>>>>> platform_get_irq on failure.
>>>>>
>>>>>  You should have probably mentioned that this function no longer  
>>>>> returns 0
>>>>> on error.
>>>>
>>>> Yeah, the patches looks good to me but I'd really appreciate more
>>>> context in the changelogs.  Gustavo, can you please respin the
>>>> patches?
>>>>
>>>
>>> Absolutely.
>>>
>>> What do you think about the following changelog:
>>>
>>> platform_get_irq() returns an error code, but the sata_rcar driver
>>> ignores it and always returns -EINVAL. This is not correct, and
>>> prevents -EPROBE_DEFER from being propagated properly.
>>>
>>> Print error message and propagate the return value of platform_get_irq
>>> on failure. Also, with this change function sata_rcar_probe() no longer
>>> returns 0 on error.
>>>
>>
>> Errata, this would be final the chagelog:
>>
>> platform_get_irq() returns an error code, but the sata_rcar driver
>> ignores it and always returns -EINVAL. This is not correct, and
>> prevents -EPROBE_DEFER from being propagated properly. Also,
>> notice that platform_get_irq() no longer returns 0 on error.
>>
>> Print error message and propagate the return value of platform_get_irq
>> on failure.
>
>    Now I'm OK with that.
>

Great :)

Thank you
--
Gustavo A. R. Silva

[toc] | [prev] | [next] | [standalone]

#1679131 — [PATCH v2] sata_rcar: fix error return code in sata_rcar_probe()

platform_get_irq() returns an error code, but the sata_rcar driver
ignores it and always returns -EINVAL. This is not correct, and
prevents -EPROBE_DEFER from being propagated properly. Also,
notice that platform_get_irq() no longer returns 0 on error.

Print error message and propagate the return value of platform_get_irq
on failure.

Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
Changes in v2:
 Rewrite commit message.

 drivers/ata/sata_rcar.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index ee98447..769bfdd 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -872,8 +872,10 @@ static int sata_rcar_probe(struct platform_device *pdev)
 	int ret = 0;
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0)
-		return -EINVAL;
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
+		return irq;
+	}
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(struct sata_rcar_priv),
 			   GFP_KERNEL);
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1679089

On 06/30/2017 10:36 PM, Gustavo A. R. Silva wrote:

>>> > Print error message and propagate the return value of
>>> > platform_get_irq on failure.
>>>
>>>    You should have probably mentioned that this function no longer returns 0
>>> on error.
>>
>> Yeah, the patches looks good to me but I'd really appreciate more
>> context in the changelogs.  Gustavo, can you please respin the
>> patches?
>>
>
> Absolutely.
>
> What do you think about the following changelog:
>
> platform_get_irq() returns an error code, but the sata_rcar driver
> ignores it and always returns -EINVAL. This is not correct, and

    This *was* correct, because it prevented returning 0 on error.

> prevents -EPROBE_DEFER from being propagated properly.

    Yes, this is a real problem.

> Print error message and propagate the return value of platform_get_irq
> on failure. Also, with this change function sata_rcar_probe() no longer
> returns 0 on error.

    It never did -- I was talking about platform_get_irq() which might return 
0 on error until I fixed it:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af

> Thank you!

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1679102

Hi Sergei,

Quoting Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:

> On 06/30/2017 10:36 PM, Gustavo A. R. Silva wrote:
>
>>>>> Print error message and propagate the return value of
>>>>> platform_get_irq on failure.
>>>>
>>>>   You should have probably mentioned that this function no longer  
>>>> returns 0
>>>> on error.
>>>
>>> Yeah, the patches looks good to me but I'd really appreciate more
>>> context in the changelogs.  Gustavo, can you please respin the
>>> patches?
>>>
>>
>> Absolutely.
>>
>> What do you think about the following changelog:
>>
>> platform_get_irq() returns an error code, but the sata_rcar driver
>> ignores it and always returns -EINVAL. This is not correct, and
>
>    This *was* correct, because it prevented returning 0 on error.
>

Yeah, I got it.

>> prevents -EPROBE_DEFER from being propagated properly.
>
>    Yes, this is a real problem.
>
>> Print error message and propagate the return value of platform_get_irq
>> on failure. Also, with this change function sata_rcar_probe() no longer
>> returns 0 on error.
>
>    It never did -- I was talking about platform_get_irq() which  
> might return 0 on error until I fixed it:
>

Yep, I sent a new email immediately after I realized this was  
incorrect. Please,
check it out.

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af
>

Great work!

Thank you
--
Gustavo A. R. Silva

[toc] | [prev] | [next] | [standalone]

#1678768

On 06/30/2017 12:42 PM, Sergei Shtylyov wrote:

>> Print error message and propagate the return value of
>> platform_get_irq on failure.
>
>    You should have probably mentioned that this function no longer returns 0
> on error.

    It's prolly also worth mentioning that enforcing the error # on return 
from probe defeats the deferred probing.

MBR, Sergei

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web