[PATCH V4 6/6] cpufreq: ti-cpufreq: Update efuse/rev offsets in AM62 family

Dhruva Gole posted 6 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH V4 6/6] cpufreq: ti-cpufreq: Update efuse/rev offsets in AM62 family
Posted by Dhruva Gole 2 months, 1 week ago
With the Silicon revision being taken directly from socinfo, there's no
longer any need for reading any SOC register for revision from this driver.
Hence, we do not require any rev_offset for AM62 family of devices.
The efuse offset should be 0x0 for AM625 as well, as the syscon
register being used from DT refers to the efuse_offset directly.

Signed-off-by: Dhruva Gole <d-gole@ti.com>
---

Viresh, Nishanth, Vignesh,

This driver fix is better to go with PATCH 5/6.

Subject: [PATCH V4 5/6] arm64: dts: ti: k3-am62: use opp_efuse_table for
 opp-table syscon

That patch fixes the efuse offset in the AM625 DT.
Without it, the driver will read from an incorrect efuse offset, and end
up breaking things in -next till all the DT changes make it in.
Hence, it would be preferrable if this entire series goes via a single
maintainer's tree.
Viresh, perhaps if you can ack this single patch, then Vignesh/Nishanth
could take it up if there are no objections?

I am sorry that this break compatibility with older AM625 devicetree.
However, the old devicetree was marking the entire wkup_conf as "syscon",
"simple-mfd" which was wrong and needed to be fixed.

This series finally tries to bring order to DT and the driver.

However, if there is still any way to maintain the backward
compatibility, then I am open to suggestions. Please try
and understand here that the ask for backward compatibility here
is to ask the driver to support a case where the register offset itself
was to be picked from a different node. I am not sure there's any
clean way to do this.

---
 drivers/cpufreq/ti-cpufreq.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
index ba621ce1cdda..870ab0b376c1 100644
--- a/drivers/cpufreq/ti-cpufreq.c
+++ b/drivers/cpufreq/ti-cpufreq.c
@@ -313,10 +313,9 @@ static const struct soc_device_attribute k3_cpufreq_soc[] = {
 
 static struct ti_cpufreq_soc_data am625_soc_data = {
 	.efuse_xlate = am625_efuse_xlate,
-	.efuse_offset = 0x0018,
+	.efuse_offset = 0x0,
 	.efuse_mask = 0x07c0,
 	.efuse_shift = 0x6,
-	.rev_offset = 0x0014,
 	.multi_regulator = false,
 };
 
@@ -325,7 +324,6 @@ static struct ti_cpufreq_soc_data am62a7_soc_data = {
 	.efuse_offset = 0x0,
 	.efuse_mask = 0x07c0,
 	.efuse_shift = 0x6,
-	.rev_offset = 0x0014,
 	.multi_regulator = false,
 };
 
@@ -334,7 +332,6 @@ static struct ti_cpufreq_soc_data am62p5_soc_data = {
 	.efuse_offset = 0x0,
 	.efuse_mask = 0x07c0,
 	.efuse_shift = 0x6,
-	.rev_offset = 0x0014,
 	.multi_regulator = false,
 };
 
-- 
2.34.1
Re: [PATCH V4 6/6] cpufreq: ti-cpufreq: Update efuse/rev offsets in AM62 family
Posted by Viresh Kumar 1 month, 4 weeks ago
On 19-09-24, 13:58, Dhruva Gole wrote:
> With the Silicon revision being taken directly from socinfo, there's no
> longer any need for reading any SOC register for revision from this driver.
> Hence, we do not require any rev_offset for AM62 family of devices.
> The efuse offset should be 0x0 for AM625 as well, as the syscon
> register being used from DT refers to the efuse_offset directly.
> 
> Signed-off-by: Dhruva Gole <d-gole@ti.com>
> ---
> 
> Viresh, Nishanth, Vignesh,
> 
> This driver fix is better to go with PATCH 5/6.
> 
> Subject: [PATCH V4 5/6] arm64: dts: ti: k3-am62: use opp_efuse_table for
>  opp-table syscon
> 
> That patch fixes the efuse offset in the AM625 DT.
> Without it, the driver will read from an incorrect efuse offset, and end
> up breaking things in -next till all the DT changes make it in.
> Hence, it would be preferrable if this entire series goes via a single
> maintainer's tree.
> Viresh, perhaps if you can ack this single patch, then Vignesh/Nishanth
> could take it up if there are no objections?
> 
> I am sorry that this break compatibility with older AM625 devicetree.
> However, the old devicetree was marking the entire wkup_conf as "syscon",
> "simple-mfd" which was wrong and needed to be fixed.
> 
> This series finally tries to bring order to DT and the driver.
> 
> However, if there is still any way to maintain the backward
> compatibility, then I am open to suggestions. Please try
> and understand here that the ask for backward compatibility here
> is to ask the driver to support a case where the register offset itself
> was to be picked from a different node. I am not sure there's any
> clean way to do this.
> 
> ---
>  drivers/cpufreq/ti-cpufreq.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
> index ba621ce1cdda..870ab0b376c1 100644
> --- a/drivers/cpufreq/ti-cpufreq.c
> +++ b/drivers/cpufreq/ti-cpufreq.c
> @@ -313,10 +313,9 @@ static const struct soc_device_attribute k3_cpufreq_soc[] = {
>  
>  static struct ti_cpufreq_soc_data am625_soc_data = {
>  	.efuse_xlate = am625_efuse_xlate,
> -	.efuse_offset = 0x0018,
> +	.efuse_offset = 0x0,
>  	.efuse_mask = 0x07c0,
>  	.efuse_shift = 0x6,
> -	.rev_offset = 0x0014,
>  	.multi_regulator = false,
>  };
>  
> @@ -325,7 +324,6 @@ static struct ti_cpufreq_soc_data am62a7_soc_data = {
>  	.efuse_offset = 0x0,
>  	.efuse_mask = 0x07c0,
>  	.efuse_shift = 0x6,
> -	.rev_offset = 0x0014,
>  	.multi_regulator = false,
>  };
>  
> @@ -334,7 +332,6 @@ static struct ti_cpufreq_soc_data am62p5_soc_data = {
>  	.efuse_offset = 0x0,
>  	.efuse_mask = 0x07c0,
>  	.efuse_shift = 0x6,
> -	.rev_offset = 0x0014,
>  	.multi_regulator = false,
>  };

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh
Re: [PATCH V4 6/6] cpufreq: ti-cpufreq: Update efuse/rev offsets in AM62 family
Posted by Dhruva Gole 1 month, 4 weeks ago
On Oct 01, 2024 at 11:59:19 +0530, Viresh Kumar wrote:
> On 19-09-24, 13:58, Dhruva Gole wrote:
> > With the Silicon revision being taken directly from socinfo, there's no
> > longer any need for reading any SOC register for revision from this driver.
> > Hence, we do not require any rev_offset for AM62 family of devices.
> > The efuse offset should be 0x0 for AM625 as well, as the syscon
> > register being used from DT refers to the efuse_offset directly.
> > 
> > Signed-off-by: Dhruva Gole <d-gole@ti.com>
> > ---
> > 
> > Viresh, Nishanth, Vignesh,
> > 
> > This driver fix is better to go with PATCH 5/6.
> > 
> > Subject: [PATCH V4 5/6] arm64: dts: ti: k3-am62: use opp_efuse_table for
> >  opp-table syscon
> > 
> > That patch fixes the efuse offset in the AM625 DT.
> > Without it, the driver will read from an incorrect efuse offset, and end
> > up breaking things in -next till all the DT changes make it in.
> > Hence, it would be preferrable if this entire series goes via a single
> > maintainer's tree.
> > Viresh, perhaps if you can ack this single patch, then Vignesh/Nishanth
> > could take it up if there are no objections?
> > 
> > I am sorry that this break compatibility with older AM625 devicetree.
> > However, the old devicetree was marking the entire wkup_conf as "syscon",
> > "simple-mfd" which was wrong and needed to be fixed.
> > 
> > This series finally tries to bring order to DT and the driver.
> > 
> > However, if there is still any way to maintain the backward
> > compatibility, then I am open to suggestions. Please try
> > and understand here that the ask for backward compatibility here
> > is to ask the driver to support a case where the register offset itself
> > was to be picked from a different node. I am not sure there's any
> > clean way to do this.
> > 
> > ---
> >  drivers/cpufreq/ti-cpufreq.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
> > index ba621ce1cdda..870ab0b376c1 100644
> > --- a/drivers/cpufreq/ti-cpufreq.c
> > +++ b/drivers/cpufreq/ti-cpufreq.c
> > @@ -313,10 +313,9 @@ static const struct soc_device_attribute k3_cpufreq_soc[] = {
> >  
> >  static struct ti_cpufreq_soc_data am625_soc_data = {
> >  	.efuse_xlate = am625_efuse_xlate,
> > -	.efuse_offset = 0x0018,
> > +	.efuse_offset = 0x0,
> >  	.efuse_mask = 0x07c0,
> >  	.efuse_shift = 0x6,
> > -	.rev_offset = 0x0014,
> >  	.multi_regulator = false,
> >  };
> >  
> > @@ -325,7 +324,6 @@ static struct ti_cpufreq_soc_data am62a7_soc_data = {
> >  	.efuse_offset = 0x0,
> >  	.efuse_mask = 0x07c0,
> >  	.efuse_shift = 0x6,
> > -	.rev_offset = 0x0014,
> >  	.multi_regulator = false,
> >  };
> >  
> > @@ -334,7 +332,6 @@ static struct ti_cpufreq_soc_data am62p5_soc_data = {
> >  	.efuse_offset = 0x0,
> >  	.efuse_mask = 0x07c0,
> >  	.efuse_shift = 0x6,
> > -	.rev_offset = 0x0014,
> >  	.multi_regulator = false,
> >  };
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Really sorry for any confusion here, but this series has been overall
split up and posted afresh here:
https://lore.kernel.org/all/20240930-b4-ti-cpufreq-am62-quirk-v1-0-b5e04f0f899b@ti.com/

This was done to avoid breaking any backward compatibility issues with
older DTs in a clean way.

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated