Skip to content

Commit

Permalink
net: stmmac: don't create a MDIO bus if unnecessary
Browse files Browse the repository at this point in the history
Currently a MDIO bus is created if the devicetree description is either:

    1. Not fixed-link
    2. fixed-link but contains a MDIO bus as well

The "1" case above isn't always accurate. If there's a phy-handle,
it could be referencing a phy on another MDIO controller's bus[1]. In
this case, where the MDIO bus is not described at all, currently
stmmac will make a MDIO bus and scan its address space to discover
phys (of which there are none). This process takes time scanning a bus
that is known to be empty, delaying time to complete probe.

There are also a lot of upstream devicetrees[2] that expect a MDIO bus
to be created, scanned for phys, and the first one found connected
to the MAC. This case can be inferred from the platform description by
not having a phy-handle && not being fixed-link. This hits case "1" in
the current driver's logic, and must be handled in any logic change here
since it is a valid legacy dt-binding.

Let's improve the logic to create a MDIO bus if either:

    - Devicetree contains a MDIO bus
    - !fixed-link && !phy-handle (legacy handling)

This way the case where no MDIO bus should be made is handled, as well
as retaining backwards compatibility with the valid cases.

Below devicetree snippets can be found that explain some of
the cases above more concretely.

Here's[0] a devicetree example where the MAC is both fixed-link and
driving a switch on MDIO (case "2" above). This needs a MDIO bus to
be created:

    &fec1 {
            phy-mode = "rmii";

            fixed-link {
                    speed = <100>;
                    full-duplex;
            };

            mdio1: mdio {
                    switch0: switch0@0 {
                            compatible = "marvell,mv88e6190";
                            pinctrl-0 = <&pinctrl_gpio_switch0>;
                    };
            };
    };

Here's[1] an example where there is no MDIO bus or fixed-link for
the ethernet1 MAC, so no MDIO bus should be created since ethernet0
is the MDIO master for ethernet1's phy:

    &ethernet0 {
            phy-mode = "sgmii";
            phy-handle = <&sgmii_phy0>;

            mdio {
                    compatible = "snps,dwmac-mdio";
                    sgmii_phy0: phy@8 {
                            compatible = "ethernet-phy-id0141.0dd4";
                            reg = <0x8>;
                            device_type = "ethernet-phy";
                    };

                    sgmii_phy1: phy@a {
                            compatible = "ethernet-phy-id0141.0dd4";
                            reg = <0xa>;
                            device_type = "ethernet-phy";
                    };
            };
    };

    &ethernet1 {
            phy-mode = "sgmii";
            phy-handle = <&sgmii_phy1>;
    };

Finally there's descriptions like this[2] which don't describe the
MDIO bus but expect it to be created and the whole address space
scanned for a phy since there's no phy-handle or fixed-link described:

    &gmac {
            phy-supply = <&vcc_lan>;
            phy-mode = "rmii";
            snps,reset-gpio = <&gpio3 RK_PB4 GPIO_ACTIVE_HIGH>;
            snps,reset-active-low;
            snps,reset-delays-us = <0 10000 1000000>;
    };

[0] https://elixir.bootlin.com/linux/v6.5-rc5/source/arch/arm/boot/dts/nxp/vf/vf610-zii-ssmb-dtu.dts
[1] https://elixir.bootlin.com/linux/v6.6-rc5/source/arch/arm64/boot/dts/qcom/sa8775p-ride.dts
[2] https://elixir.bootlin.com/linux/v6.6-rc5/source/arch/arm64/boot/dts/rockchip/rk3368-r88.dts#L164

Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
Co-developed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
Signed-off-by: NipaLocal <nipa@local>
  • Loading branch information
ahalaney authored and NipaLocal committed Dec 15, 2023
1 parent abaf992 commit e9c7d51
Showing 1 changed file with 54 additions and 37 deletions.
91 changes: 54 additions & 37 deletions drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -296,62 +296,80 @@ static int stmmac_mtl_setup(struct platform_device *pdev,
}

/**
* stmmac_dt_phy - parse device-tree driver parameters to allocate PHY resources
* @plat: driver data platform structure
* @np: device tree node
* @dev: device pointer
* Description:
* The mdio bus will be allocated in case of a phy transceiver is on board;
* it will be NULL if the fixed-link is configured.
* If there is the "snps,dwmac-mdio" sub-node the mdio will be allocated
* in any case (for DSA, mdio must be registered even if fixed-link).
* The table below sums the supported configurations:
* -------------------------------
* snps,phy-addr | Y
* -------------------------------
* phy-handle | Y
* -------------------------------
* fixed-link | N
* -------------------------------
* snps,dwmac-mdio |
* even if | Y
* fixed-link |
* -------------------------------
* stmmac_of_get_mdio() - Gets the MDIO bus from the devicetree.
* @np: devicetree node
*
* It returns 0 in case of success otherwise -ENODEV.
* The MDIO bus will be searched for in the following ways:
* 1. The compatible is "snps,dwc-qos-ethernet-4.10" && a "mdio" named
* child node exists
* 2. A child node with the "snps,dwmac-mdio" compatible is present
*
* Return: The MDIO node if present otherwise NULL
*/
static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
struct device_node *np, struct device *dev)
static struct device_node *stmmac_of_get_mdio(struct device_node *np)
{
bool mdio = !of_phy_is_fixed_link(np);
static const struct of_device_id need_mdio_ids[] = {
{ .compatible = "snps,dwc-qos-ethernet-4.10" },
{},
};
struct device_node *mdio_node = NULL;

if (of_match_node(need_mdio_ids, np)) {
plat->mdio_node = of_get_child_by_name(np, "mdio");
mdio_node = of_get_child_by_name(np, "mdio");
} else {
/**
* If snps,dwmac-mdio is passed from DT, always register
* the MDIO
*/
for_each_child_of_node(np, plat->mdio_node) {
if (of_device_is_compatible(plat->mdio_node,
for_each_child_of_node(np, mdio_node) {
if (of_device_is_compatible(mdio_node,
"snps,dwmac-mdio"))
break;
}
}

if (plat->mdio_node) {
return mdio_node;
}

/**
* stmmac_mdio_setup() - Populate platform related MDIO structures.
* @plat: driver data platform structure
* @np: devicetree node
* @dev: device pointer
*
* This searches for MDIO information from the devicetree.
* If an MDIO node is found, it's assigned to plat->mdio_node and
* plat->mdio_bus_data is allocated.
* If no connection can be determined, just plat->mdio_bus_data is allocated
* to indicate a bus should be created and scanned for a phy.
* If it's determined there's no MDIO bus needed, both are left NULL.
*
* This expects that plat->phy_node has already been searched for.
*
* Return: 0 on success, errno otherwise.
*/
static int stmmac_mdio_setup(struct plat_stmmacenet_data *plat,
struct device_node *np, struct device *dev)
{
bool legacy_mdio;

plat->mdio_node = stmmac_of_get_mdio(np);
if (plat->mdio_node)
dev_dbg(dev, "Found MDIO subnode\n");
mdio = true;
}

if (mdio) {
plat->mdio_bus_data =
devm_kzalloc(dev, sizeof(struct stmmac_mdio_bus_data),
GFP_KERNEL);
/* Legacy devicetrees allowed for no MDIO bus description and expect
* the bus to be scanned for devices. If there's no phy or fixed-link
* described assume this is the case since there must be something
* connected to the MAC.
*/
legacy_mdio = !of_phy_is_fixed_link(np) && !plat->phy_node;
if (legacy_mdio)
dev_info(dev, "Deprecated MDIO bus assumption used\n");

if (plat->mdio_node || legacy_mdio) {
plat->mdio_bus_data = devm_kzalloc(dev,
sizeof(*plat->mdio_bus_data),
GFP_KERNEL);
if (!plat->mdio_bus_data)
return -ENOMEM;

Expand Down Expand Up @@ -471,8 +489,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
if (of_property_read_u32(np, "snps,phy-addr", &plat->phy_addr) == 0)
dev_warn(&pdev->dev, "snps,phy-addr property is deprecated\n");

/* To Configure PHY by using all device-tree supported properties */
rc = stmmac_dt_phy(plat, np, &pdev->dev);
rc = stmmac_mdio_setup(plat, np, &pdev->dev);
if (rc)
return ERR_PTR(rc);

Expand Down

0 comments on commit e9c7d51

Please sign in to comment.