[1/6] rockchip: efuse: Add support for rk3288 efuse
diff mbox series

Message ID 20200220031055.5407-2-finley.xiao@rock-chips.com
State Changes Requested
Delegated to: Kever Yang
Headers show
Series
  • rockchip: efuse: add support for more paltforms
Related show

Commit Message

Finley Xiao Feb. 20, 2020, 3:10 a.m. UTC
This adds the necessary data for handling eFuse on the rk3288.

Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
---
 drivers/misc/rockchip-efuse.c | 76 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 4 deletions(-)

Comments

Kever Yang March 2, 2020, 9:41 a.m. UTC | #1
Hi Finley,

     Thanks for your patches, and see below commends.

On 2020/2/20 上午11:10, Finley Xiao wrote:
> This adds the necessary data for handling eFuse on the rk3288.
>
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> ---
>   drivers/misc/rockchip-efuse.c | 76 ++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/misc/rockchip-efuse.c b/drivers/misc/rockchip-efuse.c
> index 2520c6a38e..f01d877e33 100644
> --- a/drivers/misc/rockchip-efuse.c
> +++ b/drivers/misc/rockchip-efuse.c
> @@ -15,6 +15,15 @@
>   #include <linux/delay.h>
>   #include <misc.h>
>   
> +#define RK3288_A_SHIFT          6
> +#define RK3288_A_MASK           0x3ff
> +#define RK3288_NFUSES           32
> +#define RK3288_BYTES_PER_FUSE   1
> +#define RK3288_PGENB            BIT(3)
> +#define RK3288_LOAD             BIT(2)
> +#define RK3288_STROBE           BIT(1)
> +#define RK3288_CSB              BIT(0)
> +
>   #define RK3399_A_SHIFT          16
>   #define RK3399_A_MASK           0x3ff
>   #define RK3399_NFUSES           32
> @@ -27,6 +36,9 @@
>   #define RK3399_STROBE           BIT(1)
>   #define RK3399_CSB              BIT(0)
>   
> +typedef int (*EFUSE_READ)(struct udevice *dev, int offset, void *buf,
> +			  int size);
> +
>   struct rockchip_efuse_regs {
>   	u32 ctrl;      /* 0x00  efuse control register */
>   	u32 dout;      /* 0x04  efuse data out register */
> @@ -53,7 +65,7 @@ static int dump_efuses(cmd_tbl_t *cmdtp, int flag,
>   	 */
>   
>   	struct udevice *dev;
> -	u8 fuses[128];
> +	u8 fuses[128] = {0};
>   	int ret;
>   
>   	/* retrieve the device */
> @@ -77,12 +89,55 @@ static int dump_efuses(cmd_tbl_t *cmdtp, int flag,
>   }
>   
>   U_BOOT_CMD(
> -	rk3399_dump_efuses, 1, 1, dump_efuses,
> +	rockchip_dump_efuses, 1, 1, dump_efuses,
>   	"Dump the content of the efuses",
>   	""
>   );
>   #endif
>   
> +static int rockchip_rk3288_efuse_read(struct udevice *dev, int offset,


Could you use CONFIG_ROCKCHIP_RK3288 to quote the functions for rk3288 only?

Same requirement for other patches.


Thanks,

- Kever

> +				      void *buf, int size)
> +{
> +	struct rockchip_efuse_platdata *plat = dev_get_platdata(dev);
> +	struct rockchip_efuse_regs *efuse =
> +		(struct rockchip_efuse_regs *)plat->base;
> +	u8 *buffer = buf;
> +	int max_size = RK3288_NFUSES * RK3288_BYTES_PER_FUSE;
> +
> +	if (size > (max_size - offset))
> +		size = max_size - offset;
> +
> +	/* Switch to read mode */
> +	writel(RK3288_LOAD | RK3288_PGENB, &efuse->ctrl);
> +	udelay(1);
> +
> +	while (size--) {
> +		writel(readl(&efuse->ctrl) &
> +			     (~(RK3288_A_MASK << RK3288_A_SHIFT)),
> +			     &efuse->ctrl);
> +		/* set addr */
> +		writel(readl(&efuse->ctrl) |
> +			     ((offset++ & RK3288_A_MASK) << RK3288_A_SHIFT),
> +			     &efuse->ctrl);
> +		udelay(1);
> +		/* strobe low to high */
> +		writel(readl(&efuse->ctrl) |
> +			     RK3288_STROBE, &efuse->ctrl);
> +		ndelay(60);
> +		/* read data */
> +		*buffer++ = readl(&efuse->dout);
> +		/* reset strobe to low */
> +		writel(readl(&efuse->ctrl) &
> +			     (~RK3288_STROBE), &efuse->ctrl);
> +		udelay(1);
> +	}
> +
> +	/* Switch to standby mode */
> +	writel(RK3288_PGENB | RK3288_CSB, &efuse->ctrl);
> +
> +	return 0;
> +}
> +
>   static int rockchip_rk3399_efuse_read(struct udevice *dev, int offset,
>   				      void *buf, int size)
>   {
> @@ -130,7 +185,13 @@ static int rockchip_rk3399_efuse_read(struct udevice *dev, int offset,
>   static int rockchip_efuse_read(struct udevice *dev, int offset,
>   			       void *buf, int size)
>   {
> -	return rockchip_rk3399_efuse_read(dev, offset, buf, size);
> +	EFUSE_READ efuse_read = NULL;
> +
> +	efuse_read = (EFUSE_READ)dev_get_driver_data(dev);
> +	if (!efuse_read)
> +		return -EINVAL;
> +
> +	return (*efuse_read)(dev, offset, buf, size);
>   }
>   
>   static const struct misc_ops rockchip_efuse_ops = {
> @@ -146,7 +207,14 @@ static int rockchip_efuse_ofdata_to_platdata(struct udevice *dev)
>   }
>   
>   static const struct udevice_id rockchip_efuse_ids[] = {
> -	{ .compatible = "rockchip,rk3399-efuse" },
> +	{
> +		.compatible = "rockchip,rk3288-efuse",
> +		.data = (ulong)&rockchip_rk3288_efuse_read,
> +	},
> +	{
> +		.compatible = "rockchip,rk3399-efuse",
> +		.data = (ulong)&rockchip_rk3399_efuse_read,
> +	},
>   	{}
>   };
>
Philipp Tomsich March 2, 2020, 10:10 a.m. UTC | #2
> On 20.02.2020, at 04:10, Finley Xiao <finley.xiao@rock-chips.com> wrote:
> 
> This adds the necessary data for handling eFuse on the rk3288.
> 
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

Major rework required: see below.

> ---
> drivers/misc/rockchip-efuse.c | 76 ++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 72 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/misc/rockchip-efuse.c b/drivers/misc/rockchip-efuse.c
> index 2520c6a38e..f01d877e33 100644
> --- a/drivers/misc/rockchip-efuse.c
> +++ b/drivers/misc/rockchip-efuse.c
> @@ -15,6 +15,15 @@
> #include <linux/delay.h>
> #include <misc.h>
> 
> +#define RK3288_A_SHIFT          6
> +#define RK3288_A_MASK           0x3ff
> +#define RK3288_NFUSES           32
> +#define RK3288_BYTES_PER_FUSE   1
> +#define RK3288_PGENB            BIT(3)
> +#define RK3288_LOAD             BIT(2)
> +#define RK3288_STROBE           BIT(1)
> +#define RK3288_CSB              BIT(0)
> +
> #define RK3399_A_SHIFT          16
> #define RK3399_A_MASK           0x3ff
> #define RK3399_NFUSES           32
> @@ -27,6 +36,9 @@
> #define RK3399_STROBE           BIT(1)
> #define RK3399_CSB              BIT(0)
> 
> +typedef int (*EFUSE_READ)(struct udevice *dev, int offset, void *buf,
> +			  int size);
> +
> struct rockchip_efuse_regs {
> 	u32 ctrl;      /* 0x00  efuse control register */
> 	u32 dout;      /* 0x04  efuse data out register */
> @@ -53,7 +65,7 @@ static int dump_efuses(cmd_tbl_t *cmdtp, int flag,
> 	 */
> 
> 	struct udevice *dev;
> -	u8 fuses[128];
> +	u8 fuses[128] = {0};
> 	int ret;
> 
> 	/* retrieve the device */
> @@ -77,12 +89,55 @@ static int dump_efuses(cmd_tbl_t *cmdtp, int flag,
> }
> 
> U_BOOT_CMD(
> -	rk3399_dump_efuses, 1, 1, dump_efuses,
> +	rockchip_dump_efuses, 1, 1, dump_efuses,
> 	"Dump the content of the efuses",
> 	""
> );
> #endif
> 
> +static int rockchip_rk3288_efuse_read(struct udevice *dev, int offset,
> +				      void *buf, int size)
> +{
> +	struct rockchip_efuse_platdata *plat = dev_get_platdata(dev);
> +	struct rockchip_efuse_regs *efuse =
> +		(struct rockchip_efuse_regs *)plat->base;
> +	u8 *buffer = buf;
> +	int max_size = RK3288_NFUSES * RK3288_BYTES_PER_FUSE;
> +
> +	if (size > (max_size - offset))
> +		size = max_size - offset;
> +
> +	/* Switch to read mode */
> +	writel(RK3288_LOAD | RK3288_PGENB, &efuse->ctrl);
> +	udelay(1);
> +
> +	while (size--) {
> +		writel(readl(&efuse->ctrl) &
> +			     (~(RK3288_A_MASK << RK3288_A_SHIFT)),
> +			     &efuse->ctrl);
> +		/* set addr */
> +		writel(readl(&efuse->ctrl) |
> +			     ((offset++ & RK3288_A_MASK) << RK3288_A_SHIFT),
> +			     &efuse->ctrl);
> +		udelay(1);
> +		/* strobe low to high */
> +		writel(readl(&efuse->ctrl) |
> +			     RK3288_STROBE, &efuse->ctrl);
> +		ndelay(60);
> +		/* read data */
> +		*buffer++ = readl(&efuse->dout);
> +		/* reset strobe to low */
> +		writel(readl(&efuse->ctrl) &
> +			     (~RK3288_STROBE), &efuse->ctrl);
> +		udelay(1);
> +	}
> +
> +	/* Switch to standby mode */
> +	writel(RK3288_PGENB | RK3288_CSB, &efuse->ctrl);
> +
> +	return 0;
> +}
> +
> static int rockchip_rk3399_efuse_read(struct udevice *dev, int offset,
> 				      void *buf, int size)
> {
> @@ -130,7 +185,13 @@ static int rockchip_rk3399_efuse_read(struct udevice *dev, int offset,
> static int rockchip_efuse_read(struct udevice *dev, int offset,
> 			       void *buf, int size)
> {
> -	return rockchip_rk3399_efuse_read(dev, offset, buf, size);
> +	EFUSE_READ efuse_read = NULL;
> +
> +	efuse_read = (EFUSE_READ)dev_get_driver_data(dev);
> +	if (!efuse_read)
> +		return -EINVAL;
> +
> +	return (*efuse_read)(dev, offset, buf, size);
> }
> 
> static const struct misc_ops rockchip_efuse_ops = {
> @@ -146,7 +207,14 @@ static int rockchip_efuse_ofdata_to_platdata(struct udevice *dev)
> }
> 
> static const struct udevice_id rockchip_efuse_ids[] = {
> -	{ .compatible = "rockchip,rk3399-efuse" },
> +	{
> +		.compatible = "rockchip,rk3288-efuse",
> +		.data = (ulong)&rockchip_rk3288_efuse_read,
> +	},

It doesn’t make sense to share this in a single driver if the only op that is implemented
is redirected through the driver-data.  The driver-data is to encapsulate small parameters (e.g. changed offsets), but not to override the function pointers for the major ops.

Please split into a separate driver that uses an appropriate misc_ops structure and points the read function to your rockchip_rk3288_efuse_read. I would recommend to even have this in a separate file, unless we can share infrastructure.

Note that I have a an extended version of the driver that supports writing fuses in one of our branches—imagine the mess that we’ll create if we have reading and writing supported and go through the driver-data everytime to dispatch via functon pointers.

Thanks,
Philipp.

> +	{
> +		.compatible = "rockchip,rk3399-efuse",
> +		.data = (ulong)&rockchip_rk3399_efuse_read,
> +	},
> 	{}
> };
> 
> -- 
> 2.11.0
> 
> 
>

Patch
diff mbox series

diff --git a/drivers/misc/rockchip-efuse.c b/drivers/misc/rockchip-efuse.c
index 2520c6a38e..f01d877e33 100644
--- a/drivers/misc/rockchip-efuse.c
+++ b/drivers/misc/rockchip-efuse.c
@@ -15,6 +15,15 @@ 
 #include <linux/delay.h>
 #include <misc.h>
 
+#define RK3288_A_SHIFT          6
+#define RK3288_A_MASK           0x3ff
+#define RK3288_NFUSES           32
+#define RK3288_BYTES_PER_FUSE   1
+#define RK3288_PGENB            BIT(3)
+#define RK3288_LOAD             BIT(2)
+#define RK3288_STROBE           BIT(1)
+#define RK3288_CSB              BIT(0)
+
 #define RK3399_A_SHIFT          16
 #define RK3399_A_MASK           0x3ff
 #define RK3399_NFUSES           32
@@ -27,6 +36,9 @@ 
 #define RK3399_STROBE           BIT(1)
 #define RK3399_CSB              BIT(0)
 
+typedef int (*EFUSE_READ)(struct udevice *dev, int offset, void *buf,
+			  int size);
+
 struct rockchip_efuse_regs {
 	u32 ctrl;      /* 0x00  efuse control register */
 	u32 dout;      /* 0x04  efuse data out register */
@@ -53,7 +65,7 @@  static int dump_efuses(cmd_tbl_t *cmdtp, int flag,
 	 */
 
 	struct udevice *dev;
-	u8 fuses[128];
+	u8 fuses[128] = {0};
 	int ret;
 
 	/* retrieve the device */
@@ -77,12 +89,55 @@  static int dump_efuses(cmd_tbl_t *cmdtp, int flag,
 }
 
 U_BOOT_CMD(
-	rk3399_dump_efuses, 1, 1, dump_efuses,
+	rockchip_dump_efuses, 1, 1, dump_efuses,
 	"Dump the content of the efuses",
 	""
 );
 #endif
 
+static int rockchip_rk3288_efuse_read(struct udevice *dev, int offset,
+				      void *buf, int size)
+{
+	struct rockchip_efuse_platdata *plat = dev_get_platdata(dev);
+	struct rockchip_efuse_regs *efuse =
+		(struct rockchip_efuse_regs *)plat->base;
+	u8 *buffer = buf;
+	int max_size = RK3288_NFUSES * RK3288_BYTES_PER_FUSE;
+
+	if (size > (max_size - offset))
+		size = max_size - offset;
+
+	/* Switch to read mode */
+	writel(RK3288_LOAD | RK3288_PGENB, &efuse->ctrl);
+	udelay(1);
+
+	while (size--) {
+		writel(readl(&efuse->ctrl) &
+			     (~(RK3288_A_MASK << RK3288_A_SHIFT)),
+			     &efuse->ctrl);
+		/* set addr */
+		writel(readl(&efuse->ctrl) |
+			     ((offset++ & RK3288_A_MASK) << RK3288_A_SHIFT),
+			     &efuse->ctrl);
+		udelay(1);
+		/* strobe low to high */
+		writel(readl(&efuse->ctrl) |
+			     RK3288_STROBE, &efuse->ctrl);
+		ndelay(60);
+		/* read data */
+		*buffer++ = readl(&efuse->dout);
+		/* reset strobe to low */
+		writel(readl(&efuse->ctrl) &
+			     (~RK3288_STROBE), &efuse->ctrl);
+		udelay(1);
+	}
+
+	/* Switch to standby mode */
+	writel(RK3288_PGENB | RK3288_CSB, &efuse->ctrl);
+
+	return 0;
+}
+
 static int rockchip_rk3399_efuse_read(struct udevice *dev, int offset,
 				      void *buf, int size)
 {
@@ -130,7 +185,13 @@  static int rockchip_rk3399_efuse_read(struct udevice *dev, int offset,
 static int rockchip_efuse_read(struct udevice *dev, int offset,
 			       void *buf, int size)
 {
-	return rockchip_rk3399_efuse_read(dev, offset, buf, size);
+	EFUSE_READ efuse_read = NULL;
+
+	efuse_read = (EFUSE_READ)dev_get_driver_data(dev);
+	if (!efuse_read)
+		return -EINVAL;
+
+	return (*efuse_read)(dev, offset, buf, size);
 }
 
 static const struct misc_ops rockchip_efuse_ops = {
@@ -146,7 +207,14 @@  static int rockchip_efuse_ofdata_to_platdata(struct udevice *dev)
 }
 
 static const struct udevice_id rockchip_efuse_ids[] = {
-	{ .compatible = "rockchip,rk3399-efuse" },
+	{
+		.compatible = "rockchip,rk3288-efuse",
+		.data = (ulong)&rockchip_rk3288_efuse_read,
+	},
+	{
+		.compatible = "rockchip,rk3399-efuse",
+		.data = (ulong)&rockchip_rk3399_efuse_read,
+	},
 	{}
 };