netdev - Re: [PATCH] of: of_mdio: Handle properties for non-phy mdio devices

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] [day] [month] [year] [list]

Message-ID: <YC0ZXFx98JrtEGA3@lunn.ch>
Date:   Wed, 17 Feb 2021 14:25:48 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Nathan Rossi <nathan@...hanrossi.com>
Cc:     Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org,
        Nathan Rossi <nathan.rossi@...i.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH] of: of_mdio: Handle properties for non-phy mdio devices

On Wed, Feb 17, 2021 at 02:48:30PM +1000, Nathan Rossi wrote:
> On Wed, 17 Feb 2021 at 13:19, Andrew Lunn <andrew@...n.ch> wrote:
> >
> > > > The patch does make sense though, Broadcom 53125 switches have a broken
> > > > turn around and are mdio_device instances, the broken behavior may not
> > > > show up with all MDIO controllers used to interface though. For the
> > >
> > > Yes the reason we needed this change was to enable broken turn around,
> > > specifically with a Marvell 88E6390.
> >
> > Ah, odd. I've never had problems with the 6390, either connected to a
> > Freecale FEC, or the Linux bit banging MDIO bus.
> >
> > What are you using for an MDIO bus controller? Did it already support
> > broken turn around, or did you need to add it?
> 
> Using bit bang MDIO to access the 88e6390. I suspect the issue is
> specific to the board design, another similar design we have uses bit
> bang MDIO but a 88e6193x switch and does not have any issue with turn
> around.

So to me, it sounds like changing the data pin, by the host, from
being driven to high impedance, is taking too long. So this is a bus
problem, not a per device on the bus problem. You need to indicate to
the bus controller that all addresses on the bus have broken turn
around, not just one. If you look at mdio-bitbang.c  it has:

        /* check the turnaround bit: the PHY should be driving it to zero, if this
         * PHY is listed in phy_ignore_ta_mask as having broken TA, skip that
         */
        if (mdiobb_get_bit(ctrl) != 0 &&
            !(bus->phy_ignore_ta_mask & (1 << phy))) {
                /* PHY didn't drive TA low -- flush any bits it
                 * may be trying to send.
                 */
                for (i = 0; i < 32; i++)
                        mdiobb_get_bit(ctrl);

                return 0xffff;
        }

So the property it specific to one address. And the mv88e6xxx normally
takes up multiple addresses on the bus.

So i would do this differently. Add a new property to "mdio-gpio" to
indicate the host has broken turn around, and it needs to set all 32
bits of bus->phy_ignore_ta_mask.

     Andrew