[RFC PATCH dpss_eth] Don't initialise ports with no PHY

[RFC PATCH dpss_eth] Don't initialise ports with no PHY

[Darren Stevens]

Since cbb961ca271e ("Use random MAC address when none is given")
Varisys Cyrus P5020 boards have been listing 5 ethernet ports instead of
the 2 the board has.This is because we were preventing the adding of the
unused ports by not suppling them a MAC address, which this patch now
supplies.

Prevent them from appearing in the net devices list by checking for a
'status="disabled"' entry during probe and skipping the port if we find
it. 

Signed-off-by: Darren Stevens <Darren@stevens-zone.net>

---

 drivers/net/ethernet/freescale/fman/mac.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 43427c5..c9ed411 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -606,6 +606,7 @@ static int mac_probe(struct platform_device *_of_dev)
 	struct resource		 res;
 	struct mac_priv_s	*priv;
 	const u8		*mac_addr;
+	const char 		*prop;
 	u32			 val;
 	u8			fman_id;
 	phy_interface_t          phy_if;
@@ -628,6 +629,16 @@ static int mac_probe(struct platform_device *_of_dev)
 	mac_dev->priv = priv;
 	priv->dev = dev;
 
+	/* check for disabled devices and skip them, as now a missing
+	 * MAC address will be replaced with a Random one rather than
+	 * disabling the port
+	 */
+	prop = of_get_property(mac_node, "status", NULL);
+	if (prop && !strncmp(prop, "disabled", 8) {
+		err = -ENODEV;
+		goto _return
+	}
+
 	if (of_device_is_compatible(mac_node, "fsl,fman-dtsec")) {
 		setup_dtsec(mac_dev);
 		priv->internal_phy_node = of_parse_phandle(mac_node,

Re: [RFC PATCH dpss_eth] Don't initialise ports with no PHY

[Florian Fainelli]

On 4/24/2020 3:29 PM, Darren Stevens wrote:
> Since cbb961ca271e ("Use random MAC address when none is given")
> Varisys Cyrus P5020 boards have been listing 5 ethernet ports instead of
> the 2 the board has.This is because we were preventing the adding of the
> unused ports by not suppling them a MAC address, which this patch now
> supplies.
> 
> Prevent them from appearing in the net devices list by checking for a
> 'status="disabled"' entry during probe and skipping the port if we find
> it. 
> 
> Signed-off-by: Darren Stevens <Darren@stevens-zone.net>
> 
> ---
> 
>  drivers/net/ethernet/freescale/fman/mac.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
> index 43427c5..c9ed411 100644
> --- a/drivers/net/ethernet/freescale/fman/mac.c
> +++ b/drivers/net/ethernet/freescale/fman/mac.c
> @@ -606,6 +606,7 @@ static int mac_probe(struct platform_device *_of_dev)
>  	struct resource		 res;
>  	struct mac_priv_s	*priv;
>  	const u8		*mac_addr;
> +	const char 		*prop;
>  	u32			 val;
>  	u8			fman_id;
>  	phy_interface_t          phy_if;
> @@ -628,6 +629,16 @@ static int mac_probe(struct platform_device *_of_dev)
>  	mac_dev->priv = priv;
>  	priv->dev = dev;
>  
> +	/* check for disabled devices and skip them, as now a missing
> +	 * MAC address will be replaced with a Random one rather than
> +	 * disabling the port
> +	 */
> +	prop = of_get_property(mac_node, "status", NULL);
> +	if (prop && !strncmp(prop, "disabled", 8) {
> +		err = -ENODEV;
> +		goto _return
> +	}

There is a sorter version: of_device_is_available(mac_node) which will
do the same thing.

> +
>  	if (of_device_is_compatible(mac_node, "fsl,fman-dtsec")) {
>  		setup_dtsec(mac_dev);
>  		priv->internal_phy_node = of_parse_phandle(mac_node,
> 

-- 
Florian

Re: [RFC PATCH dpss_eth] Don't initialise ports with no PHY

[Andrew Lunn]

On Fri, Apr 24, 2020 at 11:29:38PM +0100, Darren Stevens wrote:
> Since cbb961ca271e ("Use random MAC address when none is given")
> Varisys Cyrus P5020 boards have been listing 5 ethernet ports instead of
> the 2 the board has.This is because we were preventing the adding of the
> unused ports by not suppling them a MAC address, which this patch now
> supplies.
> 
> Prevent them from appearing in the net devices list by checking for a
> 'status="disabled"' entry during probe and skipping the port if we find
> it. 

Hi Darren

I'm surprised the core is probing a device which has status disabled.
Are you sure this is the correct explanation?

    Thanks
	Andrew

Re: Don't initialise ports with no PHY

[Darren Stevens]

Hello Andrew

On 25/04/2020, Andrew Lunn wrote:
> On Fri, Apr 24, 2020 at 11:29:38PM +0100, Darren Stevens wrote:
>> Since cbb961ca271e ("Use random MAC address when none is given")
>> Varisys Cyrus P5020 boards have been listing 5 ethernet ports instead of
>> the 2 the board has.This is because we were preventing the adding of the
>> unused ports by not suppling them a MAC address, which this patch now
>> supplies.
>> 
>> Prevent them from appearing in the net devices list by checking for a
>> 'status="disabled"' entry during probe and skipping the port if we find
>> it. 
>
> Hi Darren
>
> I'm surprised the core is probing a device which has status disabled.
> Are you sure this is the correct explanation?

You are correct, the core is detecting status="disabled". My mistake, the Hardware vendor's supplied dts always relied on only supplying active ports with an IP address, and it didn't occur to me to test that they could be disabled.

Sorry.

Regards
Darren

Re: [RFC PATCH dpss_eth] Don't initialise ports with no PHY

[Christian Zigotzky]

[-- Attachment #1: Type: text/plain, Size: 2385 bytes --]

Hi Darren,

Thanks a lot for your patch!

I tested it with the RC3 today.

Unfortunately it doesn't compile because a bracket is missing in the 
following line:

+    if (prop && !strncmp(prop, "disabled", 8) {

And a semicolon is missing in the following line:

+        goto _return

I added the bracket and the semicolon and after that it compiled without 
any problems. (New patch attached)

Unfortunately I see more than 2 ethernet ports with the RC3 and your 
patch on my Cyrus P5040. Maybe Skateman has an other result on his Cyrus 
P5020.

Maybe we have to modify the dtb file.

Thanks,
Christian


On 25 April 2020 at 00:29 am, Darren Stevens wrote:
> Since cbb961ca271e ("Use random MAC address when none is given")
> Varisys Cyrus P5020 boards have been listing 5 ethernet ports instead of
> the 2 the board has.This is because we were preventing the adding of the
> unused ports by not suppling them a MAC address, which this patch now
> supplies.
>
> Prevent them from appearing in the net devices list by checking for a
> 'status="disabled"' entry during probe and skipping the port if we find
> it.
>
> Signed-off-by: Darren Stevens <Darren@stevens-zone.net>
>
> ---
>
>   drivers/net/ethernet/freescale/fman/mac.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
> index 43427c5..c9ed411 100644
> --- a/drivers/net/ethernet/freescale/fman/mac.c
> +++ b/drivers/net/ethernet/freescale/fman/mac.c
> @@ -606,6 +606,7 @@ static int mac_probe(struct platform_device *_of_dev)
>   	struct resource		 res;
>   	struct mac_priv_s	*priv;
>   	const u8		*mac_addr;
> +	const char 		*prop;
>   	u32			 val;
>   	u8			fman_id;
>   	phy_interface_t          phy_if;
> @@ -628,6 +629,16 @@ static int mac_probe(struct platform_device *_of_dev)
>   	mac_dev->priv = priv;
>   	priv->dev = dev;
>   
> +	/* check for disabled devices and skip them, as now a missing
> +	 * MAC address will be replaced with a Random one rather than
> +	 * disabling the port
> +	 */
> +	prop = of_get_property(mac_node, "status", NULL);
> +	if (prop && !strncmp(prop, "disabled", 8) {
> +		err = -ENODEV;
> +		goto _return
> +	}
> +
>   	if (of_device_is_compatible(mac_node, "fsl,fman-dtsec")) {
>   		setup_dtsec(mac_dev);
>   		priv->internal_phy_node = of_parse_phandle(mac_node,


[-- Attachment #2: dpss_eth.patch --]
[-- Type: text/x-patch, Size: 1021 bytes --]

diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 43427c5..c9ed411 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -606,6 +606,7 @@ static int mac_probe(struct platform_device *_of_dev)
 	struct resource		 res;
 	struct mac_priv_s	*priv;
 	const u8		*mac_addr;
+	const char 		*prop;
 	u32			 val;
 	u8			fman_id;
 	phy_interface_t          phy_if;
@@ -628,6 +629,16 @@ static int mac_probe(struct platform_device *_of_dev)
 	mac_dev->priv = priv;
 	priv->dev = dev;
 
+	/* check for disabled devices and skip them, as now a missing
+	 * MAC address will be replaced with a Random one rather than
+	 * disabling the port
+	 */
+	prop = of_get_property(mac_node, "status", NULL);
+	if (prop && !strncmp(prop, "disabled", 8)) {
+		err = -ENODEV;
+		goto _return;
+	}
+
 	if (of_device_is_compatible(mac_node, "fsl,fman-dtsec")) {
 		setup_dtsec(mac_dev);
 		priv->internal_phy_node = of_parse_phandle(mac_node,

Re: [RFC PATCH dpss_eth] Don't initialise ports with no PHY

[Andrew Lunn]

> Maybe we have to modify the dtb file.

Hi Christian

Could you point me at the DT file.

      Thanks
	Andrew

Re: [RFC PATCH dpss_eth] Don't initialise ports with no PHY

[Christian Zigotzky]

[-- Attachment #1: Type: text/plain, Size: 394 bytes --]

Hi Andrew,

You can find some dtb and source files in our kernel package.

Download: http://www.xenosoft.de/linux-image-5.7-rc3-X1000_X5000.tar.gz

Thanks,
Christian

> On 29. Apr 2020, at 15:13, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> 
>> 
>> Maybe we have to modify the dtb file.
> 
> Hi Christian
> 
> Could you point me at the DT file.
> 
>      Thanks
>    Andrew

[-- Attachment #2: Type: text/html, Size: 961 bytes --]

Re: [RFC PATCH dpss_eth] Don't initialise ports with no PHY

[Andrew Lunn]

On Wed, Apr 29, 2020 at 03:55:28PM +0200, Christian Zigotzky wrote:
> Hi Andrew,
> 
> You can find some dtb and source files in our kernel package.
> 
> Download: http://www.xenosoft.de/linux-image-5.7-rc3-X1000_X5000.tar.gz

I have the tarball. Are we talking about
linux-image-5.7-rc3-X1000_X5000/X5000_and_QEMU_e5500/dtbs/X5000_20/cyrus.eth.dtb

I don't see any status = "disabled"; in the blob. So i would expect
the driver to probe.

    Andrew

Re: [RFC PATCH dpss_eth] Don't initialise ports with no PHY

[Christian Zigotzky]

> On 29. Apr 2020, at 17:22, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> On Wed, Apr 29, 2020 at 03:55:28PM +0200, Christian Zigotzky wrote:
>> Hi Andrew,
>> 
>> You can find some dtb and source files in our kernel package.
>> 
>> Download: http://www.xenosoft.de/linux-image-5.7-rc3-X1000_X5000.tar.gz
> 
> I have the tarball. Are we talking about
> linux-image-5.7-rc3-X1000_X5000/X5000_and_QEMU_e5500/dtbs/X5000_20/cyrus.eth.dtb
> 
> I don't see any status = "disabled"; in the blob. So i would expect
> the driver to probe.
> 
>    Andrew
> 
> 

Yes, that’s correct but maybe Darren uses another dtb file.

@Darren
Which dtb file do you use?

Re: Don't initialise ports with no PHY

[Darren Stevens]

[-- Attachment #1: Type: text/plain, Size: 178 bytes --]

  AmigaOS...........: http://yam.ch/
  Unix/MacOS/Windows: http://www.mozilla.com/thunderbird/

General information about MIME can be found at:
http://en.wikipedia.org/wiki/MIME

[-- Attachment #2: Type: text/plain, Size: 1153 bytes --]

Hello Christian

On 29/04/2020, Christian Zigotzky wrote:
>
>
>> On 29. Apr 2020, at 17:22, Andrew Lunn <andrew@lunn.ch> wrote:
>> 
>> ?On Wed, Apr 29, 2020 at 03:55:28PM +0200, Christian Zigotzky wrote:
>>> Hi Andrew,
>>> 
>>> You can find some dtb and source files in our kernel package.
>>> 
>>> Download: http://www.xenosoft.de/linux-image-5.7-rc3-X1000_X5000.tar.gz
>> 
>> I have the tarball. Are we talking about
>>
>>
linux-image-5.7-rc3-X1000_X5000/X5000_and_QEMU_e5500/dtbs/X5000_20/cyrus.eth.dtb
>
>> I don't see any status = "disabled"; in the blob. So i would expect
>> the driver to probe.

No, the vendor never added that to them.

> Yes, that's correct but maybe Darren uses another dtb file.
>
> @Darren
> Which dtb file do you use?

My current one attached, including updated cyrus_p5020.dts & p5020si-pre.dtsi
which I'm preparing patches for.

Christian, build an unmodified kernel, select board level reset or power off,
then both the GPIO drivers.
Then under LED Support: GPIO connected LED's and triggers -> disk activity

I think you still have a 5020 don't you? I'll look at 5040 later (I'll need
someone to test)

Regards
Darren

[-- Attachment #3: cyrus-dts.zip --]
[-- Type: application/zip, Size: 8289 bytes --]

Re: Don't initialise ports with no PHY

[Christian Zigotzky]

> On 30. Apr 2020, at 23:36, Darren Stevens <darren@stevens-zone.net> wrote:
> 
> Hello Christian
> 
>> On 29/04/2020, Christian Zigotzky wrote:
>> 
>> 
>>>> On 29. Apr 2020, at 17:22, Andrew Lunn <andrew@lunn.ch> wrote:
>>> 
>>> ?On Wed, Apr 29, 2020 at 03:55:28PM +0200, Christian Zigotzky wrote:
>>>> Hi Andrew,
>>>> 
>>>> You can find some dtb and source files in our kernel package.
>>>> 
>>>> Download: http://www.xenosoft.de/linux-image-5.7-rc3-X1000_X5000.tar.gz
>>> 
>>> I have the tarball. Are we talking about
>>> 
>>> 
> linux-image-5.7-rc3-X1000_X5000/X5000_and_QEMU_e5500/dtbs/X5000_20/cyrus.eth.dtb
>> 
>>> I don't see any status = "disabled"; in the blob. So i would expect
>>> the driver to probe.
> 
> No, the vendor never added that to them.
> 
>> Yes, that's correct but maybe Darren uses another dtb file.
>> 
>> @Darren
>> Which dtb file do you use?
> 
> My current one attached, including updated cyrus_p5020.dts & p5020si-pre.dtsi
> which I'm preparing patches for.
> 
> Christian, build an unmodified kernel, select board level reset or power off,
> then both the GPIO drivers.
> Then under LED Support: GPIO connected LED's and triggers -> disk activity
> 
> I think you still have a 5020 don't you? I'll look at 5040 later (I'll need
> someone to test)
> 
> Regards
> Darren
> <cyrus-dts.zip>

Darren

I use a 5040 currently.

Thanks
Christian