Commit 205e0c05 authored by Lakshmi Yadlapati's avatar Lakshmi Yadlapati Committed by Guenter Roeck
Browse files

hwmon: (pmbus/max31785) Add delay between bus accesses 



The MAX31785 has shown erratic behaviour across multiple system
designs, unexpectedly clock stretching and NAKing transactions.

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.

Signed-off-by: default avatarLakshmi Yadlapati <lakshmiy@us.ibm.com>
Link: https://lore.kernel.org/r/20231027044346.2167548-1-lakshmiy@us.ibm.com


Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
parent 2358151b

drivers/hwmon/pmbus/max31785.c

+167 −21
Original line number Diff line number Diff line
@@ -3,6 +3,7 @@ 
 * Copyright (C) 2017 IBM Corp.

 */



#include <linux/delay.h>

#include <linux/kernel.h>

#include <linux/module.h>

#include <linux/init.h>
@@ -23,18 +24,118 @@ enum max31785_regs { 


#define MAX31785_NR_PAGES		23

#define MAX31785_NR_FAN_PAGES		6

#define MAX31785_WAIT_DELAY_US		250



static int max31785_read_byte_data(struct i2c_client *client, int page,

				   int reg)

struct max31785_data {

	ktime_t access;			/* Chip access time */

	struct pmbus_driver_info info;

};



#define to_max31785_data(x)  container_of(x, struct max31785_data, info)



/*

 * MAX31785 Driver Workaround

 *

 * The MAX31785 fan controller occasionally exhibits communication issues.

 * These issues are not indicated by the device itself, except for occasional

 * NACK responses during master transactions. No error bits are set in STATUS_BYTE.

 *

 * To address this, we introduce a delay of 250us between consecutive accesses

 * to the fan controller. This delay helps mitigate communication problems by

 * allowing sufficient time between accesses.

 */

static inline void max31785_wait(const struct max31785_data *data)

{

	if (page < MAX31785_NR_PAGES)

		return -ENODATA;

	s64 delta = ktime_us_delta(ktime_get(), data->access);



	if (delta < MAX31785_WAIT_DELAY_US)

		usleep_range(MAX31785_WAIT_DELAY_US - delta,

			     MAX31785_WAIT_DELAY_US);

}



static int max31785_i2c_write_byte_data(struct i2c_client *client,

					struct max31785_data *driver_data,

					int command, u16 data)

{

	int rc;



	max31785_wait(driver_data);

	rc = i2c_smbus_write_byte_data(client, command, data);

	driver_data->access = ktime_get();

	return rc;

}



static int max31785_i2c_read_word_data(struct i2c_client *client,

				       struct max31785_data *driver_data,

				       int command)

{

	int rc;



	max31785_wait(driver_data);

	rc = i2c_smbus_read_word_data(client, command);

	driver_data->access = ktime_get();

	return rc;

}



static int _max31785_read_byte_data(struct i2c_client *client,

				    struct max31785_data *driver_data,

				    int page, int command)

{

	int rc;



	max31785_wait(driver_data);

	rc = pmbus_read_byte_data(client, page, command);

	driver_data->access = ktime_get();

	return rc;

}



static int _max31785_write_byte_data(struct i2c_client *client,

				     struct max31785_data *driver_data,

				     int page, int command, u16 data)

{

	int rc;



	max31785_wait(driver_data);

	rc = pmbus_write_byte_data(client, page, command, data);

	driver_data->access = ktime_get();

	return rc;

}



static int _max31785_read_word_data(struct i2c_client *client,

				    struct max31785_data *driver_data,

				    int page, int phase, int command)

{

	int rc;



	max31785_wait(driver_data);

	rc = pmbus_read_word_data(client, page, phase, command);

	driver_data->access = ktime_get();

	return rc;

}



static int _max31785_write_word_data(struct i2c_client *client,

				     struct max31785_data *driver_data,

				     int page, int command, u16 data)

{

	int rc;



	max31785_wait(driver_data);

	rc = pmbus_write_word_data(client, page, command, data);

	driver_data->access = ktime_get();

	return rc;

}



static int max31785_read_byte_data(struct i2c_client *client, int page, int reg)

{

	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);

	struct max31785_data *driver_data = to_max31785_data(info);



	switch (reg) {

	case PMBUS_VOUT_MODE:

		return -ENOTSUPP;

	case PMBUS_FAN_CONFIG_12:

		return pmbus_read_byte_data(client, page - MAX31785_NR_PAGES,

		return _max31785_read_byte_data(client, driver_data,

						page - MAX31785_NR_PAGES,

						reg);

	}
@@ -102,16 +203,19 @@ static int max31785_get_pwm(struct i2c_client *client, int page) 
	return rv;

}



static int max31785_get_pwm_mode(struct i2c_client *client, int page)

static int max31785_get_pwm_mode(struct i2c_client *client,

				 struct max31785_data *driver_data, int page)

{

	int config;

	int command;



	config = pmbus_read_byte_data(client, page, PMBUS_FAN_CONFIG_12);

	config = _max31785_read_byte_data(client, driver_data, page,

					  PMBUS_FAN_CONFIG_12);

	if (config < 0)

		return config;



	command = pmbus_read_word_data(client, page, 0xff, PMBUS_FAN_COMMAND_1);

	command = _max31785_read_word_data(client, driver_data, page, 0xff,

					   PMBUS_FAN_COMMAND_1);

	if (command < 0)

		return command;
@@ -129,6 +233,8 @@ static int max31785_get_pwm_mode(struct i2c_client *client, int page) 
static int max31785_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 max31785_data *driver_data = to_max31785_data(info);

	u32 val;

	int rv;
@@ -157,7 +263,7 @@ static int max31785_read_word_data(struct i2c_client *client, int page, 
		rv = max31785_get_pwm(client, page);

		break;

	case PMBUS_VIRT_PWM_ENABLE_1:

		rv = max31785_get_pwm_mode(client, page);

		rv = max31785_get_pwm_mode(client, driver_data, page);

		break;

	default:

		rv = -ENODATA;
@@ -188,7 +294,35 @@ static inline u32 max31785_scale_pwm(u32 sensor_val) 
	return (sensor_val * 100) / 255;

}



static int max31785_pwm_enable(struct i2c_client *client, int page,

static int max31785_update_fan(struct i2c_client *client,

			       struct max31785_data *driver_data, int page,

			       u8 config, u8 mask, u16 command)

{

	int from, rv;

	u8 to;



	from = _max31785_read_byte_data(client, driver_data, page,

					PMBUS_FAN_CONFIG_12);

	if (from < 0)

		return from;



	to = (from & ~mask) | (config & mask);



	if (to != from) {

		rv = _max31785_write_byte_data(client, driver_data, page,

					       PMBUS_FAN_CONFIG_12, to);

		if (rv < 0)

			return rv;

	}



	rv = _max31785_write_word_data(client, driver_data, page,

				       PMBUS_FAN_COMMAND_1, command);



	return rv;

}



static int max31785_pwm_enable(struct i2c_client *client,

			       struct max31785_data *driver_data, int page,

			       u16 word)

{

	int config = 0;
@@ -217,18 +351,23 @@ static int max31785_pwm_enable(struct i2c_client *client, int page, 
		return -EINVAL;

	}



	return pmbus_update_fan(client, page, 0, config, PB_FAN_1_RPM, rate);

	return max31785_update_fan(client, driver_data, page, config,

				   PB_FAN_1_RPM, rate);

}



static int max31785_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 max31785_data *driver_data = to_max31785_data(info);



	switch (reg) {

	case PMBUS_VIRT_PWM_1:

		return pmbus_update_fan(client, page, 0, 0, PB_FAN_1_RPM,

		return max31785_update_fan(client, driver_data, page, 0,

					   PB_FAN_1_RPM,

					   max31785_scale_pwm(word));

	case PMBUS_VIRT_PWM_ENABLE_1:

		return max31785_pwm_enable(client, page, word);

		return max31785_pwm_enable(client, driver_data, page, word);

	default:

		break;

	}
@@ -303,13 +442,16 @@ static int max31785_configure_dual_tach(struct i2c_client *client, 
{

	int ret;

	int i;

	struct max31785_data *driver_data = to_max31785_data(info);



	for (i = 0; i < MAX31785_NR_FAN_PAGES; i++) {

		ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);

		ret = max31785_i2c_write_byte_data(client, driver_data,

						   PMBUS_PAGE, i);

		if (ret < 0)

			return ret;



		ret = i2c_smbus_read_word_data(client, MFR_FAN_CONFIG);

		ret = max31785_i2c_read_word_data(client, driver_data,

						  MFR_FAN_CONFIG);

		if (ret < 0)

			return ret;
@@ -329,6 +471,7 @@ static int max31785_probe(struct i2c_client *client) 
{

	struct device *dev = &client->dev;

	struct pmbus_driver_info *info;

	struct max31785_data *driver_data;

	bool dual_tach = false;

	int ret;
@@ -337,13 +480,16 @@ static int max31785_probe(struct i2c_client *client) 
				     I2C_FUNC_SMBUS_WORD_DATA))

		return -ENODEV;



	info = devm_kzalloc(dev, sizeof(struct pmbus_driver_info), GFP_KERNEL);

	if (!info)

	driver_data = devm_kzalloc(dev, sizeof(struct max31785_data), GFP_KERNEL);

	if (!driver_data)

		return -ENOMEM;



	info = &driver_data->info;

	driver_data->access = ktime_get();

	*info = max31785_info;



	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 255);

	ret = max31785_i2c_write_byte_data(client, driver_data,

					   PMBUS_PAGE, 255);

	if (ret < 0)

		return ret;