[PATCH v3 01/10] misc: add driver for the Sifive otp controller
Pragnesh Patel
pragnesh.patel at sifive.com
Mon Feb 10 13:44:40 CET 2020
Hi Jagan,
>-----Original Message-----
>From: Pragnesh Patel
>Sent: 27 January 2020 15:48
>To: Jagan Teki <jagan at amarulasolutions.com>
>Cc: U-Boot-Denx <u-boot at lists.denx.de>; Atish Patra
><atish.patra at wdc.com>; palmerdabbelt at google.com; Bin Meng
><bmeng.cn at gmail.com>; Paul Walmsley ( Sifive)
><paul.walmsley at sifive.com>; Troy Benjegerdes ( Sifive)
><troy.benjegerdes at sifive.com>; Anup Patel <anup.patel at wdc.com>; Sagar
>Kadam <sagar.kadam at sifive.com>; Rick Chen <rick at andestech.com>; Simon
>Glass <sjg at chromium.org>; Heiko Stuebner <heiko.stuebner at theobroma-
>systems.com>; Michal Simek <michal.simek at xilinx.com>; Marcel Ziswiler
><marcel.ziswiler at toradex.com>; Finley Xiao <finley.xiao at rock-chips.com>;
>Peng Fan <peng.fan at nxp.com>; Tero Kristo <t-kristo at ti.com>; Eugen Hristev
><eugen.hristev at microchip.com>
>Subject: RE: [PATCH v3 01/10] misc: add driver for the Sifive otp controller
>
>
>>-----Original Message-----
>>From: Jagan Teki <jagan at amarulasolutions.com>
>>Sent: 24 January 2020 12:12
>>To: Pragnesh Patel <pragnesh.patel at sifive.com>
>>Cc: U-Boot-Denx <u-boot at lists.denx.de>; Atish Patra
>><atish.patra at wdc.com>; palmerdabbelt at google.com; Bin Meng
>><bmeng.cn at gmail.com>; Paul Walmsley ( Sifive)
>><paul.walmsley at sifive.com>; Troy Benjegerdes ( Sifive)
>><troy.benjegerdes at sifive.com>; Anup Patel <anup.patel at wdc.com>; Sagar
>>Kadam <sagar.kadam at sifive.com>; Rick Chen <rick at andestech.com>; Simon
>>Glass <sjg at chromium.org>; Heiko Stuebner <heiko.stuebner at theobroma-
>>systems.com>; Michal Simek <michal.simek at xilinx.com>; Marcel Ziswiler
>><marcel.ziswiler at toradex.com>; Finley Xiao
>><finley.xiao at rock-chips.com>; Peng Fan <peng.fan at nxp.com>; Tero Kristo
>><t-kristo at ti.com>; Eugen Hristev <eugen.hristev at 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 at sifive.com>
>>wrote:
>>>
>>> Added a misc driver to handle OTP memory in FU540.
>>>
>>> Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
>>> Reviewed-by: Anup Patel <anup.patel at 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.
>
>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 <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.
>
>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 <philipp at hug.cx>
>>> + * Copyright (C) 2018 Joey Hewitt <joey at 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 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.
More information about the U-Boot
mailing list