Print error message and propagate the return value of
platform_get_irq on failure.
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
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
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 <[email protected]>
Acked-by: Sergei Shtylyov <[email protected]>
MBR, Sergei
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
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
Hi Tejun, Sergei,
Quoting Tejun Heo <[email protected]>:
> 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
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
Quoting "Gustavo A. R. Silva" <[email protected]>:
> Hi Tejun, Sergei,
>
> Quoting Tejun Heo <[email protected]>:
>
>> 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
Hi Sergei,
Quoting Sergei Shtylyov <[email protected]>:
> 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
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
Quoting Sergei Shtylyov <[email protected]>:
> 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
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 <[email protected]>
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
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
On 07/01/2017 12:08 AM, Gustavo A. R. Silva wrote:
> 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 <[email protected]>
> Signed-off-by: Gustavo A. R. Silva <[email protected]>
You forgot to collect:
Acked-by: Sergei Shtylyov <[email protected]>
MBR, Sergei
Quoting Sergei Shtylyov <[email protected]>:
> On 07/01/2017 12:08 AM, Gustavo A. R. Silva wrote:
>
>> 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 <[email protected]>
>> Signed-off-by: Gustavo A. R. Silva <[email protected]>
>
> You forgot to collect:
>
> Acked-by: Sergei Shtylyov <[email protected]>
>
Oh I'm sorry. Should I send v3?
Thanks
--
Gustavo A. R. Silva