[RFC PATCH 0/2] Add support for reading D1 efuse speed bin

[RFC PATCH 0/2] Add support for reading D1 efuse speed bin

[Brandon Cheo Fusi]

Hi everyone,

This series is an attempt to get feedback on decoding D1 efuse speed bins
in the Sun50i H6 cpufreq driver, and turning the result into a meaningful
value that selects voltage ranges in an OPP table.

I want to make sure I get this right before sending in a v3 of the D1
cpufreq support series at

https://lore.kernel.org/linux-sunxi/20231218110543.64044-1-fusibrandon13@gmail.com/T/#t

which is currently stuck at

https://lore.kernel.org/linux-sunxi/aad8302d-a015-44ee-ad11-1a4c6e00074c@sholland.org/

Brandon Cheo Fusi (2):
  cpufreq: sun50i: Add support for D1's speed bin decoding
  riscv: dts: allwinner: Fill in OPPs

 arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi |  8 +-
 drivers/cpufreq/sun50i-cpufreq-nvmem.c        | 85 +++++++++++++++----
 2 files changed, 76 insertions(+), 17 deletions(-)

-- 
2.30.2

[RFC PATCH 1/2] cpufreq: sun50i: Add support for D1's speed bin decoding

[Brandon Cheo Fusi]

Adds support for decoding the efuse value read from D1 efuse speed
bins, and factors out equivalent code for sun50i.

The algorithm is gotten from

https://github.com/Tina-Linux/linux-5.4/blob/master/drivers/cpufreq/sun50i-cpufreq-nvmem.c#L293-L338

and maps an efuse value to either 0 or 1, with 1 meaning stable at
a lower supply voltage for the same clock frequency.

Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
---
 drivers/cpufreq/sun50i-cpufreq-nvmem.c | 85 +++++++++++++++++++++-----
 1 file changed, 70 insertions(+), 15 deletions(-)

diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
index ccf83780f..28afbe558 100644
--- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
+++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
@@ -25,6 +25,66 @@
 
 static struct platform_device *cpufreq_dt_pdev, *sun50i_cpufreq_pdev;
 
+struct sunxi_cpufreq_data {
+	u32 (*efuse_xlate)(u32 efuse_value);
+};
+
+static u32 sun20i_efuse_xlate(u32 efuse_value)
+{
+	u32 ret;
+
+	switch (efuse_value) {
+	case 0x5e00:
+		/* QFN package */
+		ret = 0;
+		break;
+	case 0x5c00:
+	case 0x7400:
+		/* QFN package */
+		ret = 1;
+		break;
+	case 0x5000:
+	default:
+		/* BGA package */
+		ret = 0;
+	}
+
+	return ret;
+}
+
+static u32 sun50i_efuse_xlate(u32 efuse_value)
+{
+	efuse_value = (efuse_value >> NVMEM_SHIFT) & NVMEM_MASK;
+
+	/*
+	* We treat unexpected efuse values as if the SoC was from
+	* the slowest bin. Expected efuse values are 1-3, slowest
+	* to fastest.
+	*/
+	if (efuse_value >= 1 && efuse_value <= 3)
+		return efuse_value - 1;
+	else
+		return 0;
+}
+
+struct sunxi_cpufreq_data sun20i_cpufreq_data = {
+	.efuse_xlate = sun20i_efuse_xlate,
+};
+
+struct sunxi_cpufreq_data sun50i_cpufreq_data = {
+	.efuse_xlate = sun50i_efuse_xlate,
+};
+
+static const struct of_device_id cpu_opp_match_list[] = {
+	{ .compatible = "allwinner,sun50i-h6-operating-points",
+	  .data = &sun50i_cpufreq_data,
+	},
+	{ .compatible = "allwinner,sun20i-d1-operating-points",
+	  .data = &sun20i_cpufreq_data,
+	},
+	{}
+};
+
 /**
  * sun50i_cpufreq_get_efuse() - Determine speed grade from efuse value
  * @versions: Set to the value parsed from efuse
@@ -36,9 +96,11 @@ static int sun50i_cpufreq_get_efuse(u32 *versions)
 	struct nvmem_cell *speedbin_nvmem;
 	struct device_node *np;
 	struct device *cpu_dev;
-	u32 *speedbin, efuse_value;
+	const struct of_device_id *opp_match;
+	const struct sunxi_cpufreq_data *opp_data;
+	u32 *speedbin, efuse_value = 0;
 	size_t len;
-	int ret;
+	int i;
 
 	cpu_dev = get_cpu_device(0);
 	if (!cpu_dev)
@@ -48,9 +110,8 @@ static int sun50i_cpufreq_get_efuse(u32 *versions)
 	if (!np)
 		return -ENOENT;
 
-	ret = of_device_is_compatible(np,
-				      "allwinner,sun50i-h6-operating-points");
-	if (!ret) {
+	opp_match = of_match_node(cpu_opp_match_list, np);
+	if (!opp_match) {
 		of_node_put(np);
 		return -ENOENT;
 	}
@@ -66,17 +127,11 @@ static int sun50i_cpufreq_get_efuse(u32 *versions)
 	if (IS_ERR(speedbin))
 		return PTR_ERR(speedbin);
 
-	efuse_value = (*speedbin >> NVMEM_SHIFT) & NVMEM_MASK;
+	for (i = 0; i < len; i++)
+		efuse_value |= ((u32)speedbin[i] << (i * 8));
 
-	/*
-	 * We treat unexpected efuse values as if the SoC was from
-	 * the slowest bin. Expected efuse values are 1-3, slowest
-	 * to fastest.
-	 */
-	if (efuse_value >= 1 && efuse_value <= 3)
-		*versions = efuse_value - 1;
-	else
-		*versions = 0;
+	opp_data = opp_match->data;
+	*versions = opp_data->efuse_xlate(efuse_value);
 
 	kfree(speedbin);
 	return 0;
-- 
2.30.2

Re: [RFC PATCH 1/2] cpufreq: sun50i: Add support for D1's speed bin decoding

[Andre Przywara]

On Wed, 20 Dec 2023 10:51:40 +0100
Brandon Cheo Fusi <fusibrandon13@gmail.com> wrote:

Hi Brandon,

> Adds support for decoding the efuse value read from D1 efuse speed
> bins, and factors out equivalent code for sun50i.

Thanks for taking care and sharing this approach!

So this seems to be trying to achieve the same goal as this patch:
https://lore.kernel.org/linux-sunxi/20230904-cpufreq-h616-v1-4-b8842e525c43@somainline.org/
That covers the H616, and would at least clash heavily. The H616 series is
stuck on of some stupid bit in the temperature sensor driver, so I
guess the D1 (being easier) would overtake it.

Can you please split your patch below up, to first refactor the existing
code, with just the H6 support in, then in a second patch add the D1
specific parts? Bonus points for looking at the H616 patch and checking
if there are extra requirements over the D1, which should be already
considered in this refactoring patch.

Many thanks,
Andre


> The algorithm is gotten from
> 
> https://github.com/Tina-Linux/linux-5.4/blob/master/drivers/cpufreq/sun50i-cpufreq-nvmem.c#L293-L338
> 
> and maps an efuse value to either 0 or 1, with 1 meaning stable at
> a lower supply voltage for the same clock frequency.
> 
> Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
> ---
>  drivers/cpufreq/sun50i-cpufreq-nvmem.c | 85 +++++++++++++++++++++-----
>  1 file changed, 70 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> index ccf83780f..28afbe558 100644
> --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> @@ -25,6 +25,66 @@
>  
>  static struct platform_device *cpufreq_dt_pdev, *sun50i_cpufreq_pdev;
>  
> +struct sunxi_cpufreq_data {
> +	u32 (*efuse_xlate)(u32 efuse_value);
> +};
> +
> +static u32 sun20i_efuse_xlate(u32 efuse_value)
> +{
> +	u32 ret;
> +
> +	switch (efuse_value) {
> +	case 0x5e00:
> +		/* QFN package */
> +		ret = 0;
> +		break;
> +	case 0x5c00:
> +	case 0x7400:
> +		/* QFN package */
> +		ret = 1;
> +		break;
> +	case 0x5000:
> +	default:
> +		/* BGA package */
> +		ret = 0;
> +	}
> +
> +	return ret;
> +}
> +
> +static u32 sun50i_efuse_xlate(u32 efuse_value)
> +{
> +	efuse_value = (efuse_value >> NVMEM_SHIFT) & NVMEM_MASK;
> +
> +	/*
> +	* We treat unexpected efuse values as if the SoC was from
> +	* the slowest bin. Expected efuse values are 1-3, slowest
> +	* to fastest.
> +	*/
> +	if (efuse_value >= 1 && efuse_value <= 3)
> +		return efuse_value - 1;
> +	else
> +		return 0;
> +}
> +
> +struct sunxi_cpufreq_data sun20i_cpufreq_data = {
> +	.efuse_xlate = sun20i_efuse_xlate,
> +};
> +
> +struct sunxi_cpufreq_data sun50i_cpufreq_data = {
> +	.efuse_xlate = sun50i_efuse_xlate,
> +};
> +
> +static const struct of_device_id cpu_opp_match_list[] = {
> +	{ .compatible = "allwinner,sun50i-h6-operating-points",
> +	  .data = &sun50i_cpufreq_data,
> +	},
> +	{ .compatible = "allwinner,sun20i-d1-operating-points",
> +	  .data = &sun20i_cpufreq_data,
> +	},
> +	{}
> +};
> +
>  /**
>   * sun50i_cpufreq_get_efuse() - Determine speed grade from efuse value
>   * @versions: Set to the value parsed from efuse
> @@ -36,9 +96,11 @@ static int sun50i_cpufreq_get_efuse(u32 *versions)
>  	struct nvmem_cell *speedbin_nvmem;
>  	struct device_node *np;
>  	struct device *cpu_dev;
> -	u32 *speedbin, efuse_value;
> +	const struct of_device_id *opp_match;
> +	const struct sunxi_cpufreq_data *opp_data;
> +	u32 *speedbin, efuse_value = 0;
>  	size_t len;
> -	int ret;
> +	int i;
>  
>  	cpu_dev = get_cpu_device(0);
>  	if (!cpu_dev)
> @@ -48,9 +110,8 @@ static int sun50i_cpufreq_get_efuse(u32 *versions)
>  	if (!np)
>  		return -ENOENT;
>  
> -	ret = of_device_is_compatible(np,
> -				      "allwinner,sun50i-h6-operating-points");
> -	if (!ret) {
> +	opp_match = of_match_node(cpu_opp_match_list, np);
> +	if (!opp_match) {
>  		of_node_put(np);
>  		return -ENOENT;
>  	}
> @@ -66,17 +127,11 @@ static int sun50i_cpufreq_get_efuse(u32 *versions)
>  	if (IS_ERR(speedbin))
>  		return PTR_ERR(speedbin);
>  
> -	efuse_value = (*speedbin >> NVMEM_SHIFT) & NVMEM_MASK;
> +	for (i = 0; i < len; i++)
> +		efuse_value |= ((u32)speedbin[i] << (i * 8));
>  
> -	/*
> -	 * We treat unexpected efuse values as if the SoC was from
> -	 * the slowest bin. Expected efuse values are 1-3, slowest
> -	 * to fastest.
> -	 */
> -	if (efuse_value >= 1 && efuse_value <= 3)
> -		*versions = efuse_value - 1;
> -	else
> -		*versions = 0;
> +	opp_data = opp_match->data;
> +	*versions = opp_data->efuse_xlate(efuse_value);
>  
>  	kfree(speedbin);
>  	return 0;

[RFC PATCH 2/2] riscv: dts: allwinner: Fill in OPPs

[Brandon Cheo Fusi]

Specifies two voltage ranges, in order of increasing stability,
for each OPP. This is heavily inspired by

https://github.com/Tina-Linux/linux-5.4/blob/master/arch/riscv/boot/dts/sunxi/sun20iw1p1.dtsi#L118-L133

and

https://github.com/mangopi-sbc/tina-linux-5.4/blob/0d4903ebd9d2194ad914686d5b0fc1ddacf11a9d/arch/riscv/boot/dts/sunxi/sun20iw1p1.dtsi#L118-L182

Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
---
 arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi b/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi
index 2f1771c19..8e7bc8bd0 100644
--- a/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi
+++ b/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi
@@ -48,13 +48,17 @@ opp_table_cpu: opp-table-cpu {
 		opp-408000000 {
 			clock-latency-ns = <244144>; /* 8 32k periods */
 			opp-hz = /bits/ 64 <408000000>;
-			opp-microvolt-speed0 = <900000 900000 1100000>;
+
+			opp-microvolt-speed0 = <950000 900000 1100000>;
+			opp-microvolt-speed1 = <900000 900000 1100000>;
 		};
 
 		opp-1080000000 {
 			clock-latency-ns = <244144>; /* 8 32k periods */
 			opp-hz = /bits/ 64 <1008000000>;
-			opp-microvolt-speed0 = <900000 900000 1100000>;
+
+			opp-microvolt-speed0 = <1100000 900000 1100000>;
+			opp-microvolt-speed1 = <950000 900000 1100000>;
 		};
 	};
 
-- 
2.30.2

Re: [RFC PATCH 2/2] riscv: dts: allwinner: Fill in OPPs

[Jernej Škrabec]

Dne sreda, 20. december 2023 ob 10:51:41 CET je Brandon Cheo Fusi napisal(a):
> Specifies two voltage ranges, in order of increasing stability,
> for each OPP. This is heavily inspired by
> 
> https://github.com/Tina-Linux/linux-5.4/blob/master/arch/riscv/boot/dts/sunxi/sun20iw1p1.dtsi#L118-L133
> 
> and
> 
> https://github.com/mangopi-sbc/tina-linux-5.4/blob/0d4903ebd9d2194ad914686d5b0fc1ddacf11a9d/arch/riscv/boot/dts/sunxi/sun20iw1p1.dtsi#L118-L182

Remove links from message. If you really want them, add Link tag for each.

> 
> Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
> ---
>  arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi b/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi
> index 2f1771c19..8e7bc8bd0 100644
> --- a/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi
> +++ b/arch/riscv/boot/dts/allwinner/sun20i-d1s.dtsi
> @@ -48,13 +48,17 @@ opp_table_cpu: opp-table-cpu {
>  		opp-408000000 {
>  			clock-latency-ns = <244144>; /* 8 32k periods */
>  			opp-hz = /bits/ 64 <408000000>;
> -			opp-microvolt-speed0 = <900000 900000 1100000>;
> +
> +			opp-microvolt-speed0 = <950000 900000 1100000>;

Second value should be same as first, otherwise you'll experience stability issue.

> +			opp-microvolt-speed1 = <900000 900000 1100000>;
>  		};
>  
>  		opp-1080000000 {
>  			clock-latency-ns = <244144>; /* 8 32k periods */
>  			opp-hz = /bits/ 64 <1008000000>;
> -			opp-microvolt-speed0 = <900000 900000 1100000>;
> +
> +			opp-microvolt-speed0 = <1100000 900000 1100000>;

Ditto.

Best regards,
Jernej

> +			opp-microvolt-speed1 = <950000 900000 1100000>;
>  		};
>  	};
>  
>