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: [RFC PATCH net-next v4 23/28] net: dsa: register of_mdiobus if a
 mdio node is declared
To:     Ansuel Smith <ansuelsmth@gmail.com>
Cc:     Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Russell King <linux@armlinux.org.uk>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20210508002920.19945-1-ansuelsmth@gmail.com>
 <20210508002920.19945-23-ansuelsmth@gmail.com>
From:   Florian Fainelli <f.fainelli@gmail.com>
Message-ID: <dd58ebca-c1d4-28c4-6bee-368ba6225f34@gmail.com>
Date:   Fri, 7 May 2021 21:36:39 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Firefox/78.0 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <20210508002920.19945-23-ansuelsmth@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 5/7/2021 5:29 PM, Ansuel Smith wrote:
> Some switch have phy port that use the internal switch mdio bus and can
> have different phy regs than the one declared in the ports node. Add
> support for this specific case by registering the mdiobus with the mdio
> node and permit the port to declare a phy-handle defined inside the
> switch node.
> 
> This is an example from the qca8337 switch where the 5 phy port should
> use the internal mdiobus and would benefits from this.
> 
> switch10: switch@10 {
>         compatible = "qca,qca8337";
>         #address-cells = <1>;
>         #size-cells = <0>;
>         reg = <0x10>;
> 
>         ports {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
> 
>                 port@0 {
>                         reg = <0>;
>                         label = "cpu";
>                         ethernet = <&gmac1>;
>                         phy-mode = "rgmii-id";
> 
>                         fixed-link {
>                                 speed = <1000>;
>                                 full-duplex;
>                         };
>                 };
> 
>                 port@1 {
>                         reg = <1>;
>                         label = "lan1";
> 
>                         phy-handle = <&phy_port0>;
>                         phy-mode = "internal";
>                 };
> 
>                 port@2 {
>                         reg = <2>;
>                         label = "lan2";
> 
>                         phy-handle = <&phy_port1>;
>                         phy-mode = "internal";
>                 };
> 
>                 port@3 {
>                         reg = <3>;
>                         label = "lan3";
> 
>                         phy-handle = <&phy_port2>;
>                         phy-mode = "internal";
>                 };
> 
>                 port@4 {
>                         reg = <4>;
>                         label = "lan4";
> 
>                         phy-handle = <&phy_port3>;
>                         phy-mode = "internal";
>                 };
> 
>                 port@5 {
>                         reg = <5>;
>                         label = "wan";
> 
>                         phy-handle = <&phy_port4>;
>                         phy-mode = "internal";
>                 };
> 
>                 port@6 {
>                         reg = <6>;
>                         label = "cpu";
>                         ethernet = <&gmac2>;
>                         phy-mode = "sgmii";
> 
>                         fixed-link {
>                                         speed = <1000>;
>                                         full-duplex;
>                         };
>                 };
>         };
> 
>         mdio {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
> 
>                 phy_port0: phy@0 {
>                         reg = <0>;
>                 };
> 
>                 phy_port1: phy@1 {
>                         reg = <1>;
>                 };
> 
>                 phy_port2: phy@2 {
>                         reg = <2>;
>                 };
> 
>                 phy_port3: phy@3 {
>                         reg = <3>;
>                 };
> 
>                 phy_port4: phy@4 {
>                         reg = <4>;
>                 };
>         };
> };
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  net/dsa/dsa2.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index 3c3e56a1f34d..79adabe3e2a7 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -14,6 +14,7 @@
>  #include <linux/rtnetlink.h>
>  #include <linux/of.h>
>  #include <linux/of_net.h>
> +#include <linux/of_mdio.h>
>  #include <net/devlink.h>
>  
>  #include "dsa_priv.h"
> @@ -721,6 +722,8 @@ static int dsa_switch_setup(struct dsa_switch *ds)
>  	devlink_params_publish(ds->devlink);
>  
>  	if (!ds->slave_mii_bus && ds->ops->phy_read) {
> +		struct device_node *mdio;
> +
>  		ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
>  		if (!ds->slave_mii_bus) {
>  			err = -ENOMEM;
> @@ -729,7 +732,15 @@ static int dsa_switch_setup(struct dsa_switch *ds)
>  
>  		dsa_slave_mii_bus_init(ds);
>  
> -		err = mdiobus_register(ds->slave_mii_bus);
> +		mdio = of_get_child_by_name(ds->dev->of_node, "mdio");
> +
> +		if (mdio) {

This probably needs to be if (of_device_is_available(mdio)), since one
could conceivably declare the switch internal MDIO bus but put a
disabled status property not to use it.
-- 
Florian