Skip to content

Commit

Permalink
hwmon: (ucd90320) Add minimum delay between bus accesses
Browse files Browse the repository at this point in the history
When probing the ucd90320 access to some of the registers randomly fails.
Sometimes it NACKs a transfer, sometimes it returns just random data and
the PEC check fails.

Experimentation shows that this seems to be triggered by a register access
directly back to back with a previous register write. Experimentation also
shows that inserting a small delay after register writes makes the issue go
away.

Use a similar solution to what the max15301 driver does to solve the same
problem. Create a custom set of bus read and write functions that make sure
that the delay is added.

Fixes: a470f11 ("hwmon: (pmbus/ucd9000) Add support for UCD90320 Power Sequencer")
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Link: https://lore.kernel.org/r/20230312160312.2227405-1-lars@metafoo.de
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
  • Loading branch information
larsclausen authored and groeck committed Mar 12, 2023
1 parent c93f5e2 commit 8d655e6
Showing 1 changed file with 75 additions and 0 deletions.
75 changes: 75 additions & 0 deletions drivers/hwmon/pmbus/ucd9000.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/of_device.h>
Expand All @@ -16,6 +17,7 @@
#include <linux/i2c.h>
#include <linux/pmbus.h>
#include <linux/gpio/driver.h>
#include <linux/timekeeping.h>
#include "pmbus.h"

enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd90320, ucd9090,
Expand Down Expand Up @@ -65,6 +67,7 @@ struct ucd9000_data {
struct gpio_chip gpio;
#endif
struct dentry *debugfs;
ktime_t write_time;
};
#define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, info)

Expand All @@ -73,6 +76,73 @@ struct ucd9000_debugfs_entry {
u8 index;
};

/*
* It has been observed that the UCD90320 randomly fails register access when
* doing another access right on the back of a register write. To mitigate this
* make sure that there is a minimum delay between a write access and the
* following access. The 250us is based on experimental data. At a delay of
* 200us the issue seems to go away. Add a bit of extra margin to allow for
* system to system differences.
*/
#define UCD90320_WAIT_DELAY_US 250

static inline void ucd90320_wait(const struct ucd9000_data *data)
{
s64 delta = ktime_us_delta(ktime_get(), data->write_time);

if (delta < UCD90320_WAIT_DELAY_US)
udelay(UCD90320_WAIT_DELAY_US - delta);
}

static int ucd90320_read_word_data(struct i2c_client *client, int page,
int phase, int reg)
{
const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
struct ucd9000_data *data = to_ucd9000_data(info);

if (reg >= PMBUS_VIRT_BASE)
return -ENXIO;

ucd90320_wait(data);
return pmbus_read_word_data(client, page, phase, reg);
}

static int ucd90320_read_byte_data(struct i2c_client *client, int page, int reg)
{
const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
struct ucd9000_data *data = to_ucd9000_data(info);

ucd90320_wait(data);
return pmbus_read_byte_data(client, page, reg);
}

static int ucd90320_write_word_data(struct i2c_client *client, int page,
int reg, u16 word)
{
const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
struct ucd9000_data *data = to_ucd9000_data(info);
int ret;

ucd90320_wait(data);
ret = pmbus_write_word_data(client, page, reg, word);
data->write_time = ktime_get();

return ret;
}

static int ucd90320_write_byte(struct i2c_client *client, int page, u8 value)
{
const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
struct ucd9000_data *data = to_ucd9000_data(info);
int ret;

ucd90320_wait(data);
ret = pmbus_write_byte(client, page, value);
data->write_time = ktime_get();

return ret;
}

static int ucd9000_get_fan_config(struct i2c_client *client, int fan)
{
int fan_config = 0;
Expand Down Expand Up @@ -598,6 +668,11 @@ static int ucd9000_probe(struct i2c_client *client)
info->read_byte_data = ucd9000_read_byte_data;
info->func[0] |= PMBUS_HAVE_FAN12 | PMBUS_HAVE_STATUS_FAN12
| PMBUS_HAVE_FAN34 | PMBUS_HAVE_STATUS_FAN34;
} else if (mid->driver_data == ucd90320) {
info->read_byte_data = ucd90320_read_byte_data;
info->read_word_data = ucd90320_read_word_data;
info->write_byte = ucd90320_write_byte;
info->write_word_data = ucd90320_write_word_data;
}

ucd9000_probe_gpio(client, mid, data);
Expand Down

0 comments on commit 8d655e6

Please sign in to comment.