lkml.org 
[lkml]   [2017]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH net-next] stmmac: call stmmac_init_phy from stmmac_dvr_probe
    From
    Date
    On 03/20/2017 11:07 PM, Florian Fainelli wrote:
    >
    > (snip)
    >>
    >> However, it is kind of sad that drivers are so inconsistent of what goes
    >> in probe and what goes in ndo_open...which is tied together with the
    >> whole mess of when certain ethtool commands work or do not work.
    > Well, inconsistent here is kind of big statement, what I meant to say is
    > that your proposed change actually makes thing inconsistent.

    What I mean is that it is about 50/50 if something e.g. phy_connect
    is called from probe or from ndo_open.
    This is what I think is inconsistent.

    git grep -p -E "phy_connect\(|phy_connect_direct\(" drivers/net/ethernet/ | grep open | wc -l
    12

    git grep -p -E "phy_connect\(|phy_connect_direct\(" drivers/net/ethernet/ | grep probe | wc -l
    27
    even if we exclude *_mii_probe/*_mdio_probe which are done
    git grep -p mii_probe drivers/net/ethernet/ | grep open | wc -l
    6
    git grep -p mdio_probe drivers/net/ethernet/ | grep open | wc -l
    2

    phy_connect done from probe:
    27-6-2=19

    phy_connect done from ndo_open:
    12+6+2=20


    >
    >> Do you know of a good way to avoid the -EBUSY in stmmac_ethtool_get_link_ksettings,
    >> but still keep phy_connect in ndo_open?
    > Let me rephrase this: doing an ethtool operation on an interface that is
    > not open is not a defined behavior which adheres to a contract with the
    > kernel. ethtool operations on interfaces that are DOWN, may succeed if
    > they do not involve a piece of hardware that needs to be active (e.g:
    > the PHY) but there is no guarantee that a) the change is immediately
    > applied, or b) that the results are consistent.
    >
    > The best thing to do IMHO is just silence the warning, return an error
    > coed, and come back configuring the interface at a later time when it is
    > guaranteed to be operational.

    Thank you for your feedback. I agree with you,
    silencing the warning would be the best way forward.
    David, please ignore this patch.



    However, since ethtool is a user interface, I think that it would
    have been nice if it wasn't so driver specific regarding to if a
    command works before the interface is open or not.

    If the user does some ethtool operation that affects e.g. the PHY
    before the interface is open, perhaps there is a way to save this
    setting so it could be applied when the resource is available.

    For the PHY case, net_device has a pointer to a phy_device,
    register_netdev could create a dummy phy_device if
    ndev->phy_device is NULL, that way changes could be saved
    and applied when phy_connect replaces the dummy device
    (or perhaps when phy_start() is called).
    (If it isn't a dummy phy_device, changes could be applied immediately.)
    This way it would not matter if phy_connect is done from probe
    or from ndo_open.

    \
     
     \ /
      Last update: 2017-03-21 10:05    [W:3.366 / U:0.096 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site