2024-03-05 06:48:31

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net v2 1/1] net: dsa: microchip: make sure drive strength configuration is not lost by soft reset

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 <[email protected]>
---
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



2024-03-05 08:03:47

by Arun Ramadoss

[permalink] [raw]
Subject: Re: [PATCH net v2 1/1] net: dsa: microchip: make sure drive strength configuration is not lost by soft reset

Hi Oleksij,

On Tue, 2024-03-05 at 07:48 +0100, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> 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 <[email protected]>
>

Acked-by: Arun Ramadoss <[email protected]>

2024-03-06 03:15:09

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net v2 1/1] net: dsa: microchip: make sure drive strength configuration is not lost by soft reset

On Tue, 5 Mar 2024 07:48:02 +0100 Oleksij Rempel wrote:
> 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 <[email protected]>

Hm, this is much larger than I anticipated, and also doesn't apply,
so there will be conflict with -next. Andrew, should we go back to
the v1?

2024-03-06 03:29:56

by Arun Ramadoss

[permalink] [raw]
Subject: Re: [PATCH net v2 1/1] net: dsa: microchip: make sure drive strength configuration is not lost by soft reset

Hi Jakub,

On Tue, 2024-03-05 at 19:14 -0800, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Tue, 5 Mar 2024 07:48:02 +0100 Oleksij Rempel wrote:
> > 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 <[email protected]>
>
> Hm, this is much larger than I anticipated, and also doesn't apply,
> so there will be conflict with -next. Andrew, should we go back to
> the v1?

Suggestion: Instead of moving ksz_parse_drive_strength() from end of
file to above, can we move ksz_setup() & ksz_teardown() down. So that
the changes will be minimal. Will that work?


2024-03-06 06:04:05

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH net v2 1/1] net: dsa: microchip: make sure drive strength configuration is not lost by soft reset

On Wed, Mar 06, 2024 at 03:29:32AM +0000, [email protected] wrote:
> Hi Jakub,
>
> On Tue, 2024-03-05 at 19:14 -0800, Jakub Kicinski wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
> > On Tue, 5 Mar 2024 07:48:02 +0100 Oleksij Rempel wrote:
> > > 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 <[email protected]>
> >
> > Hm, this is much larger than I anticipated, and also doesn't apply,
> > so there will be conflict with -next. Andrew, should we go back to
> > the v1?
>
> Suggestion: Instead of moving ksz_parse_drive_strength() from end of
> file to above, can we move ksz_setup() & ksz_teardown() down. So that
> the changes will be minimal. Will that work?

This will make it hard portable to stable too. As alternative I can
offer to use v1 patch for stable and send bigger patch for next after
in the next net-next window.

--
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 |

2024-03-07 09:03:26

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH net v2 1/1] net: dsa: microchip: make sure drive strength configuration is not lost by soft reset

Hi Jakub,

On Wed, Mar 06, 2024 at 07:03:39AM +0100, Oleksij Rempel wrote:
> On Wed, Mar 06, 2024 at 03:29:32AM +0000, [email protected] wrote:
> > Hi Jakub,
> >
> > On Tue, 2024-03-05 at 19:14 -0800, Jakub Kicinski wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > know the content is safe
> > >
> > > On Tue, 5 Mar 2024 07:48:02 +0100 Oleksij Rempel wrote:
> > > > 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 <[email protected]>
> > >
> > > Hm, this is much larger than I anticipated, and also doesn't apply,
> > > so there will be conflict with -next. Andrew, should we go back to
> > > the v1?
> >
> > Suggestion: Instead of moving ksz_parse_drive_strength() from end of
> > file to above, can we move ksz_setup() & ksz_teardown() down. So that
> > the changes will be minimal. Will that work?
>
> This will make it hard portable to stable too. As alternative I can
> offer to use v1 patch for stable and send bigger patch for next after
> in the next net-next window.

Are any changes on my side required?

Regards,
Oleksij
--
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 |

2024-03-07 17:12:33

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net v2 1/1] net: dsa: microchip: make sure drive strength configuration is not lost by soft reset

On Thu, 7 Mar 2024 10:03:06 +0100 Oleksij Rempel wrote:
> > > Suggestion: Instead of moving ksz_parse_drive_strength() from end of
> > > file to above, can we move ksz_setup() & ksz_teardown() down. So that
> > > the changes will be minimal. Will that work?
> >
> > This will make it hard portable to stable too. As alternative I can
> > offer to use v1 patch for stable and send bigger patch for next after
> > in the next net-next window.
>
> Are any changes on my side required?

Not really, just waiting for anyone to speak up, I'll apply v1 soon.
(I can't revive v1 in patchwork because Konstantin's bot will set it
to Superseded, immediately, again :|)