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