Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <dad09430-4f33-7f1d-76c7-4dbd0710e950@arinc9.com>
Date:   Fri, 16 Sep 2022 18:25:52 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: Move MT7530 phy muxing from DSA to PHY driver
Content-Language: en-US
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     netdev <netdev@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Thibaut <hacks@slashdirt.org>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Sean Wang <sean.wang@mediatek.com>,
        Landen Chao <Landen.Chao@mediatek.com>,
        DENG Qingfang <dqfext@gmail.com>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Sergio Paracuellos <sergio.paracuellos@gmail.com>
References: <0e3ca573-2190-57b0-0e98-7f5b890d328e@arinc9.com>
 <YyKQKRIYDIVeczl1@lunn.ch>
From:   =?UTF-8?B?QXLEsW7DpyDDnE5BTA==?= <arinc.unal@arinc9.com>
In-Reply-To: <YyKQKRIYDIVeczl1@lunn.ch>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hey Andrew,

On 15.09.2022 05:38, Andrew Lunn wrote:
> On Thu, Sep 15, 2022 at 01:07:13AM +0300, Arınç ÜNAL wrote:
>> Hello folks.
>>
>> MediaTek MT7530 switch has got 5 phys and 7 gmacs. gmac5 and gmac6 are
>> treated as CPU ports.
>>
>> This switch has got a feature which phy0 or phy4 can be muxed to gmac5 of
>> the switch. This allows an ethernet mac connected to gmac5 to directly
>> connect to the phy.
>>
>> PHY muxing works by looking for the compatible string "mediatek,eth-mac"
>> then the mac address to find the gmac1 node. Then, it checks the mdio
>> address on the node which "phy-handle" on the gmac1 node points to. If the
>> mdio address is 0, phy0 is muxed to gmac5 of the switch. If it's 4, phy4 is
>> muxed.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/mt7530.c?id=1f9a6abecf538cc73635f6082677a2f4dc9c89a4#n2238
>>
>> Because that DSA probes the switch before muxing the phy, this won't work on
>> devices which only use a single switch phy because probing will fail.
>>
>> I'd like this operation to be done from the MediaTek Gigabit PHY driver
>> instead. The motives for this change are that we solve the behaviour above,
>> liberate the need to use DSA for this operation and get rid of the DSA
>> overhead.
>>
>> Would a change like this make sense and be accepted into netdev?
> 
> Where in the address range is the mux register? Officially, PHY
> drivers only have access to PHY registers, via MDIO. If the mux
> register is in the switch address space, it would be better if the
> switch did the mux configuration. An alternative might be to represent
> the mux in DT somewhere, and have a mux driver.

I don't know this part very well but it's in the register for hw trap 
modification which, I think, is in the switch address space.

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/mt7530.c?id=1f9a6abecf538cc73635f6082677a2f4dc9c89a4#n941

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/mt7530.h?id=1f9a6abecf538cc73635f6082677a2f4dc9c89a4#n500

Like you said, I don't think we can move away from the DSA driver, and 
would rather keep the driver do the mux configuration.

We could change the check for phy muxing to define the phy muxing 
bindings in the DSA node instead. If I understand correctly, the mdio 
address for PHYs is fake, it's for the sole purpose of making the driver 
check if there's request for phy muxing and which phy to mux. I'm saying 
this because the MT7530 switch works fine at address 0 while also using 
phy0 as a slave interface.

A property could be introduced on the DSA node for the MT7530 DSA driver:

     mdio {
         #address-cells = <1>;
         #size-cells = <0>;

         switch@0 {
             compatible = "mediatek,mt7530";
             reg = <0>;

             reset-gpios = <&pio 33 0>;

             core-supply = <&mt6323_vpa_reg>;
             io-supply = <&mt6323_vemc3v3_reg>;

             mt7530,mux-phy = <&sw0_p0>;

             ethernet-ports {
                 #address-cells = <1>;
                 #size-cells = <0>;

                 sw0_p0: port@0 {
                     reg = <0>;
                 };
             };
         };
     };

This would also allow using the phy muxing feature with any ethernet 
mac. Currently, phy muxing check wants the ethernet mac to be gmac1 of a 
MediaTek SoC. However, on a standalone MT7530, the switch can be wired 
to any SoC's ethernet mac.

For the port which is set for PHY muxing, do not bring it as a slave 
interface, just do the phy muxing operation.

Do not fail because there's no CPU port (ethernet property) defined when 
there's only one port defined and it's set for PHY muxing.

I don't know if the ethernet mac needs phy-handle defined in this case.

Arınç