[PATCH v2 1/3] dt-bindings: nvmem: rockchip,otp: Add support for rk3562 and rk3568

[PATCH v2 1/3] dt-bindings: nvmem: rockchip,otp: Add support for rk3562 and rk3568

[Kever Yang]

Add compatible entry for the otp controller in rk3562 and rk3568, add schema
for different clock names for new entry.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v2:
- Update the commit message and add maxItems in schema.

 .../bindings/nvmem/rockchip,otp.yaml          | 53 ++++++++++++++++---
 1 file changed, 46 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml b/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml
index a44d44b32809..7572f4a1d73b 100644
--- a/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml
+++ b/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml
@@ -14,6 +14,8 @@ properties:
     enum:
       - rockchip,px30-otp
       - rockchip,rk3308-otp
+      - rockchip,rk3562-otp
+      - rockchip,rk3568-otp
       - rockchip,rk3588-otp
 
   reg:
@@ -25,19 +27,15 @@ properties:
 
   clock-names:
     minItems: 3
-    items:
-      - const: otp
-      - const: apb_pclk
-      - const: phy
-      - const: arb
+    maxItems: 4
 
   resets:
     minItems: 1
-    maxItems: 3
+    maxItems: 4
 
   reset-names:
     minItems: 1
-    maxItems: 3
+    maxItems: 4
 
 required:
   - compatible
@@ -62,12 +60,45 @@ allOf:
       properties:
         clocks:
           maxItems: 3
+        clock-names:
+          items:
+            - const: otp
+            - const: apb_pclk
+            - const: phy
         resets:
           maxItems: 1
         reset-names:
           items:
             - const: phy
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - rockchip,rk3562-otp
+              - rockchip,rk3568-otp
+    then:
+      properties:
+        clocks:
+          minItems: 4
+          maxItems: 4
+        clock-names:
+          items:
+            - const: usr
+            - const: sbpi
+            - const: apb_pclk
+            - const: phy
+        resets:
+          minItems: 4
+          maxItems: 4
+        reset-names:
+          items:
+            - const: usr
+            - const: sbpi
+            - const: apb
+            - const: phy
+
   - if:
       properties:
         compatible:
@@ -78,8 +109,16 @@ allOf:
       properties:
         clocks:
           minItems: 4
+          maxItems: 4
+        clock-names:
+          items:
+            - const: otp
+            - const: apb_pclk
+            - const: phy
+            - const: arb
         resets:
           minItems: 3
+          maxItems: 3
         reset-names:
           items:
             - const: otp
-- 
2.25.1

[PATCH v2 2/3] nvmem: rockchip-otp: Add support for rk3568-otp

[Kever Yang]

From: Finley Xiao <finley.xiao@rock-chips.com>

This adds the necessary data for handling efuse on the rk3568.

Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v2: None

 drivers/nvmem/rockchip-otp.c | 82 ++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/drivers/nvmem/rockchip-otp.c b/drivers/nvmem/rockchip-otp.c
index ebc3f0b24166..a04bce89ecc8 100644
--- a/drivers/nvmem/rockchip-otp.c
+++ b/drivers/nvmem/rockchip-otp.c
@@ -27,6 +27,7 @@
 #define OTPC_USER_CTRL			0x0100
 #define OTPC_USER_ADDR			0x0104
 #define OTPC_USER_ENABLE		0x0108
+#define OTPC_USER_QP			0x0120
 #define OTPC_USER_Q			0x0124
 #define OTPC_INT_STATUS			0x0304
 #define OTPC_SBPI_CMD0_OFFSET		0x1000
@@ -53,6 +54,8 @@
 #define SBPI_ENABLE_MASK		GENMASK(16, 16)
 
 #define OTPC_TIMEOUT			10000
+#define OTPC_TIMEOUT_PROG		100000
+#define RK3568_NBYTES			2
 
 /* RK3588 Register */
 #define RK3588_OTPC_AUTO_CTRL		0x04
@@ -184,6 +187,70 @@ static int px30_otp_read(void *context, unsigned int offset,
 	return ret;
 }
 
+static int rk3568_otp_read(void *context, unsigned int offset, void *val,
+			   size_t bytes)
+{
+	struct rockchip_otp *otp = context;
+	unsigned int addr_start, addr_end, addr_offset, addr_len;
+	unsigned int otp_qp;
+	u32 out_value;
+	u8 *buf;
+	int ret = 0, i = 0;
+
+	addr_start = rounddown(offset, RK3568_NBYTES) / RK3568_NBYTES;
+	addr_end = roundup(offset + bytes, RK3568_NBYTES) / RK3568_NBYTES;
+	addr_offset = offset % RK3568_NBYTES;
+	addr_len = addr_end - addr_start;
+
+	buf = kzalloc(array3_size(addr_len, RK3568_NBYTES, sizeof(*buf)),
+		      GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = rockchip_otp_reset(otp);
+	if (ret) {
+		dev_err(otp->dev, "failed to reset otp phy\n");
+		return ret;
+	}
+
+	ret = rockchip_otp_ecc_enable(otp, true);
+	if (ret < 0) {
+		dev_err(otp->dev, "rockchip_otp_ecc_enable err\n");
+		return ret;
+	}
+
+	writel(OTPC_USE_USER | OTPC_USE_USER_MASK, otp->base + OTPC_USER_CTRL);
+	udelay(5);
+	while (addr_len--) {
+		writel(addr_start++ | OTPC_USER_ADDR_MASK,
+		       otp->base + OTPC_USER_ADDR);
+		writel(OTPC_USER_FSM_ENABLE | OTPC_USER_FSM_ENABLE_MASK,
+		       otp->base + OTPC_USER_ENABLE);
+		ret = rockchip_otp_wait_status(otp, OTPC_INT_STATUS, OTPC_USER_DONE);
+		if (ret < 0) {
+			dev_err(otp->dev, "timeout during read setup\n");
+			goto read_end;
+		}
+		otp_qp = readl(otp->base + OTPC_USER_QP);
+		if (((otp_qp & 0xc0) == 0xc0) || (otp_qp & 0x20)) {
+			ret = -EIO;
+			dev_err(otp->dev, "ecc check error during read setup\n");
+			goto read_end;
+		}
+		out_value = readl(otp->base + OTPC_USER_Q);
+		memcpy(&buf[i], &out_value, RK3568_NBYTES);
+		i += RK3568_NBYTES;
+	}
+
+	memcpy(val, buf + addr_offset, bytes);
+
+read_end:
+	writel(0x0 | OTPC_USE_USER_MASK, otp->base + OTPC_USER_CTRL);
+	kfree(buf);
+
+	return ret;
+}
+
 static int rk3588_otp_read(void *context, unsigned int offset,
 			   void *val, size_t bytes)
 {
@@ -274,6 +341,17 @@ static const struct rockchip_data px30_data = {
 	.reg_read = px30_otp_read,
 };
 
+static const char * const rk3568_otp_clocks[] = {
+	"usr", "sbpi", "apb_pclk", "phy",
+};
+
+static const struct rockchip_data rk3568_data = {
+	.size = 0x80,
+	.clks = rk3568_otp_clocks,
+	.num_clks = ARRAY_SIZE(rk3568_otp_clocks),
+	.reg_read = rk3568_otp_read,
+};
+
 static const char * const rk3588_otp_clocks[] = {
 	"otp", "apb_pclk", "phy", "arb",
 };
@@ -294,6 +372,10 @@ static const struct of_device_id rockchip_otp_match[] = {
 		.compatible = "rockchip,rk3308-otp",
 		.data = &px30_data,
 	},
+	{
+		.compatible = "rockchip,rk3568-otp",
+		.data = &rk3568_data,
+	},
 	{
 		.compatible = "rockchip,rk3588-otp",
 		.data = &rk3588_data,
-- 
2.25.1

[PATCH v2 3/3] nvmem: rockchip-otp: Add support for rk3562

[Kever Yang]

From: Finley Xiao <finley.xiao@rock-chips.com>

This adds the necessary data for handling otp on the rk3562.

Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v2: None

 drivers/nvmem/rockchip-otp.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/nvmem/rockchip-otp.c b/drivers/nvmem/rockchip-otp.c
index a04bce89ecc8..6f86cf8ec390 100644
--- a/drivers/nvmem/rockchip-otp.c
+++ b/drivers/nvmem/rockchip-otp.c
@@ -341,6 +341,17 @@ static const struct rockchip_data px30_data = {
 	.reg_read = px30_otp_read,
 };
 
+static const char * const rk3562_otp_clocks[] = {
+	"usr", "sbpi", "apb_pclk", "phy",
+};
+
+static const struct rockchip_data rk3562_data = {
+	.size = 0x80,
+	.clks = rk3562_otp_clocks,
+	.num_clks = ARRAY_SIZE(rk3562_otp_clocks),
+	.reg_read = rk3568_otp_read,
+};
+
 static const char * const rk3568_otp_clocks[] = {
 	"usr", "sbpi", "apb_pclk", "phy",
 };
@@ -372,6 +383,10 @@ static const struct of_device_id rockchip_otp_match[] = {
 		.compatible = "rockchip,rk3308-otp",
 		.data = &px30_data,
 	},
+	{
+		.compatible = "rockchip,rk3562-otp",
+		.data = &rk3562_data,
+	},
 	{
 		.compatible = "rockchip,rk3568-otp",
 		.data = &rk3568_data,
-- 
2.25.1

Re: [PATCH v2 1/3] dt-bindings: nvmem: rockchip,otp: Add support for rk3562 and rk3568

[Rob Herring (Arm)]

On Thu, 27 Feb 2025 19:08:02 +0800, Kever Yang wrote:
> Add compatible entry for the otp controller in rk3562 and rk3568, add schema
> for different clock names for new entry.
> 
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
> 
> Changes in v2:
> - Update the commit message and add maxItems in schema.
> 
>  .../bindings/nvmem/rockchip,otp.yaml          | 53 ++++++++++++++++---
>  1 file changed, 46 insertions(+), 7 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v2 1/3] dt-bindings: nvmem: rockchip,otp: Add support for rk3562 and rk3568

[Srinivas Kandagatla]

Hi Kever,


On 27/02/2025 11:08, Kever Yang wrote:
> Add compatible entry for the otp controller in rk3562 and rk3568, add schema
> for different clock names for new entry.
> 
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---

Am unable to apply this patch, Could you rebase it on top of linux-next 
or 
https://web.git.kernel.org/pub/scm/linux/kernel/git/srini/nvmem.git/log/?h=for-next


--srini

> 
> Changes in v2:
> - Update the commit message and add maxItems in schema.
> 
>   .../bindings/nvmem/rockchip,otp.yaml          | 53 ++++++++++++++++---
>   1 file changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml b/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml
> index a44d44b32809..7572f4a1d73b 100644
> --- a/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml
> @@ -14,6 +14,8 @@ properties:
>       enum:
>         - rockchip,px30-otp
>         - rockchip,rk3308-otp
> +      - rockchip,rk3562-otp
> +      - rockchip,rk3568-otp
>         - rockchip,rk3588-otp
>   
>     reg:
> @@ -25,19 +27,15 @@ properties:
>   
>     clock-names:
>       minItems: 3
> -    items:
> -      - const: otp
> -      - const: apb_pclk
> -      - const: phy
> -      - const: arb
> +    maxItems: 4
>   
>     resets:
>       minItems: 1
> -    maxItems: 3
> +    maxItems: 4
>   
>     reset-names:
>       minItems: 1
> -    maxItems: 3
> +    maxItems: 4
>   
>   required:
>     - compatible
> @@ -62,12 +60,45 @@ allOf:
>         properties:
>           clocks:
>             maxItems: 3
> +        clock-names:
> +          items:
> +            - const: otp
> +            - const: apb_pclk
> +            - const: phy
>           resets:
>             maxItems: 1
>           reset-names:
>             items:
>               - const: phy
>   
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - rockchip,rk3562-otp
> +              - rockchip,rk3568-otp
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 4
> +          maxItems: 4
> +        clock-names:
> +          items:
> +            - const: usr
> +            - const: sbpi
> +            - const: apb_pclk
> +            - const: phy
> +        resets:
> +          minItems: 4
> +          maxItems: 4
> +        reset-names:
> +          items:
> +            - const: usr
> +            - const: sbpi
> +            - const: apb
> +            - const: phy
> +
>     - if:
>         properties:
>           compatible:
> @@ -78,8 +109,16 @@ allOf:
>         properties:
>           clocks:
>             minItems: 4
> +          maxItems: 4
> +        clock-names:
> +          items:
> +            - const: otp
> +            - const: apb_pclk
> +            - const: phy
> +            - const: arb
>           resets:
>             minItems: 3
> +          maxItems: 3
>           reset-names:
>             items:
>               - const: otp

Re: [PATCH v2 3/3] nvmem: rockchip-otp: Add support for rk3562

[Jonas Karlman]

Hi Kever,

On 2025-02-27 12:08, Kever Yang wrote:
> From: Finley Xiao <finley.xiao@rock-chips.com>
> 
> This adds the necessary data for handling otp on the rk3562.
> 
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
> 
> Changes in v2: None
> 
>  drivers/nvmem/rockchip-otp.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/nvmem/rockchip-otp.c b/drivers/nvmem/rockchip-otp.c
> index a04bce89ecc8..6f86cf8ec390 100644
> --- a/drivers/nvmem/rockchip-otp.c
> +++ b/drivers/nvmem/rockchip-otp.c
> @@ -341,6 +341,17 @@ static const struct rockchip_data px30_data = {
>  	.reg_read = px30_otp_read,
>  };
>  
> +static const char * const rk3562_otp_clocks[] = {
> +	"usr", "sbpi", "apb_pclk", "phy",
> +};

These are the same clocks as used for rk3568 below.

> +
> +static const struct rockchip_data rk3562_data = {
> +	.size = 0x80,
> +	.clks = rk3562_otp_clocks,
> +	.num_clks = ARRAY_SIZE(rk3562_otp_clocks),
> +	.reg_read = rk3568_otp_read,
> +};
> +
>  static const char * const rk3568_otp_clocks[] = {
>  	"usr", "sbpi", "apb_pclk", "phy",
>  };
> @@ -372,6 +383,10 @@ static const struct of_device_id rockchip_otp_match[] = {
>  		.compatible = "rockchip,rk3308-otp",
>  		.data = &px30_data,
>  	},
> +	{
> +		.compatible = "rockchip,rk3562-otp",
> +		.data = &rk3562_data,

Here we can use rk3568_data directly without having to add a new
rk3562_data struct.

Regards,
Jonas

> +	},
>  	{
>  		.compatible = "rockchip,rk3568-otp",
>  		.data = &rk3568_data,

Re: [PATCH v2 2/3] nvmem: rockchip-otp: Add support for rk3568-otp

[Jonas Karlman]

Hi Kever,

On 2025-02-27 12:08, Kever Yang wrote:
> From: Finley Xiao <finley.xiao@rock-chips.com>
> 
> This adds the necessary data for handling efuse on the rk3568.
> 
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
> 
> Changes in v2: None
> 
>  drivers/nvmem/rockchip-otp.c | 82 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/drivers/nvmem/rockchip-otp.c b/drivers/nvmem/rockchip-otp.c
> index ebc3f0b24166..a04bce89ecc8 100644
> --- a/drivers/nvmem/rockchip-otp.c
> +++ b/drivers/nvmem/rockchip-otp.c
> @@ -27,6 +27,7 @@
>  #define OTPC_USER_CTRL			0x0100
>  #define OTPC_USER_ADDR			0x0104
>  #define OTPC_USER_ENABLE		0x0108
> +#define OTPC_USER_QP			0x0120
>  #define OTPC_USER_Q			0x0124
>  #define OTPC_INT_STATUS			0x0304
>  #define OTPC_SBPI_CMD0_OFFSET		0x1000
> @@ -53,6 +54,8 @@
>  #define SBPI_ENABLE_MASK		GENMASK(16, 16)
>  
>  #define OTPC_TIMEOUT			10000
> +#define OTPC_TIMEOUT_PROG		100000

This is not used anywhere in this patch, please drop it.

> +#define RK3568_NBYTES			2
>  
>  /* RK3588 Register */
>  #define RK3588_OTPC_AUTO_CTRL		0x04
> @@ -184,6 +187,70 @@ static int px30_otp_read(void *context, unsigned int offset,
>  	return ret;
>  }
>  
> +static int rk3568_otp_read(void *context, unsigned int offset, void *val,
> +			   size_t bytes)
> +{
> +	struct rockchip_otp *otp = context;
> +	unsigned int addr_start, addr_end, addr_offset, addr_len;
> +	unsigned int otp_qp;
> +	u32 out_value;
> +	u8 *buf;
> +	int ret = 0, i = 0;
> +
> +	addr_start = rounddown(offset, RK3568_NBYTES) / RK3568_NBYTES;
> +	addr_end = roundup(offset + bytes, RK3568_NBYTES) / RK3568_NBYTES;
> +	addr_offset = offset % RK3568_NBYTES;
> +	addr_len = addr_end - addr_start;
> +
> +	buf = kzalloc(array3_size(addr_len, RK3568_NBYTES, sizeof(*buf)),
> +		      GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	ret = rockchip_otp_reset(otp);
> +	if (ret) {
> +		dev_err(otp->dev, "failed to reset otp phy\n");
> +		return ret;

This is leaking the kzalloc memory above.

> +	}
> +
> +	ret = rockchip_otp_ecc_enable(otp, true);
> +	if (ret < 0) {
> +		dev_err(otp->dev, "rockchip_otp_ecc_enable err\n");
> +		return ret;

Same here.

> +	}
> +
> +	writel(OTPC_USE_USER | OTPC_USE_USER_MASK, otp->base + OTPC_USER_CTRL);
> +	udelay(5);
> +	while (addr_len--) {
> +		writel(addr_start++ | OTPC_USER_ADDR_MASK,
> +		       otp->base + OTPC_USER_ADDR);
> +		writel(OTPC_USER_FSM_ENABLE | OTPC_USER_FSM_ENABLE_MASK,
> +		       otp->base + OTPC_USER_ENABLE);
> +		ret = rockchip_otp_wait_status(otp, OTPC_INT_STATUS, OTPC_USER_DONE);
> +		if (ret < 0) {
> +			dev_err(otp->dev, "timeout during read setup\n");
> +			goto read_end;
> +		}
> +		otp_qp = readl(otp->base + OTPC_USER_QP);
> +		if (((otp_qp & 0xc0) == 0xc0) || (otp_qp & 0x20)) {
> +			ret = -EIO;
> +			dev_err(otp->dev, "ecc check error during read setup\n");
> +			goto read_end;
> +		}
> +		out_value = readl(otp->base + OTPC_USER_Q);
> +		memcpy(&buf[i], &out_value, RK3568_NBYTES);
> +		i += RK3568_NBYTES;
> +	}
> +
> +	memcpy(val, buf + addr_offset, bytes);
> +
> +read_end:
> +	writel(0x0 | OTPC_USE_USER_MASK, otp->base + OTPC_USER_CTRL);
> +	kfree(buf);
> +
> +	return ret;
> +}

This can be simplified if this is rebased on top of "nvmem: rockchip-otp:
Handle internal word_size in main reg_read op" [1].

[1] https://lore.kernel.org/r/20250316191900.1858944-1-jonas@kwiboo.se

Something like following could be squashed in with this:

diff --git a/drivers/nvmem/rockchip-otp.c b/drivers/nvmem/rockchip-otp.c
index ea48d51bc2ff..0991a4047bec 100644
--- a/drivers/nvmem/rockchip-otp.c
+++ b/drivers/nvmem/rockchip-otp.c
@@ -54,8 +54,6 @@
 #define SBPI_ENABLE_MASK		GENMASK(16, 16)
 
 #define OTPC_TIMEOUT			10000
-#define OTPC_TIMEOUT_PROG		100000
-#define RK3568_NBYTES			2
 
 /* RK3588 Register */
 #define RK3588_OTPC_AUTO_CTRL		0x04
@@ -188,24 +186,12 @@ static int px30_otp_read(void *context, unsigned int offset,
 }
 
 static int rk3568_otp_read(void *context, unsigned int offset, void *val,
-			   size_t bytes)
+			   size_t count)
 {
 	struct rockchip_otp *otp = context;
-	unsigned int addr_start, addr_end, addr_offset, addr_len;
-	unsigned int otp_qp;
-	u32 out_value;
-	u8 *buf;
-	int ret = 0, i = 0;
-
-	addr_start = rounddown(offset, RK3568_NBYTES) / RK3568_NBYTES;
-	addr_end = roundup(offset + bytes, RK3568_NBYTES) / RK3568_NBYTES;
-	addr_offset = offset % RK3568_NBYTES;
-	addr_len = addr_end - addr_start;
-
-	buf = kzalloc(array3_size(addr_len, RK3568_NBYTES, sizeof(*buf)),
-		      GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
+	u16 *buf = val;
+	u32 otp_qp;
+	int ret;
 
 	ret = rockchip_otp_reset(otp);
 	if (ret) {
@@ -214,39 +200,39 @@ static int rk3568_otp_read(void *context, unsigned int offset, void *val,
 	}
 
 	ret = rockchip_otp_ecc_enable(otp, true);
-	if (ret < 0) {
+	if (ret) {
 		dev_err(otp->dev, "rockchip_otp_ecc_enable err\n");
 		return ret;
 	}
 
 	writel(OTPC_USE_USER | OTPC_USE_USER_MASK, otp->base + OTPC_USER_CTRL);
 	udelay(5);
-	while (addr_len--) {
-		writel(addr_start++ | OTPC_USER_ADDR_MASK,
+
+	while (count--) {
+		writel(offset++ | OTPC_USER_ADDR_MASK,
 		       otp->base + OTPC_USER_ADDR);
 		writel(OTPC_USER_FSM_ENABLE | OTPC_USER_FSM_ENABLE_MASK,
 		       otp->base + OTPC_USER_ENABLE);
-		ret = rockchip_otp_wait_status(otp, OTPC_INT_STATUS, OTPC_USER_DONE);
-		if (ret < 0) {
+
+		ret = rockchip_otp_wait_status(otp, OTPC_INT_STATUS,
+					       OTPC_USER_DONE);
+		if (ret) {
 			dev_err(otp->dev, "timeout during read setup\n");
 			goto read_end;
 		}
+
 		otp_qp = readl(otp->base + OTPC_USER_QP);
 		if (((otp_qp & 0xc0) == 0xc0) || (otp_qp & 0x20)) {
 			ret = -EIO;
 			dev_err(otp->dev, "ecc check error during read setup\n");
 			goto read_end;
 		}
-		out_value = readl(otp->base + OTPC_USER_Q);
-		memcpy(&buf[i], &out_value, RK3568_NBYTES);
-		i += RK3568_NBYTES;
-	}
 
-	memcpy(val, buf + addr_offset, bytes);
+		*buf++ = readl(otp->base + OTPC_USER_Q);
+	}
 
 read_end:
 	writel(0x0 | OTPC_USE_USER_MASK, otp->base + OTPC_USER_CTRL);
-	kfree(buf);
 
 	return ret;
 }

> +
>  static int rk3588_otp_read(void *context, unsigned int offset,
>  			   void *val, size_t bytes)
>  {
> @@ -274,6 +341,17 @@ static const struct rockchip_data px30_data = {
>  	.reg_read = px30_otp_read,
>  };
>  
> +static const char * const rk3568_otp_clocks[] = {
> +	"usr", "sbpi", "apb_pclk", "phy",

Why do we change from using the "otp"-name for the main clock?

I suggest we keep the main clock named "otp" instead of "usr" for
consistency.

> +};
> +
> +static const struct rockchip_data rk3568_data = {
> +	.size = 0x80,

If this is rebased on top of [1] you should also add:

	.word_size = sizeof(u16),

Above suggested changes can also be found in a FIXUP commit at my
linux-rockchip tree of pending RK3528 patches [2].

[2] https://github.com/Kwiboo/linux-rockchip/commits/next-20250314-rk3528/

Regards,
Jonas

> +	.clks = rk3568_otp_clocks,
> +	.num_clks = ARRAY_SIZE(rk3568_otp_clocks),
> +	.reg_read = rk3568_otp_read,
> +};
> +
>  static const char * const rk3588_otp_clocks[] = {
>  	"otp", "apb_pclk", "phy", "arb",
>  };
> @@ -294,6 +372,10 @@ static const struct of_device_id rockchip_otp_match[] = {
>  		.compatible = "rockchip,rk3308-otp",
>  		.data = &px30_data,
>  	},
> +	{
> +		.compatible = "rockchip,rk3568-otp",
> +		.data = &rk3568_data,
> +	},
>  	{
>  		.compatible = "rockchip,rk3588-otp",
>  		.data = &rk3588_data,

Re: [PATCH v2 1/3] dt-bindings: nvmem: rockchip,otp: Add support for rk3562 and rk3568

[Jonas Karlman]

Hi Kever,

On 2025-02-27 12:08, Kever Yang wrote:
> Add compatible entry for the otp controller in rk3562 and rk3568, add schema
> for different clock names for new entry.
> 
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
> 
> Changes in v2:
> - Update the commit message and add maxItems in schema.
> 
>  .../bindings/nvmem/rockchip,otp.yaml          | 53 ++++++++++++++++---
>  1 file changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml b/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml
> index a44d44b32809..7572f4a1d73b 100644
> --- a/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/rockchip,otp.yaml
> @@ -14,6 +14,8 @@ properties:
>      enum:
>        - rockchip,px30-otp
>        - rockchip,rk3308-otp
> +      - rockchip,rk3562-otp
> +      - rockchip,rk3568-otp
>        - rockchip,rk3588-otp
>  
>    reg:
> @@ -25,19 +27,15 @@ properties:
>  
>    clock-names:
>      minItems: 3
> -    items:
> -      - const: otp
> -      - const: apb_pclk
> -      - const: phy
> -      - const: arb
> +    maxItems: 4
>  
>    resets:
>      minItems: 1
> -    maxItems: 3
> +    maxItems: 4
>  
>    reset-names:
>      minItems: 1
> -    maxItems: 3
> +    maxItems: 4

For clock-names above and reset-names we could possible skip the
min/max-items, e.g. something like:

  clock-names: true
  reset-names: true

and then only keep the items prop in the different conditions below.

>  
>  required:
>    - compatible
> @@ -62,12 +60,45 @@ allOf:
>        properties:
>          clocks:
>            maxItems: 3
> +        clock-names:
> +          items:
> +            - const: otp
> +            - const: apb_pclk
> +            - const: phy
>          resets:
>            maxItems: 1
>          reset-names:
>            items:
>              - const: phy
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - rockchip,rk3562-otp
> +              - rockchip,rk3568-otp
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 4
> +          maxItems: 4
> +        clock-names:
> +          items:
> +            - const: usr

Why do we not use the name "otp" here ? For px30 and rk3308 the clock is
named clk_otp_usr and we still use the "otp" name.

> +            - const: sbpi
> +            - const: apb_pclk
> +            - const: phy
> +        resets:
> +          minItems: 4
> +          maxItems: 4
> +        reset-names:
> +          items:
> +            - const: usr

Same here.

This needs to be rebased to also include rk3576 clock-names.

I prepared a FIXUP commit at [1] with the suggested changes, feel free
to squash any changes you agree with in a v3.

I am planing to send out a OTP series for RK3528, and that will depend
on next revision of this series :-)

[1] https://github.com/Kwiboo/linux-rockchip/commits/next-20250314-rk3528/

Regards,
Jonas

> +            - const: sbpi
> +            - const: apb
> +            - const: phy
> +
>    - if:
>        properties:
>          compatible:
> @@ -78,8 +109,16 @@ allOf:
>        properties:
>          clocks:
>            minItems: 4
> +          maxItems: 4
> +        clock-names:
> +          items:
> +            - const: otp
> +            - const: apb_pclk
> +            - const: phy
> +            - const: arb
>          resets:
>            minItems: 3
> +          maxItems: 3
>          reset-names:
>            items:
>              - const: otp

Re: [PATCH v2 2/3] nvmem: rockchip-otp: Add support for rk3568-otp

[Kever Yang]

Hi Jonas,

On 2025/3/17 05:52, Jonas Karlman wrote:
> Hi Kever,
>
> On 2025-02-27 12:08, Kever Yang wrote:
>> From: Finley Xiao <finley.xiao@rock-chips.com>
>>
>> This adds the necessary data for handling efuse on the rk3568.
>>
>> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>
>> Changes in v2: None
>>
>>   drivers/nvmem/rockchip-otp.c | 82 ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 82 insertions(+)
>>
>> diff --git a/drivers/nvmem/rockchip-otp.c b/drivers/nvmem/rockchip-otp.c
>> index ebc3f0b24166..a04bce89ecc8 100644
>> --- a/drivers/nvmem/rockchip-otp.c
>> +++ b/drivers/nvmem/rockchip-otp.c
>> @@ -27,6 +27,7 @@
>>   #define OTPC_USER_CTRL			0x0100
>>   #define OTPC_USER_ADDR			0x0104
>>   #define OTPC_USER_ENABLE		0x0108
>> +#define OTPC_USER_QP			0x0120
>>   #define OTPC_USER_Q			0x0124
>>   #define OTPC_INT_STATUS			0x0304
>>   #define OTPC_SBPI_CMD0_OFFSET		0x1000
>> @@ -53,6 +54,8 @@
>>   #define SBPI_ENABLE_MASK		GENMASK(16, 16)
>>   
>>   #define OTPC_TIMEOUT			10000
>> +#define OTPC_TIMEOUT_PROG		100000
> This is not used anywhere in this patch, please drop it.
>
>> +#define RK3568_NBYTES			2
>>   
>>   /* RK3588 Register */
>>   #define RK3588_OTPC_AUTO_CTRL		0x04
>> @@ -184,6 +187,70 @@ static int px30_otp_read(void *context, unsigned int offset,
>>   	return ret;
>>   }
>>   
>> +static int rk3568_otp_read(void *context, unsigned int offset, void *val,
>> +			   size_t bytes)
>> +{
>> +	struct rockchip_otp *otp = context;
>> +	unsigned int addr_start, addr_end, addr_offset, addr_len;
>> +	unsigned int otp_qp;
>> +	u32 out_value;
>> +	u8 *buf;
>> +	int ret = 0, i = 0;
>> +
>> +	addr_start = rounddown(offset, RK3568_NBYTES) / RK3568_NBYTES;
>> +	addr_end = roundup(offset + bytes, RK3568_NBYTES) / RK3568_NBYTES;
>> +	addr_offset = offset % RK3568_NBYTES;
>> +	addr_len = addr_end - addr_start;
>> +
>> +	buf = kzalloc(array3_size(addr_len, RK3568_NBYTES, sizeof(*buf)),
>> +		      GFP_KERNEL);
>> +	if (!buf)
>> +		return -ENOMEM;
>> +
>> +	ret = rockchip_otp_reset(otp);
>> +	if (ret) {
>> +		dev_err(otp->dev, "failed to reset otp phy\n");
>> +		return ret;
> This is leaking the kzalloc memory above.
>
>> +	}
>> +
>> +	ret = rockchip_otp_ecc_enable(otp, true);
>> +	if (ret < 0) {
>> +		dev_err(otp->dev, "rockchip_otp_ecc_enable err\n");
>> +		return ret;
> Same here.
>
>> +	}
>> +
>> +	writel(OTPC_USE_USER | OTPC_USE_USER_MASK, otp->base + OTPC_USER_CTRL);
>> +	udelay(5);
>> +	while (addr_len--) {
>> +		writel(addr_start++ | OTPC_USER_ADDR_MASK,
>> +		       otp->base + OTPC_USER_ADDR);
>> +		writel(OTPC_USER_FSM_ENABLE | OTPC_USER_FSM_ENABLE_MASK,
>> +		       otp->base + OTPC_USER_ENABLE);
>> +		ret = rockchip_otp_wait_status(otp, OTPC_INT_STATUS, OTPC_USER_DONE);
>> +		if (ret < 0) {
>> +			dev_err(otp->dev, "timeout during read setup\n");
>> +			goto read_end;
>> +		}
>> +		otp_qp = readl(otp->base + OTPC_USER_QP);
>> +		if (((otp_qp & 0xc0) == 0xc0) || (otp_qp & 0x20)) {
>> +			ret = -EIO;
>> +			dev_err(otp->dev, "ecc check error during read setup\n");
>> +			goto read_end;
>> +		}
>> +		out_value = readl(otp->base + OTPC_USER_Q);
>> +		memcpy(&buf[i], &out_value, RK3568_NBYTES);
>> +		i += RK3568_NBYTES;
>> +	}
>> +
>> +	memcpy(val, buf + addr_offset, bytes);
>> +
>> +read_end:
>> +	writel(0x0 | OTPC_USE_USER_MASK, otp->base + OTPC_USER_CTRL);
>> +	kfree(buf);
>> +
>> +	return ret;
>> +}
> This can be simplified if this is rebased on top of "nvmem: rockchip-otp:
> Handle internal word_size in main reg_read op" [1].
>
> [1] https://lore.kernel.org/r/20250316191900.1858944-1-jonas@kwiboo.se
This look good to me, I will do it.
>
> Something like following could be squashed in with this:
>
> diff --git a/drivers/nvmem/rockchip-otp.c b/drivers/nvmem/rockchip-otp.c
> index ea48d51bc2ff..0991a4047bec 100644
> --- a/drivers/nvmem/rockchip-otp.c
> +++ b/drivers/nvmem/rockchip-otp.c
> @@ -54,8 +54,6 @@
>   #define SBPI_ENABLE_MASK		GENMASK(16, 16)
>   
>   #define OTPC_TIMEOUT			10000
> -#define OTPC_TIMEOUT_PROG		100000
> -#define RK3568_NBYTES			2
>   
>   /* RK3588 Register */
>   #define RK3588_OTPC_AUTO_CTRL		0x04
> @@ -188,24 +186,12 @@ static int px30_otp_read(void *context, unsigned int offset,
>   }
>   
>   static int rk3568_otp_read(void *context, unsigned int offset, void *val,
> -			   size_t bytes)
> +			   size_t count)
>   {
>   	struct rockchip_otp *otp = context;
> -	unsigned int addr_start, addr_end, addr_offset, addr_len;
> -	unsigned int otp_qp;
> -	u32 out_value;
> -	u8 *buf;
> -	int ret = 0, i = 0;
> -
> -	addr_start = rounddown(offset, RK3568_NBYTES) / RK3568_NBYTES;
> -	addr_end = roundup(offset + bytes, RK3568_NBYTES) / RK3568_NBYTES;
> -	addr_offset = offset % RK3568_NBYTES;
> -	addr_len = addr_end - addr_start;
> -
> -	buf = kzalloc(array3_size(addr_len, RK3568_NBYTES, sizeof(*buf)),
> -		      GFP_KERNEL);
> -	if (!buf)
> -		return -ENOMEM;
> +	u16 *buf = val;
> +	u32 otp_qp;
> +	int ret;
>   
>   	ret = rockchip_otp_reset(otp);
>   	if (ret) {
> @@ -214,39 +200,39 @@ static int rk3568_otp_read(void *context, unsigned int offset, void *val,
>   	}
>   
>   	ret = rockchip_otp_ecc_enable(otp, true);
> -	if (ret < 0) {
> +	if (ret) {
>   		dev_err(otp->dev, "rockchip_otp_ecc_enable err\n");
>   		return ret;
>   	}
>   
>   	writel(OTPC_USE_USER | OTPC_USE_USER_MASK, otp->base + OTPC_USER_CTRL);
>   	udelay(5);
> -	while (addr_len--) {
> -		writel(addr_start++ | OTPC_USER_ADDR_MASK,
> +
> +	while (count--) {
> +		writel(offset++ | OTPC_USER_ADDR_MASK,
>   		       otp->base + OTPC_USER_ADDR);
>   		writel(OTPC_USER_FSM_ENABLE | OTPC_USER_FSM_ENABLE_MASK,
>   		       otp->base + OTPC_USER_ENABLE);
> -		ret = rockchip_otp_wait_status(otp, OTPC_INT_STATUS, OTPC_USER_DONE);
> -		if (ret < 0) {
> +
> +		ret = rockchip_otp_wait_status(otp, OTPC_INT_STATUS,
> +					       OTPC_USER_DONE);
> +		if (ret) {
>   			dev_err(otp->dev, "timeout during read setup\n");
>   			goto read_end;
>   		}
> +
>   		otp_qp = readl(otp->base + OTPC_USER_QP);
>   		if (((otp_qp & 0xc0) == 0xc0) || (otp_qp & 0x20)) {
>   			ret = -EIO;
>   			dev_err(otp->dev, "ecc check error during read setup\n");
>   			goto read_end;
>   		}
> -		out_value = readl(otp->base + OTPC_USER_Q);
> -		memcpy(&buf[i], &out_value, RK3568_NBYTES);
> -		i += RK3568_NBYTES;
> -	}
>   
> -	memcpy(val, buf + addr_offset, bytes);
> +		*buf++ = readl(otp->base + OTPC_USER_Q);
> +	}
>   
>   read_end:
>   	writel(0x0 | OTPC_USE_USER_MASK, otp->base + OTPC_USER_CTRL);
> -	kfree(buf);
>   
>   	return ret;
>   }
>
>> +
>>   static int rk3588_otp_read(void *context, unsigned int offset,
>>   			   void *val, size_t bytes)
>>   {
>> @@ -274,6 +341,17 @@ static const struct rockchip_data px30_data = {
>>   	.reg_read = px30_otp_read,
>>   };
>>   
>> +static const char * const rk3568_otp_clocks[] = {
>> +	"usr", "sbpi", "apb_pclk", "phy",
> Why do we change from using the "otp"-name for the main clock?
>
> I suggest we keep the main clock named "otp" instead of "usr" for
> consistency.

The name is from the hardware design, it follows the hardware naming by 
default.

To be honest the driver does not care the clock/reset naming and the 
sequence at all,

and the clock/reset name and number are keep changing due to the 
controller IP

may come from different vendor or process change.

I can rename the "usr" to "otp" to make it looks better, and also 
reorder it so like it

look like rk3588.


Thanks,
- Kever
>> +};
>> +
>> +static const struct rockchip_data rk3568_data = {
>> +	.size = 0x80,
> If this is rebased on top of [1] you should also add:
>
> 	.word_size = sizeof(u16),
>
> Above suggested changes can also be found in a FIXUP commit at my
> linux-rockchip tree of pending RK3528 patches [2].
>
> [2] https://github.com/Kwiboo/linux-rockchip/commits/next-20250314-rk3528/
>
> Regards,
> Jonas
>
>> +	.clks = rk3568_otp_clocks,
>> +	.num_clks = ARRAY_SIZE(rk3568_otp_clocks),
>> +	.reg_read = rk3568_otp_read,
>> +};
>> +
>>   static const char * const rk3588_otp_clocks[] = {
>>   	"otp", "apb_pclk", "phy", "arb",
>>   };
>> @@ -294,6 +372,10 @@ static const struct of_device_id rockchip_otp_match[] = {
>>   		.compatible = "rockchip,rk3308-otp",
>>   		.data = &px30_data,
>>   	},
>> +	{
>> +		.compatible = "rockchip,rk3568-otp",
>> +		.data = &rk3568_data,
>> +	},
>>   	{
>>   		.compatible = "rockchip,rk3588-otp",
>>   		.data = &rk3588_data,
>