[PATCH] powerpc: define the fman node for the kmcoge4 DTS

[PATCH] powerpc: define the fman node for the kmcoge4 DTS

[Valentin Longchamp]

Now that the FMAN mac driver has been merged the fman node is relevant.

The kmcoge4 board implements 3 ethernet interfaces, 1 with a RGMII phy
and 2 with fixed 1 Giga SGMII links.

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
---
 arch/powerpc/boot/dts/fsl/kmcoge4.dts | 39 +++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/powerpc/boot/dts/fsl/kmcoge4.dts b/arch/powerpc/boot/dts/fsl/kmcoge4.dts
index 6858ec9..1cec66d 100644
--- a/arch/powerpc/boot/dts/fsl/kmcoge4.dts
+++ b/arch/powerpc/boot/dts/fsl/kmcoge4.dts
@@ -106,6 +106,45 @@
 		sata@221000 {
 			status = "disabled";
 		};
+
+		fman0: fman@400000 {
+			enet0: ethernet@e0000 {
+				phy-connection-type = "sgmii";
+				local-mac-address = [00 11 22 33 44 55];
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
+			};
+			mdio0: mdio@e1120 {
+				front_phy: ethernet-phy@11 {
+					reg = <0x11>;
+				};
+			};
+
+			enet1: ethernet@e2000 {
+				phy-connection-type = "sgmii";
+				local-mac-address = [00 11 22 33 44 56];
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
+			};
+			enet2: ethernet@e4000 {
+				status = "disabled";
+			};
+
+			enet3: ethernet@e6000 {
+				status = "disabled";
+			};
+			enet4: ethernet@e8000 {
+				phy-handle = <&front_phy>;
+				phy-connection-type = "rgmii";
+			};
+			enet5: ethernet@f0000 {
+				status = "disabled";
+			};
+		};
 	};
 
 	rio: rapidio@ffe0c0000 {
-- 
1.8.3.1

Re: [PATCH] powerpc: define the fman node for the kmcoge4 DTS

[Scott Wood]

On Wed, 2016-04-06 at 15:37 +0200, Valentin Longchamp wrote:
> Now that the FMAN mac driver has been merged the fman node is relevant.
> 
> The kmcoge4 board implements 3 ethernet interfaces, 1 with a RGMII phy
> and 2 with fixed 1 Giga SGMII links.
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> ---
>  arch/powerpc/boot/dts/fsl/kmcoge4.dts | 39
> +++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/arch/powerpc/boot/dts/fsl/kmcoge4.dts
> b/arch/powerpc/boot/dts/fsl/kmcoge4.dts
> index 6858ec9..1cec66d 100644
> --- a/arch/powerpc/boot/dts/fsl/kmcoge4.dts
> +++ b/arch/powerpc/boot/dts/fsl/kmcoge4.dts
> @@ -106,6 +106,45 @@
>  		sata@221000 {
>  			status = "disabled";
>  		};
> +
> +		fman0: fman@400000 {
> +			enet0: ethernet@e0000 {
> +				phy-connection-type = "sgmii";
> +				local-mac-address = [00 11 22 33 44 55];
> +				fixed-link {
> +					speed = <1000>;
> +					full-duplex;
> +				};
> +			};
> +			mdio0: mdio@e1120 {
> +				front_phy: ethernet-phy@11 {
> +					reg = <0x11>;
> +				};
> +			};
> +
> +			enet1: ethernet@e2000 {
> +				phy-connection-type = "sgmii";
> +				local-mac-address = [00 11 22 33 44 56];
> +				fixed-link {
> +					speed = <1000>;
> +					full-duplex;
> +				};
> +			};

No hardcoded MAC addresses.

-Scott

Re: [PATCH] powerpc: define the fman node for the kmcoge4 DTS

[Valentin Longchamp]

On 06/04/16 23:49, Scott Wood wrote:
> On Wed, 2016-04-06 at 15:37 +0200, Valentin Longchamp wrote:
>> Now that the FMAN mac driver has been merged the fman node is relevant.
>>
>> The kmcoge4 board implements 3 ethernet interfaces, 1 with a RGMII phy
>> and 2 with fixed 1 Giga SGMII links.
>>
>> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
>> ---
>>  arch/powerpc/boot/dts/fsl/kmcoge4.dts | 39
>> +++++++++++++++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/arch/powerpc/boot/dts/fsl/kmcoge4.dts
>> b/arch/powerpc/boot/dts/fsl/kmcoge4.dts
>> index 6858ec9..1cec66d 100644
>> --- a/arch/powerpc/boot/dts/fsl/kmcoge4.dts
>> +++ b/arch/powerpc/boot/dts/fsl/kmcoge4.dts
>> @@ -106,6 +106,45 @@
>>  		sata@221000 {
>>  			status = "disabled";
>>  		};
>> +
>> +		fman0: fman@400000 {
>> +			enet0: ethernet@e0000 {
>> +				phy-connection-type = "sgmii";
>> +				local-mac-address = [00 11 22 33 44 55];
>> +				fixed-link {
>> +					speed = <1000>;
>> +					full-duplex;
>> +				};
>> +			};
>> +			mdio0: mdio@e1120 {
>> +				front_phy: ethernet-phy@11 {
>> +					reg = <0x11>;
>> +				};
>> +			};
>> +
>> +			enet1: ethernet@e2000 {
>> +				phy-connection-type = "sgmii";
>> +				local-mac-address = [00 11 22 33 44 56];
>> +				fixed-link {
>> +					speed = <1000>;
>> +					full-duplex;
>> +				};
>> +			};
> 
> No hardcoded MAC addresses.
> 

For these 2 interfaces where I have the local-mac-address field, the MAC
addresses are set later by an application that reads the real address in some
EEPROM. However, in order to let the fman mac_probe to run successfully in the
first place I have set non-zero MAC addresses since the local-mac-address fields
are not set by u-boot.

I have found several local-mac-address fields in other DTS files that are all
zeros, and thus are rejected by of_get_mac_address. Are they leftovers from the
past or should they be used here as well ? If not, I will simply drop these 2
fields.

Thanks

Valentin

Re: [PATCH] powerpc: define the fman node for the kmcoge4 DTS

[Scott Wood]

On Thu, 2016-04-07 at 08:14 +0200, Valentin Longchamp wrote:
> On 06/04/16 23:49, Scott Wood wrote:
> > On Wed, 2016-04-06 at 15:37 +0200, Valentin Longchamp wrote:
> > > Now that the FMAN mac driver has been merged the fman node is relevant.
> > > 
> > > The kmcoge4 board implements 3 ethernet interfaces, 1 with a RGMII phy
> > > and 2 with fixed 1 Giga SGMII links.
> > > 
> > > Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> > > ---
> > >  arch/powerpc/boot/dts/fsl/kmcoge4.dts | 39
> > > +++++++++++++++++++++++++++++++++++
> > >  1 file changed, 39 insertions(+)
> > > 
> > > diff --git a/arch/powerpc/boot/dts/fsl/kmcoge4.dts
> > > b/arch/powerpc/boot/dts/fsl/kmcoge4.dts
> > > index 6858ec9..1cec66d 100644
> > > --- a/arch/powerpc/boot/dts/fsl/kmcoge4.dts
> > > +++ b/arch/powerpc/boot/dts/fsl/kmcoge4.dts
> > > @@ -106,6 +106,45 @@
> > >  		sata@221000 {
> > >  			status = "disabled";
> > >  		};
> > > +
> > > +		fman0: fman@400000 {
> > > +			enet0: ethernet@e0000 {
> > > +				phy-connection-type = "sgmii";
> > > +				local-mac-address = [00 11 22 33 44
> > > 55];
> > > +				fixed-link {
> > > +					speed = <1000>;
> > > +					full-duplex;
> > > +				};
> > > +			};
> > > +			mdio0: mdio@e1120 {
> > > +				front_phy: ethernet-phy@11 {
> > > +					reg = <0x11>;
> > > +				};
> > > +			};
> > > +
> > > +			enet1: ethernet@e2000 {
> > > +				phy-connection-type = "sgmii";
> > > +				local-mac-address = [00 11 22 33 44
> > > 56];
> > > +				fixed-link {
> > > +					speed = <1000>;
> > > +					full-duplex;
> > > +				};
> > > +			};
> > 
> > No hardcoded MAC addresses.
> > 
> 
> For these 2 interfaces where I have the local-mac-address field, the MAC
> addresses are set later by an application that reads the real address in
> some
> EEPROM. However, in order to let the fman mac_probe to run successfully in
> the
> first place I have set non-zero MAC addresses since the local-mac-address
> fields
> are not set by u-boot.

Why can't it be set from U-Boot?

If you absolutely must hardcode a mac address, use one with the locally
-administered bit set (0x02 in the first byte).

> I have found several local-mac-address fields in other DTS files that are
> all
> zeros, and thus are rejected by of_get_mac_address. Are they leftovers from
> the
> past or should they be used here as well ? If not, I will simply drop these
> 2
> fields.

That's a relic from ancient U-Boots that could only overwrite existing
properties rather than insert them from scratch.

-Scott

Re: [PATCH] powerpc: define the fman node for the kmcoge4 DTS

[Valentin Longchamp]

On 17/04/16 03:49, Scott Wood wrote:
> On Thu, 2016-04-07 at 08:14 +0200, Valentin Longchamp wrote:
>> On 06/04/16 23:49, Scott Wood wrote:
>>> On Wed, 2016-04-06 at 15:37 +0200, Valentin Longchamp wrote:
>>>> Now that the FMAN mac driver has been merged the fman node is relevant.
>>>>
>>>> The kmcoge4 board implements 3 ethernet interfaces, 1 with a RGMII phy
>>>> and 2 with fixed 1 Giga SGMII links.
>>>>
>>>> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
>>>> ---
>>>>  arch/powerpc/boot/dts/fsl/kmcoge4.dts | 39
>>>> +++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 39 insertions(+)
>>>>
>>>> diff --git a/arch/powerpc/boot/dts/fsl/kmcoge4.dts
>>>> b/arch/powerpc/boot/dts/fsl/kmcoge4.dts
>>>> index 6858ec9..1cec66d 100644
>>>> --- a/arch/powerpc/boot/dts/fsl/kmcoge4.dts
>>>> +++ b/arch/powerpc/boot/dts/fsl/kmcoge4.dts
>>>> @@ -106,6 +106,45 @@
>>>>  		sata@221000 {
>>>>  			status = "disabled";
>>>>  		};
>>>> +
>>>> +		fman0: fman@400000 {
>>>> +			enet0: ethernet@e0000 {
>>>> +				phy-connection-type = "sgmii";
>>>> +				local-mac-address = [00 11 22 33 44
>>>> 55];
>>>> +				fixed-link {
>>>> +					speed = <1000>;
>>>> +					full-duplex;
>>>> +				};
>>>> +			};
>>>> +			mdio0: mdio@e1120 {
>>>> +				front_phy: ethernet-phy@11 {
>>>> +					reg = <0x11>;
>>>> +				};
>>>> +			};
>>>> +
>>>> +			enet1: ethernet@e2000 {
>>>> +				phy-connection-type = "sgmii";
>>>> +				local-mac-address = [00 11 22 33 44
>>>> 56];
>>>> +				fixed-link {
>>>> +					speed = <1000>;
>>>> +					full-duplex;
>>>> +				};
>>>> +			};
>>>
>>> No hardcoded MAC addresses.
>>>
>>
>> For these 2 interfaces where I have the local-mac-address field, the MAC
>> addresses are set later by an application that reads the real address in
>> some
>> EEPROM. However, in order to let the fman mac_probe to run successfully in
>> the
>> first place I have set non-zero MAC addresses since the local-mac-address
>> fields
>> are not set by u-boot.
> 
> Why can't it be set from U-Boot?

It can and should be set from u-boot. It's an old leftover from the vxworks time
where all the MAC addresses where set from the vxworks application itself. When
the company migrated to embedded Linux, they only implemented setting the MAC
address for the "debug" Ethernet interface in u-boot (for NFS boot). The other
were still controlled by the application after the Kernel boot and since then it
has remained so.

I will write this down on the (long) list of remaining vxworks migration cleanups.

> 
> If you absolutely must hardcode a mac address, use one with the locally
> -administered bit set (0x02 in the first byte).

I don't think this is necessary. As you hinted above, the correct way would be
to set them from u-boot and there is nothing that would prevent it other than
actually implementing it (u-boot actually has access to the MAC-address list of
the board).

I send a v2 patch without these fields.

> 
>> I have found several local-mac-address fields in other DTS files that are
>> all
>> zeros, and thus are rejected by of_get_mac_address. Are they leftovers from
>> the
>> past or should they be used here as well ? If not, I will simply drop these
>> 2
>> fields.
> 
> That's a relic from ancient U-Boots that could only overwrite existing
> properties rather than insert them from scratch.
> 

OK, interesting to know, thanks for the explanation.

Valentin