lkml.org 
[lkml]   [2014]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH V3] net/dt: Add support for overriding phy configuration from device tree
    Hi,

    I'm currently trying to fix an issue for which this patch provides a
    partial solution, so apologies in advance for jumping into the
    discussion for my own purposes...

    On 02/04/2014 09:39 PM, Florian Fainelli wrote:> 2014-01-17 Matthew
    Garrett <matthew.garrett@nebula.com>:
    >> Some hardware may be broken in interesting and board-specific ways, such
    >> that various bits of functionality don't work. This patch provides a
    >> mechanism for overriding mii registers during init based on the
    contents of
    >> the device tree data, allowing board-specific fixups without having to
    >> pollute generic code.
    >
    > It would be good to explain exactly how your hardware is broken
    > exactly. I really do not think that such a fine-grained setting where
    > you could disable, e.g: 100BaseT_Full, but allow 100BaseT_Half to
    > remain usable makes that much sense. In general, Gigabit might be
    > badly broken, but 100 and 10Mbits/sec should work fine. How about the
    > MASTER-SLAVE bit, is overriding it really required?
    >
    > Is not a PHY fixup registered for a specific OUI the solution you are
    > looking for? I am also concerned that this creates PHY troubleshooting
    > issues much harder to debug than before as we may have no idea about
    > how much information has been put in Device Tree to override that.
    >
    > Finally, how about making this more general just like the BCM87xx PHY
    > driver, which is supplied value/reg pairs directly? There are 16
    > common MII registers, and 16 others for vendor specific registers,
    > this is just covering for about 2% of the possible changes.

    Good point. That would easily help me with my current issue, which
    requires autoneg to be disabled to begin with (by clearing BMCR_ANENABLE
    from register 0).
    This would not however fix it entirely (I tried a quick hardwired
    implementation), as the whole PHY machinery would not take that into
    account and would re-enable autoneg anyway.
    I also tried changing the patch so that phydev->support gets updated
    (instead of phydev->advertising):

    >> + if (!of_property_read_u32(np, override->prop, &tmp)) {
    >> + if (tmp) {
    >> + *val |= override->value;
    >> + phydev->advertising |=
    override->supported;
    >> + } else {
    >> + phydev->advertising &=
    ~(override->supported);
    >> + }
    >> +
    >> + *mask |= override->value;

    What I find weird is that the only way phydev->autoneg could ever be set
    to disabled is from here (phy.c):

    static void phy_sanitize_settings(struct phy_device *phydev)
    {
    u32 features = phydev->supported;
    int idx;

    /* Sanitize settings based on PHY capabilities */
    if ((features & SUPPORTED_Autoneg) == 0)
    phydev->autoneg = AUTONEG_DISABLE;

    which is in turn only called when phydev->autoneg is set to
    AUTONEG_DISABLE to begin with:

    int phy_start_aneg(struct phy_device *phydev)
    {
    int err;

    mutex_lock(&phydev->lock);

    if (AUTONEG_DISABLE == phydev->autoneg)
    phy_sanitize_settings(phydev);

    So could someone please help me figure out what I'm missing here?

    Thanks!
    Gerlando


    \
     
     \ /
      Last update: 2014-02-10 18:01    [W:6.125 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site