[PATCH 1/6] rockchip: efuse: Add support for rk3288 efuse

[Philipp Tomsich]

> On 20.02.2020, at 04:10, Finley Xiao <finley.xiao at rock-chips.com> wrote:
> 
> This adds the necessary data for handling eFuse on the rk3288.
> 
> Signed-off-by: Finley Xiao <finley.xiao at rock-chips.com>

Reviewed-by: Philipp Tomsich <philipp.tomsich at 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
> 
> 
>