On Fri, Jan 24, 2020 at 11:21 AM Pragnesh Patel <pragnesh.pa...@sifive.com> wrote: > > Added a misc driver to handle OTP memory in FU540. > > Signed-off-by: Pragnesh Patel <pragnesh.pa...@sifive.com> > Reviewed-by: Anup Patel <anup.pa...@wdc.com> > --- > arch/riscv/dts/fu540-c000-u-boot.dtsi | 13 ++ > .../dts/hifive-unleashed-a00-u-boot.dtsi | 6 + > board/sifive/fu540/fu540.c | 113 ++++------ > configs/sifive_fu540_defconfig | 2 + > drivers/misc/Kconfig | 7 + > drivers/misc/Makefile | 1 + > drivers/misc/ememory-otp.c | 207 ++++++++++++++++++
This patch need to break into 1. add sifive otp driver 2. enable the otp driver - board, dts, defconfig changes. > 7 files changed, 276 insertions(+), 73 deletions(-) > create mode 100644 arch/riscv/dts/fu540-c000-u-boot.dtsi > create mode 100644 arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > create mode 100644 drivers/misc/ememory-otp.c > > diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi > b/arch/riscv/dts/fu540-c000-u-boot.dtsi > new file mode 100644 > index 0000000000..615a68c0e9 > --- /dev/null > +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi > @@ -0,0 +1,13 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * (C) Copyright 2019 SiFive, Inc > + */ > + > +/ { > + soc { > + otp: otp@10070000 { > + compatible = "sifive,fu540-otp"; > + reg = <0x0 0x10070000 0x0 0x0FFF>; > + }; > + }; > +}; > diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > new file mode 100644 > index 0000000000..bec0d19134 > --- /dev/null > +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > @@ -0,0 +1,6 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2019 SiFive, Inc > + */ > + > +#include "fu540-c000-u-boot.dtsi" > diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c > index 47a2090251..3a5e74f1fb 100644 > --- a/board/sifive/fu540/fu540.c > +++ b/board/sifive/fu540/fu540.c > @@ -10,94 +10,61 @@ > #include <dm.h> > #include <linux/delay.h> > #include <linux/io.h> > +#include <misc.h> > > -#ifdef CONFIG_MISC_INIT_R > - > -#define FU540_OTP_BASE_ADDR 0x10070000 > - > -struct fu540_otp_regs { > - u32 pa; /* Address input */ > - u32 paio; /* Program address input */ > - u32 pas; /* Program redundancy cell selection input */ > - u32 pce; /* OTP Macro enable input */ > - u32 pclk; /* Clock input */ > - u32 pdin; /* Write data input */ > - u32 pdout; /* Read data output */ > - u32 pdstb; /* Deep standby mode enable input (active low) */ > - u32 pprog; /* Program mode enable input */ > - u32 ptc; /* Test column enable input */ > - u32 ptm; /* Test mode enable input */ > - u32 ptm_rep;/* Repair function test mode enable input */ > - u32 ptr; /* Test row enable input */ > - u32 ptrim; /* Repair function enable input */ > - u32 pwe; /* Write enable input (defines program cycle) */ > -} __packed; > - > -#define BYTES_PER_FUSE 4 > -#define NUM_FUSES 0x1000 > - > -static int fu540_otp_read(int offset, void *buf, int size) > -{ > - struct fu540_otp_regs *regs = (void __iomem *)FU540_OTP_BASE_ADDR; > - unsigned int i; > - int fuseidx = offset / BYTES_PER_FUSE; > - int fusecount = size / BYTES_PER_FUSE; > - u32 fusebuf[fusecount]; > - > - /* check bounds */ > - if (offset < 0 || size < 0) > - return -EINVAL; > - if (fuseidx >= NUM_FUSES) > - return -EINVAL; > - if ((fuseidx + fusecount) > NUM_FUSES) > - return -EINVAL; > - > - /* init OTP */ > - writel(0x01, ®s->pdstb); /* wake up from stand-by */ > - writel(0x01, ®s->ptrim); /* enable repair function */ > - writel(0x01, ®s->pce); /* enable input */ > - > - /* read all requested fuses */ > - for (i = 0; i < fusecount; i++, fuseidx++) { > - writel(fuseidx, ®s->pa); > - > - /* cycle clock to read */ > - writel(0x01, ®s->pclk); > - mdelay(1); > - writel(0x00, ®s->pclk); > - mdelay(1); > - > - /* read the value */ > - fusebuf[i] = readl(®s->pdout); > - } > - > - /* shut down */ > - writel(0, ®s->pce); > - writel(0, ®s->ptrim); > - writel(0, ®s->pdstb); > - > - /* copy out */ > - memcpy(buf, fusebuf, size); > +/* > + * This define is a value used for error/unknown serial. > + * If we really care about distinguishing errors and 0 is > + * valid, we'll need a different one. > + */ > +#define ERROR_READING_SERIAL_NUMBER 0 > > - return 0; > -} > +#ifdef CONFIG_MISC_INIT_R > > -static u32 fu540_read_serialnum(void) > +#if CONFIG_IS_ENABLED(EMEMORY_OTP) > +static u32 otp_read_serialnum(struct udevice *dev) > { > int ret; > u32 serial[2] = {0}; > > for (int i = 0xfe * 4; i > 0; i -= 8) { > - ret = fu540_otp_read(i, serial, sizeof(serial)); > + ret = misc_read(dev, i, serial, sizeof(serial)); > + > if (ret) { > - printf("%s: error reading from OTP\n", __func__); > + printf("%s: error reading serial from OTP\n", > __func__); > break; > } > + > if (serial[0] == ~serial[1]) > return serial[0]; > } > > - return 0; > + return ERROR_READING_SERIAL_NUMBER; > +} > +#endif > + > +static u32 fu540_read_serialnum(void) > +{ > + u32 serial = ERROR_READING_SERIAL_NUMBER; > + > +#if CONFIG_IS_ENABLED(EMEMORY_OTP) > + struct udevice *dev; > + int ret; > + > + // init OTP > + ret = uclass_get_device_by_driver(UCLASS_MISC, > + DM_GET_DRIVER(hifive_otp), &dev); > + > + if (ret) { > + debug("%s: could not find otp device\n", __func__); > + return serial; > + } > + > + // read serial from OTP and set env var > + serial = otp_read_serialnum(dev); > +#endif > + > + return serial; > } > > static void fu540_setup_macaddr(u32 serialnum) > diff --git a/configs/sifive_fu540_defconfig b/configs/sifive_fu540_defconfig > index 6d61e6c960..40e78f12a2 100644 > --- a/configs/sifive_fu540_defconfig > +++ b/configs/sifive_fu540_defconfig > @@ -6,6 +6,8 @@ CONFIG_ARCH_RV64I=y > CONFIG_RISCV_SMODE=y > CONFIG_DISTRO_DEFAULTS=y > CONFIG_FIT=y > +CONFIG_MISC=y > +CONFIG_EMEMORY_OTP=y Look like this opt is needed global to SiFive, If so select MISC in arch Kconfig. > CONFIG_MISC_INIT_R=y > CONFIG_DISPLAY_CPUINFO=y > CONFIG_DISPLAY_BOARDINFO=y > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index f18aa8f7ba..cbda0deb14 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -68,6 +68,13 @@ config ROCKCHIP_OTP > addressing and a length or through child-nodes that are generated > based on the e-fuse map retrieved from the DTS. > > +config EMEMORY_OTP What about SIFIVE_OTP or SIFIVE_EMEM_OTP or similar? > + bool "Sifive Ememory OTP driver" > + depends on RISCV && MISC > + help > + Enable support for reading the Ememory OTP on the HiFive Unleashed > + OTP storage. > + > config VEXPRESS_CONFIG > bool "Enable support for Arm Versatile Express config bus" > depends on MISC > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index 2b843de93c..dcf9b628c8 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -58,6 +58,7 @@ obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o > obj-$(CONFIG_QFW) += qfw.o > obj-$(CONFIG_ROCKCHIP_EFUSE) += rockchip-efuse.o > obj-$(CONFIG_ROCKCHIP_OTP) += rockchip-otp.o > +obj-$(CONFIG_EMEMORY_OTP) += ememory-otp.o > obj-$(CONFIG_SANDBOX) += syscon_sandbox.o misc_sandbox.o > obj-$(CONFIG_SMSC_LPC47M) += smsc_lpc47m.o > obj-$(CONFIG_SMSC_SIO1007) += smsc_sio1007.o > diff --git a/drivers/misc/ememory-otp.c b/drivers/misc/ememory-otp.c > new file mode 100644 > index 0000000000..73e7af496a > --- /dev/null > +++ b/drivers/misc/ememory-otp.c Again file name, would have platform or soc naming convention. > @@ -0,0 +1,207 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * This is a driver for the eMemory EG004K32TQ028XW01 NeoFuse > + * One-Time-Programmable (OTP) memory used within the SiFive FU540. > + * It is documented in the FU540 manual here: > + * https://www.sifive.com/documentation/chips/freedom-u540-c000-manual/ > + * > + * Copyright (C) 2018 Philipp Hug <phil...@hug.cx> > + * Copyright (C) 2018 Joey Hewitt <j...@joeyhewitt.com> > + * > + * Copyright (C) 2020 SiFive, Inc > + */ > + > +/* > + * The FU540 stores 4096x32 bit (16KiB) values. > + * Index 0x00-0xff are reserved for SiFive internal use. (first 1KiB) > + */ Sorry, I didn't understand this comments. If it is memory map detail that occupy otp, try to elaborate more by giving full details. Would helpful to understand anyone in future. > + > +#include <common.h> > +#include <dm/device.h> > +#include <dm/read.h> > +#include <linux/io.h> > +#include <misc.h> > + > +struct hifive_otp_regs { > + u32 pa; /* Address input */ > + u32 paio; /* Program address input */ > + u32 pas; /* Program redundancy cell selection input */ > + u32 pce; /* OTP Macro enable input */ > + u32 pclk; /* Clock input */ > + u32 pdin; /* Write data input */ > + u32 pdout; /* Read data output */ > + u32 pdstb; /* Deep standby mode enable input (active low) */ > + u32 pprog; /* Program mode enable input */ > + u32 ptc; /* Test column enable input */ > + u32 ptm; /* Test mode enable input */ > + u32 ptm_rep;/* Repair function test mode enable input */ > + u32 ptr; /* Test row enable input */ > + u32 ptrim; /* Repair function enable input */ > + u32 pwe; /* Write enable input (defines program cycle) */ > +} __packed; > + > +struct hifive_otp_platdata { hifive here represent the board name, butter use soc name that has this otp. > + struct hifive_otp_regs __iomem *regs; > +}; > + > +typedef u32 fuse_value_t; No typdef please. No global variables(unless it really need) > +#define BYTES_PER_FUSE 4 > + > +#define NUM_FUSES 0x1000 May be use dt properties. > + > +/* > + * offset and size are assumed aligned to the size of the fuses (32bit). > + */ > +static int hifive_otp_read(struct udevice *dev, int offset, > + void *buf, int size) > +{ > + struct hifive_otp_platdata *plat = dev_get_platdata(dev); > + struct hifive_otp_regs *regs = (struct hifive_otp_regs *)plat->regs; > + > + int fuseidx = offset / BYTES_PER_FUSE; > + int fusecount = size / BYTES_PER_FUSE; > + fuse_value_t fusebuf[fusecount]; > + > + // check bounds use proper comment style. > + if (offset < 0 || size < 0) > + return -EINVAL; > + if (fuseidx >= NUM_FUSES) > + return -EINVAL; > + if ((fuseidx + fusecount) > NUM_FUSES) > + return -EINVAL; > + > + // init OTP > + iowrite32(0x01, ®s->pdstb); // wake up from stand-by > + iowrite32(0x01, ®s->ptrim); // enable repair function > + iowrite32(0x01, ®s->pce); // enable input use macros with proper bit names, like 0x01 would have proper macro. > + > + // read all requested fuses > + for (unsigned int i = 0; i < fusecount; i++, fuseidx++) { > + iowrite32(fuseidx, ®s->pa); > + > + // cycle clock to read > + iowrite32(0x01, ®s->pclk); > + mdelay(1); > + iowrite32(0x00, ®s->pclk); > + mdelay(1); > + > + // read the value > + fusebuf[i] = ioread32(®s->pdout); > + } > + > + // shut down > + iowrite32(0, ®s->pce); > + iowrite32(0, ®s->ptrim); > + iowrite32(0, ®s->pdstb); > + > + // copy out > + memcpy(buf, fusebuf, size); > + > + return 0; > +} > + > +/* > + * Caution: > + * OTP can be write only once, so use carefully. can be written > + * > + * offset and size are assumed aligned to the size of the fuses (32bit). > + */ > +static int hifive_otp_write(struct udevice *dev, int offset, > + const void *buf, int size) > +{ > + struct hifive_otp_platdata *plat = dev_get_platdata(dev); > + struct hifive_otp_regs *regs = (struct hifive_otp_regs *)plat->regs; > + > + int fuseidx = offset / BYTES_PER_FUSE; > + int fusecount = size / BYTES_PER_FUSE; > + u32 *write_buf = (u32 *)buf; > + u32 write_data; > + int i, pas, bit; > + > + // check bounds > + if (offset < 0 || size < 0) > + return -EINVAL; > + if (fuseidx >= NUM_FUSES) > + return -EINVAL; > + if ((fuseidx + fusecount) > NUM_FUSES) > + return -EINVAL; We have lib functions to do this bound, would you please try to use that.