From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0A4FEC77B61 for ; Mon, 1 May 2023 09:29:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=7ZRYro9MvAg1zoTwvvf4iEPdVZRnhHVogGV7UmTiafo=; b=1j/FatSk+uvYtG8KQw8tPGUT6P e4GDzbEI7tXSBT46vXKDZsE3Bg+wlAuT87YExvrDdrfsBum/cM/utyRH/ggnfEo3KGiN+P++AfHW7 JtzcN3722BRI7fpXHq7aiepu7PKYXcq4RYQGXpxHzIhqsyq7WRnl9/jE3nh66I90ZANHw0RinAmI/ PlQ0EUZ+L2qtj6YCkKC/cuYJZRhw9W7vt2D+mtJDylTPHgw/gEowIptN873Tf0tatI3dsMT8wyTfo tkJqO5SWtm/KZTuhybZTcnuN+1mXfQi3oobgK8pncHHI2w1aIaPdQuN2I02xby5C2Iz3m6kaLbnTO rgNofwCg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1ptPqo-00FXmo-0B; Mon, 01 May 2023 09:29:42 +0000 Received: from sender3-op-o17.zoho.com ([136.143.184.17]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1ptPqj-00FXmD-1z; Mon, 01 May 2023 09:29:39 +0000 ARC-Seal: i=1; a=rsa-sha256; t=1682933358; cv=none; d=zohomail.com; s=zohoarc; b=BaI/uLE+od4YdpchaIomh85K06+ogmefgWj6TRu5o+a+ftsK04Esbp69QHDw633XEv5kRIiPt4z8w4wyJZNENoJM5lesxcSG2v3TfcAKy8ANd58eYREKb9uvWIgtNLXXAciPDv112/+bntn5MgjzcT4szwFizJ7Y1AfL60O45OM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1682933358; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=7ZRYro9MvAg1zoTwvvf4iEPdVZRnhHVogGV7UmTiafo=; b=lOdkStpKQpTL9/uJdATB3CcE/ntpCklawsIL2ga2wldbpalqXPEVHX+jEZvhe56VF36IJUbt5x5TT7ZaC4oFYhrO5ptPDB2Nhoj4KITzDcZ3nu6XRwA5AJrm3UCzEyc6utHtvuKHJd3GHFXBfYrpXh60BzxoxC0wVgZsK4M6MoM= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=arinc9.com; spf=pass smtp.mailfrom=arinc.unal@arinc9.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1682933358; s=zmail; d=arinc9.com; i=arinc.unal@arinc9.com; h=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:Cc:Cc:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To; bh=7ZRYro9MvAg1zoTwvvf4iEPdVZRnhHVogGV7UmTiafo=; b=R2+QeE8XUXKa1k3JRwSRX4X4+C2nsaFnj7j5a6mJP5JeZlBAE7SETg8O365lMkxv R+Nm6oiZp8yws9dai6dRddf7HHoMeMrwMH4YoLbHWbLisfTLzh7p6SpEXDaKpap9fNI L1b82UU33FLDwFZHKl93jlIY+FlOFjSVKr3tewWc= Received: from [10.10.10.3] (149.91.1.15 [149.91.1.15]) by mx.zohomail.com with SMTPS id 1682933356826381.56027190272596; Mon, 1 May 2023 02:29:16 -0700 (PDT) Message-ID: <0f501bb6-18a0-1713-b08c-6ad244c022ec@arinc9.com> Date: Mon, 1 May 2023 12:28:55 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [PATCH 2/2] dt-bindings: net: dsa: mediatek,mt7530: document MDIO-bus To: David Bauer , Andrew Lunn Cc: Florian Fainelli , Vladimir Oltean , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rob Herring , Krzysztof Kozlowski , Matthias Brugger , AngeloGioacchino Del Regno , Landen Chao , DENG Qingfang , Sean Wang , Daniel Golle , netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org References: <20230430112834.11520-1-mail@david-bauer.net> <20230430112834.11520-2-mail@david-bauer.net> <396fad42-89d0-114d-c02e-ac483c1dd1ed@arinc9.com> <04cc2904-6d61-416e-bfbe-c24d96fe261b@lunn.ch> <207753d6-cffd-4a23-be16-658d7c9ceb4a@lunn.ch> <1f759370-af97-e2a4-4b93-183eb854f7cd@david-bauer.net> Content-Language: en-US From: =?UTF-8?B?QXLEsW7DpyDDnE5BTA==?= In-Reply-To: <1f759370-af97-e2a4-4b93-183eb854f7cd@david-bauer.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ZohoMailClient: External X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230501_022937_709670_3548A97C X-CRM114-Status: GOOD ( 28.61 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On 1.05.2023 12:22, David Bauer wrote: > Hi Arinc, > > thanks for spotting this issue. > > On 4/30/23 21:54, Arınç ÜNAL wrote: >> On 30.04.2023 21:48, Andrew Lunn wrote: >>>>> Try setting ds->slave_mii_bus to the MDIO bus you register via >>>>> of_mdiobus_register(). >>>> >>>> That seems to be the case already, under mt7530_setup_mdio(): >>>> >>>>     bus = devm_mdiobus_alloc(dev); >>>>     if (!bus) >>>>         return -ENOMEM; >>>> >>>>     ds->slave_mii_bus = bus; >>>> >>>> The bus is registered with devm_of_mdiobus_register(), if that matters. (My >>>> current knowledge about OF or OF helpers for MDIO is next to nothing.) >>>> >>>> The same behaviour is there. >>> >>> Maybe take a look at what is going on in dsa_slave_phy_setup() and >>> dsa_slave_phy_connect(). >>> >>> The way i understand it, is it first looks in DT to see if there is a >>> phy-handle, and if there is, it uses it. If not, it assumes there is a >>> 1:1 mapping between port number and PHY address, and looks to see if a >>> PHY has been found on ds->slave_mii_bus at that address, and uses it. >>> >>> So i don't think you need to list the PHY, the fallback should be >>> used. >> >> Thanks for pointing me in the right direction Andrew. >> >> I applied this diff: >> >> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c >> index 389f33a12534..19d0c209e7e9 100644 >> --- a/drivers/net/phy/mdio_bus.c >> +++ b/drivers/net/phy/mdio_bus.c >> @@ -117,8 +117,12 @@ struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr) >> >>       mdiodev = bus->mdio_map[addr]; >> >> -    if (!mdiodev) >> +    if (!mdiodev) { >> +        dev_info(&bus->dev, "mdio device doesn't exist\n"); >>           return NULL; >> +    } >> + >> +    dev_info(&bus->dev, "mdio device exists\n"); >> >>       if (!(mdiodev->flags & MDIO_DEVICE_FLAG_PHY)) >>           return NULL; >> diff --git a/net/dsa/slave.c b/net/dsa/slave.c >> index 165bb2cb8431..0be408e32a76 100644 >> --- a/net/dsa/slave.c >> +++ b/net/dsa/slave.c >> @@ -2487,6 +2487,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev) >>           /* We could not connect to a designated PHY or SFP, so try to >>            * use the switch internal MDIO bus instead >>            */ >> +        netdev_err(slave_dev, "using switch's internal MDIO bus\n"); >>           ret = dsa_slave_phy_connect(slave_dev, dp->index, phy_flags); >>       } >>       if (ret) { >> >> With or without this patch, the switch's internal MDIO bus is used to set >> up the PHYs. >> >> DT that defines ethphy0 only, without this patch applied: >> >> [    4.660784] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus >> [    4.669026] mdio_bus mt7530-0: mdio device exists >> [    4.677693] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) >> [    4.693238] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus >> [    4.701589] mdio_bus mt7530-0: mdio device exists >> [    4.707101] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=POLL) >> [    4.718550] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus >> [    4.726856] mdio_bus mt7530-0: mdio device exists >> [    4.732384] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=POLL) >> [    4.743822] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus >> [    4.752154] mdio_bus mt7530-0: mdio device exists >> [    4.757662] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=POLL) >> [    4.769099] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus >> [    4.781872] mdio_bus mt7530-0: mdio device exists >> [    4.787413] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=POLL) >> >> Same DT but with this patch applied: >> >> [    4.621547] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode >> [    4.631524] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus >> [    4.639764] mdio_bus mt7530-0: mdio device exists >> [    4.647770] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) >> [    4.663898] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus >> [    4.672253] mdio_bus mt7530-0: mdio device doesn't exist >> [    4.677597] mt7530-mdio mdio-bus:1f lan0 (uninitialized): no phy at 1 >> [    4.684053] mt7530-mdio mdio-bus:1f lan0 (uninitialized): failed to connect to PHY: -ENODEV >> [    4.692435] mt7530-mdio mdio-bus:1f lan0 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 1 >> [    4.703087] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus >> [    4.711408] mdio_bus mt7530-0: mdio device doesn't exist >> [    4.716731] mt7530-mdio mdio-bus:1f lan1 (uninitialized): no phy at 2 >> [    4.723214] mt7530-mdio mdio-bus:1f lan1 (uninitialized): failed to connect to PHY: -ENODEV >> [    4.731597] mt7530-mdio mdio-bus:1f lan1 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 2 >> [    4.742199] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus >> [    4.755431] mdio_bus mt7530-0: mdio device doesn't exist >> [    4.760793] mt7530-mdio mdio-bus:1f lan2 (uninitialized): no phy at 3 >> [    4.767263] mt7530-mdio mdio-bus:1f lan2 (uninitialized): failed to connect to PHY: -ENODEV >> [    4.775632] mt7530-mdio mdio-bus:1f lan2 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 3 >> [    4.786270] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus >> [    4.794591] mdio_bus mt7530-0: mdio device doesn't exist >> [    4.799944] mt7530-mdio mdio-bus:1f lan3 (uninitialized): no phy at 4 >> [    4.806397] mt7530-mdio mdio-bus:1f lan3 (uninitialized): failed to connect to PHY: -ENODEV >> [    4.814782] mt7530-mdio mdio-bus:1f lan3 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 4 >> >> DT without the mdio node defined, with this patch applied: >> >> [    4.650766] mt7530-mdio mdio-bus:1f: configuring for fixed/trgmii link mode >> [    4.660687] mt7530-mdio mdio-bus:1f wan (uninitialized): using switch's internal MDIO bus >> [    4.668937] mdio_bus mt7530-0: mdio device exists >> [    4.677787] mt7530-mdio mdio-bus:1f wan (uninitialized): PHY [mt7530-0:00] driver [MediaTek MT7530 PHY] (irq=POLL) >> [    4.693165] mt7530-mdio mdio-bus:1f lan0 (uninitialized): using switch's internal MDIO bus >> [    4.701517] mdio_bus mt7530-0: mdio device exists >> [    4.707029] mt7530-mdio mdio-bus:1f lan0 (uninitialized): PHY [mt7530-0:01] driver [MediaTek MT7530 PHY] (irq=POLL) >> [    4.718469] mt7530-mdio mdio-bus:1f lan1 (uninitialized): using switch's internal MDIO bus >> [    4.726773] mdio_bus mt7530-0: mdio device exists >> [    4.732322] mt7530-mdio mdio-bus:1f lan1 (uninitialized): PHY [mt7530-0:02] driver [MediaTek MT7530 PHY] (irq=POLL) >> [    4.743793] mt7530-mdio mdio-bus:1f lan2 (uninitialized): using switch's internal MDIO bus >> [    4.752143] mdio_bus mt7530-0: mdio device exists >> [    4.757662] mt7530-mdio mdio-bus:1f lan2 (uninitialized): PHY [mt7530-0:03] driver [MediaTek MT7530 PHY] (irq=POLL) >> [    4.769105] mt7530-mdio mdio-bus:1f lan3 (uninitialized): using switch's internal MDIO bus >> [    4.781905] mdio_bus mt7530-0: mdio device exists >> [    4.787459] mt7530-mdio mdio-bus:1f lan3 (uninitialized): PHY [mt7530-0:04] driver [MediaTek MT7530 PHY] (irq=POLL) >> >> This is how I define it, mind you no phandles. >> >> switch@1f { >>      ... >>      mdio { >>          #address-cells = <0x01>; >>          #size-cells = <0x00>; >> >>          ethernet-phy@0 { >>              reg = <0x00>; >>          }; >>      }; >> }; >> >> Like you said, if the mdio node is not defined, the driver will assume 1:1 >> mapping. If not, it will need all the PHYs to be defined on the mdio node >> along with on the ports node. Hence back to my original statement, we can >> either force defining the PHYs on the mdio node which would break the ABI, >> or forget about doing PHY muxing this way. > > While i was not aware of this side effect, I don't see how this breaks the ABI. Your patch doesn't break it, my then-intention of doing PHY muxing by utilising this would. Your first patch is perfectly fine as is. > > Existing device-trees not defining the MDIO node will still continue to work. Agreed, I also confirmed this with my test above. > > Wouldn't we just skip the whole issue by documenting the need for defining all PHYs > used on the switch when defining the MDIO bus? Good idea, please do that. Arınç