Re: [RFC PATCH net-next v4 23/28] net: dsa: register of_mdiobus if a mdio node is declared

Florian Fainelli
2021-05-08T04:36:48Z

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