| Message ID | 20210707131306.4098443-4-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | thermal: rcar_gen3_thermal: Add support for trip points | expand |
Hi Niklas, Thanks for your patch! On Wed, Jul 7, 2021 at 3:14 PM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > The loop counter 'i' should be unsigned int to match struct > rcar_gen3_thermal_priv num_tscs where it's stored. It is also stored in rcar_gen3_thermal_tsc.id, which is still (signed) int. > --- a/drivers/thermal/rcar_gen3_thermal.c > +++ b/drivers/thermal/rcar_gen3_thermal.c > @@ -423,7 +423,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct resource *res; > struct thermal_zone_device *zone; > - int ret, i; > + unsigned int i; > + int ret; > > /* default values if FUSEs are missing */ > /* TODO: Read values from hardware on supported platforms */ And perhaps: - dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret); + dev_info(dev, "TSC%u: Loaded %d trip points\n", i, ret); ? Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for your feedback. On 2021-07-07 16:42:10 +0200, Geert Uytterhoeven wrote: > Hi Niklas, > > Thanks for your patch! > > On Wed, Jul 7, 2021 at 3:14 PM Niklas Söderlund > <niklas.soderlund+renesas@ragnatech.se> wrote: > > The loop counter 'i' should be unsigned int to match struct > > rcar_gen3_thermal_priv num_tscs where it's stored. > > It is also stored in rcar_gen3_thermal_tsc.id, which is still (signed) int. Thanks. > > > --- a/drivers/thermal/rcar_gen3_thermal.c > > +++ b/drivers/thermal/rcar_gen3_thermal.c > > @@ -423,7 +423,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev) > > struct device *dev = &pdev->dev; > > struct resource *res; > > struct thermal_zone_device *zone; > > - int ret, i; > > + unsigned int i; > > + int ret; > > > > /* default values if FUSEs are missing */ > > /* TODO: Read values from hardware on supported platforms */ > > And perhaps: > > - dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret); > + dev_info(dev, "TSC%u: Loaded %d trip points\n", i, ret); > > ? Good catch, why do my compiler not warn for this? Did you catch this by review or have you some compiler magic I'm missing? > > 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
Hi Niklas, On Wed, Jul 7, 2021 at 8:56 PM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > On 2021-07-07 16:42:10 +0200, Geert Uytterhoeven wrote: > > On Wed, Jul 7, 2021 at 3:14 PM Niklas Söderlund > > <niklas.soderlund+renesas@ragnatech.se> wrote: > > > The loop counter 'i' should be unsigned int to match struct > > > rcar_gen3_thermal_priv num_tscs where it's stored. > > > > It is also stored in rcar_gen3_thermal_tsc.id, which is still (signed) int. > > Thanks. > > > > > > --- a/drivers/thermal/rcar_gen3_thermal.c > > > +++ b/drivers/thermal/rcar_gen3_thermal.c > > > @@ -423,7 +423,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev) > > > struct device *dev = &pdev->dev; > > > struct resource *res; > > > struct thermal_zone_device *zone; > > > - int ret, i; > > > + unsigned int i; > > > + int ret; > > > > > > /* default values if FUSEs are missing */ > > > /* TODO: Read values from hardware on supported platforms */ > > > > And perhaps: > > > > - dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret); > > + dev_info(dev, "TSC%u: Loaded %d trip points\n", i, ret); > > > > ? > > Good catch, why do my compiler not warn for this? Did you catch this by Because the types are (somewhat) compatible. > review or have you some compiler magic I'm missing? Review. You changed the type of a variable, I checked all places where the variable is used. Gr{oetje,eeting}s, Geert
diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c index a438e05e7ca7f20e..8ca4bfee7bca4122 100644 --- a/drivers/thermal/rcar_gen3_thermal.c +++ b/drivers/thermal/rcar_gen3_thermal.c @@ -423,7 +423,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct resource *res; struct thermal_zone_device *zone; - int ret, i; + unsigned int i; + int ret; /* default values if FUSEs are missing */ /* TODO: Read values from hardware on supported platforms */
The loop counter 'i' should be unsigned int to match struct rcar_gen3_thermal_priv num_tscs where it's stored. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/thermal/rcar_gen3_thermal.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)