diff mbox

ARM: dts: rockchip: Setup ethernet0 alias for u-boot

Message ID 1446151497-29833-1-git-send-email-sjoerd.simons@collabora.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Sjoerd Simons Oct. 29, 2015, 8:44 p.m. UTC
Add an ethernet0 alias for the wired network card and an all 0 default
mac address so that u-boot can find the device-node and fill in the
mac address.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---

 arch/arm/boot/dts/rk3288-rock2-square.dts | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Heiko Stuebner Oct. 29, 2015, 8:52 p.m. UTC | #1
Hi Sjoerd,

Am Donnerstag, 29. Oktober 2015, 21:44:57 schrieb Sjoerd Simons:
> Add an ethernet0 alias for the wired network card and an all 0 default
> mac address so that u-boot can find the device-node and fill in the
> mac address.
> 
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> ---
> 
>  arch/arm/boot/dts/rk3288-rock2-square.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts b/arch/arm/boot/dts/rk3288-rock2-square.dts
> index 0ef065d..a029ebf 100644
> --- a/arch/arm/boot/dts/rk3288-rock2-square.dts
> +++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
> @@ -45,6 +45,10 @@
>  	model = "Radxa Rock 2 Square";
>  	compatible = "radxa,rock2-square", "rockchip,rk3288";
>  
> +	aliases {
> +		ethernet0 = &gmac;
> +	};
> +

wouldn't it make more sense to have this in the rk3288.dtsi, so to keep
every board from having to add this alias?


>  	chosen {
>  		stdout-path = "serial2:115200n8";
>  	};
> @@ -130,6 +134,8 @@
>  };
>  
>  &gmac {
> +	/* To be filled in by U-Boot */
> +	mac-address = [00 00 00 00 00 00];
>  	status = "ok";
>  };

I guess the same applies for the mac placeholder.

Heiko
Sjoerd Simons Oct. 29, 2015, 9:39 p.m. UTC | #2
On Thu, 2015-10-29 at 21:52 +0100, Heiko Stuebner wrote:
> Hi Sjoerd,
> 
> Am Donnerstag, 29. Oktober 2015, 21:44:57 schrieb Sjoerd Simons:
> > Add an ethernet0 alias for the wired network card and an all 0
> > default
> > mac address so that u-boot can find the device-node and fill in the
> > mac address.
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> > ---
> > 
> >  arch/arm/boot/dts/rk3288-rock2-square.dts | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts
> > b/arch/arm/boot/dts/rk3288-rock2-square.dts
> > index 0ef065d..a029ebf 100644
> > --- a/arch/arm/boot/dts/rk3288-rock2-square.dts
> > +++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
> > @@ -45,6 +45,10 @@
> >  	model = "Radxa Rock 2 Square";
> >  	compatible = "radxa,rock2-square", "rockchip,rk3288";
> >  
> > +	aliases {
> > +		ethernet0 = &gmac;
> > +	};
> > +
> 
> wouldn't it make more sense to have this in the rk3288.dtsi, so to
> keep
> every board from having to add this alias?

Hmm, that would work. Thought it would add an alias point to a disabled
node for boards that don't have a wired interface. If that isn't an
issue it can indeed be moved there.
> 
> >  	chosen {
> >  		stdout-path = "serial2:115200n8";
> >  	};
> > @@ -130,6 +134,8 @@
> >  };
> >  
> >  &gmac {
> > +	/* To be filled in by U-Boot */
> > +	mac-address = [00 00 00 00 00 00];
> >  	status = "ok";
> >  };
> 
> I guess the same applies for the mac placeholder.

In any case they should be in the same dts{i}.

-- 
Sjoerd Simons
Collabora Ltd.
Heiko Stuebner Oct. 29, 2015, 10:57 p.m. UTC | #3
Am Donnerstag, 29. Oktober 2015, 22:39:24 schrieb Sjoerd Simons:
> On Thu, 2015-10-29 at 21:52 +0100, Heiko Stuebner wrote:
> > Hi Sjoerd,
> > 
> > Am Donnerstag, 29. Oktober 2015, 21:44:57 schrieb Sjoerd Simons:
> > > Add an ethernet0 alias for the wired network card and an all 0
> > > default
> > > mac address so that u-boot can find the device-node and fill in the
> > > mac address.
> > > 
> > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> > > ---
> > > 
> > >  arch/arm/boot/dts/rk3288-rock2-square.dts | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts
> > > b/arch/arm/boot/dts/rk3288-rock2-square.dts
> > > index 0ef065d..a029ebf 100644
> > > --- a/arch/arm/boot/dts/rk3288-rock2-square.dts
> > > +++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
> > > @@ -45,6 +45,10 @@
> > >  	model = "Radxa Rock 2 Square";
> > >  	compatible = "radxa,rock2-square", "rockchip,rk3288";
> > >  
> > > +	aliases {
> > > +		ethernet0 = &gmac;
> > > +	};
> > > +
> > 
> > wouldn't it make more sense to have this in the rk3288.dtsi, so to
> > keep
> > every board from having to add this alias?
> 
> Hmm, that would work. Thought it would add an alias point to a disabled
> node for boards that don't have a wired interface. If that isn't an
> issue it can indeed be moved there.

we also have aliases for all i2c, mmc, spi, uart etc nodes ... some of
which may stay disabled. But i2c5 stays i2c5 everywhere
and similarly the internal ethernet will stay ethernet0 everywhere too.

And for the "fluffy" good feeling, a lot of other socs also seem to keep their
ethernet0 aliases in the central dtsi (allwinner, freescale, socfpga),
so it's not obviously wrong :-) .


Heiko
Geert Uytterhoeven Oct. 30, 2015, 9:15 a.m. UTC | #4
Hi Sjoerd,

On Thu, Oct 29, 2015 at 9:44 PM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> Add an ethernet0 alias for the wired network card and an all 0 default
> mac address so that u-boot can find the device-node and fill in the
> mac address.

Thanks a lot!

I had the same issue on another board, and adding an "ethernet0" alias
fixes it.

> @@ -130,6 +134,8 @@
>  };
>
>  &gmac {
> +       /* To be filled in by U-Boot */
> +       mac-address = [00 00 00 00 00 00];
>         status = "ok";
>  };

Note that in my case I didn't have to add an empty mac-address property.
U-Boot (2015.04-something --- don't have the sources) seems to add a
"local-mac-address" property automatically.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Sjoerd Simons Oct. 30, 2015, 9:47 a.m. UTC | #5
On Fri, 2015-10-30 at 10:15 +0100, Geert Uytterhoeven wrote:
> Hi Sjoerd,
> 
> On Thu, Oct 29, 2015 at 9:44 PM, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
> > Add an ethernet0 alias for the wired network card and an all 0
> > default
> > mac address so that u-boot can find the device-node and fill in the
> > mac address.
> 
> Thanks a lot!
> 
> I had the same issue on another board, and adding an "ethernet0"
> alias
> fixes it.

Happy to help, this stuff is really not that well document
unfortunately.

> > @@ -130,6 +134,8 @@
> >  };
> > 
> >  &gmac {
> > +       /* To be filled in by U-Boot */
> > +       mac-address = [00 00 00 00 00 00];
> >         status = "ok";
> >  };
> 
> Note that in my case I didn't have to add an empty mac-address
> property.
> U-Boot (2015.04-something --- don't have the sources) seems to add a
> "local-mac-address" property automatically.

After submitting the patch i actually dived a bit deeper at how this
all hangs together.

The linux kernel checks for mac-address, local-mac-address and address
in the fdt node, in that order of preference. u-boot (at least recent
ones like you have), will set the mac-address property *if* it's
already defined but will always create the local-mac-address property
regardless.

So indeed practially, as you noticed, just setting the alias is enough.
I'm now just very confused about the intended meanings of the
properties, Documentation/devicetree/bindings/net/ethernet.txt doesn't
shed a lot of light on it:

- local-mac-address: array of 6 bytes, specifies the MAC address that 
  was assigned to the network device;
- mac-address: array of 6 bytes, specifies the MAC address that was 
  last used by the boot program; should be used in cases where the MAC 
  address assigned to the device by the boot program is different from 
  the "local-mac-address"property;

So yeah, I'm not sure what the "correct" fix is. It's tempting to
indeed just leave out the mac-address property though :)
Geert Uytterhoeven Oct. 30, 2015, 9:57 a.m. UTC | #6
Hi Sjoerd,

On Fri, Oct 30, 2015 at 10:47 AM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
>> Note that in my case I didn't have to add an empty mac-address
>> property.
>> U-Boot (2015.04-something --- don't have the sources) seems to add a
>> "local-mac-address" property automatically.
>
> After submitting the patch i actually dived a bit deeper at how this
> all hangs together.
>
> The linux kernel checks for mac-address, local-mac-address and address
> in the fdt node, in that order of preference. u-boot (at least recent
> ones like you have), will set the mac-address property *if* it's
> already defined but will always create the local-mac-address property
> regardless.

That's also what I noticed. Originally I added the empty "mac-address" like
you did, and /sys/firmware/devicetree/base/soc/ethernet@e6800000 ended up
having both "mac-address" and "local-mac-address", both with the correct
address.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
diff mbox

Patch

diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts b/arch/arm/boot/dts/rk3288-rock2-square.dts
index 0ef065d..a029ebf 100644
--- a/arch/arm/boot/dts/rk3288-rock2-square.dts
+++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
@@ -45,6 +45,10 @@ 
 	model = "Radxa Rock 2 Square";
 	compatible = "radxa,rock2-square", "rockchip,rk3288";
 
+	aliases {
+		ethernet0 = &gmac;
+	};
+
 	chosen {
 		stdout-path = "serial2:115200n8";
 	};
@@ -130,6 +134,8 @@ 
 };
 
 &gmac {
+	/* To be filled in by U-Boot */
+	mac-address = [00 00 00 00 00 00];
 	status = "ok";
 };