| Message ID | 1323966067-28333-1-git-send-email-fabio.estevam@freescale.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
> Let imx_get_mac_from_fuse function be a common function, so that other > mx28 boards can reuse it. > > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > --- > arch/arm/cpu/arm926ejs/mx28/mx28.c | 35 > ++++++++++++++++++++++++++++ arch/arm/include/asm/arch-mx28/sys_proto.h | > 2 + > board/denx/m28evk/m28evk.c | 35 > ---------------------------- 3 files changed, 37 insertions(+), 35 > deletions(-) > > diff --git a/arch/arm/cpu/arm926ejs/mx28/mx28.c > b/arch/arm/cpu/arm926ejs/mx28/mx28.c index 088c019..785978f 100644 > --- a/arch/arm/cpu/arm926ejs/mx28/mx28.c > +++ b/arch/arm/cpu/arm926ejs/mx28/mx28.c > @@ -214,6 +214,41 @@ int cpu_eth_init(bd_t *bis) > } > #endif > > +#ifdef CONFIG_MX28_FEC_MAC_IN_OCOTP > + > +#define MXS_OCOTP_MAX_TIMEOUT 1000000 > +void imx_get_mac_from_fuse(char *mac) > +{ > + struct mx28_ocotp_regs *ocotp_regs = > + (struct mx28_ocotp_regs *)MXS_OCOTP_BASE; > + uint32_t data; > + > + memset(mac, 0, 6); > + > + writel(OCOTP_CTRL_RD_BANK_OPEN, &ocotp_regs->hw_ocotp_ctrl_set); > + > + if (mx28_wait_mask_clr(&ocotp_regs->hw_ocotp_ctrl_reg, OCOTP_CTRL_BUSY, > + MXS_OCOTP_MAX_TIMEOUT)) { > + puts("MXS FEC: Can't get MAC from OCOTP\n"); > + return; > + } > + > + data = readl(&ocotp_regs->hw_ocotp_cust0); > + > + mac[0] = 0x00; > + mac[1] = 0x04; Be careful here. 0x00 0x04 prefix might not be correct for all cases! > + mac[2] = (data >> 24) & 0xff; > + mac[3] = (data >> 16) & 0xff; > + mac[4] = (data >> 8) & 0xff; > + mac[5] = data & 0xff; > +} > +#else > +void imx_get_mac_from_fuse(char *mac) > +{ > + memset(mac, 0, 6); > +} > +#endif > + > U_BOOT_CMD( > clocks, CONFIG_SYS_MAXARGS, 1, do_mx28_showclocks, > "display clocks", > diff --git a/arch/arm/include/asm/arch-mx28/sys_proto.h > b/arch/arm/include/asm/arch-mx28/sys_proto.h index be1f7db..cf5ab16 100644 > --- a/arch/arm/include/asm/arch-mx28/sys_proto.h > +++ b/arch/arm/include/asm/arch-mx28/sys_proto.h > @@ -35,4 +35,6 @@ void mx28_common_spl_init(const iomux_cfg_t *iomux_setup, > const unsigned int iomux_size); > #endif > > +void imx_get_mac_from_fuse(char *mac); > + > #endif /* __MX28_H__ */ > diff --git a/board/denx/m28evk/m28evk.c b/board/denx/m28evk/m28evk.c > index fcee046..005446a 100644 > --- a/board/denx/m28evk/m28evk.c > +++ b/board/denx/m28evk/m28evk.c > @@ -178,39 +178,4 @@ int board_eth_init(bd_t *bis) > return ret; > } > > -#ifdef CONFIG_M28_FEC_MAC_IN_OCOTP > - > -#define MXS_OCOTP_MAX_TIMEOUT 1000000 > -void imx_get_mac_from_fuse(char *mac) > -{ > - struct mx28_ocotp_regs *ocotp_regs = > - (struct mx28_ocotp_regs *)MXS_OCOTP_BASE; > - uint32_t data; > - > - memset(mac, 0, 6); > - > - writel(OCOTP_CTRL_RD_BANK_OPEN, &ocotp_regs->hw_ocotp_ctrl_set); > - > - if (mx28_wait_mask_clr(&ocotp_regs->hw_ocotp_ctrl_reg, OCOTP_CTRL_BUSY, > - MXS_OCOTP_MAX_TIMEOUT)) { > - printf("MXS FEC: Can't get MAC from OCOTP\n"); > - return; > - } > - > - data = readl(&ocotp_regs->hw_ocotp_cust0); > - > - mac[0] = 0x00; > - mac[1] = 0x04; > - mac[2] = (data >> 24) & 0xff; > - mac[3] = (data >> 16) & 0xff; > - mac[4] = (data >> 8) & 0xff; > - mac[5] = data & 0xff; > -} > -#else > -void imx_get_mac_from_fuse(char *mac) > -{ > - memset(mac, 0, 6); > -} > -#endif > - > #endif Otherwise, sure, it seems OK. M
On 15/12/2011 18:22, Marek Vasut wrote: >> Let imx_get_mac_from_fuse function be a common function, so that other >> mx28 boards can reuse it. >> >> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> Hi Marek, >> + writel(OCOTP_CTRL_RD_BANK_OPEN, &ocotp_regs->hw_ocotp_ctrl_set); >> + >> + if (mx28_wait_mask_clr(&ocotp_regs->hw_ocotp_ctrl_reg, OCOTP_CTRL_BUSY, >> + MXS_OCOTP_MAX_TIMEOUT)) { >> + puts("MXS FEC: Can't get MAC from OCOTP\n"); >> + return; >> + } >> + >> + data = readl(&ocotp_regs->hw_ocotp_cust0); >> + >> + mac[0] = 0x00; >> + mac[1] = 0x04; > > Be careful here. 0x00 0x04 prefix might not be correct for all cases! But to be honest, it seems they are correct for the MX28EVK and not for the M28EVK. The M28EVK and the M28 module are delivered with their own MAC address that does not belong to the Freescale's range. As far as I can see, the M28EVK starts with a Freescale's MAC, and then the DENX MAC address (Vendor ID C0:E5:4E) is set later to the correct value when Linux boots. I can understand this feature in the SOC as a way to set the LSBs of the MAC address, but leaving to the customer a way to set its own vendor id, if he bought it. What about to add a weak function (board_set_mac_vendor, maybe ?) that can be called at this point to set the vendor id ? The default behavior should be to set the Freescale's vendor id. Best regards, Stefano Babic
> On 15/12/2011 18:22, Marek Vasut wrote: > >> Let imx_get_mac_from_fuse function be a common function, so that other > >> mx28 boards can reuse it. > >> > >> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > > Hi Marek, Hi Stefano, > > >> + writel(OCOTP_CTRL_RD_BANK_OPEN, &ocotp_regs->hw_ocotp_ctrl_set); > >> + > >> + if (mx28_wait_mask_clr(&ocotp_regs->hw_ocotp_ctrl_reg, > >> OCOTP_CTRL_BUSY, + MXS_OCOTP_MAX_TIMEOUT)) { > >> + puts("MXS FEC: Can't get MAC from OCOTP\n"); > >> + return; > >> + } > >> + > >> + data = readl(&ocotp_regs->hw_ocotp_cust0); > >> + > >> + mac[0] = 0x00; > >> + mac[1] = 0x04; > > > > Be careful here. 0x00 0x04 prefix might not be correct for all cases! > > But to be honest, it seems they are correct for the MX28EVK and not for > the M28EVK. The M28EVK and the M28 module are delivered with their own > MAC address that does not belong to the Freescale's range. > > As far as I can see, the M28EVK starts with a Freescale's MAC, and then > the DENX MAC address (Vendor ID C0:E5:4E) is set later to the correct > value when Linux boots. It's actually set even in uboot by ethaddr and eth1addr. This is the default behaviour. > > I can understand this feature in the SOC as a way to set the LSBs of the > MAC address, but leaving to the customer a way to set its own vendor id, > if he bought it. > > What about to add a weak function (board_set_mac_vendor, maybe ?) that > can be called at this point to set the vendor id ? The default behavior > should be to set the Freescale's vendor id. ethaddr and eth1addr is insufficient? M
On 16/12/2011 10:53, Marek Vasut wrote: >> On 15/12/2011 18:22, Marek Vasut wrote: >>>> Let imx_get_mac_from_fuse function be a common function, so that other >>>> mx28 boards can reuse it. >>>> >>>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> >> Hi Marek, >>> Be careful here. 0x00 0x04 prefix might not be correct for all cases! >> >> But to be honest, it seems they are correct for the MX28EVK and not for >> the M28EVK. The M28EVK and the M28 module are delivered with their own >> MAC address that does not belong to the Freescale's range. >> >> As far as I can see, the M28EVK starts with a Freescale's MAC, and then >> the DENX MAC address (Vendor ID C0:E5:4E) is set later to the correct >> value when Linux boots. > > It's actually set even in uboot by ethaddr and eth1addr. This is the default > behaviour. This means ethaddr overwrites the values read from fuses - why do we need the fuses ? > >> >> I can understand this feature in the SOC as a way to set the LSBs of the >> MAC address, but leaving to the customer a way to set its own vendor id, >> if he bought it. >> >> What about to add a weak function (board_set_mac_vendor, maybe ?) that >> can be called at this point to set the vendor id ? The default behavior >> should be to set the Freescale's vendor id. > > ethaddr and eth1addr is insufficient? In a standard way, the environment must be adapted for each board to store the MAC address in the ethaddr variable. The reason to get the MAC directly from the hardware is that it is not required a specific initial setup for each board, and that simplifies the production and the delivery of the boards. If you rely on ethaddr, than we do not need imx_get_mac_from_fuse() at all, and the MAC can be set to zero, because in this case the ethaddr value is used - but we lose the feature. The reason why imx_get_mac_from_fuse was introduce is to have an automatic mechanism to set up the MAC without any customization. Best regards, Stefano Babic
> On 16/12/2011 10:53, Marek Vasut wrote: > >> On 15/12/2011 18:22, Marek Vasut wrote: > >>>> Let imx_get_mac_from_fuse function be a common function, so that other > >>>> mx28 boards can reuse it. > >>>> > >>>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > > Hi Marek, Hi Stefano, DISCLAIMER: I'm terribly tired today, had early class. > > >>> Be careful here. 0x00 0x04 prefix might not be correct for all cases! > >> > >> But to be honest, it seems they are correct for the MX28EVK and not for > >> the M28EVK. The M28EVK and the M28 module are delivered with their own > >> MAC address that does not belong to the Freescale's range. > >> > >> As far as I can see, the M28EVK starts with a Freescale's MAC, and then > >> the DENX MAC address (Vendor ID C0:E5:4E) is set later to the correct > >> value when Linux boots. > > > > It's actually set even in uboot by ethaddr and eth1addr. This is the > > default behaviour. > > This means ethaddr overwrites the values read from fuses - why do we > need the fuses ? You disable fuses and use ethaddr. Why do we need them? We don't, but the hardware is in the CPU and someone might use it so let's support it. > > >> I can understand this feature in the SOC as a way to set the LSBs of the > >> MAC address, but leaving to the customer a way to set its own vendor id, > >> if he bought it. > >> > >> What about to add a weak function (board_set_mac_vendor, maybe ?) that > >> can be called at this point to set the vendor id ? The default behavior > >> should be to set the Freescale's vendor id. > > > > ethaddr and eth1addr is insufficient? > > In a standard way, the environment must be adapted for each board to > store the MAC address in the ethaddr variable. The reason to get the MAC > directly from the hardware is that it is not required a specific initial > setup for each board, and that simplifies the production and the > delivery of the boards. Sure, but the OCOTP capacity here is limited. But there was approach from Fabio/me how to make the OCOTP good enough for two NICs even. * The idea is to let user configure the top 2 bytes per-device (which is the most likely case). * Introduce mac_modify(int fec, char *mac) function, which will be called from imx_get_mac_from_fuse() * The function will adjust the MAC, for example by setting top two bytes to preconfigured data, bottom four bytes from OCOTP and the last bit of the MAC from "int fec", which is 0 for FEC0 and 1 for FEC1. This should be sufficient for most people. * The function will be weak so it can be overridden to your hearts content. > > If you rely on ethaddr, than we do not need imx_get_mac_from_fuse() at > all, and the MAC can be set to zero, because in this case the ethaddr > value is used - but we lose the feature. > > The reason why imx_get_mac_from_fuse was introduce is to have an > automatic mechanism to set up the MAC without any customization. > > Best regards, > Stefano Babic Cheers! M
On 16/12/2011 11:39, Marek Vasut wrote: >> >> Hi Marek, > > Hi Stefano, > > DISCLAIMER: I'm terribly tired today, had early class. ok, I understand..;-) > > You disable fuses and use ethaddr. Why do we need them? We don't, but the > hardware is in the CPU and someone might use it so let's support it. Exactly - see also othe IMX SOCs. >> In a standard way, the environment must be adapted for each board to >> store the MAC address in the ethaddr variable. The reason to get the MAC >> directly from the hardware is that it is not required a specific initial >> setup for each board, and that simplifies the production and the >> delivery of the boards. > > Sure, but the OCOTP capacity here is limited. But there was approach from > Fabio/me how to make the OCOTP good enough for two NICs even. And this is nice. > * The idea is to let user configure the top 2 bytes per-device (which is the > most likely case). This is exactly what I meant. > * Introduce mac_modify(int fec, char *mac) function, which will be called from > imx_get_mac_from_fuse() > * The function will adjust the MAC, for example by setting top two bytes to > preconfigured data, bottom four bytes from OCOTP and the last bit of the MAC > from "int fec", which is 0 for FEC0 and 1 for FEC1. This should be sufficient > for most people. Exactly ! > * The function will be weak so it can be overridden to your hearts content. Right ! Stefano
> On 16/12/2011 11:39, Marek Vasut wrote: > >> Hi Marek, > > > > Hi Stefano, > > > > DISCLAIMER: I'm terribly tired today, had early class. > > ok, I understand..;-) > > > You disable fuses and use ethaddr. Why do we need them? We don't, but the > > hardware is in the CPU and someone might use it so let's support it. > > Exactly - see also othe IMX SOCs. > > >> In a standard way, the environment must be adapted for each board to > >> store the MAC address in the ethaddr variable. The reason to get the MAC > >> directly from the hardware is that it is not required a specific initial > >> setup for each board, and that simplifies the production and the > >> delivery of the boards. > > > > Sure, but the OCOTP capacity here is limited. But there was approach from > > Fabio/me how to make the OCOTP good enough for two NICs even. > > And this is nice. > > > * The idea is to let user configure the top 2 bytes per-device (which is > > the most likely case). > > This is exactly what I meant. > > > * Introduce mac_modify(int fec, char *mac) function, which will be called > > from imx_get_mac_from_fuse() > > * The function will adjust the MAC, for example by setting top two bytes > > to preconfigured data, bottom four bytes from OCOTP and the last bit of > > the MAC from "int fec", which is 0 for FEC0 and 1 for FEC1. This should > > be sufficient for most people. > > Exactly ! > > > * The function will be weak so it can be overridden to your hearts > > content. > > Right ! > > Stefano And it was mostly Fabio's idea I'd say ;-) M
diff --git a/arch/arm/cpu/arm926ejs/mx28/mx28.c b/arch/arm/cpu/arm926ejs/mx28/mx28.c index 088c019..785978f 100644 --- a/arch/arm/cpu/arm926ejs/mx28/mx28.c +++ b/arch/arm/cpu/arm926ejs/mx28/mx28.c @@ -214,6 +214,41 @@ int cpu_eth_init(bd_t *bis) } #endif +#ifdef CONFIG_MX28_FEC_MAC_IN_OCOTP + +#define MXS_OCOTP_MAX_TIMEOUT 1000000 +void imx_get_mac_from_fuse(char *mac) +{ + struct mx28_ocotp_regs *ocotp_regs = + (struct mx28_ocotp_regs *)MXS_OCOTP_BASE; + uint32_t data; + + memset(mac, 0, 6); + + writel(OCOTP_CTRL_RD_BANK_OPEN, &ocotp_regs->hw_ocotp_ctrl_set); + + if (mx28_wait_mask_clr(&ocotp_regs->hw_ocotp_ctrl_reg, OCOTP_CTRL_BUSY, + MXS_OCOTP_MAX_TIMEOUT)) { + puts("MXS FEC: Can't get MAC from OCOTP\n"); + return; + } + + data = readl(&ocotp_regs->hw_ocotp_cust0); + + mac[0] = 0x00; + mac[1] = 0x04; + mac[2] = (data >> 24) & 0xff; + mac[3] = (data >> 16) & 0xff; + mac[4] = (data >> 8) & 0xff; + mac[5] = data & 0xff; +} +#else +void imx_get_mac_from_fuse(char *mac) +{ + memset(mac, 0, 6); +} +#endif + U_BOOT_CMD( clocks, CONFIG_SYS_MAXARGS, 1, do_mx28_showclocks, "display clocks", diff --git a/arch/arm/include/asm/arch-mx28/sys_proto.h b/arch/arm/include/asm/arch-mx28/sys_proto.h index be1f7db..cf5ab16 100644 --- a/arch/arm/include/asm/arch-mx28/sys_proto.h +++ b/arch/arm/include/asm/arch-mx28/sys_proto.h @@ -35,4 +35,6 @@ void mx28_common_spl_init(const iomux_cfg_t *iomux_setup, const unsigned int iomux_size); #endif +void imx_get_mac_from_fuse(char *mac); + #endif /* __MX28_H__ */ diff --git a/board/denx/m28evk/m28evk.c b/board/denx/m28evk/m28evk.c index fcee046..005446a 100644 --- a/board/denx/m28evk/m28evk.c +++ b/board/denx/m28evk/m28evk.c @@ -178,39 +178,4 @@ int board_eth_init(bd_t *bis) return ret; } -#ifdef CONFIG_M28_FEC_MAC_IN_OCOTP - -#define MXS_OCOTP_MAX_TIMEOUT 1000000 -void imx_get_mac_from_fuse(char *mac) -{ - struct mx28_ocotp_regs *ocotp_regs = - (struct mx28_ocotp_regs *)MXS_OCOTP_BASE; - uint32_t data; - - memset(mac, 0, 6); - - writel(OCOTP_CTRL_RD_BANK_OPEN, &ocotp_regs->hw_ocotp_ctrl_set); - - if (mx28_wait_mask_clr(&ocotp_regs->hw_ocotp_ctrl_reg, OCOTP_CTRL_BUSY, - MXS_OCOTP_MAX_TIMEOUT)) { - printf("MXS FEC: Can't get MAC from OCOTP\n"); - return; - } - - data = readl(&ocotp_regs->hw_ocotp_cust0); - - mac[0] = 0x00; - mac[1] = 0x04; - mac[2] = (data >> 24) & 0xff; - mac[3] = (data >> 16) & 0xff; - mac[4] = (data >> 8) & 0xff; - mac[5] = data & 0xff; -} -#else -void imx_get_mac_from_fuse(char *mac) -{ - memset(mac, 0, 6); -} -#endif - #endif
Let imx_get_mac_from_fuse function be a common function, so that other mx28 boards can reuse it. Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> --- arch/arm/cpu/arm926ejs/mx28/mx28.c | 35 ++++++++++++++++++++++++++++ arch/arm/include/asm/arch-mx28/sys_proto.h | 2 + board/denx/m28evk/m28evk.c | 35 ---------------------------- 3 files changed, 37 insertions(+), 35 deletions(-)