| Message ID | 20200424232938.1a85d353@Cyrus.lan (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [RFC,dpss_eth] Don't initialise ports with no PHY | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (5d23fec1ac1d385a5c4f29252fd91c84c6ea4ccc) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | warning | Upstream build failed, couldn't test patch |
| snowpatch_ozlabs/checkpatch | warning | total: 1 errors, 2 warnings, 0 checks, 23 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
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, >
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
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,
> Maybe we have to modify the dtb file.
Hi Christian
Could you point me at the DT file.
Thanks
Andrew
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
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
> 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?
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
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
> 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
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,
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(+)