| Message ID | 20200224083247.29086-3-pragnesh.patel@sifive.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V SiFive FU540 support SPL | expand |
Hi,
Just a warning as I had the same issue with 8729b1ae2cbd ("misc: Update read() and write() methods to return bytes xfered")
> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Pragnesh Patel
> Sent: lundi 24 f?vrier 2020 09:33
>
> Use the OTP DM driver to set the serial environment variable.
>
> Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
> ---
> arch/riscv/dts/fu540-c000-u-boot.dtsi | 14 +++
> .../dts/hifive-unleashed-a00-u-boot.dtsi | 6 +
> board/sifive/fu540/Kconfig | 2 +
> board/sifive/fu540/fu540.c | 113 +++++++-----------
> 4 files changed, 62 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
>
> 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..31fd113c7d
> --- /dev/null
> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
> @@ -0,0 +1,14 @@
> +// 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>;
> + fuse-count = <0x1000>;
> + };
> + };
> +};
> 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/Kconfig b/board/sifive/fu540/Kconfig index
> 5ca21474de..900197bbb2 100644
> --- a/board/sifive/fu540/Kconfig
> +++ b/board/sifive/fu540/Kconfig
> @@ -48,5 +48,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy
> imply SIFIVE_GPIO
> imply CMD_GPIO
> imply SMP
> + imply MISC
> + imply SIFIVE_OTP
>
> endif
> diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c index
> 47a2090251..409471effc 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(SIFIVE_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) {
Warning here, misc_read returns now the size of the data (sizeof(serial)),
since commit 8729b1ae2cbd ("misc: Update read() and write() methods to return bytes xfered")
So I think the test need to change to
if (ret < 0)
or
if (ret != sizeof(serial))
> - 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(SIFIVE_OTP)
> + struct udevice *dev;
> + int ret;
> +
> + // init OTP
> + ret = uclass_get_device_by_driver(UCLASS_MISC,
> + DM_GET_DRIVER(sifive_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)
> --
> 2.17.1
Patrick
Hi Patrick, >-----Original Message----- >From: Patrick DELAUNAY <patrick.delaunay at st.com> >Sent: 24 February 2020 23:30 >To: Pragnesh Patel <pragnesh.patel at sifive.com>; u-boot at lists.denx.de >Cc: atish.patra at wdc.com; palmerdabbelt at google.com; >bmeng.cn at gmail.com; Paul Walmsley ( Sifive) <paul.walmsley at sifive.com>; >jagan at amarulasolutions.com; Troy Benjegerdes ( Sifive) ><troy.benjegerdes at sifive.com>; anup.patel at wdc.com; Sagar Kadam ><sagar.kadam at sifive.com>; Rick Chen <rick at andestech.com> >Subject: RE: [PATCH v4 02/14] riscv: sifive: fu540: Use OTP DM driver for serial >environment variable > >Hi, > >Just a warning as I had the same issue with 8729b1ae2cbd ("misc: Update >read() and write() methods to return bytes xfered") > >> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Pragnesh >> Patel >> Sent: lundi 24 f?vrier 2020 09:33 >> >> Use the OTP DM driver to set the serial environment variable. >> >> Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com> >> --- >> arch/riscv/dts/fu540-c000-u-boot.dtsi | 14 +++ >> .../dts/hifive-unleashed-a00-u-boot.dtsi | 6 + >> board/sifive/fu540/Kconfig | 2 + >> board/sifive/fu540/fu540.c | 113 +++++++----------- >> 4 files changed, 62 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 >> >> 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..31fd113c7d >> --- /dev/null >> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi >> @@ -0,0 +1,14 @@ >> +// 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>; >> + fuse-count = <0x1000>; >> + }; >> + }; >> +}; >> 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/Kconfig b/board/sifive/fu540/Kconfig >> index >> 5ca21474de..900197bbb2 100644 >> --- a/board/sifive/fu540/Kconfig >> +++ b/board/sifive/fu540/Kconfig >> @@ -48,5 +48,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy >> imply SIFIVE_GPIO >> imply CMD_GPIO >> imply SMP >> + imply MISC >> + imply SIFIVE_OTP >> >> endif >> diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c >> index 47a2090251..409471effc 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(SIFIVE_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) { > >Warning here, misc_read returns now the size of the data (sizeof(serial)), >since commit 8729b1ae2cbd ("misc: Update read() and write() methods to >return bytes xfered") > >So I think the test need to change to > >if (ret < 0) > >or > >if (ret != sizeof(serial)) > Yes, you are right. Will update in v5. Thanks for the review. >> - 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(SIFIVE_OTP) >> + struct udevice *dev; >> + int ret; >> + >> + // init OTP >> + ret = uclass_get_device_by_driver(UCLASS_MISC, >> + DM_GET_DRIVER(sifive_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) >> -- >> 2.17.1 > >Patrick
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..31fd113c7d --- /dev/null +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi @@ -0,0 +1,14 @@ +// 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>; + fuse-count = <0x1000>; + }; + }; +}; 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/Kconfig b/board/sifive/fu540/Kconfig index 5ca21474de..900197bbb2 100644 --- a/board/sifive/fu540/Kconfig +++ b/board/sifive/fu540/Kconfig @@ -48,5 +48,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy imply SIFIVE_GPIO imply CMD_GPIO imply SMP + imply MISC + imply SIFIVE_OTP endif diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c index 47a2090251..409471effc 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(SIFIVE_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(SIFIVE_OTP) + struct udevice *dev; + int ret; + + // init OTP + ret = uclass_get_device_by_driver(UCLASS_MISC, + DM_GET_DRIVER(sifive_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)
Use the OTP DM driver to set the serial environment variable. Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com> --- arch/riscv/dts/fu540-c000-u-boot.dtsi | 14 +++ .../dts/hifive-unleashed-a00-u-boot.dtsi | 6 + board/sifive/fu540/Kconfig | 2 + board/sifive/fu540/fu540.c | 113 +++++++----------- 4 files changed, 62 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