linux-kernel - Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170119145315.GD21805@lunn.ch>
Date:   Thu, 19 Jan 2017 15:53:15 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org,
        Jason Cooper <jason@...edaemon.net>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Gregory Clement <gregory.clement@...e-electrons.com>,
        Russell King <linux@...linux.org.uk>,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        "David S. Miller" <davem@...emloft.net>,
        "moderated list:ARM SUB-ARCHITECTURES" 
        <linux-arm-kernel@...ts.infradead.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v3 06/10] net: dsa: Migrate to
 device_find_class()

> > > 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.

> 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. 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! 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.

> 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?

       Andrew