Small fixes to the SMBus IPMI and IPMB driver

Nothing big, cleanups, fixing names, and one small deviation from the
 specification fixed.
 -----BEGIN PGP SIGNATURE-----
 
 iQIyBAABCgAdFiEE/Q1c5nzg9ZpmiCaGYfOMkJGb/4EFAmP2DPYACgkQYfOMkJGb
 /4G+1w/1E3mqdn1P4pq9pcsTZRRRfrLxMYQAaZMOZu6cb/mOL1Bj0D/Ks/q9bkkF
 1iCSYUgca6TVSmnF0o23cP2Q4cGmH4vEk/e1L5XF3A/gXJ1KK6T1p5g/pYhyIJze
 WRO/r1OKPvfsD+djkslafMlzE1lwKGQa4hlwXAkmeSZn5cijB4BKeh+i8Sv2C0ER
 PX8Ar3TTXrfSragXjzZdbXOksT4kskGpPkAYSwWj2dRM9+Sh2ReoOr0xiCy5Wnm6
 gCVtjrPiVdtlAUi3OriUR1/IJjvWXuDBVFCKpd/w8urK61BsoMmrUVb319GOYH94
 UTIz8SyxOaS8E3v46ToRrQHRvjlBNMQY3BrYMxfVp1NmR1VRJycPgKfviV0dezmK
 CvObTTK9JscSJ2BlmZ+u9sC4pN9OQCoA988B91plJ9X5GGI4VH/aQ+EN7XIjrRn7
 09QV9OyW56yFdtgIWcvH8dwpsQQD4T1H7aWe50BtL6Z+NB8HnV6h9suSh9+Q6jgr
 K7MNXWgxVvKs+MolbuUNJ6iWYTwgcAIz/21h2deZrtQ6vXKZFyQBHzGiN9We6nwz
 J/Pbz4R4pE7l/zsP+yNdPi7XRa8uieNOXbp3GoorhRBPyQC38AWJTUh4FybOEB9V
 Gj/iDAm6L6VRwTSXYarFvmU1vq/EPVHTGZ3Vz6Mam62JXFoShQ==
 =qBVV
 -----END PGP SIGNATURE-----

Merge tag 'for-linus-6.3-1' of https://github.com/cminyard/linux-ipmi

Pull IPMI updates from Corey Minyard:
 "Small fixes to the SMBus IPMI and IPMB driver.

  Nothing big, cleanups, fixing names, and one small deviation from the
  specification fixed"

* tag 'for-linus-6.3-1' of https://github.com/cminyard/linux-ipmi:
  ipmi: ipmb: Fix the MODULE_PARM_DESC associated to 'retry_time_ms'
  ipmi:ssif: Add a timer between request retries
  ipmi:ssif: Remove rtc_us_timer
  ipmi_ssif: Rename idle state and check
  ipmi:ssif: resend_msg() cannot fail
This commit is contained in:
Linus Torvalds 2023-02-22 11:41:37 -08:00
commit bc009f9382
2 changed files with 56 additions and 59 deletions

View File

@ -27,7 +27,7 @@ MODULE_PARM_DESC(bmcaddr, "Address to use for BMC.");
static unsigned int retry_time_ms = 250;
module_param(retry_time_ms, uint, 0644);
MODULE_PARM_DESC(max_retries, "Timeout time between retries, in milliseconds.");
MODULE_PARM_DESC(retry_time_ms, "Timeout time between retries, in milliseconds.");
static unsigned int max_retries = 1;
module_param(max_retries, uint, 0644);

View File

@ -74,7 +74,8 @@
/*
* Timer values
*/
#define SSIF_MSG_USEC 60000 /* 60ms between message tries. */
#define SSIF_MSG_USEC 60000 /* 60ms between message tries (T3). */
#define SSIF_REQ_RETRY_USEC 60000 /* 60ms between send retries (T6). */
#define SSIF_MSG_PART_USEC 5000 /* 5ms for a message part */
/* How many times to we retry sending/receiving the message. */
@ -82,7 +83,9 @@
#define SSIF_RECV_RETRIES 250
#define SSIF_MSG_MSEC (SSIF_MSG_USEC / 1000)
#define SSIF_REQ_RETRY_MSEC (SSIF_REQ_RETRY_USEC / 1000)
#define SSIF_MSG_JIFFIES ((SSIF_MSG_USEC * 1000) / TICK_NSEC)
#define SSIF_REQ_RETRY_JIFFIES ((SSIF_REQ_RETRY_USEC * 1000) / TICK_NSEC)
#define SSIF_MSG_PART_JIFFIES ((SSIF_MSG_PART_USEC * 1000) / TICK_NSEC)
/*
@ -92,7 +95,7 @@
#define SSIF_WATCH_WATCHDOG_TIMEOUT msecs_to_jiffies(250)
enum ssif_intf_state {
SSIF_NORMAL,
SSIF_IDLE,
SSIF_GETTING_FLAGS,
SSIF_GETTING_EVENTS,
SSIF_CLEARING_FLAGS,
@ -100,8 +103,8 @@ enum ssif_intf_state {
/* FIXME - add watchdog stuff. */
};
#define SSIF_IDLE(ssif) ((ssif)->ssif_state == SSIF_NORMAL \
&& (ssif)->curr_msg == NULL)
#define IS_SSIF_IDLE(ssif) ((ssif)->ssif_state == SSIF_IDLE \
&& (ssif)->curr_msg == NULL)
/*
* Indexes into stats[] in ssif_info below.
@ -229,6 +232,9 @@ struct ssif_info {
bool got_alert;
bool waiting_alert;
/* Used to inform the timeout that it should do a resend. */
bool do_resend;
/*
* If set to true, this will request events the next time the
* state machine is idle.
@ -241,12 +247,6 @@ struct ssif_info {
*/
bool req_flags;
/*
* Used to perform timer operations when run-to-completion
* mode is on. This is a countdown timer.
*/
int rtc_us_timer;
/* Used for sending/receiving data. +1 for the length. */
unsigned char data[IPMI_MAX_MSG_LENGTH + 1];
unsigned int data_len;
@ -348,9 +348,9 @@ static void return_hosed_msg(struct ssif_info *ssif_info,
/*
* Must be called with the message lock held. This will release the
* message lock. Note that the caller will check SSIF_IDLE and start a
* new operation, so there is no need to check for new messages to
* start in here.
* message lock. Note that the caller will check IS_SSIF_IDLE and
* start a new operation, so there is no need to check for new
* messages to start in here.
*/
static void start_clear_flags(struct ssif_info *ssif_info, unsigned long *flags)
{
@ -367,7 +367,7 @@ static void start_clear_flags(struct ssif_info *ssif_info, unsigned long *flags)
if (start_send(ssif_info, msg, 3) != 0) {
/* Error, just go to normal state. */
ssif_info->ssif_state = SSIF_NORMAL;
ssif_info->ssif_state = SSIF_IDLE;
}
}
@ -382,7 +382,7 @@ static void start_flag_fetch(struct ssif_info *ssif_info, unsigned long *flags)
mb[0] = (IPMI_NETFN_APP_REQUEST << 2);
mb[1] = IPMI_GET_MSG_FLAGS_CMD;
if (start_send(ssif_info, mb, 2) != 0)
ssif_info->ssif_state = SSIF_NORMAL;
ssif_info->ssif_state = SSIF_IDLE;
}
static void check_start_send(struct ssif_info *ssif_info, unsigned long *flags,
@ -393,7 +393,7 @@ static void check_start_send(struct ssif_info *ssif_info, unsigned long *flags,
flags = ipmi_ssif_lock_cond(ssif_info, &oflags);
ssif_info->curr_msg = NULL;
ssif_info->ssif_state = SSIF_NORMAL;
ssif_info->ssif_state = SSIF_IDLE;
ipmi_ssif_unlock_cond(ssif_info, flags);
ipmi_free_smi_msg(msg);
}
@ -407,7 +407,7 @@ static void start_event_fetch(struct ssif_info *ssif_info, unsigned long *flags)
msg = ipmi_alloc_smi_msg();
if (!msg) {
ssif_info->ssif_state = SSIF_NORMAL;
ssif_info->ssif_state = SSIF_IDLE;
ipmi_ssif_unlock_cond(ssif_info, flags);
return;
}
@ -430,7 +430,7 @@ static void start_recv_msg_fetch(struct ssif_info *ssif_info,
msg = ipmi_alloc_smi_msg();
if (!msg) {
ssif_info->ssif_state = SSIF_NORMAL;
ssif_info->ssif_state = SSIF_IDLE;
ipmi_ssif_unlock_cond(ssif_info, flags);
return;
}
@ -448,9 +448,9 @@ static void start_recv_msg_fetch(struct ssif_info *ssif_info,
/*
* Must be called with the message lock held. This will release the
* message lock. Note that the caller will check SSIF_IDLE and start a
* new operation, so there is no need to check for new messages to
* start in here.
* message lock. Note that the caller will check IS_SSIF_IDLE and
* start a new operation, so there is no need to check for new
* messages to start in here.
*/
static void handle_flags(struct ssif_info *ssif_info, unsigned long *flags)
{
@ -466,7 +466,7 @@ static void handle_flags(struct ssif_info *ssif_info, unsigned long *flags)
/* Events available. */
start_event_fetch(ssif_info, flags);
else {
ssif_info->ssif_state = SSIF_NORMAL;
ssif_info->ssif_state = SSIF_IDLE;
ipmi_ssif_unlock_cond(ssif_info, flags);
}
}
@ -530,7 +530,6 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
static void start_get(struct ssif_info *ssif_info)
{
ssif_info->rtc_us_timer = 0;
ssif_info->multi_pos = 0;
ssif_i2c_send(ssif_info, msg_done_handler, I2C_SMBUS_READ,
@ -538,22 +537,28 @@ static void start_get(struct ssif_info *ssif_info)
ssif_info->recv, I2C_SMBUS_BLOCK_DATA);
}
static void start_resend(struct ssif_info *ssif_info);
static void retry_timeout(struct timer_list *t)
{
struct ssif_info *ssif_info = from_timer(ssif_info, t, retry_timer);
unsigned long oflags, *flags;
bool waiting;
bool waiting, resend;
if (ssif_info->stopping)
return;
flags = ipmi_ssif_lock_cond(ssif_info, &oflags);
resend = ssif_info->do_resend;
ssif_info->do_resend = false;
waiting = ssif_info->waiting_alert;
ssif_info->waiting_alert = false;
ipmi_ssif_unlock_cond(ssif_info, flags);
if (waiting)
start_get(ssif_info);
if (resend)
start_resend(ssif_info);
}
static void watch_timeout(struct timer_list *t)
@ -568,7 +573,7 @@ static void watch_timeout(struct timer_list *t)
if (ssif_info->watch_timeout) {
mod_timer(&ssif_info->watch_timer,
jiffies + ssif_info->watch_timeout);
if (SSIF_IDLE(ssif_info)) {
if (IS_SSIF_IDLE(ssif_info)) {
start_flag_fetch(ssif_info, flags); /* Releases lock */
return;
}
@ -602,8 +607,6 @@ static void ssif_alert(struct i2c_client *client, enum i2c_alert_protocol type,
start_get(ssif_info);
}
static int start_resend(struct ssif_info *ssif_info);
static void msg_done_handler(struct ssif_info *ssif_info, int result,
unsigned char *data, unsigned int len)
{
@ -622,7 +625,6 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
flags = ipmi_ssif_lock_cond(ssif_info, &oflags);
ssif_info->waiting_alert = true;
ssif_info->rtc_us_timer = SSIF_MSG_USEC;
if (!ssif_info->stopping)
mod_timer(&ssif_info->retry_timer,
jiffies + SSIF_MSG_JIFFIES);
@ -756,7 +758,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
}
switch (ssif_info->ssif_state) {
case SSIF_NORMAL:
case SSIF_IDLE:
ipmi_ssif_unlock_cond(ssif_info, flags);
if (!msg)
break;
@ -774,7 +776,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
* Error fetching flags, or invalid length,
* just give up for now.
*/
ssif_info->ssif_state = SSIF_NORMAL;
ssif_info->ssif_state = SSIF_IDLE;
ipmi_ssif_unlock_cond(ssif_info, flags);
dev_warn(&ssif_info->client->dev,
"Error getting flags: %d %d, %x\n",
@ -809,7 +811,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
"Invalid response clearing flags: %x %x\n",
data[0], data[1]);
}
ssif_info->ssif_state = SSIF_NORMAL;
ssif_info->ssif_state = SSIF_IDLE;
ipmi_ssif_unlock_cond(ssif_info, flags);
break;
@ -887,7 +889,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
}
flags = ipmi_ssif_lock_cond(ssif_info, &oflags);
if (SSIF_IDLE(ssif_info) && !ssif_info->stopping) {
if (IS_SSIF_IDLE(ssif_info) && !ssif_info->stopping) {
if (ssif_info->req_events)
start_event_fetch(ssif_info, flags);
else if (ssif_info->req_flags)
@ -909,31 +911,23 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
if (result < 0) {
ssif_info->retries_left--;
if (ssif_info->retries_left > 0) {
if (!start_resend(ssif_info)) {
ssif_inc_stat(ssif_info, send_retries);
return;
}
/* request failed, just return the error. */
ssif_inc_stat(ssif_info, send_errors);
if (ssif_info->ssif_debug & SSIF_DEBUG_MSG)
dev_dbg(&ssif_info->client->dev,
"%s: Out of retries\n", __func__);
msg_done_handler(ssif_info, -EIO, NULL, 0);
/*
* Wait the retry timeout time per the spec,
* then redo the send.
*/
ssif_info->do_resend = true;
mod_timer(&ssif_info->retry_timer,
jiffies + SSIF_REQ_RETRY_JIFFIES);
return;
}
ssif_inc_stat(ssif_info, send_errors);
/*
* Got an error on transmit, let the done routine
* handle it.
*/
if (ssif_info->ssif_debug & SSIF_DEBUG_MSG)
dev_dbg(&ssif_info->client->dev,
"%s: Error %d\n", __func__, result);
"%s: Out of retries\n", __func__);
msg_done_handler(ssif_info, result, NULL, 0);
msg_done_handler(ssif_info, -EIO, NULL, 0);
return;
}
@ -987,7 +981,6 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
/* Wait a jiffie then request the next message */
ssif_info->waiting_alert = true;
ssif_info->retries_left = SSIF_RECV_RETRIES;
ssif_info->rtc_us_timer = SSIF_MSG_PART_USEC;
if (!ssif_info->stopping)
mod_timer(&ssif_info->retry_timer,
jiffies + SSIF_MSG_PART_JIFFIES);
@ -996,7 +989,7 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
}
}
static int start_resend(struct ssif_info *ssif_info)
static void start_resend(struct ssif_info *ssif_info)
{
int command;
@ -1021,7 +1014,6 @@ static int start_resend(struct ssif_info *ssif_info)
ssif_i2c_send(ssif_info, msg_written_handler, I2C_SMBUS_WRITE,
command, ssif_info->data, I2C_SMBUS_BLOCK_DATA);
return 0;
}
static int start_send(struct ssif_info *ssif_info,
@ -1036,7 +1028,8 @@ static int start_send(struct ssif_info *ssif_info,
ssif_info->retries_left = SSIF_SEND_RETRIES;
memcpy(ssif_info->data + 1, data, len);
ssif_info->data_len = len;
return start_resend(ssif_info);
start_resend(ssif_info);
return 0;
}
/* Must be called with the message lock held. */
@ -1046,7 +1039,7 @@ static void start_next_msg(struct ssif_info *ssif_info, unsigned long *flags)
unsigned long oflags;
restart:
if (!SSIF_IDLE(ssif_info)) {
if (!IS_SSIF_IDLE(ssif_info)) {
ipmi_ssif_unlock_cond(ssif_info, flags);
return;
}
@ -1269,7 +1262,7 @@ static void shutdown_ssif(void *send_info)
dev_set_drvdata(&ssif_info->client->dev, NULL);
/* make sure the driver is not looking for flags any more. */
while (ssif_info->ssif_state != SSIF_NORMAL)
while (ssif_info->ssif_state != SSIF_IDLE)
schedule_timeout(1);
ssif_info->stopping = true;
@ -1334,8 +1327,10 @@ static int do_cmd(struct i2c_client *client, int len, unsigned char *msg,
ret = i2c_smbus_write_block_data(client, SSIF_IPMI_REQUEST, len, msg);
if (ret) {
retry_cnt--;
if (retry_cnt > 0)
if (retry_cnt > 0) {
msleep(SSIF_REQ_RETRY_MSEC);
goto retry1;
}
return -ENODEV;
}
@ -1476,8 +1471,10 @@ retry_write:
32, msg);
if (ret) {
retry_cnt--;
if (retry_cnt > 0)
if (retry_cnt > 0) {
msleep(SSIF_REQ_RETRY_MSEC);
goto retry_write;
}
dev_err(&client->dev, "Could not write multi-part start, though the BMC said it could handle it. Just limit sends to one part.\n");
return ret;
}
@ -1839,7 +1836,7 @@ static int ssif_probe(struct i2c_client *client)
}
spin_lock_init(&ssif_info->lock);
ssif_info->ssif_state = SSIF_NORMAL;
ssif_info->ssif_state = SSIF_IDLE;
timer_setup(&ssif_info->retry_timer, retry_timeout, 0);
timer_setup(&ssif_info->watch_timer, watch_timeout, 0);