netdev - Re: [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller

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: <4c3f1252-50fd-8d90-3fac-009dace44813@gmail.com>
Date:   Wed, 17 Mar 2021 14:42:23 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Andrew Lunn <andrew@...n.ch>,
        Álvaro Fernández Rojas <noltari@...il.com>
Cc:     Jonas Gorski <jonas.gorski@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller



On 3/17/2021 10:49 AM, Andrew Lunn wrote:
>> BCM6368 (and newer) SoCs have an integrated ethernet switch controller with dedicated internal phys, but it also supports connecting to external phys not integrated in the internal switch.
>> Ports 0-3 are internal, ports 4-7 are external and can be connected to external switches or phys and port 8 is the CPU.
>> This MDIO bus device is integrated in the BCM63xx switch registers, which corresponds to the same registers present in drivers/net/dsa/b53/b53_regs.h.
>> As you can see in the source code, registers are the same for the internal and external bus. The only difference is that if MDIOC_EXT_MASK (bit 16) is set, the MDIO bus accessed will be the external, and on the contrary, if bit 16 isn’t set, the MDIO bus accessed will be the internal one.
>>
>> I don’t know if this answers your question, but I think that adding it as mdiomux is the way to go.
> 
> Hi Álvaro
> 
> The Marvell mv88e6390 family of switches has a very similar setup. An
> internal and an external MDIO bus, one bit difference in a
> register. When i wrote the code for that, i decided it was not a mux
> as such, but two MDIO busses. So i register two MDIO busses, and rely
> on a higher level switch register mutex to prevent parallel operations
> on the two busses.
> 
> The reason i decided it was not a mux, is that all the other mux
> drivers are separate drivers which rely on another MDIO bus
> driver. The mux driver gets a handle to the underlying MDIO bus
> driver, and and builds on it. Here you have it all combined in one, so
> it does not follow the pattern.
> 
> So if you want to use a max, please break this up into an MDIO driver,
> and a mux driver. Or have one driver which registers two mdio busses,
> no mux.

Also, if you were to support some dynamic power management in the
future, it may be desirable to have the switch drivers and the MDIO mux
controller be within the same driver, so you are in complete control of
when the resources are needed and you can fully clock gate everything.
-- 
Florian