[PATCH v1 5/7] arm: dts: s700: add node for ethernet controller

["André Przywara" <andre.przywara@arm.com>]

On 12/05/2020 15:37, Amit Tomer wrote:
> Hi,
> 
> On Tue, May 12, 2020 at 7:49 PM Andr? Przywara <andre.przywara@arm.com> wrote:
>>
>> On 09/05/2020 15:25, Amit Singh Tomar wrote:
>>> This patch adds node for ethernet controller found on Action Semi OWL
>>> S700 SoC.
>>>
>>> Since, there is no upstream Linux binding exist for S700 ethernet
>>> controller, Changes are put in u-boot specific dtsi file.
>>
>> But that should not be the S700 SoC .dtsi, instead the cubieboard .dts
>> file, since you specify the PHY mode in here (which is board specific).
> 
> But MAC is present on SoC , so I thought of adding it s700 specific
> u-boot.dtsi as
> it is already hosting few other things .

The MAC is indeed, but not the PHY and the MAC address.
And since this -u-boot.dtsi trick only works once in the chain, you have
to move this to the board version of the file.
Doesn't really make any difference looking at the vast number of boards
using the S700 ;-)

> 
>>> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
>>> ---
>>>  arch/arm/dts/s700-u-boot.dtsi | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>>
>>> diff --git a/arch/arm/dts/s700-u-boot.dtsi b/arch/arm/dts/s700-u-boot.dtsi
>>> index a527cccc75f2..1b2768272c62 100644
>>> --- a/arch/arm/dts/s700-u-boot.dtsi
>>> +++ b/arch/arm/dts/s700-u-boot.dtsi
>>> @@ -6,6 +6,19 @@
>>>  /{
>>>       soc {
>>>               u-boot,dm-pre-reloc;
>>> +
>>> +             gmac:  ethernet at e0220000 {
>>> +                     compatible = "actions,s700-ethernet";
>>> +                     reg = <0 0xe0220000 0 0x2000>;
>>> +                     interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
>>> +                     interrupt-names = "macirq";
>>> +                     local-mac-address = [ 00 18 fe 66 66 66 ];
>>
>> Is there another solution to that? Maybe put that in the environment
>> instead? Or generate this randomly or ideally by hashing some serial number?
> 
> Yeah, I tried having "CONFIG_NET_RANDOM_ETHADDR" other day which seems
> to work(would double check it).
> 
> But is there a reason not to use "local-mac-address", since driver
> does not support parsing this
> property ?

I don't think local-mac-address is meant to be in a .dts file, actually.
It's more useful to communicate some MAC address between firmware and
bootloaders/kernels. You can use this in your development setup, but
this should not be submitted. Otherwise every board on the planet would
use the same MAC address.

Cheers,
Andre