Re: [RFC net-next v2 2/2] net: dsa: microchip: Add drive strength configuration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?


-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux