[PATCH 0/2] thermal: rcar: minor fixes

[PATCH 0/2] thermal: rcar: minor fixes

[Yoshihiro Shimoda]

This patch series is based on linux-soc-thermal.git / fixes branch.
(commit id = 5a723e81923410f0d3ae4c38974607b13befdda9)

Yoshihiro Shimoda (2):
  thermal: rcar: fix ENR register value
  thermal: rcar: change type of ctemp in rcar_thermal_update_temp()

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

-- 
1.7.9.5

[PATCH 1/2] thermal: rcar: fix ENR register value

[Yoshihiro Shimoda]

Since the write value should always be 0 for bit 31-4 of ENR register
on R-Car H2, this patch fixes the value.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/thermal/rcar_thermal.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index 8803e69..bbdf64e 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -409,7 +409,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 			return PTR_ERR(common->base);
 
 		/* enable temperature comparation */
-		rcar_thermal_common_write(common, ENR, 0x00030303);
+		rcar_thermal_common_write(common, ENR, 0x00000003);
 
 		idle = 0; /* polling delay is not needed */
 	}
-- 
1.7.9.5

Re: [PATCH 1/2] thermal: rcar: fix ENR register value

[Geert Uytterhoeven]

Hi Shimoda-san,

On Tue, Jan 6, 2015 at 11:02 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Since the write value should always be 0 for bit 31-4 of ENR register
> on R-Car H2, this patch fixes the value.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  drivers/thermal/rcar_thermal.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index 8803e69..bbdf64e 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -409,7 +409,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>                         return PTR_ERR(common->base);
>
>                 /* enable temperature comparation */
> -               rcar_thermal_common_write(common, ENR, 0x00030303);
> +               rcar_thermal_common_write(common, ENR, 0x00000003);

While your patch is correct for R-Car gen2, this driver is also used on
R-Mobile APE6 (r8a73a4), which does have 3 thermal zones.
So I think you should count the other I/O resource sets to calculate
the correct mask.

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

RE: [PATCH 1/2] thermal: rcar: fix ENR register value

[yoshihiro shimoda]

Hi Geert-san,

> Hi Shimoda-san,
> 
> On Tue, Jan 6, 2015 at 11:02 AM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > Since the write value should always be 0 for bit 31-4 of ENR register
> > on R-Car H2, this patch fixes the value.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  drivers/thermal/rcar_thermal.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> > index 8803e69..bbdf64e 100644
> > --- a/drivers/thermal/rcar_thermal.c
> > +++ b/drivers/thermal/rcar_thermal.c
> > @@ -409,7 +409,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
> >                         return PTR_ERR(common->base);
> >
> >                 /* enable temperature comparation */
> > -               rcar_thermal_common_write(common, ENR, 0x00030303);
> > +               rcar_thermal_common_write(common, ENR, 0x00000003);
> 
> While your patch is correct for R-Car gen2, this driver is also used on
> R-Mobile APE6 (r8a73a4), which does have 3 thermal zones.
> So I think you should count the other I/O resource sets to calculate
> the correct mask.

Thank you for the review and suggestion!
As you say, I didn't care about R-Mobile APE6.
So, I will modify this code tomorrow.

Best regards,
Yoshihiro Shimoda

> 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

[PATCH 2/2] thermal: rcar: change type of ctemp in rcar_thermal_update_temp()

[Yoshihiro Shimoda]

Since the ctemp is used for rcar_thermal_write() in
rcar_thermal_update_temp(), the type of ctemp should be "u32" instead
of "int".

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/thermal/rcar_thermal.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index bbdf64e..6dcead1 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -63,7 +63,7 @@ struct rcar_thermal_priv {
 	struct mutex lock;
 	struct list_head list;
 	int id;
-	int ctemp;
+	u32 ctemp;
 };
 
 #define rcar_thermal_for_each_priv(pos, common)	\
@@ -145,7 +145,7 @@ static int rcar_thermal_update_temp(struct rcar_thermal_priv *priv)
 {
 	struct device *dev = rcar_priv_to_dev(priv);
 	int i;
-	int ctemp, old, new;
+	u32 ctemp, old, new;
 	int ret = -EINVAL;
 
 	mutex_lock(&priv->lock);
-- 
1.7.9.5

Re: [PATCH 2/2] thermal: rcar: change type of ctemp in rcar_thermal_update_temp()

[Geert Uytterhoeven]

On Tue, Jan 6, 2015 at 11:02 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Since the ctemp is used for rcar_thermal_write() in
> rcar_thermal_update_temp(), the type of ctemp should be "u32" instead
> of "int".
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Acked-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

Re: [PATCH 2/2] thermal: rcar: change type of ctemp in rcar_thermal_update_temp()

[Eduardo Valentin]

[-- Attachment #1: Type: text/plain, Size: 1347 bytes --]

Shimoda-san,

On Tue, Jan 06, 2015 at 07:02:08PM +0900, Yoshihiro Shimoda wrote:
> Since the ctemp is used for rcar_thermal_write() in
> rcar_thermal_update_temp(), the type of ctemp should be "u32" instead
> of "int".

In your V2, can you please include in your description that you are also
updating the helper variables 'old' and 'new'?

The description does not mention it.

> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  drivers/thermal/rcar_thermal.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index bbdf64e..6dcead1 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -63,7 +63,7 @@ struct rcar_thermal_priv {
>  	struct mutex lock;
>  	struct list_head list;
>  	int id;
> -	int ctemp;
> +	u32 ctemp;
>  };
>  
>  #define rcar_thermal_for_each_priv(pos, common)	\
> @@ -145,7 +145,7 @@ static int rcar_thermal_update_temp(struct rcar_thermal_priv *priv)
>  {
>  	struct device *dev = rcar_priv_to_dev(priv);
>  	int i;
> -	int ctemp, old, new;
> +	u32 ctemp, old, new;

But you also update them, as necessary.

>  	int ret = -EINVAL;
>  
>  	mutex_lock(&priv->lock);
> -- 
> 1.7.9.5
> 


BR, 

Eduardo Valentin

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

RE: [PATCH 2/2] thermal: rcar: change type of ctemp in rcar_thermal_update_temp()

[yoshihiro shimoda]

Hi Eduardo-san,

> Shimoda-san,
> 
> On Tue, Jan 06, 2015 at 07:02:08PM +0900, Yoshihiro Shimoda wrote:
> > Since the ctemp is used for rcar_thermal_write() in
> > rcar_thermal_update_temp(), the type of ctemp should be "u32" instead
> > of "int".
> 
> In your V2, can you please include in your description that you are also
> updating the helper variables 'old' and 'new'?
> 
> The description does not mention it.

Thank you for the point.
I will fix this description in v2 patch set.

Best regards,
Yoshihiro Shimoda