On 9/14/20 5:28 AM, Andrew Jeffery wrote:
> Short turn-around times between transfers to e.g. the UCD90320 can lead
> to problematic behaviour, including excessive clock stretching, bus
> lockups and potential corruption of the device's volatile state.
>
> Introduce transfer throttling for the device with a minimum access
> delay of 1ms.
>
Some Zilker labs devices have the same problem, though not as bad
to need a 1ms delay. See zl6100.c. Various LTS devices have a similar
problem, but there it is possible to poll the device until it is ready.
See ltc2978.c.
> Signed-off-by: Andrew Jeffery <andrew@xxxxxxxx>
> ---
> drivers/hwmon/pmbus/ucd9000.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c
> index 81f4c4f166cd..a0b97d035326 100644
> --- a/drivers/hwmon/pmbus/ucd9000.c
> +++ b/drivers/hwmon/pmbus/ucd9000.c
> @@ -9,6 +9,7 @@
> #include <linux/debugfs.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/moduleparam.h>
> #include <linux/of_device.h>
> #include <linux/init.h>
> #include <linux/err.h>
> @@ -18,6 +19,9 @@
> #include <linux/gpio/driver.h>
> #include "pmbus.h"
>
> +static unsigned long smbus_delay_us = 1000;
Is that to be on the super-safe side ? Patch 0 talks about needing 250 uS.
> +module_param(smbus_delay_us, ulong, 0664);
> +
I would not want to have this in user control, and it should not affect devices
not known to be affected. I would suggest an implementation similar to other
affected devices; again, see zl6100.c or ltc2978.c for examples.
Thanks,
Guenter
> enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd90320, ucd9090,
> ucd90910 };
>
> @@ -502,6 +506,8 @@ static int ucd9000_probe(struct i2c_client *client,
> I2C_FUNC_SMBUS_BLOCK_DATA))
> return -ENODEV;
>
> + i2c_smbus_throttle_client(client, smbus_delay_us);
> +
> ret = i2c_smbus_read_block_data(client, UCD9000_DEVICE_ID,
> block_buffer);
> if (ret < 0) {
>
