Received-SPF: pass (google.com: domain of linux-kernel+bounces-170814-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <904e5bd2-4495-4c1b-a172-ffcd737e6ee3@arinc9.com>
Date: Tue, 7 May 2024 10:25:57 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v2] net: dsa: mt7530: detect PHY muxing when PHY
 is defined on switch MDIO bus
To: Daniel Golle <daniel@makrotopia.org>
Cc: DENG Qingfang <dqfext@gmail.com>, Sean Wang <sean.wang@mediatek.com>,
 Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>,
 Vladimir Oltean <olteanv@gmail.com>, "David S. Miller"
 <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>,
 Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
 Matthias Brugger <matthias.bgg@gmail.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>,
 Bartel Eerdekens <bartel.eerdekens@constell8.be>, mithat.guner@xeront.com,
 erkin.bozoglu@xeront.com, netdev@vger.kernel.org,
 linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
 linux-mediatek@lists.infradead.org
References: <20240430-b4-for-netnext-mt7530-use-switch-mdio-bus-for-phy-muxing-v2-1-9104d886d0db@arinc9.com>
 <Zji94d4yfEBaHlzt@makrotopia.org>
Content-Language: en-US
From: =?UTF-8?B?QXLEsW7DpyDDnE5BTA==?= <arinc.unal@arinc9.com>
In-Reply-To: <Zji94d4yfEBaHlzt@makrotopia.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 06/05/2024 14:24, Daniel Golle wrote:
> On Tue, Apr 30, 2024 at 08:01:33AM +0300, Arınç ÜNAL via B4 Relay wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> Currently, the MT7530 DSA subdriver configures the MT7530 switch to provide
>> direct access to switch PHYs, meaning, the switch PHYs listen on the MDIO
>> bus the switch listens on. The PHY muxing feature makes use of this.
>>
>> This is problematic as the PHY may be attached before the switch is
>> initialised, in which case, the PHY will fail to be attached.
>>
>> Since commit 91374ba537bd ("net: dsa: mt7530: support OF-based registration
>> of switch MDIO bus"), we can describe the switch PHYs on the MDIO bus of
>> the switch on the device tree. Extend the check to detect PHY muxing when
>> the PHY is defined on the MDIO bus of the switch on the device tree.
>>
>> When the PHY is described this way, the switch will be initialised first,
>> then the switch MDIO bus will be registered. Only after these steps, the
>> PHY will be attached.
>>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> Reviewed-by: Daniel Golle <daniel@makrotopia.org>
> 
>> ---
>> Changes in v2:
>> - Address the terminology on the patch log.
>> - Link to v1: https://lore.kernel.org/r/20240429-b4-for-netnext-mt7530-use-switch-mdio-bus-for-phy-muxing-v1-1-1f775983e155@arinc9.com
>> ---
>>   drivers/net/dsa/mt7530.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
>> index 2b9f904a98f0..6cf21c9d523b 100644
>> --- a/drivers/net/dsa/mt7530.c
>> +++ b/drivers/net/dsa/mt7530.c
>> @@ -2483,7 +2483,8 @@ mt7530_setup(struct dsa_switch *ds)
>>   			if (!phy_node)
>>   				continue;
>>   
>> -			if (phy_node->parent == priv->dev->of_node->parent) {
>> +			if (phy_node->parent == priv->dev->of_node->parent ||
>> +			    phy_node->parent->parent == priv->dev->of_node) {
> 
> I had some concerns about missing check for phy_node->parent != NULL,
> but it's impossible in practise. If phy_node exists, it will have a parent
> node as well.
> 
> To be super extra safe, maybe doing
> phy_node->parent && phy_node->parent->parent == priv->dev->of_node
> would be better.

At the current state of this driver where the hardware probes on OF, I
don't see any benefit of doing this so I'm going to pass on this for now.

Arınç