netdev - Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

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: <3D5C1ACB-4338-4A9F-9BCC-0424A1793664@gmail.com>
Date:   Thu, 18 Aug 2016 09:09:17 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Timur Tabi <timur@...eaurora.org>, Al Stone <al.stone@...aro.org>,
        Rob Herring <robh@...nel.org>
CC:     netdev <netdev@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Sagar Dharia <sdharia@...eaurora.org>,
        Shanker Donthineni <shankerd@...eaurora.org>,
        Vikram Sethi <vikrams@...eaurora.org>,
        Christopher Covington <cov@...eaurora.org>,
        Gilad Avidov <gavidov@...eaurora.org>,
        Andrew Lunn <andrew@...n.ch>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Mark Langsdorf <mlangsdo@...hat.com>,
        "jcm@...hat.com" <jcm@...hat.com>,
        Andy Gross <agross@...eaurora.org>,
        David Miller <davem@...emloft.net>,
        Lino Sanfilippo <LinoSanfilippo@....de>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        "Abdulhamid, Harb" <harba@...eaurora.org>
Subject: Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

On August 17, 2016 9:19:23 PM PDT, Timur Tabi <timur@...eaurora.org> wrote:
>Florian Fainelli wrote:
>> The larger issue is that the emac_sgmii node in the form you posted
>> is going to be backed by a platform device in Linux while you want a
>> PHY device with a reg property that describes a MDIO address
>> (#address-cells = 1, #size-cells = 0).
>
>But how do I get the platform device for the emac_sgmii node?  If I 
>create an of_device_id ID for it, like this:
>
>static const struct of_device_id emac_dt_match[] = {
>	{
>		.compatible = "qcom,fsm9900-emac",
>	},
>	{}
>};
>
>static const struct of_device_id emac_sgmii_dt_match[] = {
>	{
>		.compatible = "qcom,fsm9900-emac-sgmii",
>	},
>	{}
>};
>
>Then the probe function will be called for qcom,fsm9900-emac-sgmii 
>separately from qcom,fsm9900-emac, which just confuses things. So I 
>can't create emac_sgmii_dt_match.
>
>I know this is standard DT stuff, and I used to do a lot of work on DT 
>so maybe I should know this already.  But it seems to me that I need to
>
>manually create the platform_device for qcom,fsm9900-emac-sgmii.

There is a helper function to obtain the platform device associated with a device_node: of_find_device_by_node. AFAICT probe ordering is based on DT ordering, but if there was a dependency order to solve you could use EPROBE_DEFER until the emac driver finds its dependencies.

>
>> IIRC the amd xgbe driver mainline had a similar design but still
>> implemented a PHY device anyway although it may not have been using
>> Device Tree. It should still be possible to implement a PHY driver
>> that you manually register and bind to its device_node pointer such
>> that of_phy_find_device and friends still work. You would do this
>> from the emac_sgmii platform device driver and parent devices in a
>> way that satisfy the PHY device driver lifecycle as well.
>>
>> Hope this helps.
>
>It doesn't, sorry.  The emac_sgmii is really just another register
>block 
>for the driver to program.  Creating another PHY driver for it doesn't 
>really make sense.  It's not on an MDIO bus.

First of a PHY does not have to be on a MDIO bus, it just happens to be the most common thing, and second, this is a PHY of some kind as far as the Ethernet MAC is concerned and this is the abstraction you want at the MAC driver level so you can plug arbitrary external PHYs or internal PHYs irrespective of their backing transport/bus layer.

As long as your DT representation is sane and accurate, however this ends up being implemented matters less. Whether the PHY device model suits you here, you need to tell the emac0 node that it is bound to the emac_sgmii node somehow since there is programming of this block required for operating and for that a phandle property of some kind is required, phy-handle is a standard one which would help with the driver acceptance IMHO.


-- 
Florian