diff mbox series

[v6,6/6] cpufreq: ti-cpufreq: Update efuse/rev offsets in AM62 family

Message ID 20240925-ti-cpufreq-fixes-v5-v6-6-46f41a903e01@ti.com
State New
Headers show
Series ti: k3-am62{a,p}x-sk: add opp frequencies | expand

Commit Message

Dhruva Gole Sept. 25, 2024, 2:54 p.m. UTC
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.

However, to maintain the backward compatibility with old devicetree, also
add condition to handle the case where we have the wrong offset and add
the older efuse_offset value there such that we don't end up reading the
wrong register offset.

Signed-off-by: Dhruva Gole <d-gole@ti.com>
---
 drivers/cpufreq/ti-cpufreq.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

Comments

Andrew Davis Sept. 25, 2024, 4:12 p.m. UTC | #1
On 9/25/24 9:54 AM, 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.
> 
> However, to maintain the backward compatibility with old devicetree, also
> add condition to handle the case where we have the wrong offset and add
> the older efuse_offset value there such that we don't end up reading the
> wrong register offset.
> 
> Signed-off-by: Dhruva Gole <d-gole@ti.com>
> ---
>   drivers/cpufreq/ti-cpufreq.c | 23 +++++++++++++++++------
>   1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
> index ba621ce1cdda694c98867422dbb7f10c0df2afef..8a97b95b4c44a76b12cab76ddc0f9a5b8ae73f84 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,
>   };
>   
> @@ -349,11 +346,25 @@ static int ti_cpufreq_get_efuse(struct ti_cpufreq_data *opp_data,
>   				u32 *efuse_value)
>   {
>   	struct device *dev = opp_data->cpu_dev;
> +	struct device_node *np = opp_data->opp_node;
>   	u32 efuse;
>   	int ret;
>   
> -	ret = regmap_read(opp_data->syscon, opp_data->soc_data->efuse_offset,
> -			  &efuse);
> +	/*
> +	 * The following check is used as a way to check if this is an older devicetree

"check is used as a way to check" sound redundant, maybe just:

This checks for old AM625 Devicetrees where the syscon was a phandle
to the wkup_conf parent, this required a hard-coded offset to
the efuse register.

> +	 * being used where the entire wkup_conf node was marked as "syscon",
> +	 * "simple-mfd".
> +	 * Since this bug only affects AM625, make sure it enters this condition
> +	 * only for that SoC.
> +	 */
> +	if (of_device_is_compatible(np, "simple-mfd") &&
> +	    of_device_is_compatible(np, "ti,am625")) {

Kinda hacky, but keeping backwards compat often is hacky..

Does `of_device_is_compatible(np, "ti,am625")` actually work here? I'm assuming you
tested with an old DT to make sure this path ever got taken. Maybe put a warning
here that an old DT is in use and the user should update at some point.

Andrew

> +		ret = regmap_read(opp_data->syscon, opp_data->soc_data->efuse_offset + 0x0018,
> +				  &efuse);
> +	} else {
> +		ret = regmap_read(opp_data->syscon, opp_data->soc_data->efuse_offset,
> +				  &efuse);
> +	}
>   	if (opp_data->soc_data->quirks & TI_QUIRK_SYSCON_MAY_BE_MISSING && ret == -EIO) {
>   		/* not a syscon register! */
>   		void __iomem *regs = ioremap(OMAP3_SYSCON_BASE +
>
diff mbox series

Patch

diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
index ba621ce1cdda694c98867422dbb7f10c0df2afef..8a97b95b4c44a76b12cab76ddc0f9a5b8ae73f84 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,
 };
 
@@ -349,11 +346,25 @@  static int ti_cpufreq_get_efuse(struct ti_cpufreq_data *opp_data,
 				u32 *efuse_value)
 {
 	struct device *dev = opp_data->cpu_dev;
+	struct device_node *np = opp_data->opp_node;
 	u32 efuse;
 	int ret;
 
-	ret = regmap_read(opp_data->syscon, opp_data->soc_data->efuse_offset,
-			  &efuse);
+	/*
+	 * The following check is used as a way to check if this is an older devicetree
+	 * being used where the entire wkup_conf node was marked as "syscon",
+	 * "simple-mfd".
+	 * Since this bug only affects AM625, make sure it enters this condition
+	 * only for that SoC.
+	 */
+	if (of_device_is_compatible(np, "simple-mfd") &&
+	    of_device_is_compatible(np, "ti,am625")) {
+		ret = regmap_read(opp_data->syscon, opp_data->soc_data->efuse_offset + 0x0018,
+				  &efuse);
+	} else {
+		ret = regmap_read(opp_data->syscon, opp_data->soc_data->efuse_offset,
+				  &efuse);
+	}
 	if (opp_data->soc_data->quirks & TI_QUIRK_SYSCON_MAY_BE_MISSING && ret == -EIO) {
 		/* not a syscon register! */
 		void __iomem *regs = ioremap(OMAP3_SYSCON_BASE +