| 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 | expand |
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, > + }, > {} > }; >
> 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 > > >
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, + }, {} };
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(-)