[PATCH] fsl/fman: remove of_node

[PATCH] fsl/fman: remove of_node

[Madalin Bucur]

The FMan MAC driver allocates a platform device for the Ethernet
driver to probe on. Setting pdev->dev.of_node with the MAC node
triggers the MAC driver probing of the new platform device. While
this fails quickly and does not affect the functionality of the
drivers, it is incorrect and must be removed. This was added to
address a report that DSA code using of_find_net_device_by_node()
is unable to use the DPAA interfaces. Error message seen before
this fix:

fsl_mac dpaa-ethernet.0: __devm_request_mem_region(mac) failed
fsl_mac: probe of dpaa-ethernet.0 failed with error -16

Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
 drivers/net/ethernet/freescale/fman/mac.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 387eb4a..9a265f8 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -623,7 +623,6 @@ static struct platform_device *dpaa_eth_add_device(int fman_id,
 		goto no_mem;
 	}
 
-	pdev->dev.of_node = node;
 	pdev->dev.parent = priv->dev;
 	set_dma_ops(&pdev->dev, get_dma_ops(priv->dev));
 
-- 
2.1.0

Re: [PATCH] fsl/fman: remove of_node

[David Miller]

From: Madalin Bucur <madalin.bucur@nxp.com>
Date: Mon, 2 Oct 2017 13:31:37 +0300

> The FMan MAC driver allocates a platform device for the Ethernet
> driver to probe on. Setting pdev->dev.of_node with the MAC node
> triggers the MAC driver probing of the new platform device. While
> this fails quickly and does not affect the functionality of the
> drivers, it is incorrect and must be removed. This was added to
> address a report that DSA code using of_find_net_device_by_node()
> is unable to use the DPAA interfaces. Error message seen before
> this fix:
> 
> fsl_mac dpaa-ethernet.0: __devm_request_mem_region(mac) failed
> fsl_mac: probe of dpaa-ethernet.0 failed with error -16
> 
> Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>

Is the DSA issue no longer something we need to be concerned
about?  If not, why?  You have to explain this.

RE: [PATCH] fsl/fman: remove of_node

[Madalin-cristian Bucur]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, October 03, 2017 2:05 AM
> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> Subject: Re: [PATCH] fsl/fman: remove of_node
> 
> From: Madalin Bucur <madalin.bucur@nxp.com>
> Date: Mon, 2 Oct 2017 13:31:37 +0300
> 
> > The FMan MAC driver allocates a platform device for the Ethernet
> > driver to probe on. Setting pdev->dev.of_node with the MAC node
> > triggers the MAC driver probing of the new platform device. While
> > this fails quickly and does not affect the functionality of the
> > drivers, it is incorrect and must be removed. This was added to
> > address a report that DSA code using of_find_net_device_by_node()
> > is unable to use the DPAA interfaces. Error message seen before
> > this fix:
> >
> > fsl_mac dpaa-ethernet.0: __devm_request_mem_region(mac) failed
> > fsl_mac: probe of dpaa-ethernet.0 failed with error -16
> >
> > Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> 
> Is the DSA issue no longer something we need to be concerned
> about?  If not, why?  You have to explain this.

My patch removes the of_node that was set to a device that was not an
of_device, preventing duplicated probing of both the real of_device
and the "fake" one created through this assignment.

I understand that the DSA issue that triggered the initial change
was related to DSA finding the network devices using 
of_find_net_device_by_node(), something that will not work for the
DPAA case where the netdevice does not have an of_node. I do not know
enough about DSA to come up with a solution for this problem now.
Andrew, Florian, can you please comment on this?

Thanks,
Madalin

Re: [PATCH] fsl/fman: remove of_node

[Andrew Lunn]

On Tue, Oct 03, 2017 at 08:49:31AM +0000, Madalin-cristian Bucur wrote:
> > -----Original Message-----
> > From: David Miller [mailto:davem@davemloft.net]
> > Sent: Tuesday, October 03, 2017 2:05 AM
> > To: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> > Subject: Re: [PATCH] fsl/fman: remove of_node
> > 
> > From: Madalin Bucur <madalin.bucur@nxp.com>
> > Date: Mon, 2 Oct 2017 13:31:37 +0300
> > 
> > > The FMan MAC driver allocates a platform device for the Ethernet
> > > driver to probe on. Setting pdev->dev.of_node with the MAC node
> > > triggers the MAC driver probing of the new platform device. While
> > > this fails quickly and does not affect the functionality of the
> > > drivers, it is incorrect and must be removed. This was added to
> > > address a report that DSA code using of_find_net_device_by_node()
> > > is unable to use the DPAA interfaces. Error message seen before
> > > this fix:
> > >
> > > fsl_mac dpaa-ethernet.0: __devm_request_mem_region(mac) failed
> > > fsl_mac: probe of dpaa-ethernet.0 failed with error -16
> > >
> > > Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> > 
> > Is the DSA issue no longer something we need to be concerned
> > about?  If not, why?  You have to explain this.
> 
> My patch removes the of_node that was set to a device that was not an
> of_device, preventing duplicated probing of both the real of_device
> and the "fake" one created through this assignment.
> 
> I understand that the DSA issue that triggered the initial change
> was related to DSA finding the network devices using 
> of_find_net_device_by_node(), something that will not work for the
> DPAA case where the netdevice does not have an of_node. I do not know
> enough about DSA to come up with a solution for this problem now.
> Andrew, Florian, can you please comment on this?
> 
> Thanks,

Hi Madalin

I guess the real fix is to throw away the platform device. But that is
a big change.

I've not looked at the code in detail. Why is the platform device
needed?

	Andrew

Re: [PATCH] fsl/fman: remove of_node

[David Miller]

From: Madalin-cristian Bucur <madalin.bucur@nxp.com>
Date: Tue, 3 Oct 2017 08:49:31 +0000

> My patch removes the of_node that was set to a device that was not an
> of_device, preventing duplicated probing of both the real of_device
> and the "fake" one created through this assignment.
> 
> I understand that the DSA issue that triggered the initial change
> was related to DSA finding the network devices using 
> of_find_net_device_by_node(), something that will not work for the
> DPAA case where the netdevice does not have an of_node. I do not know
> enough about DSA to come up with a solution for this problem now.
> Andrew, Florian, can you please comment on this?

It sounds like you're knowingly breaking DSA.

RE: [PATCH] fsl/fman: remove of_node

[Madalin-cristian Bucur]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, October 04, 2017 7:44 AM
> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> Cc: netdev@vger.kernel.org; andrew@lunn.ch; f.fainelli@gmail.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] fsl/fman: remove of_node
> 
> From: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> Date: Tue, 3 Oct 2017 08:49:31 +0000
> 
> > My patch removes the of_node that was set to a device that was not an
> > of_device, preventing duplicated probing of both the real of_device
> > and the "fake" one created through this assignment.
> >
> > I understand that the DSA issue that triggered the initial change
> > was related to DSA finding the network devices using
> > of_find_net_device_by_node(), something that will not work for the
> > DPAA case where the netdevice does not have an of_node. I do not know
> > enough about DSA to come up with a solution for this problem now.
> > Andrew, Florian, can you please comment on this?
> 
> It sounds like you're knowingly breaking DSA.

It never worked, even with the change I'm reverting.

RE: [PATCH] fsl/fman: remove of_node

[Madalin-cristian Bucur]

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Tuesday, October 03, 2017 4:01 PM
> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> Cc: David Miller <davem@davemloft.net>; netdev@vger.kernel.org;
> f.fainelli@gmail.com; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] fsl/fman: remove of_node
> 
> On Tue, Oct 03, 2017 at 08:49:31AM +0000, Madalin-cristian Bucur wrote:
> > > -----Original Message-----
> > > From: David Miller [mailto:davem@davemloft.net]
> > > Sent: Tuesday, October 03, 2017 2:05 AM
> > > To: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> > > Subject: Re: [PATCH] fsl/fman: remove of_node
> > >
> > > From: Madalin Bucur <madalin.bucur@nxp.com>
> > > Date: Mon, 2 Oct 2017 13:31:37 +0300
> > >
> > > > The FMan MAC driver allocates a platform device for the Ethernet
> > > > driver to probe on. Setting pdev->dev.of_node with the MAC node
> > > > triggers the MAC driver probing of the new platform device. While
> > > > this fails quickly and does not affect the functionality of the
> > > > drivers, it is incorrect and must be removed. This was added to
> > > > address a report that DSA code using of_find_net_device_by_node()
> > > > is unable to use the DPAA interfaces. Error message seen before
> > > > this fix:
> > > >
> > > > fsl_mac dpaa-ethernet.0: __devm_request_mem_region(mac) failed
> > > > fsl_mac: probe of dpaa-ethernet.0 failed with error -16
> > > >
> > > > Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> > >
> > > Is the DSA issue no longer something we need to be concerned
> > > about?  If not, why?  You have to explain this.
> >
> > My patch removes the of_node that was set to a device that was not an
> > of_device, preventing duplicated probing of both the real of_device
> > and the "fake" one created through this assignment.
> >
> > I understand that the DSA issue that triggered the initial change
> > was related to DSA finding the network devices using
> > of_find_net_device_by_node(), something that will not work for the
> > DPAA case where the netdevice does not have an of_node. I do not know
> > enough about DSA to come up with a solution for this problem now.
> > Andrew, Florian, can you please comment on this?
> >
> > Thanks,
> 
> Hi Madalin
> 
> I guess the real fix is to throw away the platform device. But that is
> a big change.
> 
> I've not looked at the code in detail. Why is the platform device
> needed?
> 
> 	Andrew

There are multiple components that are aggregated by the DPAA Ethernet
netdevice, the FMan MAC is one of them. There is no entry in the device
tree for the Ethernet device as this is just a software construct that
binds together multiple pieces from the DPAA FMan, QMan, BMan. The probing
of the Ethernet driver is performed against a platform device that is created
in the FMan MAC. Setting the of_node in the new platform device makes it
look like an of_device without being one, thus the errors at the MAC driver
probing against the platform device.

Does DSA work for systems that do not use a device tree to boot, i.e. ACPI?

Madalin

Re: [PATCH] fsl/fman: remove of_node

[Andrew Lunn]

> Does DSA work for systems that do not use a device tree to boot, i.e. ACPI?

ACPI is not supported. So far, nobody has built a board using ACPI and
Ethernet switches supported by DSA.

	 Andrew

RE: [PATCH] fsl/fman: remove of_node

[Madalin-cristian Bucur]

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Madalin-cristian Bucur
> Sent: Wednesday, October 04, 2017 12:54 PM
> To: David Miller <davem@davemloft.net>
> Subject: RE: [PATCH] fsl/fman: remove of_node
> 
> > -----Original Message-----
> > From: David Miller [mailto:davem@davemloft.net]
> > Sent: Wednesday, October 04, 2017 7:44 AM
> > To: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> > Cc: netdev@vger.kernel.org; andrew@lunn.ch; f.fainelli@gmail.com; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH] fsl/fman: remove of_node
> >
> > From: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> > Date: Tue, 3 Oct 2017 08:49:31 +0000
> >
> > > My patch removes the of_node that was set to a device that was not an
> > > of_device, preventing duplicated probing of both the real of_device
> > > and the "fake" one created through this assignment.
> > >
> > > I understand that the DSA issue that triggered the initial change
> > > was related to DSA finding the network devices using
> > > of_find_net_device_by_node(), something that will not work for the
> > > DPAA case where the netdevice does not have an of_node. I do not know
> > > enough about DSA to come up with a solution for this problem now.
> > > Andrew, Florian, can you please comment on this?
> >
> > It sounds like you're knowingly breaking DSA.
> 
> It never worked, even with the change I'm reverting.

I'll resend this change as part of a patch set that changes the device
used as net_dev dev to ensure DSA will find a of_device there. To make
that work some changes to adjust link (that also make it cleaner) were
needed. Also, to keep the old udev rules happy, I've changed the names
of the FMan kernel modules from fsl_fman_* to fsl_dpaa_fman*.

I do not have a DSA setup to test so I just tested the part related to
of_find_net_device_by_node() being able to determine the net_device
based on a device tree handle using an artificial device tree and code
construct. I hope that will help the initial reporter of the DSA issue
on DPAA (Junote Cai).

Madalin