mbox series

[RFC,net-next,v2,0/2] net: dsa: microchip: add drive strength support

Message ID 20230906105904.1477021-1-o.rempel@pengutronix.de
Headers show
Series net: dsa: microchip: add drive strength support | expand

Message

Oleksij Rempel Sept. 6, 2023, 10:59 a.m. UTC
changes v2:
- make it work on all know KSZÜ variants except of undocumented LAN*
  switches
- add io-drive-strength compatible for ksz88xx chips
- test exact drive strength instead of nearest closest.
- add comment and refactor the code

Oleksij Rempel (2):
  dt-bindings: net: dsa: microchip: Update ksz device tree bindings for
    drive strength
  net: dsa: microchip: Add drive strength configuration

 .../bindings/net/dsa/microchip,ksz.yaml       |  23 ++
 drivers/net/dsa/microchip/ksz8795_reg.h       |  14 -
 drivers/net/dsa/microchip/ksz9477_reg.h       |  13 -
 drivers/net/dsa/microchip/ksz_common.c        | 286 ++++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.h        |  20 ++
 5 files changed, 329 insertions(+), 27 deletions(-)

Comments

Andrew Lunn Sept. 6, 2023, 12:04 p.m. UTC | #1
On Wed, Sep 06, 2023 at 12:59:04PM +0200, Oleksij Rempel wrote:
> Add device tree based drive strength configuration support. It is needed to
> pass EMI validation on our hardware.
> 
> Configuration values are based on the vendor's reference driver.
> 
> Tested on KSZ9563R.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Russell King (Oracle) Sept. 6, 2023, 4:36 p.m. UTC | #2
On Wed, Sep 06, 2023 at 12:59:04PM +0200, Oleksij Rempel wrote:
> +static void ksz9477_drive_strength_error(struct ksz_device *dev, int milliamp)
> +{
> +	size_t array_size = ARRAY_SIZE(ksz9477_drive_strengths);
> +	char supported_values[100];
> +	int i;
> +
> +	for (i = 0; i < array_size; i++) {
> +		if (i == 0)
> +			snprintf(supported_values, sizeof(supported_values),
> +				 "%d", ksz9477_drive_strengths[i].milliamp);
> +		else
> +			snprintf(supported_values, sizeof(supported_values),
> +				 "%s, %d", supported_values,
> +				 ksz9477_drive_strengths[i].milliamp);

That's an interesting way to append... I note that snprintf(3) has a
note about this, suggesting that (a) the standards make this undefined
and (b) that depending on the gcc version used, this may not produce
the expected results. Taking both together seems sufficient
justification to stay away from attempting this method of appending
a string.

> +static int ksz9477_drive_strength_write(struct ksz_device *dev,
> +					struct ksz_driver_strength_prop *props,
> +					int num_props)
> +{
> +	int i, ret, reg;
> +	u8 val;
	u8 val, mask;

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

> +	ret = ksz_read8(dev, reg, &val);
> +	if (ret)
> +		return ret;
> +
Remote this.

	val = mask = 0;

> +	for (i = 0; i < num_props; i++) {
> +		if (props[i].value == -1)
> +			continue;
> +
> +		ret = ksz9477_drive_strength_to_reg(props[i].value);
> +		if (ret < 0) {
> +			ksz9477_drive_strength_error(dev, props[i].value);
> +			return ret;
> +		}
> +
> +		val &= ~(SW_DRIVE_STRENGTH_M << props[i].offset);

		mask |= SW_DRIVE_STRENGTH_M << props[i].offset;

> +		val |= ret << props[i].offset;

		val |= ret << props[i].offset;

> +	}
> +
> +	return ksz_write8(dev, reg, val);

	return ksz_rmw8(dev, reg, mask, val);

maybe safer?
Woojung.Huh@microchip.com Sept. 7, 2023, 5:35 p.m. UTC | #3
Hi Oleksij,

Thanks for the series.
Microchip can confirm that KSZ Switch series of KSZ9477, KSZ9567, KSZ8567, KSZ9897, KSZ9896, KSZ9563, KSZ9893 and KSZ8563 is using same register (drive strength) settings even though not documented.

Best regards,
Woojung


> -----Original Message-----
> From: Oleksij Rempel <o.rempel@pengutronix.de>
> Sent: Wednesday, September 6, 2023 6:59 AM
> 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
> - C21699 <Woojung.Huh@microchip.com>; Arun Ramadoss - I17769
> <Arun.Ramadoss@microchip.com>; Conor Dooley <conor+dt@kernel.org>;
> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Rob Herring
> <robh+dt@kernel.org>
> Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de;
> linux-kernel@vger.kernel.org; netdev@vger.kernel.org; UNGLinuxDriver
> <UNGLinuxDriver@microchip.com>; Russell King (Oracle)
> <linux@armlinux.org.uk>; devicetree@vger.kernel.org
> Subject: [RFC net-next v2 0/2] net: dsa: microchip: add drive strength support
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> changes v2:
> - make it work on all know KSZÜ variants except of undocumented LAN*
>   switches
> - add io-drive-strength compatible for ksz88xx chips
> - test exact drive strength instead of nearest closest.
> - add comment and refactor the code
> 
> Oleksij Rempel (2):
>   dt-bindings: net: dsa: microchip: Update ksz device tree bindings for
>     drive strength
>   net: dsa: microchip: Add drive strength configuration
> 
>  .../bindings/net/dsa/microchip,ksz.yaml       |  23 ++
>  drivers/net/dsa/microchip/ksz8795_reg.h       |  14 -
>  drivers/net/dsa/microchip/ksz9477_reg.h       |  13 -
>  drivers/net/dsa/microchip/ksz_common.c        | 286 ++++++++++++++++++
>  drivers/net/dsa/microchip/ksz_common.h        |  20 ++
>  5 files changed, 329 insertions(+), 27 deletions(-)
> 
> --
> 2.39.2
>