diff mbox series

[v3,01/10] misc: add driver for the Sifive otp controller

Message ID 20200124055026.30787-2-pragnesh.patel@sifive.com
State Superseded
Delegated to: Andes
Headers show
Series RISC-V SiFive FU540 support SPL | expand

Commit Message

Pragnesh Patel Jan. 24, 2020, 5:50 a.m. UTC
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 ++++++++++++++++++
 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

Comments

Jagan Teki Jan. 24, 2020, 6:41 a.m. UTC | #1
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
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, &regs->pdstb); /* wake up from stand-by */
> -       writel(0x01, &regs->ptrim); /* enable repair function */
> -       writel(0x01, &regs->pce);   /* enable input */
> -
> -       /* read all requested fuses */
> -       for (i = 0; i < fusecount; i++, fuseidx++) {
> -               writel(fuseidx, &regs->pa);
> -
> -               /* cycle clock to read */
> -               writel(0x01, &regs->pclk);
> -               mdelay(1);
> -               writel(0x00, &regs->pclk);
> -               mdelay(1);
> -
> -               /* read the value */
> -               fusebuf[i] = readl(&regs->pdout);
> -       }
> -
> -       /* shut down */
> -       writel(0, &regs->pce);
> -       writel(0, &regs->ptrim);
> -       writel(0, &regs->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 <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.

> +
> +#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, &regs->pdstb); // wake up from stand-by
> +       iowrite32(0x01, &regs->ptrim); // enable repair function
> +       iowrite32(0x01, &regs->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, &regs->pa);
> +
> +               // cycle clock to read
> +               iowrite32(0x01, &regs->pclk);
> +               mdelay(1);
> +               iowrite32(0x00, &regs->pclk);
> +               mdelay(1);
> +
> +               // read the value
> +               fusebuf[i] = ioread32(&regs->pdout);
> +       }
> +
> +       // shut down
> +       iowrite32(0, &regs->pce);
> +       iowrite32(0, &regs->ptrim);
> +       iowrite32(0, &regs->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.
Pragnesh Patel Jan. 27, 2020, 10:18 a.m. UTC | #2
>-----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
>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@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, &regs->pdstb); /* wake up from stand-by */
>> -       writel(0x01, &regs->ptrim); /* enable repair function */
>> -       writel(0x01, &regs->pce);   /* enable input */
>> -
>> -       /* read all requested fuses */
>> -       for (i = 0; i < fusecount; i++, fuseidx++) {
>> -               writel(fuseidx, &regs->pa);
>> -
>> -               /* cycle clock to read */
>> -               writel(0x01, &regs->pclk);
>> -               mdelay(1);
>> -               writel(0x00, &regs->pclk);
>> -               mdelay(1);
>> -
>> -               /* read the value */
>> -               fusebuf[i] = readl(&regs->pdout);
>> -       }
>> -
>> -       /* shut down */
>> -       writel(0, &regs->pce);
>> -       writel(0, &regs->ptrim);
>> -       writel(0, &regs->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@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 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, &regs->pdstb); // wake up from stand-by
>> +       iowrite32(0x01, &regs->ptrim); // enable repair function
>> +       iowrite32(0x01, &regs->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, &regs->pa);
>> +
>> +               // cycle clock to read
>> +               iowrite32(0x01, &regs->pclk);
>> +               mdelay(1);
>> +               iowrite32(0x00, &regs->pclk);
>> +               mdelay(1);
>> +
>> +               // read the value
>> +               fusebuf[i] = ioread32(&regs->pdout);
>> +       }
>> +
>> +       // shut down
>> +       iowrite32(0, &regs->pce);
>> +       iowrite32(0, &regs->ptrim);
>> +       iowrite32(0, &regs->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.

Will check and update in v4, thanks for the review.
Pragnesh Patel Feb. 10, 2020, 12:44 p.m. UTC | #3
Hi Jagan,

>-----Original Message-----
>From: Pragnesh Patel
>Sent: 27 January 2020 15:48
>To: Jagan Teki <jagan@amarulasolutions.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
>
>
>>-----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
>>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@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, &regs->pdstb); /* wake up from stand-by */
>>> -       writel(0x01, &regs->ptrim); /* enable repair function */
>>> -       writel(0x01, &regs->pce);   /* enable input */
>>> -
>>> -       /* read all requested fuses */
>>> -       for (i = 0; i < fusecount; i++, fuseidx++) {
>>> -               writel(fuseidx, &regs->pa);
>>> -
>>> -               /* cycle clock to read */
>>> -               writel(0x01, &regs->pclk);
>>> -               mdelay(1);
>>> -               writel(0x00, &regs->pclk);
>>> -               mdelay(1);
>>> -
>>> -               /* read the value */
>>> -               fusebuf[i] = readl(&regs->pdout);
>>> -       }
>>> -
>>> -       /* shut down */
>>> -       writel(0, &regs->pce);
>>> -       writel(0, &regs->ptrim);
>>> -       writel(0, &regs->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@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 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, &regs->pdstb); // wake up from stand-by
>>> +       iowrite32(0x01, &regs->ptrim); // enable repair function
>>> +       iowrite32(0x01, &regs->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, &regs->pa);
>>> +
>>> +               // cycle clock to read
>>> +               iowrite32(0x01, &regs->pclk);
>>> +               mdelay(1);
>>> +               iowrite32(0x00, &regs->pclk);
>>> +               mdelay(1);
>>> +
>>> +               // read the value
>>> +               fusebuf[i] = ioread32(&regs->pdout);
>>> +       }
>>> +
>>> +       // shut down
>>> +       iowrite32(0, &regs->pce);
>>> +       iowrite32(0, &regs->ptrim);
>>> +       iowrite32(0, &regs->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.
diff mbox series

Patch

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, &regs->pdstb); /* wake up from stand-by */
-	writel(0x01, &regs->ptrim); /* enable repair function */
-	writel(0x01, &regs->pce);   /* enable input */
-
-	/* read all requested fuses */
-	for (i = 0; i < fusecount; i++, fuseidx++) {
-		writel(fuseidx, &regs->pa);
-
-		/* cycle clock to read */
-		writel(0x01, &regs->pclk);
-		mdelay(1);
-		writel(0x00, &regs->pclk);
-		mdelay(1);
-
-		/* read the value */
-		fusebuf[i] = readl(&regs->pdout);
-	}
-
-	/* shut down */
-	writel(0, &regs->pce);
-	writel(0, &regs->ptrim);
-	writel(0, &regs->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
 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
+       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
@@ -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)
+ */
+
+#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 {
+	struct hifive_otp_regs __iomem *regs;
+};
+
+typedef u32 fuse_value_t;
+#define BYTES_PER_FUSE     4
+
+#define NUM_FUSES          0x1000
+
+/*
+ * 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
+	if (offset < 0 || size < 0)
+		return -EINVAL;
+	if (fuseidx >= NUM_FUSES)
+		return -EINVAL;
+	if ((fuseidx + fusecount) > NUM_FUSES)
+		return -EINVAL;
+
+	// init OTP
+	iowrite32(0x01, &regs->pdstb); // wake up from stand-by
+	iowrite32(0x01, &regs->ptrim); // enable repair function
+	iowrite32(0x01, &regs->pce);   // enable input
+
+	// read all requested fuses
+	for (unsigned int i = 0; i < fusecount; i++, fuseidx++) {
+		iowrite32(fuseidx, &regs->pa);
+
+		// cycle clock to read
+		iowrite32(0x01, &regs->pclk);
+		mdelay(1);
+		iowrite32(0x00, &regs->pclk);
+		mdelay(1);
+
+		// read the value
+		fusebuf[i] = ioread32(&regs->pdout);
+	}
+
+	// shut down
+	iowrite32(0, &regs->pce);
+	iowrite32(0, &regs->ptrim);
+	iowrite32(0, &regs->pdstb);
+
+	// copy out
+	memcpy(buf, fusebuf, size);
+
+	return 0;
+}
+
+/*
+ * Caution:
+ * OTP can be write only once, so use carefully.
+ *
+ * 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;
+
+	// init OTP
+	iowrite32(0x01, &regs->pdstb);  // wake up from stand-by
+	iowrite32(0x01, &regs->ptrim);  // enable repair function
+
+	iowrite32(0x00, &regs->pclk);
+	iowrite32(0x00, &regs->pa);
+	iowrite32(0x00, &regs->pas);
+	iowrite32(0x00, &regs->paio);
+	iowrite32(0x00, &regs->pdin);
+	iowrite32(0x00, &regs->pwe);
+	iowrite32(0x02, &regs->ptm);
+	mdelay(1);
+
+	iowrite32(0x01, &regs->pce);    // enable input
+	iowrite32(0x01, &regs->pprog);  // Program mode enable input
+	iowrite32(0x01, &regs->ptrim);
+
+	// write all requested fuses
+	for (i = 0; i < fusecount; i++, fuseidx++) {
+		iowrite32(fuseidx, &regs->pa);
+		write_data = *(write_buf++);
+
+		for (pas = 0; pas < 2; pas++) {
+			iowrite32(pas, &regs->pas);
+
+			for (bit = 0; bit < 32; bit++) {
+				iowrite32(bit, &regs->paio);
+				iowrite32(((write_data >> bit) & 1),
+					  &regs->pdin);
+				mdelay(1);
+
+				iowrite32(0x01, &regs->pwe);
+				mdelay(1);
+				iowrite32(0x00, &regs->pwe);
+				mdelay(1);
+			}
+		}
+
+		iowrite32(0x00, &regs->pas);
+	}
+
+	// shut down
+	iowrite32(0x00, &regs->pwe);
+	iowrite32(0x00, &regs->pprog);
+	iowrite32(0x00, &regs->pce);
+	iowrite32(0x00, &regs->ptm);
+
+	iowrite32(0x00, &regs->ptrim);
+	iowrite32(0x00, &regs->pdstb);
+
+	return 0;
+}
+
+static int hifive_otp_ofdata_to_platdata(struct udevice *dev)
+{
+	struct hifive_otp_platdata *plat = dev_get_platdata(dev);
+
+	plat->regs = dev_read_addr_ptr(dev);
+	return 0;
+}
+
+static const struct misc_ops hifive_otp_ops = {
+	.read = hifive_otp_read,
+	.write = hifive_otp_write,
+};
+
+static const struct udevice_id hifive_otp_ids[] = {
+	{ .compatible = "sifive,fu540-otp" },
+	{}
+};
+
+U_BOOT_DRIVER(hifive_otp) = {
+	.name = "hifive_otp",
+	.id = UCLASS_MISC,
+	.of_match = hifive_otp_ids,
+	.ofdata_to_platdata = hifive_otp_ofdata_to_platdata,
+	.platdata_auto_alloc_size = sizeof(struct hifive_otp_platdata),
+	.ops = &hifive_otp_ops,
+};