1. Patchew
  2. linux
  3. [PATCH net-next v3 0/6] Side MDIO Support for LAN937x Switches
  4. View patch

[PATCH net-next v3 6/6] net: dsa: microchip: parse PHY config from device tree

Oleksij Rempel posted 6 patches 2 weeks, 5 days ago
There is a newer version of this series
[PATCH net-next v3 6/6] net: dsa: microchip: parse PHY config from device tree
Posted by Oleksij Rempel 2 weeks, 5 days ago 
Introduce ksz_parse_dt_phy_config() to validate and parse PHY
configuration from the device tree for KSZ switches. This function
ensures proper setup of internal PHYs by checking `phy-handle`
properties, verifying expected PHY IDs, and handling parent node
mismatches. Sets the PHY mask on the MII bus if validation is
successful. Returns -EINVAL on configuration errors.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz_common.c | 80 ++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 3909b55857430..cd1a466504180 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2373,6 +2373,77 @@ static void ksz_irq_phy_free(struct ksz_device *dev)
 			irq_dispose_mapping(ds->user_mii_bus->irq[phy]);
 }

+/**
+ * ksz_parse_dt_phy_config - Parse and validate PHY configuration from DT
+ * @dev: pointer to the KSZ device structure
+ * @bus: pointer to the MII bus structure
+ * @mdio_np: pointer to the MDIO node in the device tree
+ *
+ * This function parses and validates PHY configurations for each user port
+ * defined in the device tree for a KSZ switch device. It verifies that the
+ * `phy-handle` properties are correctly set and that the internal PHYs match
+ * expected IDs and parent nodes. Sets up the PHY mask in the MII bus if all
+ * validations pass. Logs error messages for any mismatches or missing data.
+ *
+ * Return: 0 on success, or a negative error code on failure.
+ */
+static int ksz_parse_dt_phy_config(struct ksz_device *dev, struct mii_bus *bus,
+				   struct device_node *mdio_np)
+{
+	struct device_node *phy_node, *phy_parent_node;
+	bool phys_are_valid = true;
+	struct dsa_port *dp;
+	u32 phy_id;
+	int ret;
+
+	dsa_switch_for_each_user_port(dp, dev->ds) {
+		if (!dev->info->internal_phy[dp->index])
+			continue;
+
+		phy_node = of_parse_phandle(dp->dn, "phy-handle", 0);
+		if (!phy_node) {
+			dev_err(dev->dev, "failed to parse phy-handle for port %d.\n",
+				dp->index);
+			phys_are_valid = false;
+			continue;
+		}
+
+		phy_parent_node = of_get_parent(phy_node);
+		if (!phy_parent_node) {
+			dev_err(dev->dev, "failed to get PHY-parent node for port %d\n",
+				dp->index);
+			phys_are_valid = false;
+		} else if (dev->info->internal_phy[dp->index] &&
+			   phy_parent_node != mdio_np) {
+			dev_err(dev->dev, "PHY-parent node mismatch for port %d, expected %pOF, got %pOF\n",
+				dp->index, mdio_np, phy_parent_node);
+			phys_are_valid = false;
+		} else {
+			ret = of_property_read_u32(phy_node, "reg", &phy_id);
+			if (ret < 0) {
+				dev_err(dev->dev, "failed to read PHY ID for port %d. Error %d\n",
+					dp->index, ret);
+				phys_are_valid = false;
+			} else if (phy_id != dev->phy_addr_map[dp->index]) {
+				dev_err(dev->dev, "PHY ID mismatch for port %d, expected 0x%x, got 0x%x\n",
+					dp->index, dev->phy_addr_map[dp->index],
+					phy_id);
+				phys_are_valid = false;
+			} else {
+				bus->phy_mask |= BIT(phy_id);
+			}
+		}
+
+		of_node_put(phy_node);
+		of_node_put(phy_parent_node);
+	}
+
+	if (!phys_are_valid)
+		return -EINVAL;
+
+	return 0;
+}
+
 /**
  * ksz_mdio_register - Register and configure the MDIO bus for the KSZ device.
  * @dev: Pointer to the KSZ device structure.
@@ -2392,7 +2463,6 @@ static int ksz_mdio_register(struct ksz_device *dev)
 	struct dsa_switch *ds = dev->ds;
 	struct device_node *mdio_np;
 	struct mii_bus *bus;
-	struct dsa_port *dp;
 	int ret, i;

 	mdio_np = of_get_child_by_name(dev->dev->of_node, "mdio");
@@ -2451,11 +2521,9 @@ static int ksz_mdio_register(struct ksz_device *dev)
 		snprintf(bus->id, MII_BUS_ID_SIZE, "SMI-%d", ds->index);
 	}

-	dsa_switch_for_each_user_port(dp, dev->ds) {
-		if (dev->info->internal_phy[dp->index] &&
-		    dev->phy_addr_map[dp->index] < PHY_MAX_ADDR)
-			bus->phy_mask |= BIT(dev->phy_addr_map[dp->index]);
-	}
+	ret = ksz_parse_dt_phy_config(dev, bus, mdio_np);
+	if (ret)
+		goto put_mdio_node;

 	ds->phys_mii_mask = bus->phy_mask;
 	bus->parent = ds->dev;
--
2.39.5
Re: [PATCH net-next v3 6/6] net: dsa: microchip: parse PHY config from device tree
Posted by Maxime Chevallier 2 weeks, 5 days ago 
Hello Oleksij,

On Tue,  5 Nov 2024 10:09:44 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Introduce ksz_parse_dt_phy_config() to validate and parse PHY
> configuration from the device tree for KSZ switches. This function
> ensures proper setup of internal PHYs by checking `phy-handle`
> properties, verifying expected PHY IDs, and handling parent node
> mismatches. Sets the PHY mask on the MII bus if validation is
> successful. Returns -EINVAL on configuration errors.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/dsa/microchip/ksz_common.c | 80 ++++++++++++++++++++++++--
>  1 file changed, 74 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 3909b55857430..cd1a466504180 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -2373,6 +2373,77 @@ static void ksz_irq_phy_free(struct ksz_device *dev)
>  			irq_dispose_mapping(ds->user_mii_bus->irq[phy]);
>  }
> 
> +/**
> + * ksz_parse_dt_phy_config - Parse and validate PHY configuration from DT
> + * @dev: pointer to the KSZ device structure
> + * @bus: pointer to the MII bus structure
> + * @mdio_np: pointer to the MDIO node in the device tree
> + *
> + * This function parses and validates PHY configurations for each user port
> + * defined in the device tree for a KSZ switch device. It verifies that the
> + * `phy-handle` properties are correctly set and that the internal PHYs match
> + * expected IDs and parent nodes. Sets up the PHY mask in the MII bus if all
> + * validations pass. Logs error messages for any mismatches or missing data.
> + *
> + * Return: 0 on success, or a negative error code on failure.
> + */
> +static int ksz_parse_dt_phy_config(struct ksz_device *dev, struct mii_bus *bus,
> +				   struct device_node *mdio_np)
> +{
> +	struct device_node *phy_node, *phy_parent_node;
> +	bool phys_are_valid = true;
> +	struct dsa_port *dp;
> +	u32 phy_id;
> +	int ret;
> +
> +	dsa_switch_for_each_user_port(dp, dev->ds) {
> +		if (!dev->info->internal_phy[dp->index])
> +			continue;
> +
> +		phy_node = of_parse_phandle(dp->dn, "phy-handle", 0);
> +		if (!phy_node) {
> +			dev_err(dev->dev, "failed to parse phy-handle for port %d.\n",
> +				dp->index);
> +			phys_are_valid = false;
> +			continue;
> +		}
> +
> +		phy_parent_node = of_get_parent(phy_node);
> +		if (!phy_parent_node) {
> +			dev_err(dev->dev, "failed to get PHY-parent node for port %d\n",
> +				dp->index);
> +			phys_are_valid = false;
> +		} else if (dev->info->internal_phy[dp->index] &&
> +			   phy_parent_node != mdio_np) {

There's a check a few lines above that guarantees that at this point
dev->info->internal_phy[dp->index] will always evaluate as true,
so you could simplify that condition a bit :)

> +			dev_err(dev->dev, "PHY-parent node mismatch for port %d, expected %pOF, got %pOF\n",
> +				dp->index, mdio_np, phy_parent_node);
> +			phys_are_valid = false;
> +		} else {
> +			ret = of_property_read_u32(phy_node, "reg", &phy_id);
> +			if (ret < 0) {
> +				dev_err(dev->dev, "failed to read PHY ID for port %d. Error %d\n",
> +					dp->index, ret);
> +				phys_are_valid = false;
> +			} else if (phy_id != dev->phy_addr_map[dp->index]) {
> +				dev_err(dev->dev, "PHY ID mismatch for port %d, expected 0x%x, got 0x%x\n",
> +					dp->index, dev->phy_addr_map[dp->index],
> +					phy_id);

In this context, PHY ID might be a bit misleading, as PHY ID usually
refers to the identifier (OUI + model id used at probe to select the
driver). May I suggest phy_addr instead ?

Thanks,

Maxime
Re: [PATCH net-next v3 6/6] net: dsa: microchip: parse PHY config from device tree
Posted by Oleksij Rempel 2 weeks, 5 days ago 
Hi Maxime,

On Tue, Nov 05, 2024 at 03:28:05PM +0100, Maxime Chevallier wrote:
> > +	dsa_switch_for_each_user_port(dp, dev->ds) {
> > +		if (!dev->info->internal_phy[dp->index])
> > +			continue;
> > +
> > +		phy_node = of_parse_phandle(dp->dn, "phy-handle", 0);
> > +		if (!phy_node) {
> > +			dev_err(dev->dev, "failed to parse phy-handle for port %d.\n",
> > +				dp->index);
> > +			phys_are_valid = false;
> > +			continue;
> > +		}
> > +
> > +		phy_parent_node = of_get_parent(phy_node);
> > +		if (!phy_parent_node) {
> > +			dev_err(dev->dev, "failed to get PHY-parent node for port %d\n",
> > +				dp->index);
> > +			phys_are_valid = false;
> > +		} else if (dev->info->internal_phy[dp->index] &&
> > +			   phy_parent_node != mdio_np) {
> 
> There's a check a few lines above that guarantees that at this point
> dev->info->internal_phy[dp->index] will always evaluate as true,
> so you could simplify that condition a bit :)

good point :)

> > +			dev_err(dev->dev, "PHY-parent node mismatch for port %d, expected %pOF, got %pOF\n",
> > +				dp->index, mdio_np, phy_parent_node);
> > +			phys_are_valid = false;
> > +		} else {
> > +			ret = of_property_read_u32(phy_node, "reg", &phy_id);
> > +			if (ret < 0) {
> > +				dev_err(dev->dev, "failed to read PHY ID for port %d. Error %d\n",
> > +					dp->index, ret);
> > +				phys_are_valid = false;
> > +			} else if (phy_id != dev->phy_addr_map[dp->index]) {
> > +				dev_err(dev->dev, "PHY ID mismatch for port %d, expected 0x%x, got 0x%x\n",
> > +					dp->index, dev->phy_addr_map[dp->index],
> > +					phy_id);
> 
> In this context, PHY ID might be a bit misleading, as PHY ID usually
> refers to the identifier (OUI + model id used at probe to select the
> driver). May I suggest phy_addr instead ?

ack, will rework it.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.