[v2 PATCH] thermal: rcar_gen3_thermal: Fix init value of IRQCTL register

[v2 PATCH] thermal: rcar_gen3_thermal: Fix init value of IRQCTL register

[Nguyen An Hoan]

From: Hoan Nguyen An <na-hoan@jinso.co.jp>

Fix setting value for IRQCTL register. We are setting the last 6 bits
of (IRQCTL) to be 1 (0x3f), this is only suitable for H3ES1.*, according
to new Hardware manual values 1 are "setting prohibited" for Gen3!

Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
---
 drivers/thermal/rcar_gen3_thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 7aed533..fde3fd8 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -306,7 +306,7 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
 
 	usleep_range(1000, 2000);
 
-	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
+	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0);
 	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
 	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN, IRQ_TEMPD1 | IRQ_TEMP2);
 
-- 
2.7.4

Re: [v2 PATCH] thermal: rcar_gen3_thermal: Fix init value of IRQCTL register

[Hoan]

Dear Geert-san, Wolfram-san

I am very sorry for not being re-test for this patch v1 with H3ES1.*.

Although I tested with M3-N, M3W H3ES2.0 and did not test

with H3ES1.1.  I  discarded v1 and updated v2.

Thank you! Hoan.


On 2018/10/25 11:13, Nguyen An Hoan wrote:
> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
>
> Fix setting value for IRQCTL register. We are setting the last 6 bits
> of (IRQCTL) to be 1 (0x3f), this is only suitable for H3ES1.*, according
> to new Hardware manual values 1 are "setting prohibited" for Gen3!
>
> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> ---
>   drivers/thermal/rcar_gen3_thermal.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 7aed533..fde3fd8 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -306,7 +306,7 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
>   
>   	usleep_range(1000, 2000);
>   
> -	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
> +	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0);
>   	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
>   	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN, IRQ_TEMPD1 | IRQ_TEMP2);
>   

Re: [v2 PATCH] thermal: rcar_gen3_thermal: Fix init value of IRQCTL register

[Niklas Söderlund]

Hi Hoan-san,

Thanks for your patch.

On 2018-10-25 11:13:41 +0900, Nguyen An Hoan wrote:
> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
> 
> Fix setting value for IRQCTL register. We are setting the last 6 bits
> of (IRQCTL) to be 1 (0x3f), this is only suitable for H3ES1.*, according
> to new Hardware manual values 1 are "setting prohibited" for Gen3!

Hum, as you point out this change is not suitable for H3 ES1.x so this 
would introduce a regression, which is not good. I will leave the final 
word to Wolfram but in my view you need to introduce quirk handling to 
keep support for H3 ES1.x or blacklist that SoC in the driver.

> 
> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> ---
>  drivers/thermal/rcar_gen3_thermal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 7aed533..fde3fd8 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -306,7 +306,7 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
>  
>  	usleep_range(1000, 2000);
>  
> -	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
> +	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0);
>  	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
>  	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN, IRQ_TEMPD1 | IRQ_TEMP2);
>  
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

Re: [v2 PATCH] thermal: rcar_gen3_thermal: Fix init value of IRQCTL register

[Hoan]

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

Dear Niklas-san

Thank you for your review!

On 2018/11/05 23:09, Niklas Söderlund wrote:
> Hi Hoan-san,
>
> Thanks for your patch.
>
> On 2018-10-25 11:13:41 +0900, Nguyen An Hoan wrote:
>> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
>>
>> Fix setting value for IRQCTL register. We are setting the last 6 bits
>> of (IRQCTL) to be 1 (0x3f), this is only suitable for H3ES1.*, according
>> to new Hardware manual values 1 are "setting prohibited" for Gen3!
> Hum, as you point out this change is not suitable for H3 ES1.x so this
> would introduce a regression, which is not good. I will leave the final
> word to Wolfram but in my view you need to introduce quirk handling to
> keep support for H3 ES1.x or blacklist that SoC in the driver.

With this patch and current state of the Gen3 thermal driver, H3ES1.* is 
still correctly supported.

     priv->thermal_init = rcar_gen3_thermal_init;
     if (soc_device_match(r8a7795es1))
         priv->thermal_init = rcar_gen3_thermal_init_r8a7795es1;

General common Gen3(H3, M3) and H3ES1. * using two different 
thermal_init（） function.

As the v1 of this patch

"https://patchwork.kernel.org/patch/10654457/"

I was  to change the init_r8a7795es1() function, but then test again

(I used the patch that improves interrupts for testing) and realized

(The count of interrupts  rising or falling temperature)
Unstable with H3ES1.1! So I update v2

I also have re-read the Hardware Manual,

with Rev0.5 is consistent with ES1 , but  Since Rev0.8 or Rev1.0 HWM is 
not compatible with H3ES1.

So that this update will be correct and consistent with 
rcar_gen3_thermal_init.
H3ES1.1 is already supported correctly at rcar_gen3_thermal_init_r8a7795es1!

I'm still thinking this change is correct! Thank you very much :-)

Hoan.

>> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
>> ---
>>   drivers/thermal/rcar_gen3_thermal.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
>> index 7aed533..fde3fd8 100644
>> --- a/drivers/thermal/rcar_gen3_thermal.c
>> +++ b/drivers/thermal/rcar_gen3_thermal.c
>> @@ -306,7 +306,7 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
>>   
>>   	usleep_range(1000, 2000);
>>   
>> -	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
>> +	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0);
>>   	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
>>   	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN, IRQ_TEMPD1 | IRQ_TEMP2);
>>   
>> -- 
>> 2.7.4
>>

[-- Attachment #2: Type: text/html, Size: 3913 bytes --]

Re: [v2 PATCH] thermal: rcar_gen3_thermal: Fix init value of IRQCTL register

["グェン・アン・ホァン"]

Dear Niklas-san

Thank you for your review!


Niklas Söderlund wrote:
> Hi Hoan-san,
> 
> Thanks for your patch.
> 
> On 2018-10-25 11:13:41 +0900, Nguyen An Hoan wrote:
> > From: Hoan Nguyen An <na-hoan@jinso.co.jp>
> > 
> > Fix setting value for IRQCTL register. We are setting the last 6 bits
> > of (IRQCTL) to be 1 (0x3f), this is only suitable for H3ES1.*, according
> > to new Hardware manual values 1 are "setting prohibited" for Gen3!
> 
> Hum, as you point out this change is not suitable for H3 ES1.x so this 
> would introduce a regression, which is not good. I will leave the final 
> word to Wolfram but in my view you need to introduce quirk handling to 
> keep support for H3 ES1.x or blacklist that SoC in the driver.
> 

With this patch and current state of the Gen3 thermal driver, H3ES1.* is still correctly supported.

    priv->thermal_init = rcar_gen3_thermal_init;
    if (soc_device_match(r8a7795es1))
        priv->thermal_init = rcar_gen3_thermal_init_r8a7795es1;

General common Gen3(H3, M3) and H3ES1. * using two different thermal_init（） function.

As the v1 of this patch

"https://patchwork.kernel.org/patch/10654457/"

I was  to change the init_r8a7795es1() function, but then test again

(I used the patch that improves interrupts for testing) and realized

(The count of interrupts  rising or falling temperature)
Unstable with H3ES1.1! So I update v2

I also have re-read the Hardware Manual,

with Rev0.5 is consistent with ES1 , but  Since Rev0.8 or Rev1.0 HWM is not compatible with H3ES1.

So that this update will be correct and consistent with rcar_gen3_thermal_init.
H3ES1.1 is already supported correctly at rcar_gen3_thermal_init_r8a7795es1!

I'm still thinking this change is correct! Thank you very much :)

Hoan.


> > 
> > Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> > ---
> >  drivers/thermal/rcar_gen3_thermal.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> > index 7aed533..fde3fd8 100644
> > --- a/drivers/thermal/rcar_gen3_thermal.c
> > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > @@ -306,7 +306,7 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
> >  
> >  	usleep_range(1000, 2000);
> >  
> > -	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
> > +	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0);
> >  	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
> >  	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN, IRQ_TEMPD1 | IRQ_TEMP2);
> >  
> > -- 
> > 2.7.4
> > 
> 
> -- 
> Regards,
> Niklas Söderlund

Re: [v2 PATCH] thermal: rcar_gen3_thermal: Fix init value of IRQCTL register

[Hoan]

Thank you for your review !!!!!

Hoan

Re: [v2 PATCH] thermal: rcar_gen3_thermal: Fix init value of IRQCTL register

[Wolfram Sang]

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


> > > Fix setting value for IRQCTL register. We are setting the last 6 bits
> > > of (IRQCTL) to be 1 (0x3f), this is only suitable for H3ES1.*, according
> > > to new Hardware manual values 1 are "setting prohibited" for Gen3!
> > Hum, as you point out this change is not suitable for H3 ES1.x so this
> > would introduce a regression, which is not good. I will leave the final
> > word to Wolfram but in my view you need to introduce quirk handling to
> > keep support for H3 ES1.x or blacklist that SoC in the driver.
> 
> With this patch and current state of the Gen3 thermal driver, H3ES1.* is
> still correctly supported.
> 
>     priv->thermal_init = rcar_gen3_thermal_init;
>     if (soc_device_match(r8a7795es1))
>         priv->thermal_init = rcar_gen3_thermal_init_r8a7795es1;

Makes sense to me. Niklas?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]