Skip to content

Commit 2b67a98

Browse files
ahalaneyNipaLocal
authored and
NipaLocal
committed
net: stmmac: don't create a MDIO bus if unnecessary
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 <[email protected]> Co-developed-by: Bartosz Golaszewski <[email protected]> Signed-off-by: Bartosz Golaszewski <[email protected]> Signed-off-by: Andrew Halaney <[email protected]> Signed-off-by: NipaLocal <nipa@local>
1 parent 87dbe79 commit 2b67a98

File tree

1 file changed

+54
-37
lines changed

1 file changed

+54
-37
lines changed

drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c

+54-37
Original file line numberDiff line numberDiff line change
@@ -296,62 +296,80 @@ static int stmmac_mtl_setup(struct platform_device *pdev,
296296
}
297297

298298
/**
299-
* stmmac_dt_phy - parse device-tree driver parameters to allocate PHY resources
300-
* @plat: driver data platform structure
301-
* @np: device tree node
302-
* @dev: device pointer
303-
* Description:
304-
* The mdio bus will be allocated in case of a phy transceiver is on board;
305-
* it will be NULL if the fixed-link is configured.
306-
* If there is the "snps,dwmac-mdio" sub-node the mdio will be allocated
307-
* in any case (for DSA, mdio must be registered even if fixed-link).
308-
* The table below sums the supported configurations:
309-
* -------------------------------
310-
* snps,phy-addr | Y
311-
* -------------------------------
312-
* phy-handle | Y
313-
* -------------------------------
314-
* fixed-link | N
315-
* -------------------------------
316-
* snps,dwmac-mdio |
317-
* even if | Y
318-
* fixed-link |
319-
* -------------------------------
299+
* stmmac_of_get_mdio() - Gets the MDIO bus from the devicetree.
300+
* @np: devicetree node
320301
*
321-
* It returns 0 in case of success otherwise -ENODEV.
302+
* The MDIO bus will be searched for in the following ways:
303+
* 1. The compatible is "snps,dwc-qos-ethernet-4.10" && a "mdio" named
304+
* child node exists
305+
* 2. A child node with the "snps,dwmac-mdio" compatible is present
306+
*
307+
* Return: The MDIO node if present otherwise NULL
322308
*/
323-
static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
324-
struct device_node *np, struct device *dev)
309+
static struct device_node *stmmac_of_get_mdio(struct device_node *np)
325310
{
326-
bool mdio = !of_phy_is_fixed_link(np);
327311
static const struct of_device_id need_mdio_ids[] = {
328312
{ .compatible = "snps,dwc-qos-ethernet-4.10" },
329313
{},
330314
};
315+
struct device_node *mdio_node = NULL;
331316

332317
if (of_match_node(need_mdio_ids, np)) {
333-
plat->mdio_node = of_get_child_by_name(np, "mdio");
318+
mdio_node = of_get_child_by_name(np, "mdio");
334319
} else {
335320
/**
336321
* If snps,dwmac-mdio is passed from DT, always register
337322
* the MDIO
338323
*/
339-
for_each_child_of_node(np, plat->mdio_node) {
340-
if (of_device_is_compatible(plat->mdio_node,
324+
for_each_child_of_node(np, mdio_node) {
325+
if (of_device_is_compatible(mdio_node,
341326
"snps,dwmac-mdio"))
342327
break;
343328
}
344329
}
345330

346-
if (plat->mdio_node) {
331+
return mdio_node;
332+
}
333+
334+
/**
335+
* stmmac_mdio_setup() - Populate platform related MDIO structures.
336+
* @plat: driver data platform structure
337+
* @np: devicetree node
338+
* @dev: device pointer
339+
*
340+
* This searches for MDIO information from the devicetree.
341+
* If an MDIO node is found, it's assigned to plat->mdio_node and
342+
* plat->mdio_bus_data is allocated.
343+
* If no connection can be determined, just plat->mdio_bus_data is allocated
344+
* to indicate a bus should be created and scanned for a phy.
345+
* If it's determined there's no MDIO bus needed, both are left NULL.
346+
*
347+
* This expects that plat->phy_node has already been searched for.
348+
*
349+
* Return: 0 on success, errno otherwise.
350+
*/
351+
static int stmmac_mdio_setup(struct plat_stmmacenet_data *plat,
352+
struct device_node *np, struct device *dev)
353+
{
354+
bool legacy_mdio;
355+
356+
plat->mdio_node = stmmac_of_get_mdio(np);
357+
if (plat->mdio_node)
347358
dev_dbg(dev, "Found MDIO subnode\n");
348-
mdio = true;
349-
}
350359

351-
if (mdio) {
352-
plat->mdio_bus_data =
353-
devm_kzalloc(dev, sizeof(struct stmmac_mdio_bus_data),
354-
GFP_KERNEL);
360+
/* Legacy devicetrees allowed for no MDIO bus description and expect
361+
* the bus to be scanned for devices. If there's no phy or fixed-link
362+
* described assume this is the case since there must be something
363+
* connected to the MAC.
364+
*/
365+
legacy_mdio = !of_phy_is_fixed_link(np) && !plat->phy_node;
366+
if (legacy_mdio)
367+
dev_info(dev, "Deprecated MDIO bus assumption used\n");
368+
369+
if (plat->mdio_node || legacy_mdio) {
370+
plat->mdio_bus_data = devm_kzalloc(dev,
371+
sizeof(*plat->mdio_bus_data),
372+
GFP_KERNEL);
355373
if (!plat->mdio_bus_data)
356374
return -ENOMEM;
357375

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

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

0 commit comments

Comments
 (0)