Re: [PATCH v2] ARM: dts: exynos: Add a placeholder for a MAC address

From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Anand Moon <linux.amoon@gmail.com>
Cc: "Łukasz Stelmach" <l.stelmach@samsung.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Kukjin Kim" <kgene@kernel.org>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linux-samsung-soc@vger.kernel.org,
	"Linux Kernel" <linux-kernel@vger.kernel.org>,
	"Bartlomiej Zolnierkiewicz" <b.zolnierkie@samsung.com>
Subject: Re: [PATCH v2] ARM: dts: exynos: Add a placeholder for a MAC address
Date: Thu, 5 Nov 2020 09:18:26 +0100	[thread overview]
Message-ID: <71f06064-079c-1044-a51a-ce1ea7cc4049@samsung.com> (raw)
In-Reply-To: <CANAwSgR=20AOVxi=5L_ep=uK98dzL6Ent52Mux1_iviiK-XGPA@mail.gmail.com>

Hi Anand,

On 05.11.2020 09:06, Anand Moon wrote:
> On Mon, 2 Nov 2020 at 21:53, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> On 01.11.2020 15:07, Anand Moon wrote:
>>> On Thu, 1 Oct 2020 at 19:25, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
>>>> Add a placeholder for a MAC address. A bootloader may fill it
>>>> to set the MAC address and override EEPROM settings.
>>>>
>>>> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
>>>> ---
>>>> Changes in v2:
>>>>    - use local-mac-address and leave mac-address to be added by a bootloader
>>>>
>>>>    arch/arm/boot/dts/exynos5422-odroidxu3.dts | 18 ++++++++++++++++++
>>>>    1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3.dts b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
>>>> index db0bc17a667b..d0f6ac5fa79d 100644
>>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3.dts
>>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
>>>> @@ -70,3 +70,21 @@ &pwm {
>>>>    &usbdrd_dwc3_1 {
>>>>           dr_mode = "peripheral";
>>>>    };
>>>> +
>>>> +&usbhost2 {
>>>> +       #address-cells = <1>;
>>>> +       #size-cells = <0>;
>>>> +
>>>> +       hub@1 {
>>>> +               compatible = "usb8087,0024";
>>>> +               reg = <1>;
>>>> +               #address-cells = <1>;
>>>> +               #size-cells = <0>;
>>>> +
>>>> +               ethernet: usbether@1 {
>>>> +                       compatible = "usb0c45,6310";
>>>> +                       reg = <1>;
>>>> +                       local-mac-address = [00 00 00 00 00 00]; /* Filled in by a bootloader */
>>>> +               };
>>>> +       };
>>>> +};
>>>> --
>>>> 2.26.2
>>>>
>>> Thanks for this patch, can you share some example on how to set the
>>> mac address via u-boot bootargs
>> A little bit hacky script to set permanent board unique MAC address:
>>
>> # setexp.b u0 *0x10000014; setexp.b u1 *0x10000015; setexp.b u2
>> *0x10000016; setexp.b u3 *0x10000017; setenv ethaddr
>> 0:0:${u0}:${u1}:${u2}:${u3}; setenv usbethaddr ${ethaddr};
>>
> OK this command worked for me.
>
>> Then if there is proper ethernet0 alias set, u-boot will then
>> automatically save the configured MAC address to the device tree. I've
>> just check this on recent u-boot v2020.10 and Odroid U3 board.
>>
>> Lukasz will send updated patch soon (with proper alias entry).
>>
>> If you want to hack setting MAC address manually, this will work with
>> the current patch:
>>
>> # setexp.b u0 *0x10000014; setexp.b u1 *0x10000015; setexp.b u2
>> *0x10000016; setexp.b u3 *0x10000017; fdt addr ${fdtaddr}; fdt set
>> /soc/usb@12110000/hub@1/usbether@1 local-mac-address [ 0 0 ${u0} ${u1}
>> ${u2} ${u3} ]
>>
> So do we need a similar patch for u-boot ?

I've not sure that this ethaddr hack/workaround should be added to 
mainline uboot. Some other exynos based board have proper MAC address 
stored in EEPROM (for example Odroid XU4/HC1). I would leave it for the 
users to add it manually if it is really needed for now.

> I am getting following error on Odroid U3+ and U-Boot 2020.10
>
> Odroid #  setexp.b u0 *0x10000014; setexp.b u1 *0x10000015; setexp.b
> u2 *0x10000016; setexp.b u3 *0x10000017; fdt addr ${fdtaddr}; fdt set
> /soc/usb@12110000/hub@1/usbether@1 local-mac-address [ 0 0 ${u0} ${u1}
> ${u2} ${u3} ]
> No FDT memory address configured. Please configure
> the FDT address via "fdt addr <address>" command.
> Aborting!
>
> Also added these command to boot.scr but still observing the failure

You need to use proper env for setting fdt address (the "fdt addr 
${fdtaddr}" command). For some versions it was ${fdt_addr}, the other 
used ${fdtaddr}. Please check which one is used for loading dtb and 
adjust the script.

Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland