[U-Boot] [PATCH 5/6] arm: sunxi: Use board hooks to obtain MAC address

Olliver Schinagl o.schinagl at ultimaker.com
Tue Apr 11 20:14:46 UTC 2017


Hey Joe,

On 11-04-17 00:56, Joe Hershberger wrote:
> On Fri, Apr 7, 2017 at 8:57 AM,  <o.schinagl at ultimaker.com> wrote:
>> On Fri, 2017-04-07 at 15:45 +0200, Olliver Schinagl wrote:
>>> Hey Joe,
>>>
>>> On 30-11-16 22:36, Joe Hershberger wrote:
>>>> On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver at schinagl.
>>>> nl> wrote:
>>>>> Add board hooks allowing to get ethernet addresses in a board
>>>>> specific
>>>>> manner. Currently this is done by generating a MAC address from
>>>>> the SID and injecting the ethernet device number in the first
>>>>> octet.
>>>>>
>>>>> This usually happens as a fallback, if either the eeprom fails to
>>>>> set a
>>>>> MAC address or the FDT forces an override.
>>>>>
>>>>> Signed-off-by: Olliver Schinagl <oliver at schinagl.nl>
>>>>> ---
>>>>>   arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
>>>>>   board/sunxi/board.c                         | 161
>>>>> +++++++++++++++-------------
>>>>>   net/eth_legacy.c                            |   1 +
>>>>>   3 files changed, 98 insertions(+), 75 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> index a373319..fad7c48 100644
>>>>> --- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> +++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> @@ -30,4 +30,15 @@ void eth_init_board(void);
>>>>>   static inline void eth_init_board(void) {}
>>>>>   #endif
>>>>>
>>>>> +int board_get_enetaddr(const int i, unsigned char *mac_addr);
>>>>> +
>>>>> +#if CONFIG_SUNXI_EMAC
>>>>> +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr,
>>>>> int id);
>>>>> +#endif
>>>>> +
>>>>> +#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
>>>>> +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
>>>>> +#endif
>>>>> +
>>>>> +
>>>>>   #endif
>>>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>>>>> index 5365638..4aeab51 100644
>>>>> --- a/board/sunxi/board.c
>>>>> +++ b/board/sunxi/board.c
>>>>> @@ -21,6 +21,7 @@
>>>>>   #include <asm/arch/gpio.h>
>>>>>   #include <asm/arch/mmc.h>
>>>>>   #include <asm/arch/spl.h>
>>>>> +#include <asm/arch/sys_proto.h>
>>>>>   #include <asm/arch/usb_phy.h>
>>>>>   #ifndef CONFIG_ARM64
>>>>>   #include <asm/armv7.h>
>>>>> @@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
>>>>>   }
>>>>>   #endif
>>>>>
>>>>> +int sunxi_get_board_serial(unsigned int *serial)
>>>>> +{
>>>>> +       int ret;
>>>>> +
>>>>> +       ret = sunxi_get_sid(serial);
>>>>> +       if (!ret || serial[0])
>>>>> +               return -ENOSYS;
>>>>> +
>>>>> +       /*
>>>>> +        * The single words 1 - 3 of the SID have quite a few
>>>>> bits
>>>>> +        * which are the same on many models, so we take a crc32
>>>>> +        * of all 3 words, to get a more unique value.
>>>>> +        *
>>>>> +        * Note we only do this on newer SoCs as we cannot change
>>>>> +        * the algorithm on older SoCs since those have been
>>>>> using
>>>>> +        * fixed mac-addresses/serial based on only using word 3
>>>>> for a
>>>>> +        * long time and changing a fixed mac-address/serial with
>>>>> an
>>>>> +        * u-boot update is not good.
>>>>> +        */
>>>>> +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I)
>>>>> && \
>>>>> +    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I)
>>>>> && \
>>>>> +    !defined(CONFIG_MACH_SUN8I_A23) &&
>>>>> !defined(CONFIG_MACH_SUN8I_A33)
>>>>> +       serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
>>>>> +#endif
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>>   #ifdef CONFIG_SERIAL_TAG
>>>>>   void get_board_serial(struct tag_serialnr *serialnr)
>>>>>   {
>>>>> @@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr
>>>>> *serialnr)
>>>>>   #endif
>>>>>
>>>>>   /*
>>>>> + * Generate a MAC address based on device index and the serial
>>>>> number.
>>>>> + * The first half of the of the first octet holds the eth index.
>>>>> + *
>>>>> + * In the second octet we forcefully mark the MAC address to a
>>>>> locally
>>>>> + * administered MAC address.
>>>>> + *
>>>>> + */
>>>>> +int board_get_enetaddr(const int index, unsigned char *enetaddr)
>>>> This would be part of a board-specific eth driver.
>>> this is being called now from sunxi_gmac.c and sunxi_emac.c and
>>> supplies
>>> these board specific drivers with a mac address based on the serial
>>> number of the board. I could move this logic over, but then i'd have
>>> to
>>> add it to both eth drivers. By having it in the board.c file, we have
>>> 2
>>> simple functions in the board-specific eth driver:
>>>
>>>
>>> static int sunxi_gmac_eth_read_rom_hwaddr(struct udevice *dev)
>>> {
>>>        struct eth_pdata *pdata = dev_get_platdata(dev);
>>>
>>>        return board_get_enetaddr(dev->seq, pdata->enetaddr);
>>> }
>>>
>>> So do you propose to dupilicate the code into both board specific
>>> drivers, have it named differently or that the shared code live
>>> elsewhere?
>> Replying to myself here,
>>
>> I just realized, while this bit was not accepted, the overal
>> implementation has changed in the set. So before, I did things wrong :)
>> As Simon explained last time.
>>
>> To clarify, I now have added the logic to the sunxi gmac and emac board
>> specific drivers. But afaik they share no common code. (like
>> sunxi_common.c in drivers/net)
>>
>> With that in mind, how we did things up until now, was to have a
>> fallback scenario where we use the SoC serial number to generate a MAC
>> address.
>>
>> If this is go be done with the board specific driver, we'd need to
>> still however call board specific functions (sunxi_get_board_serial).
>>
>> The solution I think is still one of the previously mentioned, a
>> sunxi_common.c which does the serial -> MAC conversion according to the
>> previous logic, using sunxi_get_board_serial() (which really is a SoC
>> specific function, rather board) or have (sunxi)_board_get_enetaddr()
>> in the same spot where it is now.
>>
>> Writing this, I realize the sunxi_common.c approach may not be half
>> bad, even if it only contains a single function for now.
> Sounds like the common layout has promise. Care to send an RFC for how
> it would look?
Not sure if we are on the same page :) but I've sent a patch set which 
includes the change. Find it on patchwork [0].

We should discuss it there I reccon, but for now, it is a single 
function for the read_rom_hwaddr hook for sunxi_emac, sunxi_gmac and 
sun8i_emac. I saw more similar code paths, such as the write_hwaddr, 
which could move here as well. I think in time with some effort a few 
more functions can go into this shared file.

The only thing I haven't figured out yet, where I did continue the 
original thread [1], is how to solve the fdt_fixup scenario.

Olliver

[0] https://patchwork.ozlabs.org/patch/749103/
[1] https://patchwork.ozlabs.org/patch/699288/
>
> Thanks,
> -Joe


-- 
Met vriendelijke groeten, Kind regards, 与亲切的问候
Olliver Schinagl
Research & Development
Ultimaker B.V.
http://www.ultimaker.com



More information about the U-Boot mailing list