linux-kernel - Re: [PATCH 2/4] dt-bindings: net: dsa: document internal MDIO bus

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: <20230915121830.tmzuxthup7rzewhv@skbuf>
Date:   Fri, 15 Sep 2023 15:18:30 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Arınç ÜNAL <arinc.unal@...nc9.com>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Woojung Huh <woojung.huh@...rochip.com>,
        UNGLinuxDriver@...rochip.com,
        Linus Walleij <linus.walleij@...aro.org>,
        Alvin Šipraga <alsi@...g-olufsen.dk>,
        Daniel Golle <daniel@...rotopia.org>,
        Landen Chao <Landen.Chao@...iatek.com>,
        DENG Qingfang <dqfext@...il.com>,
        Sean Wang <sean.wang@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>, mithat.guner@...ont.com,
        erkin.bozoglu@...ont.com, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 2/4] dt-bindings: net: dsa: document internal MDIO bus

Hi Russell,

On Thu, Sep 14, 2023 at 07:07:13PM +0100, Russell King (Oracle) wrote:
> On Thu, Sep 14, 2023 at 07:06:11PM +0100, Russell King (Oracle) wrote:
> > On Wed, Sep 13, 2023 at 04:59:19PM +0100, Russell King (Oracle) wrote:
> > > On Wed, Sep 13, 2023 at 10:42:31AM +0300, Vladimir Oltean wrote:
> > > > On Wed, Sep 13, 2023 at 08:52:37AM +0300, Arınç ÜNAL wrote:
> > > > > One more thing, I don't recall phy-mode being required to be defined for
> > > > > user ports as it will default to GMII. I don't believe this is the same
> > > > > case for shared ports so phy-mode is required only for them?
> > > > 
> > > > phy-mode is not strictly required, but I think there is a strong
> > > > preference to set it. IIRC, when looking at the DSA device trees, there
> > > > was no case where phy-mode would be absent on CPU/DSA ports if the other
> > > > link properties were also present, so we required it too. There were no
> > > > complaints in 1 year since dsa_shared_port_validate_of() is there. The
> > > > requirement can be relaxed to just a warning and no error in the kernel,
> > > > and the removal of "required" in the schema, if it helps making it
> > > > common with user ports.
> > > 
> > > However, phylink pretty much requires phy-mode to be specified to be
> > > something sane for shared ports, so I wouldn't be in favour of relaxing
> > > the checkinng in dsa_shared_port_validate_of()... not unless you're
> > > now going to accept the approach I originally proposed to have DSA
> > > drivers tell the core (and thus phylink) what phy-mode and other link
> > > parameters should be used when they are missing from DT.
> > 
> > You mean the approach that I picked up using software nodes that got
> > thrown out by the software node people? That approach that I picked
> > up from you and tried to get merged?
> > 
> > No, that's not going to happen, and it's not a question of whether
> > _I_ am going to accept that approach or not. So don't throw that
> > back on me, please.
> > 
> > If this is something that we want to solve, we need to stop being so
> > devisive (your language above is so) and try to come up with a
> > solution that is acceptable to everyone... the swnode approach
> > doesn't seem to be it.
> 
> Oh dear. I must be going mad!

So first things first: I am not advocating for making phy-mode fully
optional in the sense that you say (if absent, then write non-OF code
through which DSA infers the phy-mode from drivers). I'm happy with the
current form of the code.

I was just trying to add some nuance to this bizarre aspect signalled by
Arınç - phy-mode is not required for user ports, presumably because when
it is absent, user ports will default to GMII. That isn't an intrinsic
feature of user ports, but rather of having a phydev, and so, because
DSA/CPU ports can also have a phydev, logically it means that phy-mode
can also be omitted in that particular case, with the same result.

Our missing_phy_mode check from dsa_shared_port_validate_of() is theoretically
more restrictive than it needs to be, because it artificially prohibits
that behavior, and it results in an inexplicable difference in the phylink
dt-bindings for user vs shared ports. So that's where my relaxation
proposal came from: we could make missing_phy_mode non-fatal, and that
would permit the configurations which can work to work, and the ones
which can't work will fail elsewhere. Just like for the user ports.

Where I wasn't absolutely clear is that I don't want Arınç to change any
of that. Right now, on DSA shared ports, phy-mode is required and on user
ports it isn't. The difference is a bit strange (arbitrary considering
the example above) and should maybe be settled at some point in the
future, but for now, the dt-bindings should document it like that.