On Mon, 14 Sep 2020, at 23:44, Guenter Roeck wrote: > 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. Ah, this kind of info is what I was hoping for. Thanks. > > > 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. Yeah, 250us was what we came to after 5 minutes of playing with values and a logic analyzer, we didn't really take the time to determine a minimum with confidence. TI mentioned the minimum time between transfers in their test environment is 2.5ms, so 1ms is aggressive by comparison. > > > +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. Certainly agree with the latter, and regarding the former, it was mostly a convenient mechanism for me to experiment with values. I agree that it's not something we would want to be changed arbitrarily by a system admin. > I would suggest an implementation similar to other > affected devices; again, see zl6100.c or ltc2978.c for examples. Yes, thanks for these pointers, I will take a look. Andrew
