-----Original Message----- From: Jagan Teki jagan@amarulasolutions.com Sent: 24 January 2020 12:12 To: Pragnesh Patel pragnesh.patel@sifive.com Cc: U-Boot-Denx u-boot@lists.denx.de; Atish Patra atish.patra@wdc.com; palmerdabbelt@google.com; Bin Meng bmeng.cn@gmail.com; Paul Walmsley ( Sifive) paul.walmsley@sifive.com; Troy Benjegerdes ( Sifive) troy.benjegerdes@sifive.com; Anup Patel anup.patel@wdc.com; Sagar Kadam sagar.kadam@sifive.com; Rick Chen rick@andestech.com; Simon Glass sjg@chromium.org; Heiko Stuebner <heiko.stuebner@theobroma- systems.com>; Michal Simek michal.simek@xilinx.com; Marcel Ziswiler marcel.ziswiler@toradex.com; Finley Xiao finley.xiao@rock-chips.com; Peng Fan peng.fan@nxp.com; Tero Kristo t-kristo@ti.com; Eugen Hristev eugen.hristev@microchip.com Subject: Re: [PATCH v3 01/10] misc: add driver for the Sifive otp controller
On Fri, Jan 24, 2020 at 11:21 AM Pragnesh Patel pragnesh.patel@sifive.com wrote:
Added a misc driver to handle OTP memory in FU540.
Signed-off-by: Pragnesh Patel pragnesh.patel@sifive.com Reviewed-by: Anup Patel anup.patel@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
- add sifive otp driver
- enable the otp driver - board, dts, defconfig changes.
Will split in v4.
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 OTPret = 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 varserial = otp_read_serialnum(dev); #endifreturn 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.
Will update the "board/Sifive/fu540/Kconfig" in v4.
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?
Will change it to SIFIVE_OTP in v4.
bool "Sifive Ememory OTP driver"depends on RISCV && MISChelpEnable support for reading the Ememory OTP on the HiFive UnleashedOTP 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.
Agreed, will change in v4. Thanks for pointing me.
@@ -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 philipp@hug.cx
- Copyright (C) 2018 Joey Hewitt joey@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.
Right now, OTP is used only for serial number. First 1 KB is reserved for Sifive Internal use, serial number is one of them.
+#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.
Will update in v4.
struct hifive_otp_regs __iomem *regs; };+typedef u32 fuse_value_t;
No typdef please. No global variables(unless it really need)
Will update in v4, thanks.
+#define BYTES_PER_FUSE 4
+#define NUM_FUSES 0x1000
May be use dt properties.
Will check this.
+/*
- 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 boundsuse proper comment style.
Will update in v4.
if (offset < 0 || size < 0)return -EINVAL;if (fuseidx >= NUM_FUSES)return -EINVAL;if ((fuseidx + fusecount) > NUM_FUSES)return -EINVAL;// init OTPiowrite32(0x01, ®s->pdstb); // wake up from stand-byiowrite32(0x01, ®s->ptrim); // enable repair functioniowrite32(0x01, ®s->pce); // enable inputuse macros with proper bit names, like 0x01 would have proper macro.
Will update in v4.
// read all requested fusesfor (unsigned int i = 0; i < fusecount; i++, fuseidx++) {iowrite32(fuseidx, ®s->pa);// cycle clock to readiowrite32(0x01, ®s->pclk);mdelay(1);iowrite32(0x00, ®s->pclk);mdelay(1);// read the valuefusebuf[i] = ioread32(®s->pdout);}// shut downiowrite32(0, ®s->pce);iowrite32(0, ®s->ptrim);iowrite32(0, ®s->pdstb);// copy outmemcpy(buf, fusebuf, size);return 0;+}
+/*
- Caution:
- OTP can be write only once, so use carefully.
can be written
Thanks, will update in v4.
- 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 boundsif (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.
Will check and update in v4, thanks for the review.