sata_rcar: fix error return code in sata_rcar_probe()

PATCH Superseded Gustavo A. R. Silva 3 participants 13 messages Jun 30, 2017

Subsystems (3)

libata Arm/RISC-V/Renesas architecture Renesas R-Car SATA driver

lore

Share & tools

lore

Gustavo A. R. Silva

author

2017-06-30 05:44 UTC #0 lore raw

PATCH 1/1 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 <[email protected]>

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

drivers/ata/sata_rcar.c +4 -2

index ee98447..c936b2a 100644
@@ -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);

Signature · 1 line

2.5.0

Sergei Shtylyov

Acked-by on v1 1/1

2017-06-30 09:42 UTC #1 lore raw

Hello!

Quoted 2 lines · Gustavo A. R. Silva

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.

Quoted 1 line

Signed-off-by Gustavo A. R. Silva <[email protected]>

Acked-by Sergei Shtylyov <[email protected]>

MBR, Sergei

Tejun Heo

2017-06-30 12:01 UTC #2 lore raw

Quoted 9 lines · Sergei Shtylyov

Hello!
Quoted 2 lines · Gustavo A. R. Silva
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.

Signature · 1 line

tejun

Gustavo A. R. Silva

author

2017-06-30 19:36 UTC #3 lore raw

Hi Tejun, Sergei,

Quoting Tejun Heo <[email protected]>:

Quoted 8 lines · Sergei Shtylyov

Quoted 7 lines
Hello!
Quoted 2 lines · Gustavo A. R. Silva
Print error message and propagate the return value of platform_get_irq on failure.

Quoted 7 lines

Quoted 2 lines
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

Sergei Shtylyov

2017-06-30 19:42 UTC #4 lore raw

Quoted 11 lines · Gustavo A. R. Silva

Quoted 10 lines
Quoted 5 lines
Quoted 2 lines
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?

Quoted 6 lines

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.

Quoted 1 line

prevents -EPROBE_DEFER from being propagated properly.

Yes, this is a real problem.

Quoted 3 lines

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

Quoted 1 line

Thank you!

MBR, Sergei

Gustavo A. R. Silva

author

2017-06-30 19:54 UTC #5 lore raw

Hi Sergei,

Quoting Sergei Shtylyov <[email protected]>:

Quoted 14 lines · Gustavo A. R. Silva

Quoted 13 lines
Quoted 12 lines
Quoted 7 lines
Quoted 3 lines
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?

Quoted 9 lines

Quoted 6 lines
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.

Quoted 11 lines

Quoted 1 line
prevents -EPROBE_DEFER from being propagated properly.
Yes, this is a real problem.
Quoted 3 lines
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.

Quoted 2 lines

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

Great work!

Thank you
--
Gustavo A. R. Silva

Gustavo A. R. Silva

author

2017-06-30 19:46 UTC #6 lore raw

In reply to Gustavo A. R. Silva

Quoting "Gustavo A. R. Silva" <[email protected]>:

Quoted 24 lines

Hi Tejun, Sergei,
Quoting Tejun Heo <[email protected]>:
Quoted 8 lines · Sergei Shtylyov
Quoted 7 lines
Hello!
Quoted 2 lines · Gustavo A. R. Silva
Print error message and propagate the return value of platform_get_irq on failure.
Quoted 7 lines
Quoted 2 lines
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:

Quoted 8 lines

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

Sergei Shtylyov

2017-06-30 20:03 UTC #7 lore raw

Quoted 26 lines · Gustavo A. R. Silva

Quoted 15 lines
Quoted 10 lines
Quoted 5 lines
Quoted 2 lines
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:
Quoted 8 lines
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:

Quoted 7 lines

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.

Quoted 1 line

Thanks

MBR, Sergei

Gustavo A. R. Silva

author

2017-06-30 20:34 UTC #8 lore raw

Quoting Sergei Shtylyov <[email protected]>:

Quoted 34 lines · Gustavo A. R. Silva

Quoted 33 lines
Quoted 17 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Print error message and propagate the return value of platform_get_irq on failure.
Quoted 8 lines
Quoted 3 lines
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:
Quoted 8 lines
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.

Quoted 5 lines

Quoted 2 lines
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

Gustavo A. R. Silva

author

2017-06-30 21:08 UTC #9 lore raw

PATCH v2 1/1 sata_rcar: fix error return code in sata_rcar_probe()

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(-)

drivers/ata/sata_rcar.c +4 -2

index ee98447..769bfdd 100644
@@ -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);

Signature · 1 line

2.5.0

Sergei Shtylyov

Acked-by on v2 1/1

2017-06-30 21:09 UTC #10 lore raw

Quoted 10 lines · 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]>

You forgot to collect:

Acked-by Sergei Shtylyov <[email protected]>

MBR, Sergei

Gustavo A. R. Silva

author

2017-06-30 21:11 UTC #11 lore raw

Quoting Sergei Shtylyov <[email protected]>:

Quoted 10 lines · Gustavo A. R. Silva

Quoted 9 lines
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.

Quoted 7 lines

Quoted 2 lines
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

Sergei Shtylyov

2017-06-30 12:35 UTC #12 lore raw

In reply to Sergei Shtylyov

Quoted 5 lines · Sergei Shtylyov

Quoted 2 lines
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