Received-SPF: pass (google.com: domain of linux-kernel+bounces-91748-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Oleksij Rempel <o.rempel@pengutronix.de>
To: "David S. Miller" <davem@davemloft.net>,
	Andrew Lunn <andrew@lunn.ch>,
	Eric Dumazet <edumazet@google.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	Woojung Huh <woojung.huh@microchip.com>,
	Arun Ramadoss <arun.ramadoss@microchip.com>
Cc: Oleksij Rempel <o.rempel@pengutronix.de>,
	kernel@pengutronix.de,
	linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org,
	UNGLinuxDriver@microchip.com
Subject: [PATCH net v2 1/1] net: dsa: microchip: make sure drive strength configuration is not lost by soft reset
Date: Tue,  5 Mar 2024 07:48:02 +0100
Message-Id: <20240305064802.2478971-1-o.rempel@pengutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

This driver has two separate reset sequence in different places:
- gpio/HW reset on start of ksz_switch_register()
- SW reset on start of ksz_setup()

The second one will overwrite drive strength configuration made in the
ksz_switch_register().

To fix it, move ksz_parse_drive_strength() from ksz_switch_register() to
ksz_setup().

Fixes: d67d7247f641 ("net: dsa: microchip: Add drive strength configuration")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- move code to avoid forward declaration

 drivers/net/dsa/microchip/ksz_common.c | 488 ++++++++++++-------------
 1 file changed, 244 insertions(+), 244 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index d58cc685478b1..030b167764b39 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2260,6 +2260,246 @@ static int ksz_pirq_setup(struct ksz_device *dev, u8 p)
 	return ksz_irq_common_setup(dev, pirq);
 }
 
+/**
+ * ksz_drive_strength_to_reg() - Convert drive strength value to corresponding
+ *				 register value.
+ * @array:	The array of drive strength values to search.
+ * @array_size:	The size of the array.
+ * @microamp:	The drive strength value in microamp to be converted.
+ *
+ * This function searches the array of drive strength values for the given
+ * microamp value and returns the corresponding register value for that drive.
+ *
+ * Returns: If found, the corresponding register value for that drive strength
+ * is returned. Otherwise, -EINVAL is returned indicating an invalid value.
+ */
+static int ksz_drive_strength_to_reg(const struct ksz_drive_strength *array,
+				     size_t array_size, int microamp)
+{
+	int i;
+
+	for (i = 0; i < array_size; i++) {
+		if (array[i].microamp == microamp)
+			return array[i].reg_val;
+	}
+
+	return -EINVAL;
+}
+
+/**
+ * ksz_drive_strength_error() - Report invalid drive strength value
+ * @dev:	ksz device
+ * @array:	The array of drive strength values to search.
+ * @array_size:	The size of the array.
+ * @microamp:	Invalid drive strength value in microamp
+ *
+ * This function logs an error message when an unsupported drive strength value
+ * is detected. It lists out all the supported drive strength values for
+ * reference in the error message.
+ */
+static void ksz_drive_strength_error(struct ksz_device *dev,
+				     const struct ksz_drive_strength *array,
+				     size_t array_size, int microamp)
+{
+	char supported_values[100];
+	size_t remaining_size;
+	int added_len;
+	char *ptr;
+	int i;
+
+	remaining_size = sizeof(supported_values);
+	ptr = supported_values;
+
+	for (i = 0; i < array_size; i++) {
+		added_len = snprintf(ptr, remaining_size,
+				     i == 0 ? "%d" : ", %d", array[i].microamp);
+
+		if (added_len >= remaining_size)
+			break;
+
+		ptr += added_len;
+		remaining_size -= added_len;
+	}
+
+	dev_err(dev->dev, "Invalid drive strength %d, supported values are %s\n",
+		microamp, supported_values);
+}
+
+/**
+ * ksz9477_drive_strength_write() - Set the drive strength for specific KSZ9477
+ *				    chip variants.
+ * @dev:       ksz device
+ * @props:     Array of drive strength properties to be applied
+ * @num_props: Number of properties in the array
+ *
+ * This function configures the drive strength for various KSZ9477 chip variants
+ * based on the provided properties. It handles chip-specific nuances and
+ * ensures only valid drive strengths are written to the respective chip.
+ *
+ * Return: 0 on successful configuration, a negative error code on failure.
+ */
+static int ksz9477_drive_strength_write(struct ksz_device *dev,
+					struct ksz_driver_strength_prop *props,
+					int num_props)
+{
+	size_t array_size = ARRAY_SIZE(ksz9477_drive_strengths);
+	int i, ret, reg;
+	u8 mask = 0;
+	u8 val = 0;
+
+	if (props[KSZ_DRIVER_STRENGTH_IO].value != -1)
+		dev_warn(dev->dev, "%s is not supported by this chip variant\n",
+			 props[KSZ_DRIVER_STRENGTH_IO].name);
+
+	if (dev->chip_id == KSZ8795_CHIP_ID ||
+	    dev->chip_id == KSZ8794_CHIP_ID ||
+	    dev->chip_id == KSZ8765_CHIP_ID)
+		reg = KSZ8795_REG_SW_CTRL_20;
+	else
+		reg = KSZ9477_REG_SW_IO_STRENGTH;
+
+	for (i = 0; i < num_props; i++) {
+		if (props[i].value == -1)
+			continue;
+
+		ret = ksz_drive_strength_to_reg(ksz9477_drive_strengths,
+						array_size, props[i].value);
+		if (ret < 0) {
+			ksz_drive_strength_error(dev, ksz9477_drive_strengths,
+						 array_size, props[i].value);
+			return ret;
+		}
+
+		mask |= SW_DRIVE_STRENGTH_M << props[i].offset;
+		val |= ret << props[i].offset;
+	}
+
+	return ksz_rmw8(dev, reg, mask, val);
+}
+
+/**
+ * ksz8830_drive_strength_write() - Set the drive strength configuration for
+ *				    KSZ8830 compatible chip variants.
+ * @dev:       ksz device
+ * @props:     Array of drive strength properties to be set
+ * @num_props: Number of properties in the array
+ *
+ * This function applies the specified drive strength settings to KSZ8830 chip
+ * variants (KSZ8873, KSZ8863).
+ * It ensures the configurations align with what the chip variant supports and
+ * warns or errors out on unsupported settings.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int ksz8830_drive_strength_write(struct ksz_device *dev,
+					struct ksz_driver_strength_prop *props,
+					int num_props)
+{
+	size_t array_size = ARRAY_SIZE(ksz8830_drive_strengths);
+	int microamp;
+	int i, ret;
+
+	for (i = 0; i < num_props; i++) {
+		if (props[i].value == -1 || i == KSZ_DRIVER_STRENGTH_IO)
+			continue;
+
+		dev_warn(dev->dev, "%s is not supported by this chip variant\n",
+			 props[i].name);
+	}
+
+	microamp = props[KSZ_DRIVER_STRENGTH_IO].value;
+	ret = ksz_drive_strength_to_reg(ksz8830_drive_strengths, array_size,
+					microamp);
+	if (ret < 0) {
+		ksz_drive_strength_error(dev, ksz8830_drive_strengths,
+					 array_size, microamp);
+		return ret;
+	}
+
+	return ksz_rmw8(dev, KSZ8873_REG_GLOBAL_CTRL_12,
+			KSZ8873_DRIVE_STRENGTH_16MA, ret);
+}
+
+/**
+ * ksz_parse_drive_strength() - Extract and apply drive strength configurations
+ *				from device tree properties.
+ * @dev:	ksz device
+ *
+ * This function reads the specified drive strength properties from the
+ * device tree, validates against the supported chip variants, and sets
+ * them accordingly. An error should be critical here, as the drive strength
+ * settings are crucial for EMI compliance.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int ksz_parse_drive_strength(struct ksz_device *dev)
+{
+	struct ksz_driver_strength_prop of_props[] = {
+		[KSZ_DRIVER_STRENGTH_HI] = {
+			.name = "microchip,hi-drive-strength-microamp",
+			.offset = SW_HI_SPEED_DRIVE_STRENGTH_S,
+			.value = -1,
+		},
+		[KSZ_DRIVER_STRENGTH_LO] = {
+			.name = "microchip,lo-drive-strength-microamp",
+			.offset = SW_LO_SPEED_DRIVE_STRENGTH_S,
+			.value = -1,
+		},
+		[KSZ_DRIVER_STRENGTH_IO] = {
+			.name = "microchip,io-drive-strength-microamp",
+			.offset = 0, /* don't care */
+			.value = -1,
+		},
+	};
+	struct device_node *np = dev->dev->of_node;
+	bool have_any_prop = false;
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(of_props); i++) {
+		ret = of_property_read_u32(np, of_props[i].name,
+					   &of_props[i].value);
+		if (ret && ret != -EINVAL)
+			dev_warn(dev->dev, "Failed to read %s\n",
+				 of_props[i].name);
+		if (ret)
+			continue;
+
+		have_any_prop = true;
+	}
+
+	if (!have_any_prop)
+		return 0;
+
+	switch (dev->chip_id) {
+	case KSZ8830_CHIP_ID:
+		return ksz8830_drive_strength_write(dev, of_props,
+						    ARRAY_SIZE(of_props));
+	case KSZ8795_CHIP_ID:
+	case KSZ8794_CHIP_ID:
+	case KSZ8765_CHIP_ID:
+	case KSZ8563_CHIP_ID:
+	case KSZ8567_CHIP_ID:
+	case KSZ9477_CHIP_ID:
+	case KSZ9563_CHIP_ID:
+	case KSZ9567_CHIP_ID:
+	case KSZ9893_CHIP_ID:
+	case KSZ9896_CHIP_ID:
+	case KSZ9897_CHIP_ID:
+		return ksz9477_drive_strength_write(dev, of_props,
+						    ARRAY_SIZE(of_props));
+	default:
+		for (i = 0; i < ARRAY_SIZE(of_props); i++) {
+			if (of_props[i].value == -1)
+				continue;
+
+			dev_warn(dev->dev, "%s is not supported by this chip variant\n",
+				 of_props[i].name);
+		}
+	}
+
+	return 0;
+}
+
 static int ksz_setup(struct dsa_switch *ds)
 {
 	struct ksz_device *dev = ds->priv;
@@ -2281,6 +2521,10 @@ static int ksz_setup(struct dsa_switch *ds)
 		return ret;
 	}
 
+	ret = ksz_parse_drive_strength(dev);
+	if (ret)
+		return ret;
+
 	/* set broadcast storm protection 10% rate */
 	regmap_update_bits(ksz_regmap_16(dev), regs[S_BROADCAST_CTRL],
 			   BROADCAST_STORM_RATE,
@@ -4018,246 +4262,6 @@ static void ksz_parse_rgmii_delay(struct ksz_device *dev, int port_num,
 	dev->ports[port_num].rgmii_tx_val = tx_delay;
 }
 
-/**
- * ksz_drive_strength_to_reg() - Convert drive strength value to corresponding
- *				 register value.
- * @array:	The array of drive strength values to search.
- * @array_size:	The size of the array.
- * @microamp:	The drive strength value in microamp to be converted.
- *
- * This function searches the array of drive strength values for the given
- * microamp value and returns the corresponding register value for that drive.
- *
- * Returns: If found, the corresponding register value for that drive strength
- * is returned. Otherwise, -EINVAL is returned indicating an invalid value.
- */
-static int ksz_drive_strength_to_reg(const struct ksz_drive_strength *array,
-				     size_t array_size, int microamp)
-{
-	int i;
-
-	for (i = 0; i < array_size; i++) {
-		if (array[i].microamp == microamp)
-			return array[i].reg_val;
-	}
-
-	return -EINVAL;
-}
-
-/**
- * ksz_drive_strength_error() - Report invalid drive strength value
- * @dev:	ksz device
- * @array:	The array of drive strength values to search.
- * @array_size:	The size of the array.
- * @microamp:	Invalid drive strength value in microamp
- *
- * This function logs an error message when an unsupported drive strength value
- * is detected. It lists out all the supported drive strength values for
- * reference in the error message.
- */
-static void ksz_drive_strength_error(struct ksz_device *dev,
-				     const struct ksz_drive_strength *array,
-				     size_t array_size, int microamp)
-{
-	char supported_values[100];
-	size_t remaining_size;
-	int added_len;
-	char *ptr;
-	int i;
-
-	remaining_size = sizeof(supported_values);
-	ptr = supported_values;
-
-	for (i = 0; i < array_size; i++) {
-		added_len = snprintf(ptr, remaining_size,
-				     i == 0 ? "%d" : ", %d", array[i].microamp);
-
-		if (added_len >= remaining_size)
-			break;
-
-		ptr += added_len;
-		remaining_size -= added_len;
-	}
-
-	dev_err(dev->dev, "Invalid drive strength %d, supported values are %s\n",
-		microamp, supported_values);
-}
-
-/**
- * ksz9477_drive_strength_write() - Set the drive strength for specific KSZ9477
- *				    chip variants.
- * @dev:       ksz device
- * @props:     Array of drive strength properties to be applied
- * @num_props: Number of properties in the array
- *
- * This function configures the drive strength for various KSZ9477 chip variants
- * based on the provided properties. It handles chip-specific nuances and
- * ensures only valid drive strengths are written to the respective chip.
- *
- * Return: 0 on successful configuration, a negative error code on failure.
- */
-static int ksz9477_drive_strength_write(struct ksz_device *dev,
-					struct ksz_driver_strength_prop *props,
-					int num_props)
-{
-	size_t array_size = ARRAY_SIZE(ksz9477_drive_strengths);
-	int i, ret, reg;
-	u8 mask = 0;
-	u8 val = 0;
-
-	if (props[KSZ_DRIVER_STRENGTH_IO].value != -1)
-		dev_warn(dev->dev, "%s is not supported by this chip variant\n",
-			 props[KSZ_DRIVER_STRENGTH_IO].name);
-
-	if (dev->chip_id == KSZ8795_CHIP_ID ||
-	    dev->chip_id == KSZ8794_CHIP_ID ||
-	    dev->chip_id == KSZ8765_CHIP_ID)
-		reg = KSZ8795_REG_SW_CTRL_20;
-	else
-		reg = KSZ9477_REG_SW_IO_STRENGTH;
-
-	for (i = 0; i < num_props; i++) {
-		if (props[i].value == -1)
-			continue;
-
-		ret = ksz_drive_strength_to_reg(ksz9477_drive_strengths,
-						array_size, props[i].value);
-		if (ret < 0) {
-			ksz_drive_strength_error(dev, ksz9477_drive_strengths,
-						 array_size, props[i].value);
-			return ret;
-		}
-
-		mask |= SW_DRIVE_STRENGTH_M << props[i].offset;
-		val |= ret << props[i].offset;
-	}
-
-	return ksz_rmw8(dev, reg, mask, val);
-}
-
-/**
- * ksz8830_drive_strength_write() - Set the drive strength configuration for
- *				    KSZ8830 compatible chip variants.
- * @dev:       ksz device
- * @props:     Array of drive strength properties to be set
- * @num_props: Number of properties in the array
- *
- * This function applies the specified drive strength settings to KSZ8830 chip
- * variants (KSZ8873, KSZ8863).
- * It ensures the configurations align with what the chip variant supports and
- * warns or errors out on unsupported settings.
- *
- * Return: 0 on success, error code otherwise
- */
-static int ksz8830_drive_strength_write(struct ksz_device *dev,
-					struct ksz_driver_strength_prop *props,
-					int num_props)
-{
-	size_t array_size = ARRAY_SIZE(ksz8830_drive_strengths);
-	int microamp;
-	int i, ret;
-
-	for (i = 0; i < num_props; i++) {
-		if (props[i].value == -1 || i == KSZ_DRIVER_STRENGTH_IO)
-			continue;
-
-		dev_warn(dev->dev, "%s is not supported by this chip variant\n",
-			 props[i].name);
-	}
-
-	microamp = props[KSZ_DRIVER_STRENGTH_IO].value;
-	ret = ksz_drive_strength_to_reg(ksz8830_drive_strengths, array_size,
-					microamp);
-	if (ret < 0) {
-		ksz_drive_strength_error(dev, ksz8830_drive_strengths,
-					 array_size, microamp);
-		return ret;
-	}
-
-	return ksz_rmw8(dev, KSZ8873_REG_GLOBAL_CTRL_12,
-			KSZ8873_DRIVE_STRENGTH_16MA, ret);
-}
-
-/**
- * ksz_parse_drive_strength() - Extract and apply drive strength configurations
- *				from device tree properties.
- * @dev:	ksz device
- *
- * This function reads the specified drive strength properties from the
- * device tree, validates against the supported chip variants, and sets
- * them accordingly. An error should be critical here, as the drive strength
- * settings are crucial for EMI compliance.
- *
- * Return: 0 on success, error code otherwise
- */
-static int ksz_parse_drive_strength(struct ksz_device *dev)
-{
-	struct ksz_driver_strength_prop of_props[] = {
-		[KSZ_DRIVER_STRENGTH_HI] = {
-			.name = "microchip,hi-drive-strength-microamp",
-			.offset = SW_HI_SPEED_DRIVE_STRENGTH_S,
-			.value = -1,
-		},
-		[KSZ_DRIVER_STRENGTH_LO] = {
-			.name = "microchip,lo-drive-strength-microamp",
-			.offset = SW_LO_SPEED_DRIVE_STRENGTH_S,
-			.value = -1,
-		},
-		[KSZ_DRIVER_STRENGTH_IO] = {
-			.name = "microchip,io-drive-strength-microamp",
-			.offset = 0, /* don't care */
-			.value = -1,
-		},
-	};
-	struct device_node *np = dev->dev->of_node;
-	bool have_any_prop = false;
-	int i, ret;
-
-	for (i = 0; i < ARRAY_SIZE(of_props); i++) {
-		ret = of_property_read_u32(np, of_props[i].name,
-					   &of_props[i].value);
-		if (ret && ret != -EINVAL)
-			dev_warn(dev->dev, "Failed to read %s\n",
-				 of_props[i].name);
-		if (ret)
-			continue;
-
-		have_any_prop = true;
-	}
-
-	if (!have_any_prop)
-		return 0;
-
-	switch (dev->chip_id) {
-	case KSZ8830_CHIP_ID:
-		return ksz8830_drive_strength_write(dev, of_props,
-						    ARRAY_SIZE(of_props));
-	case KSZ8795_CHIP_ID:
-	case KSZ8794_CHIP_ID:
-	case KSZ8765_CHIP_ID:
-	case KSZ8563_CHIP_ID:
-	case KSZ8567_CHIP_ID:
-	case KSZ9477_CHIP_ID:
-	case KSZ9563_CHIP_ID:
-	case KSZ9567_CHIP_ID:
-	case KSZ9893_CHIP_ID:
-	case KSZ9896_CHIP_ID:
-	case KSZ9897_CHIP_ID:
-		return ksz9477_drive_strength_write(dev, of_props,
-						    ARRAY_SIZE(of_props));
-	default:
-		for (i = 0; i < ARRAY_SIZE(of_props); i++) {
-			if (of_props[i].value == -1)
-				continue;
-
-			dev_warn(dev->dev, "%s is not supported by this chip variant\n",
-				 of_props[i].name);
-		}
-	}
-
-	return 0;
-}
-
 int ksz_switch_register(struct ksz_device *dev)
 {
 	const struct ksz_chip_data *info;
@@ -4345,10 +4349,6 @@ int ksz_switch_register(struct ksz_device *dev)
 	for (port_num = 0; port_num < dev->info->port_cnt; ++port_num)
 		dev->ports[port_num].interface = PHY_INTERFACE_MODE_NA;
 	if (dev->dev->of_node) {
-		ret = ksz_parse_drive_strength(dev);
-		if (ret)
-			return ret;
-
 		ret = of_get_phy_mode(dev->dev->of_node, &interface);
 		if (ret == 0)
 			dev->compat_interface = interface;
-- 
2.39.2