diff mbox

[2/7] thermal: rcar_gen3_thermal: fix probe error path

Message ID 20170306200401.29923-3-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State Superseded, archived
Delegated to: Eduardo Valentin
Headers show

Commit Message

Niklas Söderlund March 6, 2017, 8:03 p.m. UTC
If the memory resource for a TSC is unviable probe should fail not try
to go ahead with the remaining TSC. This fix is aligned with other
checks in probe where probe fails if they are unavailable.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/thermal/rcar_gen3_thermal.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Geert Uytterhoeven March 7, 2017, 3:19 p.m. UTC | #1
On Mon, Mar 6, 2017 at 9:03 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> If the memory resource for a TSC is unviable probe should fail not try

unavailable

> to go ahead with the remaining TSC. This fix is aligned with other

TSCs

> checks in probe where probe fails if they are unavailable.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Wolfram Sang March 7, 2017, 7:52 p.m. UTC | #2
On Mon, Mar 06, 2017 at 09:03:56PM +0100, Niklas Söderlund wrote:
> If the memory resource for a TSC is unviable probe should fail not try
> to go ahead with the remaining TSC. This fix is aligned with other
> checks in probe where probe fails if they are unavailable.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

I disagree. There are likely SoCs in the future which have less than
TSC_MAX_NUM sensors (V3M shall have only 1 according to chapter 10C in
v0.52 documentation). So, the code exits the loop for this case. We
should move it before the devm_kzalloc(), however.

That also means that you can't really iterate over TSC_MAX_NUM in later
patches, but rather store the amount of instances in the main struct and
iterate over that value.
Niklas Söderlund March 17, 2017, 10:12 a.m. UTC | #3
Hi Wolfram,

Thanks for your feedback.

On 2017-03-07 20:52:03 +0100, Wolfram Sang wrote:
> On Mon, Mar 06, 2017 at 09:03:56PM +0100, Niklas Söderlund wrote:
> > If the memory resource for a TSC is unviable probe should fail not try
> > to go ahead with the remaining TSC. This fix is aligned with other
> > checks in probe where probe fails if they are unavailable.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> I disagree. There are likely SoCs in the future which have less than
> TSC_MAX_NUM sensors (V3M shall have only 1 according to chapter 10C in
> v0.52 documentation). So, the code exits the loop for this case. We
> should move it before the devm_kzalloc(), however.

Ahh I did not think of that. I will drop this patch and instead do as 
you suggest move this before the devm_kzalloc().

> 
> That also means that you can't really iterate over TSC_MAX_NUM in later
> patches, but rather store the amount of instances in the main struct and
> iterate over that value.
> 

I will see how to best solve this, probably we need then to recoded in 
struct rcar_gen3_thermal_priv how many zones are in use so we can 
iterate over all active zones.
diff mbox

Patch

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index ec477d47d0bae8e5..97958f91047b4c3a 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -289,8 +289,10 @@  static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 		}
 
 		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
-		if (!res)
-			break;
+		if (!res) {
+			ret = -ENODEV;
+			goto error_unregister;
+		}
 
 		tsc->base = devm_ioremap_resource(dev, res);
 		if (IS_ERR(tsc->base)) {