| Message ID | 20190620064753.32391-8-anup.patel@wdc.com |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | Update SiFive Unleashed Drivers | expand |
Hi Anup, On Thu, Jun 20, 2019 at 2:49 PM Anup Patel <Anup.Patel@wdc.com> wrote: > > This patch extends SiFive FU540 board support to setup ethaddr > env variable based on board serialnum read from OTP. > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > --- > board/sifive/fu540/fu540.c | 118 +++++++++++++++++++++++++++++++++ > configs/sifive_fu540_defconfig | 1 + > 2 files changed, 119 insertions(+) > > diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c > index 5adc4a3d4a..81f2025402 100644 > --- a/board/sifive/fu540/fu540.c > +++ b/board/sifive/fu540/fu540.c > @@ -8,6 +8,124 @@ > > #include <common.h> > #include <dm.h> > +#include <linux/delay.h> > +#include <linux/io.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); > + > + return 0; > +} > + > +static u32 fu540_read_serialnum(void) > +{ > + int ret; > + u32 serial[2] = {0}; > + > + for (int i = 0xfe * 4; i > 0; i -= 8) { > + ret = fu540_otp_read(i, serial, sizeof(serial)); > + if (ret) { > + printf("%s: error reading from OTP\n", __func__); > + break; > + } > + if (serial[0] == ~serial[1]) > + return serial[0]; > + } > + > + return 0; > +} > + > +static void fu540_setup_macaddr(u32 serialnum) > +{ > + /* Default MAC address */ > + unsigned char mac[6] = { 0x70, 0xb3, 0xd5, 0x92, 0xf0, 0x00 }; Why are the last 3 bytes of the default MAC address not zero? My understanding is that the first 3 numbers indicate the vendor and vendor is free to use the last 3 bytes for different devices. > + > + /* OR the serial into the MAC -- see SiFive FSBL */ > + mac[5] |= (serialnum >> 0) & 0xff; > + mac[4] |= (serialnum >> 8) & 0xff; > + mac[3] |= (serialnum >> 16) & 0xff; > + > + /* Skip if environment variable already set */ > + if (env_get("ethaddr")) > + return; I think we should make the function clear by: * ether move this logic to out of this function, and do such check in misc_init_r() OR * unconditionally overwrite the "ethaddr" by removing this check I am OK with either way. > + > + /* Update environment variable */ > + eth_env_set_enetaddr("ethaddr", mac); > +} > + > +int misc_init_r(void) > +{ > + fu540_setup_macaddr(fu540_read_serialnum()); > + > + return 0; > +} > + > +#endif > Regards, Bin
On Fri, Jun 21, 2019 at 12:48 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Anup, > > On Thu, Jun 20, 2019 at 2:49 PM Anup Patel <Anup.Patel@wdc.com> wrote: > > > > This patch extends SiFive FU540 board support to setup ethaddr > > env variable based on board serialnum read from OTP. > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > --- > > board/sifive/fu540/fu540.c | 118 +++++++++++++++++++++++++++++++++ > > configs/sifive_fu540_defconfig | 1 + > > 2 files changed, 119 insertions(+) > > > > diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c > > index 5adc4a3d4a..81f2025402 100644 > > --- a/board/sifive/fu540/fu540.c > > +++ b/board/sifive/fu540/fu540.c > > @@ -8,6 +8,124 @@ > > > > #include <common.h> > > #include <dm.h> > > +#include <linux/delay.h> > > +#include <linux/io.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); > > + > > + return 0; > > +} > > + > > +static u32 fu540_read_serialnum(void) > > +{ > > + int ret; > > + u32 serial[2] = {0}; > > + > > + for (int i = 0xfe * 4; i > 0; i -= 8) { > > + ret = fu540_otp_read(i, serial, sizeof(serial)); > > + if (ret) { > > + printf("%s: error reading from OTP\n", __func__); > > + break; > > + } > > + if (serial[0] == ~serial[1]) > > + return serial[0]; > > + } > > + > > + return 0; > > +} > > + > > +static void fu540_setup_macaddr(u32 serialnum) > > +{ > > + /* Default MAC address */ > > + unsigned char mac[6] = { 0x70, 0xb3, 0xd5, 0x92, 0xf0, 0x00 }; > > Why are the last 3 bytes of the default MAC address not zero? My > understanding is that the first 3 numbers indicate the vendor and > vendor is free to use the last 3 bytes for different devices. Your understanding about MAC address is correct. I don't know why last three bytes are ORed and not assigned. The logic is taken from FSBL. I did not want users to see different MAC address after this patch so I have kept the logic same. > > > + > > + /* OR the serial into the MAC -- see SiFive FSBL */ > > + mac[5] |= (serialnum >> 0) & 0xff; > > + mac[4] |= (serialnum >> 8) & 0xff; > > + mac[3] |= (serialnum >> 16) & 0xff; > > + > > + /* Skip if environment variable already set */ > > + if (env_get("ethaddr")) > > + return; > > I think we should make the function clear by: > * ether move this logic to out of this function, and do such check in > misc_init_r() > > OR > * unconditionally overwrite the "ethaddr" by removing this check > > I am OK with either way. Sure, I will move the env_get() check to misc_init_r(). > > > + > > + /* Update environment variable */ > > + eth_env_set_enetaddr("ethaddr", mac); > > +} > > + > > +int misc_init_r(void) > > +{ > > + fu540_setup_macaddr(fu540_read_serialnum()); > > + > > + return 0; > > +} > > + > > +#endif > > > > Regards, > Bin > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot Regards, Anup
Hi Anup, On Fri, Jun 21, 2019 at 8:49 PM Anup Patel <anup@brainfault.org> wrote: > > On Fri, Jun 21, 2019 at 12:48 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Anup, > > > > On Thu, Jun 20, 2019 at 2:49 PM Anup Patel <Anup.Patel@wdc.com> wrote: > > > > > > This patch extends SiFive FU540 board support to setup ethaddr > > > env variable based on board serialnum read from OTP. > > > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > > --- > > > board/sifive/fu540/fu540.c | 118 +++++++++++++++++++++++++++++++++ > > > configs/sifive_fu540_defconfig | 1 + > > > 2 files changed, 119 insertions(+) > > > > > > diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c > > > index 5adc4a3d4a..81f2025402 100644 > > > --- a/board/sifive/fu540/fu540.c > > > +++ b/board/sifive/fu540/fu540.c > > > @@ -8,6 +8,124 @@ > > > > > > #include <common.h> > > > #include <dm.h> > > > +#include <linux/delay.h> > > > +#include <linux/io.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); > > > + > > > + return 0; > > > +} > > > + > > > +static u32 fu540_read_serialnum(void) > > > +{ > > > + int ret; > > > + u32 serial[2] = {0}; > > > + > > > + for (int i = 0xfe * 4; i > 0; i -= 8) { > > > + ret = fu540_otp_read(i, serial, sizeof(serial)); > > > + if (ret) { > > > + printf("%s: error reading from OTP\n", __func__); > > > + break; > > > + } > > > + if (serial[0] == ~serial[1]) > > > + return serial[0]; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static void fu540_setup_macaddr(u32 serialnum) > > > +{ > > > + /* Default MAC address */ > > > + unsigned char mac[6] = { 0x70, 0xb3, 0xd5, 0x92, 0xf0, 0x00 }; > > > > Why are the last 3 bytes of the default MAC address not zero? My > > understanding is that the first 3 numbers indicate the vendor and > > vendor is free to use the last 3 bytes for different devices. > > Your understanding about MAC address is correct. > > I don't know why last three bytes are ORed and not assigned. The > logic is taken from FSBL. > > I did not want users to see different MAC address after this patch > so I have kept the logic same. > OK, thanks for the clarification. Please put some comments here. > > > > > + > > > + /* OR the serial into the MAC -- see SiFive FSBL */ > > > + mac[5] |= (serialnum >> 0) & 0xff; > > > + mac[4] |= (serialnum >> 8) & 0xff; > > > + mac[3] |= (serialnum >> 16) & 0xff; > > > + > > > + /* Skip if environment variable already set */ > > > + if (env_get("ethaddr")) > > > + return; > > > > I think we should make the function clear by: > > * ether move this logic to out of this function, and do such check in > > misc_init_r() > > > > OR > > * unconditionally overwrite the "ethaddr" by removing this check > > > > I am OK with either way. > > Sure, I will move the env_get() check to misc_init_r(). Regards, Bin
diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c index 5adc4a3d4a..81f2025402 100644 --- a/board/sifive/fu540/fu540.c +++ b/board/sifive/fu540/fu540.c @@ -8,6 +8,124 @@ #include <common.h> #include <dm.h> +#include <linux/delay.h> +#include <linux/io.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); + + return 0; +} + +static u32 fu540_read_serialnum(void) +{ + int ret; + u32 serial[2] = {0}; + + for (int i = 0xfe * 4; i > 0; i -= 8) { + ret = fu540_otp_read(i, serial, sizeof(serial)); + if (ret) { + printf("%s: error reading from OTP\n", __func__); + break; + } + if (serial[0] == ~serial[1]) + return serial[0]; + } + + return 0; +} + +static void fu540_setup_macaddr(u32 serialnum) +{ + /* Default MAC address */ + unsigned char mac[6] = { 0x70, 0xb3, 0xd5, 0x92, 0xf0, 0x00 }; + + /* OR the serial into the MAC -- see SiFive FSBL */ + mac[5] |= (serialnum >> 0) & 0xff; + mac[4] |= (serialnum >> 8) & 0xff; + mac[3] |= (serialnum >> 16) & 0xff; + + /* Skip if environment variable already set */ + if (env_get("ethaddr")) + return; + + /* Update environment variable */ + eth_env_set_enetaddr("ethaddr", mac); +} + +int misc_init_r(void) +{ + fu540_setup_macaddr(fu540_read_serialnum()); + + return 0; +} + +#endif int board_init(void) { diff --git a/configs/sifive_fu540_defconfig b/configs/sifive_fu540_defconfig index f78412398e..f19203745e 100644 --- a/configs/sifive_fu540_defconfig +++ b/configs/sifive_fu540_defconfig @@ -7,4 +7,5 @@ CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y CONFIG_DISPLAY_CPUINFO=y CONFIG_DISPLAY_BOARDINFO=y +CONFIG_MISC_INIT_R=y CONFIG_OF_PRIOR_STAGE=y
This patch extends SiFive FU540 board support to setup ethaddr env variable based on board serialnum read from OTP. Signed-off-by: Anup Patel <anup.patel@wdc.com> --- board/sifive/fu540/fu540.c | 118 +++++++++++++++++++++++++++++++++ configs/sifive_fu540_defconfig | 1 + 2 files changed, 119 insertions(+)