From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pragnesh Patel Date: Mon, 10 Feb 2020 12:44:40 +0000 Subject: [PATCH v3 01/10] misc: add driver for the Sifive otp controller In-Reply-To: References: <20200124055026.30787-1-pragnesh.patel@sifive.com> <20200124055026.30787-2-pragnesh.patel@sifive.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Jagan, >-----Original Message----- >From: Pragnesh Patel >Sent: 27 January 2020 15:48 >To: Jagan Teki >Cc: U-Boot-Denx ; Atish Patra >; palmerdabbelt at google.com; Bin Meng >; Paul Walmsley ( Sifive) >; Troy Benjegerdes ( Sifive) >; Anup Patel ; Sagar >Kadam ; Rick Chen ; Simon >Glass ; Heiko Stuebner systems.com>; Michal Simek ; Marcel Ziswiler >; Finley Xiao ; >Peng Fan ; Tero Kristo ; Eugen Hristev > >Subject: RE: [PATCH v3 01/10] misc: add driver for the Sifive otp controller > > >>-----Original Message----- >>From: Jagan Teki >>Sent: 24 January 2020 12:12 >>To: Pragnesh Patel >>Cc: U-Boot-Denx ; Atish Patra >>; palmerdabbelt at google.com; Bin Meng >>; Paul Walmsley ( Sifive) >>; Troy Benjegerdes ( Sifive) >>; Anup Patel ; Sagar >>Kadam ; Rick Chen ; Simon >>Glass ; Heiko Stuebner >systems.com>; Michal Simek ; Marcel Ziswiler >>; Finley Xiao >>; Peng Fan ; Tero Kristo >>; Eugen Hristev >>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 >> >>wrote: >>> >>> Added a misc driver to handle OTP memory in FU540. >>> >>> Signed-off-by: Pragnesh Patel >>> Reviewed-by: Anup Patel >>> --- >>> 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. > >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 at 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 >>> #include >>> #include >>> +#include >>> >>> -#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. > >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 && 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. > >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 >>> + * Copyright (C) 2018 Joey Hewitt >>> + * >>> + * 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 >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +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 bounds >> >>use 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 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. > >Will update in v4. > >> >>> + >>> + // 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 > >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 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. Which lib functions you want me to use here ? > >Will check and update in v4, thanks for the review.