On Thu, Jan 19, 2017 at 03:53:15PM +0100, Andrew Lunn wrote:
> > > > struct dsa_platform_data {
> > > > /*
> > > > * Reference to a Linux network interface that connects
> > > > * to the root switch chip of the tree.
> > > > */
> > > > struct device *netdev;
> >
> > This I think is the oddest thing, why do you need to have the "root
> > switch" here? You seem to have dropped the next value in this
> > structure:
> > struct net_device *of_netdev;
>
> We are implementing platform_data for devices which don't support
> device tree. When using OF, we don't have any of these issues. We can
> go straight to the device.
>
> It is a bit convoluted, but look at
> arch/arm/mach-orion5x/rd88f5181l-ge-setup.c. It defines the start of
> the dsa_platform_data in that file. It then gets passed through
> common.c: orion5x_eth_switch_init() to
> arch/arm/plat-orion/common.c:orion_ge00_switch_init() :
>
> void __init orion_ge00_switch_init(struct dsa_platform_data *d)
> {
> int i;
>
> d->netdev = &orion_ge00.dev;
> for (i = 0; i < d->nr_chips; i++)
> d->chip[i].host_dev = &orion_ge_mvmdio.dev;
>
> platform_device_register_data(NULL, "dsa", 0, d, sizeof(d));
> }
>
> Where we have
>
> static struct platform_device orion_ge00 = {
> .name = MV643XX_ETH_NAME,
> .id = 0,
> .num_resources = 1,
> .resource = orion_ge00_resources,
> .dev = {
> .coherent_dma_mask = DMA_BIT_MASK(32),
> },
> };
>
> So this is the platform device for the Ethernet device. We cannot go
> to the net_device, because it does not exist until this Ethernet
> platform device is instantiated.
Ok, fine, but why isn't the ethernet device a child of this platform
device? Why is it floating around somewhere else? You don't see that
happening for other devices.
> > Shouldn't you have a bus for RGMII devices? Is that the real problem
> > here, you don't have a representation for your RGMII "bus" with a
> > controller to bundle everything under (like a USB host controller, it
> > bridges from one bus to another).
>
> RGMII is not a bus. It is a point to point link.
That's fine, but you have multiple devices talking across it, so in the
kernel driver model "naming", it's a bus. Anything can be a bus, it's
just a way to group together devices of the same type.
> Normally, it is
> between the Ethernet MAC and the Ethernet PHY. But you can also have
> it between an Ethernet MAC and another Ethernet MAC. I'm not sure
> describing this is a bus would be practical. It would mean every
> ethernet driver also becomes a bus driver!
Instead of a custom platform device driver, yes. Is that a big deal?
How many do you have?
> Every Ethernet PHY would become a bus device. That is a huge change,
> for a few legacy boards which are not getting converted to device
> tree.
How many different drivers are we talking about here?
> > If so, why is eth1 not below f1072004.mdio-mi in the heirachy already?
>
> See the initial diagram above. The switch has two parents. It hangs of
> an MDIO bus, and you would like to make RGMII also a bus. Can the
> device model handle that? I thought it was a tree, not a graph?
It is a tree, you are correct. But right now you are picking and
choosing where you want to put that network device. Why not put it over
on the mdio bus? Or, like I mentioned, make it a custom bus where you
can properly show this relationship, not just in a generic "let's jump
to the parent and poke around randomly."
Again, it's that last sentance that I object the most to here. You all
keep ignoring it for some reason...
thanks,
greg k-h
