* [PATCH linux dev-5.10 01/14] i2c: Allow throttling of transfers to client devices
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 02/14] pmbus: (ucd9000) Throttle SMBus transfers to avoid poor behaviour Eddie James
` (12 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Andrew Jeffery
From: Andrew Jeffery <andrew@aj.id.au>
Some devices fail to cope in disastrous ways with the small command
turn-around times enabled by in-kernel device drivers.
Introduce per-client throttling of transfers to avoid these issues.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/i2c/i2c-core-base.c | 8 +-
drivers/i2c/i2c-core-smbus.c | 169 +++++++++++++++++++++++++++++------
drivers/i2c/i2c-core.h | 21 +++++
include/linux/i2c.h | 5 ++
4 files changed, 172 insertions(+), 31 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index bdce6d3e5327..bf35710d0407 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -871,13 +871,17 @@ int i2c_dev_irq_from_resources(const struct resource *resources,
struct i2c_client *
i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
{
+ struct i2c_client_priv *priv;
struct i2c_client *client;
int status;
- client = kzalloc(sizeof *client, GFP_KERNEL);
- if (!client)
+ priv = kzalloc(sizeof *priv, GFP_KERNEL);
+ if (!priv)
return ERR_PTR(-ENOMEM);
+ mutex_init(&priv->throttle_lock);
+ client = &priv->client;
+
client->adapter = adap;
client->dev.platform_data = info->platform_data;
diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index f5c9787992e9..1dde58c8a387 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -10,6 +10,7 @@
* SMBus 2.0 support by Mark Studebaker <mdsxyz123@yahoo.com> and
* Jean Delvare <jdelvare@suse.de>
*/
+#include <linux/delay.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/i2c.h>
@@ -21,6 +22,9 @@
#define CREATE_TRACE_POINTS
#include <trace/events/smbus.h>
+static s32 i2c_smbus_throttle_xfer(const struct i2c_client *client,
+ char read_write, u8 command, int protocol,
+ union i2c_smbus_data *data);
/* The SMBus parts */
@@ -95,9 +99,9 @@ s32 i2c_smbus_read_byte(const struct i2c_client *client)
union i2c_smbus_data data;
int status;
- status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_READ, 0,
- I2C_SMBUS_BYTE, &data);
+ status = i2c_smbus_throttle_xfer(client, I2C_SMBUS_READ, 0,
+ I2C_SMBUS_BYTE, &data);
+
return (status < 0) ? status : data.byte;
}
EXPORT_SYMBOL(i2c_smbus_read_byte);
@@ -112,8 +116,8 @@ EXPORT_SYMBOL(i2c_smbus_read_byte);
*/
s32 i2c_smbus_write_byte(const struct i2c_client *client, u8 value)
{
- return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_WRITE, value, I2C_SMBUS_BYTE, NULL);
+ return i2c_smbus_throttle_xfer(client, I2C_SMBUS_WRITE, value,
+ I2C_SMBUS_BYTE, NULL);
}
EXPORT_SYMBOL(i2c_smbus_write_byte);
@@ -130,9 +134,9 @@ s32 i2c_smbus_read_byte_data(const struct i2c_client *client, u8 command)
union i2c_smbus_data data;
int status;
- status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_READ, command,
- I2C_SMBUS_BYTE_DATA, &data);
+ status = i2c_smbus_throttle_xfer(client, I2C_SMBUS_READ, command,
+ I2C_SMBUS_BYTE_DATA, &data);
+
return (status < 0) ? status : data.byte;
}
EXPORT_SYMBOL(i2c_smbus_read_byte_data);
@@ -150,10 +154,10 @@ s32 i2c_smbus_write_byte_data(const struct i2c_client *client, u8 command,
u8 value)
{
union i2c_smbus_data data;
+
data.byte = value;
- return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_WRITE, command,
- I2C_SMBUS_BYTE_DATA, &data);
+ return i2c_smbus_throttle_xfer(client, I2C_SMBUS_WRITE, command,
+ I2C_SMBUS_BYTE_DATA, &data);
}
EXPORT_SYMBOL(i2c_smbus_write_byte_data);
@@ -170,9 +174,9 @@ s32 i2c_smbus_read_word_data(const struct i2c_client *client, u8 command)
union i2c_smbus_data data;
int status;
- status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_READ, command,
- I2C_SMBUS_WORD_DATA, &data);
+ status = i2c_smbus_throttle_xfer(client, I2C_SMBUS_READ, command,
+ I2C_SMBUS_WORD_DATA, &data);
+
return (status < 0) ? status : data.word;
}
EXPORT_SYMBOL(i2c_smbus_read_word_data);
@@ -190,10 +194,10 @@ s32 i2c_smbus_write_word_data(const struct i2c_client *client, u8 command,
u16 value)
{
union i2c_smbus_data data;
+
data.word = value;
- return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_WRITE, command,
- I2C_SMBUS_WORD_DATA, &data);
+ return i2c_smbus_throttle_xfer(client, I2C_SMBUS_WRITE, command,
+ I2C_SMBUS_WORD_DATA, &data);
}
EXPORT_SYMBOL(i2c_smbus_write_word_data);
@@ -218,9 +222,9 @@ s32 i2c_smbus_read_block_data(const struct i2c_client *client, u8 command,
union i2c_smbus_data data;
int status;
- status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_READ, command,
- I2C_SMBUS_BLOCK_DATA, &data);
+ status = i2c_smbus_throttle_xfer(client, I2C_SMBUS_READ, command,
+ I2C_SMBUS_BLOCK_DATA, &data);
+
if (status)
return status;
@@ -248,9 +252,8 @@ s32 i2c_smbus_write_block_data(const struct i2c_client *client, u8 command,
length = I2C_SMBUS_BLOCK_MAX;
data.block[0] = length;
memcpy(&data.block[1], values, length);
- return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_WRITE, command,
- I2C_SMBUS_BLOCK_DATA, &data);
+ return i2c_smbus_throttle_xfer(client, I2C_SMBUS_WRITE, command,
+ I2C_SMBUS_BLOCK_DATA, &data);
}
EXPORT_SYMBOL(i2c_smbus_write_block_data);
@@ -264,9 +267,9 @@ s32 i2c_smbus_read_i2c_block_data(const struct i2c_client *client, u8 command,
if (length > I2C_SMBUS_BLOCK_MAX)
length = I2C_SMBUS_BLOCK_MAX;
data.block[0] = length;
- status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_READ, command,
- I2C_SMBUS_I2C_BLOCK_DATA, &data);
+ status = i2c_smbus_throttle_xfer(client, I2C_SMBUS_READ, command,
+ I2C_SMBUS_I2C_BLOCK_DATA, &data);
+
if (status < 0)
return status;
@@ -284,9 +287,8 @@ s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client, u8 command,
length = I2C_SMBUS_BLOCK_MAX;
data.block[0] = length;
memcpy(data.block + 1, values, length);
- return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_WRITE, command,
- I2C_SMBUS_I2C_BLOCK_DATA, &data);
+ return i2c_smbus_throttle_xfer(client, I2C_SMBUS_WRITE, command,
+ I2C_SMBUS_I2C_BLOCK_DATA, &data);
}
EXPORT_SYMBOL(i2c_smbus_write_i2c_block_data);
@@ -547,6 +549,71 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
}
EXPORT_SYMBOL(i2c_smbus_xfer);
+static int i2c_smbus_throttle_enter(const struct i2c_client *client)
+ __acquires(&priv->throttle_lock)
+{
+ struct i2c_client_priv *priv;
+ ktime_t earliest;
+ int rc;
+
+ priv = to_i2c_client_priv(client);
+
+ if (i2c_in_atomic_xfer_mode()) {
+ if (!mutex_trylock(&priv->throttle_lock))
+ return -EAGAIN;
+ } else {
+ rc = mutex_lock_interruptible(&priv->throttle_lock);
+ if (rc)
+ return rc;
+ }
+ earliest = ktime_add_us(priv->last, priv->delay_us);
+
+ if (priv->delay_us && ktime_before(ktime_get(), earliest)) {
+ if (i2c_in_atomic_xfer_mode()) {
+ mutex_unlock(&priv->throttle_lock);
+ return -EAGAIN;
+ }
+
+ usleep_range(priv->delay_us, 2 * priv->delay_us);
+ }
+
+ return 0;
+}
+
+static void i2c_smbus_throttle_exit(const struct i2c_client *client)
+ __releases(&priv->throttle_lock)
+{
+ struct i2c_client_priv *priv;
+
+ priv = to_i2c_client_priv(client);
+
+ if (priv->delay_us)
+ priv->last = ktime_get();
+ mutex_unlock(&priv->throttle_lock);
+}
+
+static s32 i2c_smbus_throttle_xfer(const struct i2c_client *client,
+ char read_write, u8 command, int protocol,
+ union i2c_smbus_data *data)
+{
+ s32 res;
+
+ res = i2c_smbus_throttle_enter(client);
+ if (res)
+ return res;
+
+ res = __i2c_lock_bus_helper(client->adapter);
+ if (!res)
+ res = __i2c_smbus_xfer(client->adapter, client->addr,
+ client->flags, read_write, command,
+ protocol, data);
+ i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
+
+ i2c_smbus_throttle_exit(client);
+
+ return res;
+}
+
s32 __i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
unsigned short flags, char read_write,
u8 command, int protocol, union i2c_smbus_data *data)
@@ -715,3 +782,47 @@ int of_i2c_setup_smbus_alert(struct i2c_adapter *adapter)
}
EXPORT_SYMBOL_GPL(of_i2c_setup_smbus_alert);
#endif
+
+int i2c_smbus_throttle_client(struct i2c_client *client,
+ unsigned long delay_us)
+{
+ struct i2c_client_priv *priv;
+ int rc;
+
+ priv = to_i2c_client_priv(client);
+
+ if (i2c_in_atomic_xfer_mode()) {
+ if (!mutex_trylock(&priv->throttle_lock))
+ return -EAGAIN;
+ } else {
+ rc = mutex_lock_interruptible(&priv->throttle_lock);
+ if (rc)
+ return rc;
+ }
+ priv->delay_us = delay_us;
+ priv->last = ktime_get();
+ mutex_unlock(&priv->throttle_lock);
+
+ return 0;
+}
+
+int i2c_smbus_throttle_value(struct i2c_client *client, unsigned long *delay_us)
+{
+ struct i2c_client_priv *priv;
+ int rc;
+
+ priv = to_i2c_client_priv(client);
+
+ if (i2c_in_atomic_xfer_mode()) {
+ if (!mutex_trylock(&priv->throttle_lock))
+ return -EAGAIN;
+ } else {
+ rc = mutex_lock_interruptible(&priv->throttle_lock);
+ if (rc)
+ return rc;
+ }
+ *delay_us = priv->delay_us;
+ mutex_unlock(&priv->throttle_lock);
+
+ return 0;
+}
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
index 8ce261167a2d..e916624c7b98 100644
--- a/drivers/i2c/i2c-core.h
+++ b/drivers/i2c/i2c-core.h
@@ -4,6 +4,27 @@
*/
#include <linux/rwsem.h>
+#include <linux/i2c.h>
+#include <linux/timekeeping.h>
+#include <linux/kernel.h>
+#include <linux/mutex.h>
+
+struct i2c_client_priv {
+ struct i2c_client client;
+
+ /*
+ * Per-client access throttling, described in terms of microsecond
+ * delay between the end of the nth transfer and the start of the
+ * (n+1)th transfer
+ *
+ * Do it in a wrapper struct to preserve const-ness of the i2c_smbus_*
+ * interfaces.
+ */
+ struct mutex throttle_lock;
+ unsigned long delay_us;
+ ktime_t last;
+};
+#define to_i2c_client_priv(c) container_of(c, struct i2c_client_priv, client)
struct i2c_devinfo {
struct list_head list;
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index a670ae129f4b..89dbe147040f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -181,8 +181,13 @@ s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client,
s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
u8 command, u8 length,
u8 *values);
+int i2c_smbus_throttle_client(struct i2c_client *client,
+ unsigned long delay_us);
+int i2c_smbus_throttle_value(struct i2c_client *client,
+ unsigned long *delay_us);
int i2c_get_device_id(const struct i2c_client *client,
struct i2c_device_identity *id);
+
#endif /* I2C */
/**
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH linux dev-5.10 02/14] pmbus: (ucd9000) Throttle SMBus transfers to avoid poor behaviour
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 01/14] i2c: Allow throttling of transfers to client devices Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 03/14] ucd9000: Add a throttle delay attribute in debugfs Eddie James
` (11 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Andrew Jeffery
From: Andrew Jeffery <andrew@aj.id.au>
Short turn-around times between transfers to UCD9000 devices can lead to
problematic behaviour, including unnecessary 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.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/pmbus/ucd9000.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c
index a15e6fe3e425..9687bc3609d1 100644
--- a/drivers/hwmon/pmbus/ucd9000.c
+++ b/drivers/hwmon/pmbus/ucd9000.c
@@ -487,6 +487,8 @@ static int ucd9000_init_debugfs(struct i2c_client *client,
}
#endif /* CONFIG_DEBUG_FS */
+#define UCD9000_SMBUS_THROTTLE_US 1000
+
static int ucd9000_probe(struct i2c_client *client)
{
u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1];
@@ -501,6 +503,8 @@ static int ucd9000_probe(struct i2c_client *client)
I2C_FUNC_SMBUS_BLOCK_DATA))
return -ENODEV;
+ i2c_smbus_throttle_client(client, UCD9000_SMBUS_THROTTLE_US);
+
ret = i2c_smbus_read_block_data(client, UCD9000_DEVICE_ID,
block_buffer);
if (ret < 0) {
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH linux dev-5.10 03/14] ucd9000: Add a throttle delay attribute in debugfs
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 01/14] i2c: Allow throttling of transfers to client devices Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 02/14] pmbus: (ucd9000) Throttle SMBus transfers to avoid poor behaviour Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 04/14] fsi: run clock at 100MHz Eddie James
` (10 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Andrew Jeffery
From: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/pmbus/ucd9000.c | 37 ++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c
index 9687bc3609d1..cbfafebf1ded 100644
--- a/drivers/hwmon/pmbus/ucd9000.c
+++ b/drivers/hwmon/pmbus/ucd9000.c
@@ -401,6 +401,37 @@ static int ucd9000_debugfs_show_mfr_status_bit(void *data, u64 *val)
DEFINE_DEBUGFS_ATTRIBUTE(ucd9000_debugfs_mfr_status_bit,
ucd9000_debugfs_show_mfr_status_bit, NULL, "%1lld\n");
+#define UCD9000_SMBUS_THROTTLE_US 1000
+static int throttle_delay_us = UCD9000_SMBUS_THROTTLE_US;
+
+static int ucd9000_debugfs_show_smbus_throttle_delay(void *data,
+ u64 *val)
+{
+ struct i2c_client *client = data;
+ unsigned long ulval;
+ int rc;
+
+ rc = i2c_smbus_throttle_value(client, &ulval);
+ if (rc)
+ return rc;
+
+ *val = ulval;
+
+ return 0;
+}
+
+static int ucd9000_debugfs_store_smbus_throttle_delay(void *data,
+ u64 val)
+{
+ struct i2c_client *client = data;
+
+ throttle_delay_us = val;
+ return i2c_smbus_throttle_client(client, val);
+}
+DEFINE_DEBUGFS_ATTRIBUTE(ucd9000_debugfs_smbus_throttle_delay,
+ ucd9000_debugfs_show_smbus_throttle_delay,
+ ucd9000_debugfs_store_smbus_throttle_delay, "%llu\n");
+
static ssize_t ucd9000_debugfs_read_mfr_status(struct file *file,
char __user *buf, size_t count,
loff_t *ppos)
@@ -475,6 +506,8 @@ static int ucd9000_init_debugfs(struct i2c_client *client,
scnprintf(name, UCD9000_DEBUGFS_NAME_LEN, "mfr_status");
debugfs_create_file(name, 0444, data->debugfs, client,
&ucd9000_debugfs_show_mfr_status_fops);
+ debugfs_create_file("smbus_throttle_delay", 0664, data->debugfs, client,
+ &ucd9000_debugfs_smbus_throttle_delay);
return 0;
}
@@ -503,7 +536,9 @@ static int ucd9000_probe(struct i2c_client *client)
I2C_FUNC_SMBUS_BLOCK_DATA))
return -ENODEV;
- i2c_smbus_throttle_client(client, UCD9000_SMBUS_THROTTLE_US);
+ ret = i2c_smbus_throttle_client(client, throttle_delay_us);
+ if (ret)
+ return ret;
ret = i2c_smbus_read_block_data(client, UCD9000_DEVICE_ID,
block_buffer);
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH linux dev-5.10 04/14] fsi: run clock at 100MHz
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
` (2 preceding siblings ...)
2021-08-11 15:42 ` [PATCH linux dev-5.10 03/14] ucd9000: Add a throttle delay attribute in debugfs Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 05/14] pmbus: (core) Add a one-shot retry in pmbus_set_page() Eddie James
` (9 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Andrew Geissler
From: Andrew Geissler <geisonator@yahoo.com>
This is a workaround requested by bringup team to see if some of the
instabilities being seen in the lab clear up.
This should not be upstreamed until the hardware team gets some more
data on the issue.
Signed-off-by: Andrew Geissler <geisonator@yahoo.com>
---
drivers/fsi/fsi-master-aspeed.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index 8606e55c1721..12d36ede6e4b 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -87,7 +87,8 @@ static const u32 fsi_base = 0xa0000000;
#define FSI_LINK_ENABLE_SETUP_TIME 10 /* in mS */
/* Run the bus at maximum speed by default */
-#define FSI_DIVISOR_DEFAULT 1
+/* TODO - Run at 100MHz for now to help hw bringup debug */
+#define FSI_DIVISOR_DEFAULT 2
#define FSI_DIVISOR_CABLED 2
static u16 aspeed_fsi_divisor = FSI_DIVISOR_DEFAULT;
module_param_named(bus_div,aspeed_fsi_divisor, ushort, 0);
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH linux dev-5.10 05/14] pmbus: (core) Add a one-shot retry in pmbus_set_page()
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
` (3 preceding siblings ...)
2021-08-11 15:42 ` [PATCH linux dev-5.10 04/14] fsi: run clock at 100MHz Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 06/14] pmbus: (max31785) Add a local pmbus_set_page() implementation Eddie James
` (8 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Andrew Jeffery
From: Andrew Jeffery <andrew@aj.id.au>
From extensive testing and tracing it was discovered that the MAX31785
occasionally fails to switch pages despite ACK'ing the PAGE PMBus data
write. I suspect this behaviour had been seen on other devices as well,
as pmbus_set_page() already read-back the freshly set value and errored
out if it wasn't what we requested.
In the case of the MAX31785 it was shown that a one-shot retry was
enough to get the PAGE write to stick if the inital command failed. To
improve robustness, only error out if the one-shot retry also fails to
stick.
OpenBMC-Staging-Count: 1
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/hwmon/pmbus/pmbus_core.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index def8be831456..1a13d6d3f8b5 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -151,25 +151,34 @@ int pmbus_set_page(struct i2c_client *client, int page, int phase)
if (!(data->info->func[page] & PMBUS_PAGE_VIRTUAL) &&
data->info->pages > 1 && page != data->currpage) {
+ int i;
+
dev_dbg(&client->dev, "Want page %u, %u cached\n", page,
data->currpage);
- rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
- if (rv < 0) {
+ for (i = 0; i < 2; i++) {
rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE,
page);
- dev_dbg(&client->dev,
- "Failed to set page %u, performed one-shot retry %s: %d\n",
- page, rv ? "and failed" : "with success", rv);
+ if (rv)
+ continue;
+
+ rv = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
if (rv < 0)
- return rv;
- }
+ continue;
- rv = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
- if (rv < 0)
- return rv;
+ /* Success, exit loop */
+ if (rv == page)
+ break;
+
+ rv = i2c_smbus_read_byte_data(client, PMBUS_STATUS_CML);
+ if (rv < 0)
+ continue;
+
+ if (rv & PB_CML_FAULT_INVALID_DATA)
+ return -EIO;
+ }
- if (rv != page)
+ if (i == 2)
return -EIO;
}
data->currpage = page;
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH linux dev-5.10 06/14] pmbus: (max31785) Add a local pmbus_set_page() implementation
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
` (4 preceding siblings ...)
2021-08-11 15:42 ` [PATCH linux dev-5.10 05/14] pmbus: (core) Add a one-shot retry in pmbus_set_page() Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 16:13 ` Matthew Barth
2021-08-11 15:42 ` [PATCH linux dev-5.10 07/14] pmbus: (max31785) Retry enabling fans after writing MFR_FAN_CONFIG Eddie James
` (7 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Andrew Jeffery
From: Andrew Jeffery <andrew@aj.id.au>
Extensive testing and tracing has shown that the MAX31785 is unreliable
in the face of PAGE write commands, ACK'ing the PAGE request but
reporting a value of 0 on some subsequent PAGE reads. The trace data
suggests that a one-shot retry of the PAGE write is enough to get the
requested value to stick.
As we configure the device before registering with the PMBus core,
centralise PAGE handling inside the driver and implement the one-shot
retry semantics there.
OpenBMC-Staging-Count: 1
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/hwmon/pmbus/max31785.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
index d097f72d4d47..7518fff356f9 100644
--- a/drivers/hwmon/pmbus/max31785.c
+++ b/drivers/hwmon/pmbus/max31785.c
@@ -362,6 +362,27 @@ static int max31785_write_word_data(struct i2c_client *client, int page,
return -ENXIO;
}
+static int max31785_pmbus_set_page(struct i2c_client *client, int page)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < 2; i++) {
+ ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
+ if (ret < 0)
+ return ret;
+
+ ret = max31785_i2c_smbus_read_byte_data(client, PMBUS_PAGE);
+ if (ret < 0)
+ return ret;
+
+ if (ret == page)
+ return 0;
+ }
+
+ return -EIO;
+}
+
/*
* Returns negative error codes if an unrecoverable problem is detected, 0 if a
* recoverable problem is detected, or a positive value on success.
@@ -392,7 +413,7 @@ static int max31785_of_fan_config(struct i2c_client *client,
return -ENXIO;
}
- ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
+ ret = max31785_pmbus_set_page(client, page);
if (ret < 0)
return ret;
@@ -599,7 +620,7 @@ static int max31785_of_tmp_config(struct i2c_client *client,
return -ENXIO;
}
- ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
+ ret = max31785_pmbus_set_page(client, page);
if (ret < 0)
return ret;
@@ -700,7 +721,7 @@ static int max31785_configure_dual_tach(struct i2c_client *client,
int i;
for (i = 0; i < MAX31785_NR_FAN_PAGES; i++) {
- ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);
+ ret = max31785_pmbus_set_page(client, i);
if (ret < 0)
return ret;
@@ -741,7 +762,7 @@ static int max31785_probe(struct i2c_client *client)
*info = max31785_info;
- ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, 255);
+ ret = max31785_pmbus_set_page(client, 255);
if (ret < 0)
return ret;
@@ -785,8 +806,7 @@ static int max31785_probe(struct i2c_client *client)
if (!have_fan || fan_configured)
continue;
- ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE,
- i);
+ ret = max31785_pmbus_set_page(client, i);
if (ret < 0)
return ret;
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH linux dev-5.10 06/14] pmbus: (max31785) Add a local pmbus_set_page() implementation
2021-08-11 15:42 ` [PATCH linux dev-5.10 06/14] pmbus: (max31785) Add a local pmbus_set_page() implementation Eddie James
@ 2021-08-11 16:13 ` Matthew Barth
0 siblings, 0 replies; 20+ messages in thread
From: Matthew Barth @ 2021-08-11 16:13 UTC (permalink / raw)
To: Eddie James, openbmc; +Cc: Andrew Jeffery
On 8/11/21 10:42 AM, Eddie James wrote:
> From: Andrew Jeffery <andrew@aj.id.au>
>
> Extensive testing and tracing has shown that the MAX31785 is unreliable
> in the face of PAGE write commands, ACK'ing the PAGE request but
> reporting a value of 0 on some subsequent PAGE reads. The trace data
> suggests that a one-shot retry of the PAGE write is enough to get the
> requested value to stick.
>
> As we configure the device before registering with the PMBus core,
> centralise PAGE handling inside the driver and implement the one-shot
> retry semantics there.
>
> OpenBMC-Staging-Count: 1
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Matthew Barth <msbarth@linux.ibm.com>
> ---
> drivers/hwmon/pmbus/max31785.c | 32 ++++++++++++++++++++++++++------
> 1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
> index d097f72d4d47..7518fff356f9 100644
> --- a/drivers/hwmon/pmbus/max31785.c
> +++ b/drivers/hwmon/pmbus/max31785.c
> @@ -362,6 +362,27 @@ static int max31785_write_word_data(struct i2c_client *client, int page,
> return -ENXIO;
> }
>
> +static int max31785_pmbus_set_page(struct i2c_client *client, int page)
> +{
> + int ret;
> + int i;
> +
> + for (i = 0; i < 2; i++) {
> + ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> + if (ret < 0)
> + return ret;
> +
> + ret = max31785_i2c_smbus_read_byte_data(client, PMBUS_PAGE);
> + if (ret < 0)
> + return ret;
> +
> + if (ret == page)
> + return 0;
> + }
> +
> + return -EIO;
> +}
> +
> /*
> * Returns negative error codes if an unrecoverable problem is detected, 0 if a
> * recoverable problem is detected, or a positive value on success.
> @@ -392,7 +413,7 @@ static int max31785_of_fan_config(struct i2c_client *client,
> return -ENXIO;
> }
>
> - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> + ret = max31785_pmbus_set_page(client, page);
> if (ret < 0)
> return ret;
>
> @@ -599,7 +620,7 @@ static int max31785_of_tmp_config(struct i2c_client *client,
> return -ENXIO;
> }
>
> - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> + ret = max31785_pmbus_set_page(client, page);
> if (ret < 0)
> return ret;
>
> @@ -700,7 +721,7 @@ static int max31785_configure_dual_tach(struct i2c_client *client,
> int i;
>
> for (i = 0; i < MAX31785_NR_FAN_PAGES; i++) {
> - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);
> + ret = max31785_pmbus_set_page(client, i);
> if (ret < 0)
> return ret;
>
> @@ -741,7 +762,7 @@ static int max31785_probe(struct i2c_client *client)
>
> *info = max31785_info;
>
> - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, 255);
> + ret = max31785_pmbus_set_page(client, 255);
> if (ret < 0)
> return ret;
>
> @@ -785,8 +806,7 @@ static int max31785_probe(struct i2c_client *client)
> if (!have_fan || fan_configured)
> continue;
>
> - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE,
> - i);
> + ret = max31785_pmbus_set_page(client, i);
> if (ret < 0)
> return ret;
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH linux dev-5.10 07/14] pmbus: (max31785) Retry enabling fans after writing MFR_FAN_CONFIG
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
` (5 preceding siblings ...)
2021-08-11 15:42 ` [PATCH linux dev-5.10 06/14] pmbus: (max31785) Add a local pmbus_set_page() implementation Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 16:14 ` Matthew Barth
2021-08-11 15:42 ` [PATCH linux dev-5.10 08/14] ARM: dts: aspeed: Rainier: Add fan controller properties Eddie James
` (6 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Andrew Jeffery
From: Andrew Jeffery <andrew@aj.id.au>
It has been observed across large fleets of systems that a small subset
of those systems occasionally loose control of some number of fans
across a BMC reboot (their hwmon fan attributes are missing from sysfs).
>From extensive testing and tracing it was discovered that writes
enabling a fan in FAN_CONFIG_1_2 failed to stick on the system under
test with a frequency of about 1 in 1000 re-binds of the driver.
The MAX31785 datasheet recommends in the documentation for
MFR_FAN_CONFIG that the asssociated fan(s) be disabled before updating
the register. The sequence in question implements this suggestion, and
the observed loss-of-fans symptom occurs when the write to re-enable the
fan in FAN_CONFIG_1_2 fails to stick.
The trace data suggests a one-shot retry is enough to successfully
update FAN_CONFIG_1_2. With the workaround, no loss of fans was observed
in over 20,000 consecutive rebinds of the driver.
OpenBMC-Staging-Count: 1
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/hwmon/pmbus/max31785.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
index 7518fff356f9..b37da2ec1ce4 100644
--- a/drivers/hwmon/pmbus/max31785.c
+++ b/drivers/hwmon/pmbus/max31785.c
@@ -398,6 +398,7 @@ static int max31785_of_fan_config(struct i2c_client *client,
u32 page;
u32 uval;
int ret;
+ int i;
if (!of_device_is_compatible(child, "pmbus-fan"))
return 0;
@@ -574,10 +575,24 @@ static int max31785_of_fan_config(struct i2c_client *client,
if (ret < 0)
return ret;
- ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_FAN_CONFIG_12,
- pb_cfg);
- if (ret < 0)
- return ret;
+ for (i = 0; i < 2; i++) {
+ ret = max31785_i2c_smbus_write_byte_data(client,
+ PMBUS_FAN_CONFIG_12,
+ pb_cfg);
+ if (ret < 0)
+ continue;
+
+ ret = max31785_i2c_smbus_read_byte_data(client,
+ PMBUS_FAN_CONFIG_12);
+ if (ret < 0)
+ continue;
+
+ if (ret == pb_cfg)
+ break;
+ }
+
+ if (i == 2)
+ return -EIO;
/*
* Fans are on pages 0 - 5. If the page property of a fan node is
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH linux dev-5.10 07/14] pmbus: (max31785) Retry enabling fans after writing MFR_FAN_CONFIG
2021-08-11 15:42 ` [PATCH linux dev-5.10 07/14] pmbus: (max31785) Retry enabling fans after writing MFR_FAN_CONFIG Eddie James
@ 2021-08-11 16:14 ` Matthew Barth
0 siblings, 0 replies; 20+ messages in thread
From: Matthew Barth @ 2021-08-11 16:14 UTC (permalink / raw)
To: Eddie James, openbmc; +Cc: Andrew Jeffery
On 8/11/21 10:42 AM, Eddie James wrote:
> From: Andrew Jeffery <andrew@aj.id.au>
>
> It has been observed across large fleets of systems that a small subset
> of those systems occasionally loose control of some number of fans
> across a BMC reboot (their hwmon fan attributes are missing from sysfs).
>
> >From extensive testing and tracing it was discovered that writes
> enabling a fan in FAN_CONFIG_1_2 failed to stick on the system under
> test with a frequency of about 1 in 1000 re-binds of the driver.
>
> The MAX31785 datasheet recommends in the documentation for
> MFR_FAN_CONFIG that the asssociated fan(s) be disabled before updating
> the register. The sequence in question implements this suggestion, and
> the observed loss-of-fans symptom occurs when the write to re-enable the
> fan in FAN_CONFIG_1_2 fails to stick.
>
> The trace data suggests a one-shot retry is enough to successfully
> update FAN_CONFIG_1_2. With the workaround, no loss of fans was observed
> in over 20,000 consecutive rebinds of the driver.
>
> OpenBMC-Staging-Count: 1
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Matthew Barth <msbarth@linux.ibm.com>
> ---
> drivers/hwmon/pmbus/max31785.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
> index 7518fff356f9..b37da2ec1ce4 100644
> --- a/drivers/hwmon/pmbus/max31785.c
> +++ b/drivers/hwmon/pmbus/max31785.c
> @@ -398,6 +398,7 @@ static int max31785_of_fan_config(struct i2c_client *client,
> u32 page;
> u32 uval;
> int ret;
> + int i;
>
> if (!of_device_is_compatible(child, "pmbus-fan"))
> return 0;
> @@ -574,10 +575,24 @@ static int max31785_of_fan_config(struct i2c_client *client,
> if (ret < 0)
> return ret;
>
> - ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_FAN_CONFIG_12,
> - pb_cfg);
> - if (ret < 0)
> - return ret;
> + for (i = 0; i < 2; i++) {
> + ret = max31785_i2c_smbus_write_byte_data(client,
> + PMBUS_FAN_CONFIG_12,
> + pb_cfg);
> + if (ret < 0)
> + continue;
> +
> + ret = max31785_i2c_smbus_read_byte_data(client,
> + PMBUS_FAN_CONFIG_12);
> + if (ret < 0)
> + continue;
> +
> + if (ret == pb_cfg)
> + break;
> + }
> +
> + if (i == 2)
> + return -EIO;
>
> /*
> * Fans are on pages 0 - 5. If the page property of a fan node is
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH linux dev-5.10 08/14] ARM: dts: aspeed: Rainier: Add fan controller properties
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
` (6 preceding siblings ...)
2021-08-11 15:42 ` [PATCH linux dev-5.10 07/14] pmbus: (max31785) Retry enabling fans after writing MFR_FAN_CONFIG Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 16:14 ` Matthew Barth
2021-08-11 15:42 ` [PATCH linux dev-5.10 09/14] ARM: dts: aspeed: Everest: " Eddie James
` (5 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Eddie James
Add the necessary Max chip specific fan properties.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 42 ++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
index f87bc5dc8aba..6fd3ddf97a21 100644
--- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
@@ -1282,36 +1282,78 @@ fan0: fan@0 {
compatible = "pmbus-fan";
reg = <0>;
tach-pulses = <2>;
+ maxim,fan-rotor-input = "tach";
+ maxim,fan-pwm-freq = <25000>;
+ maxim,fan-dual-tach;
+ maxim,fan-no-watchdog;
+ maxim,fan-no-fault-ramp;
+ maxim,fan-ramp = <2>;
+ maxim,fan-fault-pin-mon;
};
fan1: fan@1 {
compatible = "pmbus-fan";
reg = <1>;
tach-pulses = <2>;
+ maxim,fan-rotor-input = "tach";
+ maxim,fan-pwm-freq = <25000>;
+ maxim,fan-dual-tach;
+ maxim,fan-no-watchdog;
+ maxim,fan-no-fault-ramp;
+ maxim,fan-ramp = <2>;
+ maxim,fan-fault-pin-mon;
};
fan2: fan@2 {
compatible = "pmbus-fan";
reg = <2>;
tach-pulses = <2>;
+ maxim,fan-rotor-input = "tach";
+ maxim,fan-pwm-freq = <25000>;
+ maxim,fan-dual-tach;
+ maxim,fan-no-watchdog;
+ maxim,fan-no-fault-ramp;
+ maxim,fan-ramp = <2>;
+ maxim,fan-fault-pin-mon;
};
fan3: fan@3 {
compatible = "pmbus-fan";
reg = <3>;
tach-pulses = <2>;
+ maxim,fan-rotor-input = "tach";
+ maxim,fan-pwm-freq = <25000>;
+ maxim,fan-dual-tach;
+ maxim,fan-no-watchdog;
+ maxim,fan-no-fault-ramp;
+ maxim,fan-ramp = <2>;
+ maxim,fan-fault-pin-mon;
};
fan4: fan@4 {
compatible = "pmbus-fan";
reg = <4>;
tach-pulses = <2>;
+ maxim,fan-rotor-input = "tach";
+ maxim,fan-pwm-freq = <25000>;
+ maxim,fan-dual-tach;
+ maxim,fan-no-watchdog;
+ maxim,fan-no-fault-ramp;
+ maxim,fan-ramp = <2>;
+ maxim,fan-fault-pin-mon;
};
fan5: fan@5 {
compatible = "pmbus-fan";
reg = <5>;
tach-pulses = <2>;
+ maxim,fan-rotor-input = "tach";
+ maxim,fan-pwm-freq = <25000>;
+ maxim,fan-dual-tach;
+ maxim,fan-no-watchdog;
+ maxim,fan-no-fault-ramp;
+ maxim,fan-ramp = <2>;
+ maxim,fan-fault-pin-mon;
};
};
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH linux dev-5.10 08/14] ARM: dts: aspeed: Rainier: Add fan controller properties
2021-08-11 15:42 ` [PATCH linux dev-5.10 08/14] ARM: dts: aspeed: Rainier: Add fan controller properties Eddie James
@ 2021-08-11 16:14 ` Matthew Barth
0 siblings, 0 replies; 20+ messages in thread
From: Matthew Barth @ 2021-08-11 16:14 UTC (permalink / raw)
To: Eddie James, openbmc
On 8/11/21 10:42 AM, Eddie James wrote:
> Add the necessary Max chip specific fan properties.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Matthew Barth <msbarth@linux.ibm.com>
> ---
> arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 42 ++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
> index f87bc5dc8aba..6fd3ddf97a21 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
> @@ -1282,36 +1282,78 @@ fan0: fan@0 {
> compatible = "pmbus-fan";
> reg = <0>;
> tach-pulses = <2>;
> + maxim,fan-rotor-input = "tach";
> + maxim,fan-pwm-freq = <25000>;
> + maxim,fan-dual-tach;
> + maxim,fan-no-watchdog;
> + maxim,fan-no-fault-ramp;
> + maxim,fan-ramp = <2>;
> + maxim,fan-fault-pin-mon;
> };
>
> fan1: fan@1 {
> compatible = "pmbus-fan";
> reg = <1>;
> tach-pulses = <2>;
> + maxim,fan-rotor-input = "tach";
> + maxim,fan-pwm-freq = <25000>;
> + maxim,fan-dual-tach;
> + maxim,fan-no-watchdog;
> + maxim,fan-no-fault-ramp;
> + maxim,fan-ramp = <2>;
> + maxim,fan-fault-pin-mon;
> };
>
> fan2: fan@2 {
> compatible = "pmbus-fan";
> reg = <2>;
> tach-pulses = <2>;
> + maxim,fan-rotor-input = "tach";
> + maxim,fan-pwm-freq = <25000>;
> + maxim,fan-dual-tach;
> + maxim,fan-no-watchdog;
> + maxim,fan-no-fault-ramp;
> + maxim,fan-ramp = <2>;
> + maxim,fan-fault-pin-mon;
> };
>
> fan3: fan@3 {
> compatible = "pmbus-fan";
> reg = <3>;
> tach-pulses = <2>;
> + maxim,fan-rotor-input = "tach";
> + maxim,fan-pwm-freq = <25000>;
> + maxim,fan-dual-tach;
> + maxim,fan-no-watchdog;
> + maxim,fan-no-fault-ramp;
> + maxim,fan-ramp = <2>;
> + maxim,fan-fault-pin-mon;
> };
>
> fan4: fan@4 {
> compatible = "pmbus-fan";
> reg = <4>;
> tach-pulses = <2>;
> + maxim,fan-rotor-input = "tach";
> + maxim,fan-pwm-freq = <25000>;
> + maxim,fan-dual-tach;
> + maxim,fan-no-watchdog;
> + maxim,fan-no-fault-ramp;
> + maxim,fan-ramp = <2>;
> + maxim,fan-fault-pin-mon;
> };
>
> fan5: fan@5 {
> compatible = "pmbus-fan";
> reg = <5>;
> tach-pulses = <2>;
> + maxim,fan-rotor-input = "tach";
> + maxim,fan-pwm-freq = <25000>;
> + maxim,fan-dual-tach;
> + maxim,fan-no-watchdog;
> + maxim,fan-no-fault-ramp;
> + maxim,fan-ramp = <2>;
> + maxim,fan-fault-pin-mon;
> };
> };
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH linux dev-5.10 09/14] ARM: dts: aspeed: Everest: Add fan controller properties
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
` (7 preceding siblings ...)
2021-08-11 15:42 ` [PATCH linux dev-5.10 08/14] ARM: dts: aspeed: Rainier: Add fan controller properties Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 16:15 ` Matthew Barth
2021-08-11 15:42 ` [PATCH linux dev-5.10 10/14] ARM: dts: aspeed: Rainier 4U: Delete fan dual-tach properties Eddie James
` (4 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Eddie James
Add the necessary Max chip specific fan properties.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts | 28 ++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
index 01a0b2b03ddd..83fc29309154 100644
--- a/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
@@ -2510,24 +2510,52 @@ fan@0 {
compatible = "pmbus-fan";
reg = <0>;
tach-pulses = <2>;
+ maxim,fan-rotor-input = "tach";
+ maxim,fan-pwm-freq = <25000>;
+ maxim,fan-dual-tach;
+ maxim,fan-no-watchdog;
+ maxim,fan-no-fault-ramp;
+ maxim,fan-ramp = <2>;
+ maxim,fan-fault-pin-mon;
};
fan@1 {
compatible = "pmbus-fan";
reg = <1>;
tach-pulses = <2>;
+ maxim,fan-rotor-input = "tach";
+ maxim,fan-pwm-freq = <25000>;
+ maxim,fan-dual-tach;
+ maxim,fan-no-watchdog;
+ maxim,fan-no-fault-ramp;
+ maxim,fan-ramp = <2>;
+ maxim,fan-fault-pin-mon;
};
fan@2 {
compatible = "pmbus-fan";
reg = <2>;
tach-pulses = <2>;
+ maxim,fan-rotor-input = "tach";
+ maxim,fan-pwm-freq = <25000>;
+ maxim,fan-dual-tach;
+ maxim,fan-no-watchdog;
+ maxim,fan-no-fault-ramp;
+ maxim,fan-ramp = <2>;
+ maxim,fan-fault-pin-mon;
};
fan@3 {
compatible = "pmbus-fan";
reg = <3>;
tach-pulses = <2>;
+ maxim,fan-rotor-input = "tach";
+ maxim,fan-pwm-freq = <25000>;
+ maxim,fan-dual-tach;
+ maxim,fan-no-watchdog;
+ maxim,fan-no-fault-ramp;
+ maxim,fan-ramp = <2>;
+ maxim,fan-fault-pin-mon;
};
};
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH linux dev-5.10 09/14] ARM: dts: aspeed: Everest: Add fan controller properties
2021-08-11 15:42 ` [PATCH linux dev-5.10 09/14] ARM: dts: aspeed: Everest: " Eddie James
@ 2021-08-11 16:15 ` Matthew Barth
0 siblings, 0 replies; 20+ messages in thread
From: Matthew Barth @ 2021-08-11 16:15 UTC (permalink / raw)
To: Eddie James, openbmc
On 8/11/21 10:42 AM, Eddie James wrote:
> Add the necessary Max chip specific fan properties.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Matthew Barth <msbarth@linux.ibm.com>
> ---
> arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts | 28 ++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
> index 01a0b2b03ddd..83fc29309154 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
> @@ -2510,24 +2510,52 @@ fan@0 {
> compatible = "pmbus-fan";
> reg = <0>;
> tach-pulses = <2>;
> + maxim,fan-rotor-input = "tach";
> + maxim,fan-pwm-freq = <25000>;
> + maxim,fan-dual-tach;
> + maxim,fan-no-watchdog;
> + maxim,fan-no-fault-ramp;
> + maxim,fan-ramp = <2>;
> + maxim,fan-fault-pin-mon;
> };
>
> fan@1 {
> compatible = "pmbus-fan";
> reg = <1>;
> tach-pulses = <2>;
> + maxim,fan-rotor-input = "tach";
> + maxim,fan-pwm-freq = <25000>;
> + maxim,fan-dual-tach;
> + maxim,fan-no-watchdog;
> + maxim,fan-no-fault-ramp;
> + maxim,fan-ramp = <2>;
> + maxim,fan-fault-pin-mon;
> };
>
> fan@2 {
> compatible = "pmbus-fan";
> reg = <2>;
> tach-pulses = <2>;
> + maxim,fan-rotor-input = "tach";
> + maxim,fan-pwm-freq = <25000>;
> + maxim,fan-dual-tach;
> + maxim,fan-no-watchdog;
> + maxim,fan-no-fault-ramp;
> + maxim,fan-ramp = <2>;
> + maxim,fan-fault-pin-mon;
> };
>
> fan@3 {
> compatible = "pmbus-fan";
> reg = <3>;
> tach-pulses = <2>;
> + maxim,fan-rotor-input = "tach";
> + maxim,fan-pwm-freq = <25000>;
> + maxim,fan-dual-tach;
> + maxim,fan-no-watchdog;
> + maxim,fan-no-fault-ramp;
> + maxim,fan-ramp = <2>;
> + maxim,fan-fault-pin-mon;
> };
> };
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH linux dev-5.10 10/14] ARM: dts: aspeed: Rainier 4U: Delete fan dual-tach properties
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
` (8 preceding siblings ...)
2021-08-11 15:42 ` [PATCH linux dev-5.10 09/14] ARM: dts: aspeed: Everest: " Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 16:17 ` Matthew Barth
2021-08-11 15:42 ` [PATCH linux dev-5.10 11/14] ARM: dts: aspeed: Add Rainier 2U and 4U device trees for pass 1 hardware Eddie James
` (3 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Eddie James
The fans in the 4U chassis do not have dual tachometers, so remove those
properties in the device tree.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
.../boot/dts/aspeed-bmc-ibm-rainier-4u.dts | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u.dts
index 342546a3c0f5..24283cc3d486 100644
--- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u.dts
@@ -19,3 +19,27 @@ power-supply@6b {
reg = <0x6b>;
};
};
+
+&fan0 {
+ /delete-property/ maxim,fan-dual-tach;
+};
+
+&fan1 {
+ /delete-property/ maxim,fan-dual-tach;
+};
+
+&fan2 {
+ /delete-property/ maxim,fan-dual-tach;
+};
+
+&fan3 {
+ /delete-property/ maxim,fan-dual-tach;
+};
+
+&fan4 {
+ /delete-property/ maxim,fan-dual-tach;
+};
+
+&fan5 {
+ /delete-property/ maxim,fan-dual-tach;
+};
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH linux dev-5.10 10/14] ARM: dts: aspeed: Rainier 4U: Delete fan dual-tach properties
2021-08-11 15:42 ` [PATCH linux dev-5.10 10/14] ARM: dts: aspeed: Rainier 4U: Delete fan dual-tach properties Eddie James
@ 2021-08-11 16:17 ` Matthew Barth
0 siblings, 0 replies; 20+ messages in thread
From: Matthew Barth @ 2021-08-11 16:17 UTC (permalink / raw)
To: Eddie James, openbmc
On 8/11/21 10:42 AM, Eddie James wrote:
> The fans in the 4U chassis do not have dual tachometers, so remove those
> properties in the device tree.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Matthew Barth <msbarth@linux.ibm.com>
> ---
> .../boot/dts/aspeed-bmc-ibm-rainier-4u.dts | 24 +++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u.dts
> index 342546a3c0f5..24283cc3d486 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u.dts
> @@ -19,3 +19,27 @@ power-supply@6b {
> reg = <0x6b>;
> };
> };
> +
> +&fan0 {
> + /delete-property/ maxim,fan-dual-tach;
> +};
> +
> +&fan1 {
> + /delete-property/ maxim,fan-dual-tach;
> +};
> +
> +&fan2 {
> + /delete-property/ maxim,fan-dual-tach;
> +};
> +
> +&fan3 {
> + /delete-property/ maxim,fan-dual-tach;
> +};
> +
> +&fan4 {
> + /delete-property/ maxim,fan-dual-tach;
> +};
> +
> +&fan5 {
> + /delete-property/ maxim,fan-dual-tach;
> +};
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH linux dev-5.10 11/14] ARM: dts: aspeed: Add Rainier 2U and 4U device trees for pass 1 hardware
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
` (9 preceding siblings ...)
2021-08-11 15:42 ` [PATCH linux dev-5.10 10/14] ARM: dts: aspeed: Rainier 4U: Delete fan dual-tach properties Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 12/14] fsi: sbefifo: Increase command timeouts to 30 seconds Eddie James
` (2 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Eddie James
The original BMC card must be supported in the device tree, so add
new pass 1 device trees for the two relevant Rainier systems.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
arch/arm/boot/dts/Makefile | 2 +
.../boot/dts/aspeed-bmc-ibm-rainier-4u-p1.dts | 94 +++++++++++++++++++
.../boot/dts/aspeed-bmc-ibm-rainier-p1.dts | 94 +++++++++++++++++++
3 files changed, 190 insertions(+)
create mode 100644 arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u-p1.dts
create mode 100644 arch/arm/boot/dts/aspeed-bmc-ibm-rainier-p1.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 48d48c85de9e..36b4798e7cd4 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1402,8 +1402,10 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
aspeed-bmc-facebook-yosemitev2.dtb \
aspeed-bmc-ibm-everest.dtb \
aspeed-bmc-ibm-rainier.dtb \
+ aspeed-bmc-ibm-rainier-p1.dtb \
aspeed-bmc-ibm-rainier-1s4u.dtb \
aspeed-bmc-ibm-rainier-4u.dtb \
+ aspeed-bmc-ibm-rainier-4u-p1.dtb \
aspeed-bmc-intel-s2600wf.dtb \
aspeed-bmc-inspur-fp5280g2.dtb \
aspeed-bmc-inspur-nf5280m6.dtb \
diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u-p1.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u-p1.dts
new file mode 100644
index 000000000000..e35a3efd9d22
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-4u-p1.dts
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright 2021 IBM Corp.
+/dts-v1/;
+
+#include "aspeed-bmc-ibm-rainier-4u.dts"
+
+/ {
+ model = "Rainier 4U Pass 1";
+
+ aliases {
+ /delete-property/ i2c20;
+ /delete-property/ i2c21;
+ /delete-property/ i2c22;
+ /delete-property/ i2c23;
+ /delete-property/ i2c24;
+ /delete-property/ i2c25;
+ /delete-property/ i2c26;
+ /delete-property/ i2c27;
+ /delete-property/ i2c28;
+ /delete-property/ i2c29;
+ /delete-property/ i2c30;
+ };
+};
+
+&i2c4 {
+ /delete-node/ pca9546@70;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c64";
+ reg = <0x51>;
+ };
+
+ eeprom@52 {
+ compatible = "atmel,24c64";
+ reg = <0x52>;
+ };
+};
+
+&i2c5 {
+ /delete-node/ pca9546@70;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c64";
+ reg = <0x51>;
+ };
+};
+
+&i2c6 {
+ /delete-node/ pca9546@70;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c64";
+ reg = <0x51>;
+ };
+
+ eeprom@52 {
+ compatible = "atmel,24c64";
+ reg = <0x52>;
+ };
+
+ eeprom@53 {
+ compatible = "atmel,24c64";
+ reg = <0x53>;
+ };
+};
+
+&i2c11 {
+ /delete-node/ pca9546@70;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c64";
+ reg = <0x51>;
+ };
+};
diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-p1.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-p1.dts
new file mode 100644
index 000000000000..b3c923f1838b
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier-p1.dts
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright 2021 IBM Corp.
+/dts-v1/;
+
+#include "aspeed-bmc-ibm-rainier.dts"
+
+/ {
+ model = "Rainier 2U Pass 1";
+
+ aliases {
+ /delete-property/ i2c20;
+ /delete-property/ i2c21;
+ /delete-property/ i2c22;
+ /delete-property/ i2c23;
+ /delete-property/ i2c24;
+ /delete-property/ i2c25;
+ /delete-property/ i2c26;
+ /delete-property/ i2c27;
+ /delete-property/ i2c28;
+ /delete-property/ i2c29;
+ /delete-property/ i2c30;
+ };
+};
+
+&i2c4 {
+ /delete-node/ pca9546@70;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c64";
+ reg = <0x51>;
+ };
+
+ eeprom@52 {
+ compatible = "atmel,24c64";
+ reg = <0x52>;
+ };
+};
+
+&i2c5 {
+ /delete-node/ pca9546@70;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c64";
+ reg = <0x51>;
+ };
+};
+
+&i2c6 {
+ /delete-node/ pca9546@70;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c64";
+ reg = <0x51>;
+ };
+
+ eeprom@52 {
+ compatible = "atmel,24c64";
+ reg = <0x52>;
+ };
+
+ eeprom@53 {
+ compatible = "atmel,24c64";
+ reg = <0x53>;
+ };
+};
+
+&i2c11 {
+ /delete-node/ pca9546@70;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c64";
+ reg = <0x51>;
+ };
+};
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH linux dev-5.10 12/14] fsi: sbefifo: Increase command timeouts to 30 seconds
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
` (10 preceding siblings ...)
2021-08-11 15:42 ` [PATCH linux dev-5.10 11/14] ARM: dts: aspeed: Add Rainier 2U and 4U device trees for pass 1 hardware Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 13/14] fsi: occ: Add dynamic debug to dump command and response Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 14/14] fsi: sbefifo: Use interruptible mutex locking Eddie James
13 siblings, 0 replies; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Eddie James
Different commands (with different sizes) require different timeouts,
the longest of which can be up to 30 seconds. Adjust the command
timeout accordingly.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-sbefifo.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index 84cb965bfed5..8645a092af89 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -102,12 +102,12 @@ enum sbe_state
#define sbefifo_eot_set(sts) (((sts) & SBEFIFO_STS_EOT_MASK) >> SBEFIFO_STS_EOT_SHIFT)
/* Reset request timeout in ms */
-#define SBEFIFO_RESET_TIMEOUT 10000
+#define SBEFIFO_RESET_TIMEOUT 30000
/* Timeouts for commands in ms */
-#define SBEFIFO_TIMEOUT_START_CMD 10000
+#define SBEFIFO_TIMEOUT_START_CMD 30000
#define SBEFIFO_TIMEOUT_IN_CMD 1000
-#define SBEFIFO_TIMEOUT_START_RSP 10000
+#define SBEFIFO_TIMEOUT_START_RSP 30000
#define SBEFIFO_TIMEOUT_IN_RSP 1000
/* Other constants */
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH linux dev-5.10 13/14] fsi: occ: Add dynamic debug to dump command and response
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
` (11 preceding siblings ...)
2021-08-11 15:42 ` [PATCH linux dev-5.10 12/14] fsi: sbefifo: Increase command timeouts to 30 seconds Eddie James
@ 2021-08-11 15:42 ` Eddie James
2021-08-11 15:42 ` [PATCH linux dev-5.10 14/14] fsi: sbefifo: Use interruptible mutex locking Eddie James
13 siblings, 0 replies; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Eddie James
Use the dynamic branching capability of the dynamic debug subsystem
to dump the command and response with the correct OCC device name.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-occ.c | 44 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
index 90795325b7d9..b0c9322078a1 100644
--- a/drivers/fsi/fsi-occ.c
+++ b/drivers/fsi/fsi-occ.c
@@ -21,6 +21,15 @@
#include <linux/uaccess.h>
#include <asm/unaligned.h>
+#if !defined(CONFIG_DYNAMIC_DEBUG_CORE)
+#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)
+#if defined(DEBUG)
+#define DYNAMIC_DEBUG_BRANCH(descriptor) true
+#else /* DEBUG */
+#define DYNAMIC_DEBUG_BRANCH(descriptor) false
+#endif /* DEBUG */
+#endif /* CONFIG_DYNAMIC_DEBUG_CORE */
+
#define OCC_SRAM_BYTES 4096
#define OCC_CMD_DATA_BYTES 4090
#define OCC_RESP_DATA_BYTES 4089
@@ -359,6 +368,20 @@ static int occ_putsram(struct occ *occ, const void *data, ssize_t len,
byte_buf[len - 2] = checksum >> 8;
byte_buf[len - 1] = checksum & 0xff;
+ {
+ DEFINE_DYNAMIC_DEBUG_METADATA(ddm_occ_cmd, "OCC command");
+
+ if (DYNAMIC_DEBUG_BRANCH(ddm_occ_cmd)) {
+ char prefix[64];
+
+ snprintf(prefix, sizeof(prefix), "%s %s: cmd ",
+ dev_driver_string(occ->dev),
+ dev_name(occ->dev));
+ print_hex_dump(KERN_DEBUG, prefix, DUMP_PREFIX_OFFSET,
+ 16, 4, byte_buf, len, false);
+ }
+ }
+
rc = sbefifo_submit(occ->sbefifo, buf, cmd_len, buf, &resp_len);
if (rc)
goto free;
@@ -556,6 +579,27 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
}
*resp_len = resp_data_length + 7;
+
+ {
+ DEFINE_DYNAMIC_DEBUG_METADATA(ddm_occ_rsp,
+ "OCC response");
+ DEFINE_DYNAMIC_DEBUG_METADATA(ddm_occ_full_rsp,
+ "OCC full response");
+
+ if (DYNAMIC_DEBUG_BRANCH(ddm_occ_full_rsp) ||
+ DYNAMIC_DEBUG_BRANCH(ddm_occ_rsp)) {
+ char prefix[64];
+ size_t l = DYNAMIC_DEBUG_BRANCH(ddm_occ_full_rsp) ?
+ *resp_len : 16;
+
+ snprintf(prefix, sizeof(prefix), "%s %s: rsp ",
+ dev_driver_string(occ->dev),
+ dev_name(occ->dev));
+ print_hex_dump(KERN_DEBUG, prefix, DUMP_PREFIX_OFFSET,
+ 16, 4, resp, l, false);
+ }
+ }
+
rc = occ_verify_checksum(occ, resp, resp_data_length);
done:
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH linux dev-5.10 14/14] fsi: sbefifo: Use interruptible mutex locking
2021-08-11 15:42 [PATCH linux dev-5.10 00/14] Rainier and Everest system fixes Eddie James
` (12 preceding siblings ...)
2021-08-11 15:42 ` [PATCH linux dev-5.10 13/14] fsi: occ: Add dynamic debug to dump command and response Eddie James
@ 2021-08-11 15:42 ` Eddie James
13 siblings, 0 replies; 20+ messages in thread
From: Eddie James @ 2021-08-11 15:42 UTC (permalink / raw)
To: openbmc; +Cc: Eddie James
Some SBE operations have extremely large responses and can require
several minutes to process the response. During this time, the device
lock must be held. If another process attempts an operation, it will
wait for the mutex for longer than the kernel hung task watchdog
allows. Therefore, use the interruptible function to lock the mutex.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-sbefifo.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index 8645a092af89..060a790989f9 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -740,7 +740,9 @@ int sbefifo_submit(struct device *dev, const __be32 *command, size_t cmd_len,
iov_iter_kvec(&resp_iter, WRITE, &resp_iov, 1, rbytes);
/* Perform the command */
- mutex_lock(&sbefifo->lock);
+ rc = mutex_lock_interruptible(&sbefifo->lock);
+ if (rc)
+ return rc;
rc = __sbefifo_submit(sbefifo, command, cmd_len, &resp_iter);
mutex_unlock(&sbefifo->lock);
@@ -820,7 +822,9 @@ static ssize_t sbefifo_user_read(struct file *file, char __user *buf,
iov_iter_init(&resp_iter, WRITE, &resp_iov, 1, len);
/* Perform the command */
- mutex_lock(&sbefifo->lock);
+ rc = mutex_lock_interruptible(&sbefifo->lock);
+ if (rc)
+ goto bail;
rc = __sbefifo_submit(sbefifo, user->pending_cmd, cmd_len, &resp_iter);
mutex_unlock(&sbefifo->lock);
if (rc < 0)
@@ -875,7 +879,9 @@ static ssize_t sbefifo_user_write(struct file *file, const char __user *buf,
user->pending_len = 0;
/* Trigger reset request */
- mutex_lock(&sbefifo->lock);
+ rc = mutex_lock_interruptible(&sbefifo->lock);
+ if (rc)
+ goto bail;
rc = sbefifo_request_reset(user->sbefifo);
mutex_unlock(&sbefifo->lock);
if (rc == 0)
--
2.27.0
^ permalink raw reply related [flat|nested] 20+ messages in thread