Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller
To:     Andrew Lunn <andrew@lunn.ch>,
        =?UTF-8?Q?=c3=81lvaro_Fern=c3=a1ndez_Rojas?= <noltari@gmail.com>
Cc:     Jonas Gorski <jonas.gorski@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>, netdev@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20210308184102.3921-1-noltari@gmail.com>
 <20210308184102.3921-3-noltari@gmail.com> <YEaQdXwrmVekXp4G@lunn.ch>
 <D39D163A-C6B3-4B66-B650-8FF0A06EF7A2@gmail.com> <YFJBJ1IHpkXXaGvc@lunn.ch>
From:   Florian Fainelli <f.fainelli@gmail.com>
Message-ID: <4c3f1252-50fd-8d90-3fac-009dace44813@gmail.com>
Date:   Wed, 17 Mar 2021 14:42:23 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Firefox/78.0 Thunderbird/78.8.1
MIME-Version: 1.0
In-Reply-To: <YFJBJ1IHpkXXaGvc@lunn.ch>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk


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