| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20231207-stmmac-no-mdio-node-v3-1-34b870f2bafb@redhat.com>
Date: Thu, 07 Dec 2023 15:12:40 -0600
From: Andrew Halaney <ahalaney@...hat.com>
To: Alexandre Torgue <alexandre.torgue@...s.st.com>,
Jose Abreu <joabreu@...opsys.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>
Cc: netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
Andrew Halaney <ahalaney@...hat.com>
Subject: [PATCH net-next v3] net: stmmac: don't create a MDIO bus if
unnecessary
The stmmac_dt_phy() function, which parses the devicetree node of the
MAC and ultimately causes MDIO bus allocation, misinterprets what
fixed-link means in relation to the MAC's MDIO bus. This results in
a MDIO bus being created in situations it need not be.
Currently a MDIO bus is created if the 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 currently the MAC will make a MDIO bus and scan it all
anyways unnecessarily.
There's also a lot of upstream devicetrees[2] that expect a MDIO bus to
be created and scanned for a phy. This case can also 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.
Let's improve the logic to create a MDIO bus if either:
- Devicetree contains a MDIO bus
- !fixed-link && !phy-handle (legacy handling)
Below upstream 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:
ðernet0 {
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";
};
};
};
ðernet1 {
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
Co-developed-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Signed-off-by: Andrew Halaney <ahalaney@...hat.com>
---
Changes in v3:
- Keep logic out of stmmac_probe_config_dt() since it's already massive (Serge)
Changes in v2:
- Handle the fixed-link + mdio case (Andrew Lunn)
- Reworded commit message
- Still handle the "legacy" case mentioned in the commit
- Bit further refactoring of the function for readability
- Link to v2: https://lore.kernel.org/r/20231206-stmmac-no-mdio-node-v2-1-333cae49b1ca@redhat.com
- Link to v1: https://lore.kernel.org/netdev/20230808120254.11653-1-brgl@bgdev.pl/
---
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 91 +++++++++++++---------
1 file changed, 54 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 1ffde555da47..d6079c1432c7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -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(struct stmmac_mdio_bus_data),
+ GFP_KERNEL);
if (!plat->mdio_bus_data)
return -ENOMEM;
@@ -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);
---
base-commit: fd8a79b63710acb84321be3ce74be23c876873fb
change-id: 20231127-stmmac-no-mdio-node-1db9da8a25d9
Best regards,
--
Andrew Halaney <ahalaney@...hat.com>
Powered by blists - more mailing lists