devfreq: tegra: fix error code in tegra_devfreq_probe()

PATCH Superseded Gustavo A. R. Silva 4 participants 9 messages Jun 30, 2017 – Jul 17, 2017

Subsystems (2)

Tegra architecture support Device frequency (devfreq)

lore

Share & tools

lore

Gustavo A. R. Silva

author

2017-06-30 07:22 UTC #0 lore raw

PATCH 1/1 devfreq: tegra: fix error code in tegra_devfreq_probe()

Print and propagate the return value of platform_get_irq on failure.

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

---
 drivers/devfreq/tegra-devfreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/devfreq/tegra-devfreq.c +3 -3

index 214fff9..ae71215 100644
@@ -688,9 +688,9 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	}
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
-		dev_err(&pdev->dev, "Failed to get IRQ\n");
-		return -ENODEV;
+	if (irq < 0) {
+		dev_err(&pdev->dev, "Failed to get IRQ: %d\n", irq);
+		return irq;
 	}
 
 	platform_set_drvdata(pdev, tegra);

Signature · 1 line

2.5.0

Chanwoo Choi

maintainer Reviewed-by on v1 1/1

2017-06-30 08:02 UTC #1 lore raw

Hi,

Quoted 15 lines · Gustavo A. R. Silva

Print and propagate the return value of platform_get_irq on failure.
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/devfreq/tegra-devfreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/devfreq/tegra-devfreq.c +0 -0
index 214fff9..ae71215 100644
@@ -688,9 +688,9 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	}
 

drivers/devfreq/tegra-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 696

Quoted 10 lines

drivers/devfreq/tegra-devfreq.c +3 -3

 	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
-		dev_err(&pdev->dev, "Failed to get IRQ\n");
-		return -ENODEV;
+	if (irq < 0) {
+		dev_err(&pdev->dev, "Failed to get IRQ: %d\n", irq);
+		return irq;
 	}
 
 	platform_set_drvdata(pdev, tegra);

Reviewed-by Chanwoo Choi <[email protected]>

Signature · 3 lines

Best Regards,
Chanwoo Choi
Samsung Electronics

MyungJoo Ham

maintainer Acked-by

2017-07-01 07:42 UTC #2 lore raw

Quoted 26 lines · Chanwoo Choi

Hi,

Quoted 23 lines · Gustavo A. R. Silva

Print and propagate the return value of platform_get_irq on failure.

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

---
 drivers/devfreq/tegra-devfreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/devfreq/tegra-devfreq.c +3 -3

index 214fff9..ae71215 100644
@@ -688,9 +688,9 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
      }
 
      irq = platform_get_irq(pdev, 0);
-     if (irq <= 0) {
-             dev_err(&pdev->dev, "Failed to get IRQ\n");
-             return -ENODEV;
+     if (irq < 0) {
+             dev_err(&pdev->dev, "Failed to get IRQ: %d\n", irq);
+             return irq;
      }

Quoted 4 lines

drivers/devfreq/tegra-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 696
Quoted 2 lines
drivers/devfreq/tegra-devfreq.c +0 -0
      platform_set_drvdata(pdev, tegra);
Reviewed-by Chanwoo Choi <[email protected]>

Acked-by MyungJoo Ham <[email protected]>

Quoted 5 lines

--
Best Regards,
Chanwoo Choi
Samsung Electronics

Signature · 2 lines

MyungJoo Ham, Ph.D.
S/W Center, Samsung Electronics

Jon Hunter

maintainer

2017-07-03 11:35 UTC #3 lore raw

In reply to Gustavo A. R. Silva

Quoted 12 lines · Gustavo A. R. Silva

Print and propagate the return value of platform_get_irq on failure.
Signed-off-by Gustavo A. R. Silva <garsilva-L1vi/[email protected]>
---
 drivers/devfreq/tegra-devfreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/devfreq/tegra-devfreq.c +0 -0
index 214fff9..ae71215 100644

drivers/devfreq/tegra-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 691

Quoted 7 lines

drivers/devfreq/tegra-devfreq.c +1 -3

@@ -688,9 +688,9 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	}
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
-		dev_err(&pdev->dev, "Failed to get IRQ\n");
-		return -ENODEV;
+	if (irq < 0) {

The changelog does not describe the above change and if/why this is ok. However, the original test looks fine to me and so I don't see a need to change this.

drivers/devfreq/tegra-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 693

Quoted 2 lines

drivers/devfreq/tegra-devfreq.c +2 -0

+		dev_err(&pdev->dev, "Failed to get IRQ: %d\n", irq);
+		return irq;

Why not have ...

		return irq ? irq : -ENODEV;

Cheers
Jon

Signature · 1 line

nvpublic

Gustavo A. R. Silva

author

2017-07-03 11:59 UTC #4 lore raw

Hi Jon,

Quoting Jon Hunter <[email protected]>:

Quoted 16 lines · Gustavo A. R. Silva

Quoted 15 lines
Print and propagate the return value of platform_get_irq on failure.
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/devfreq/tegra-devfreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/devfreq/tegra-devfreq.c +0 -0
index 214fff9..ae71215 100644
@@ -688,9 +688,9 @@ static int tegra_devfreq_probe(struct  
 platform_device *pdev)
 	}
 

Quoted 10 lines

drivers/devfreq/tegra-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 691
Quoted 5 lines
drivers/devfreq/tegra-devfreq.c +1 -3
 	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
-		dev_err(&pdev->dev, "Failed to get IRQ\n");
-		return -ENODEV;
+	if (irq < 0) {
The changelog does not describe the above change and if/why this is ok. However, the original test looks fine to me and so I don't see a need to change this.

You are right about the changelog. I think this one is much better:

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

Notice that platform_get_irq() no longer returns 0 on error:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af

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

What do you think?

Thank you!
--
Gustavo A. R. Silva

Jon Hunter

maintainer

2017-07-03 12:22 UTC #5 lore raw

Quoted 32 lines · Gustavo A. R. Silva

Hi Jon,
Quoting Jon Hunter <[email protected]>:
Quoted 17 lines · Gustavo A. R. Silva
Quoted 16 lines
Print and propagate the return value of platform_get_irq on failure.
Signed-off-by Gustavo A. R. Silva <garsilva-L1vi/[email protected]>
---
 drivers/devfreq/tegra-devfreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/devfreq/tegra-devfreq.c +0 -0
index 214fff9..ae71215 100644
@@ -688,9 +688,9 @@ static int tegra_devfreq_probe(struct
 platform_device *pdev)
     }
 
Quoted 10 lines
drivers/devfreq/tegra-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 691
Quoted 5 lines
drivers/devfreq/tegra-devfreq.c +1 -3
     irq = platform_get_irq(pdev, 0);
-    if (irq <= 0) {
-        dev_err(&pdev->dev, "Failed to get IRQ\n");
-        return -ENODEV;
+    if (irq < 0) {
The changelog does not describe the above change and if/why this is ok. However, the original test looks fine to me and so I don't see a need to change this.

Quoted 8 lines

You are right about the changelog. I think this one is much better:
platform_get_irq() returns an error code, but the tegra-devfreq driver ignores it and always returns -ENODEV. This is not correct, and prevents -EPROBE_DEFER from being propagated properly.
Notice that platform_get_irq() no longer returns 0 on error:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af

Yes the above should be added to the changelog.

Looks like someone should update the following for consistency too ...

141 int platform_irq_count(struct platform_device *dev) 142 {
143 int ret, nr = 0;
144
145 while ((ret = platform_get_irq(dev, nr)) >= 0)
146 nr++;

Quoted 5 lines

Print error message and propagate the return value of platform_get_irq on failure.
What do you think?

Fine with me.

Thanks!
Jon

Signature · 1 line

nvpublic

Gustavo A. R. Silva

author

2017-07-03 12:47 UTC #6 lore raw

PATCH v2 1/1 devfreq: tegra: fix error return code in tegra_devfreq_probe()

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

Notice that platform_get_irq() no longer returns 0 on error:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af

Print and propagate the return value of platform_get_irq on failure.

Reviewed-by Chanwoo Choi <[email protected]>

Acked-by MyungJoo Ham <[email protected]>

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

---
Changes in v2:
Rewrite commit message.

 drivers/devfreq/tegra-devfreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/devfreq/tegra-devfreq.c +3 -3

index 214fff9..ae71215 100644
@@ -688,9 +688,9 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	}
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
-		dev_err(&pdev->dev, "Failed to get IRQ\n");
-		return -ENODEV;
+	if (irq < 0) {
+		dev_err(&pdev->dev, "Failed to get IRQ: %d\n", irq);
+		return irq;
 	}
 
 	platform_set_drvdata(pdev, tegra);

Signature · 1 line

2.5.0

Jon Hunter

maintainer Acked-by on v2 1/1

2017-07-04 08:35 UTC #7 lore raw

Re: [PATCH v2] devfreq: tegra: fix error return code in tegra_devfreq_probe()

Quoted 26 lines · Gustavo A. R. Silva

platform_get_irq() returns an error code, but the tegra-devfreq driver ignores it and always returns -ENODEV. This is not correct, and prevents -EPROBE_DEFER from being propagated properly.
Notice that platform_get_irq() no longer returns 0 on error:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af
Print and propagate the return value of platform_get_irq on failure.
Reviewed-by Chanwoo Choi <[email protected]>
Acked-by MyungJoo Ham <[email protected]>
Signed-off-by Gustavo A. R. Silva <garsilva-L1vi/[email protected]>
---
Changes in v2:
Rewrite commit message.
 drivers/devfreq/tegra-devfreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/devfreq/tegra-devfreq.c +0 -0
index 214fff9..ae71215 100644
@@ -688,9 +688,9 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	}
 

drivers/devfreq/tegra-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 696

Quoted 10 lines

drivers/devfreq/tegra-devfreq.c +3 -3

 	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
-		dev_err(&pdev->dev, "Failed to get IRQ\n");
-		return -ENODEV;
+	if (irq < 0) {
+		dev_err(&pdev->dev, "Failed to get IRQ: %d\n", irq);
+		return irq;
 	}
 
 	platform_set_drvdata(pdev, tegra);

Thanks.

Acked-by Jon Hunter <[email protected]>

Cheers
Jon

Signature · 1 line

nvpublic

Gustavo A. R. Silva

author

2017-07-17 04:52 UTC #8 lore raw

Re: [PATCH v2] devfreq: tegra: fix error return code in tegra_devfreq_probe()

Quoted 37 lines · Jon Hunter

Quoted 35 lines · Gustavo A. R. Silva
platform_get_irq() returns an error code, but the tegra-devfreq driver ignores it and always returns -ENODEV. This is not correct, and prevents -EPROBE_DEFER from being propagated properly.
Notice that platform_get_irq() no longer returns 0 on error:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af
Print and propagate the return value of platform_get_irq on failure.
Reviewed-by Chanwoo Choi <[email protected]>
Acked-by MyungJoo Ham <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
Changes in v2:
Rewrite commit message.
 drivers/devfreq/tegra-devfreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/devfreq/tegra-devfreq.c +3 -3
index 214fff9..ae71215 100644
@@ -688,9 +688,9 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	}
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0) {
-		dev_err(&pdev->dev, "Failed to get IRQ\n");
-		return -ENODEV;
+	if (irq < 0) {
+		dev_err(&pdev->dev, "Failed to get IRQ: %d\n", irq);
+		return irq;
 	}
 

Quoted 6 lines

drivers/devfreq/tegra-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 696
Quoted 1 line
drivers/devfreq/tegra-devfreq.c +0 -0
 	platform_set_drvdata(pdev, tegra);
Thanks.
Acked-by Jon Hunter <[email protected]>

Thank you, Jon.

Signature · 1 line

Gustavo A. R. Silva