| Message ID | 1561576395-6429-1-git-send-email-eajames@linux.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | OCC: FSI and hwmon: Add sequence numbering | expand |
On Wed, Jun 26, 2019 at 02:13:15PM -0500, Eddie James wrote: > Sequence numbering of the commands submitted to the OCC is required by > the OCC interface specification. Add sequence numbering and check for > the correct sequence number on the response. > > Signed-off-by: Eddie James <eajames@linux.ibm.com> For hwmon: Acked-by: Guenter Roeck <linux@roeck-us.net> I assume this will be pushed through drivers/fsi. Guenter > --- > drivers/fsi/fsi-occ.c | 15 ++++++++++++--- > drivers/hwmon/occ/common.c | 4 ++-- > drivers/hwmon/occ/common.h | 1 + > 3 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c > index a2301ce..7da9c81 100644 > --- a/drivers/fsi/fsi-occ.c > +++ b/drivers/fsi/fsi-occ.c > @@ -412,6 +412,7 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, > msecs_to_jiffies(OCC_CMD_IN_PRG_WAIT_MS); > struct occ *occ = dev_get_drvdata(dev); > struct occ_response *resp = response; > + u8 seq_no; > u16 resp_data_length; > unsigned long start; > int rc; > @@ -426,6 +427,8 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, > > mutex_lock(&occ->occ_lock); > > + /* Extract the seq_no from the command (first byte) */ > + seq_no = *(const u8 *)request; > rc = occ_putsram(occ, OCC_SRAM_CMD_ADDR, request, req_len); > if (rc) > goto done; > @@ -441,11 +444,17 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, > if (rc) > goto done; > > - if (resp->return_status == OCC_RESP_CMD_IN_PRG) { > + if (resp->return_status == OCC_RESP_CMD_IN_PRG || > + resp->seq_no != seq_no) { > rc = -ETIMEDOUT; > > - if (time_after(jiffies, start + timeout)) > - break; > + if (time_after(jiffies, start + timeout)) { > + dev_err(occ->dev, "resp timeout status=%02x " > + "resp seq_no=%d our seq_no=%d\n", > + resp->return_status, resp->seq_no, > + seq_no); > + goto done; > + } > > set_current_state(TASK_UNINTERRUPTIBLE); > schedule_timeout(wait_time); > diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c > index d593517..a7d2b16 100644 > --- a/drivers/hwmon/occ/common.c > +++ b/drivers/hwmon/occ/common.c > @@ -124,12 +124,12 @@ struct extended_sensor { > static int occ_poll(struct occ *occ) > { > int rc; > - u16 checksum = occ->poll_cmd_data + 1; > + u16 checksum = occ->poll_cmd_data + occ->seq_no + 1; > u8 cmd[8]; > struct occ_poll_response_header *header; > > /* big endian */ > - cmd[0] = 0; /* sequence number */ > + cmd[0] = occ->seq_no++; /* sequence number */ > cmd[1] = 0; /* cmd type */ > cmd[2] = 0; /* data length msb */ > cmd[3] = 1; /* data length lsb */ > diff --git a/drivers/hwmon/occ/common.h b/drivers/hwmon/occ/common.h > index fc13f3c..67e6968 100644 > --- a/drivers/hwmon/occ/common.h > +++ b/drivers/hwmon/occ/common.h > @@ -95,6 +95,7 @@ struct occ { > struct occ_sensors sensors; > > int powr_sample_time_us; /* average power sample time */ > + u8 seq_no; > u8 poll_cmd_data; /* to perform OCC poll command */ > int (*send_cmd)(struct occ *occ, u8 *cmd); > > -- > 1.8.3.1 >
On Wed, 26 Jun 2019 at 19:41, Guenter Roeck <linux@roeck-us.net> wrote: > > On Wed, Jun 26, 2019 at 02:13:15PM -0500, Eddie James wrote: > > Sequence numbering of the commands submitted to the OCC is required by > > the OCC interface specification. Add sequence numbering and check for > > the correct sequence number on the response. > > > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > > For hwmon: > > Acked-by: Guenter Roeck <linux@roeck-us.net> > > I assume this will be pushed through drivers/fsi. Yes. Eddie, can you please collect the acks and send to Greg? Cheers, Joel > > Guenter > > > --- > > drivers/fsi/fsi-occ.c | 15 ++++++++++++--- > > drivers/hwmon/occ/common.c | 4 ++-- > > drivers/hwmon/occ/common.h | 1 + > > 3 files changed, 15 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c > > index a2301ce..7da9c81 100644 > > --- a/drivers/fsi/fsi-occ.c > > +++ b/drivers/fsi/fsi-occ.c > > @@ -412,6 +412,7 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, > > msecs_to_jiffies(OCC_CMD_IN_PRG_WAIT_MS); > > struct occ *occ = dev_get_drvdata(dev); > > struct occ_response *resp = response; > > + u8 seq_no; > > u16 resp_data_length; > > unsigned long start; > > int rc; > > @@ -426,6 +427,8 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, > > > > mutex_lock(&occ->occ_lock); > > > > + /* Extract the seq_no from the command (first byte) */ > > + seq_no = *(const u8 *)request; > > rc = occ_putsram(occ, OCC_SRAM_CMD_ADDR, request, req_len); > > if (rc) > > goto done; > > @@ -441,11 +444,17 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, > > if (rc) > > goto done; > > > > - if (resp->return_status == OCC_RESP_CMD_IN_PRG) { > > + if (resp->return_status == OCC_RESP_CMD_IN_PRG || > > + resp->seq_no != seq_no) { > > rc = -ETIMEDOUT; > > > > - if (time_after(jiffies, start + timeout)) > > - break; > > + if (time_after(jiffies, start + timeout)) { > > + dev_err(occ->dev, "resp timeout status=%02x " > > + "resp seq_no=%d our seq_no=%d\n", > > + resp->return_status, resp->seq_no, > > + seq_no); > > + goto done; > > + } > > > > set_current_state(TASK_UNINTERRUPTIBLE); > > schedule_timeout(wait_time); > > diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c > > index d593517..a7d2b16 100644 > > --- a/drivers/hwmon/occ/common.c > > +++ b/drivers/hwmon/occ/common.c > > @@ -124,12 +124,12 @@ struct extended_sensor { > > static int occ_poll(struct occ *occ) > > { > > int rc; > > - u16 checksum = occ->poll_cmd_data + 1; > > + u16 checksum = occ->poll_cmd_data + occ->seq_no + 1; > > u8 cmd[8]; > > struct occ_poll_response_header *header; > > > > /* big endian */ > > - cmd[0] = 0; /* sequence number */ > > + cmd[0] = occ->seq_no++; /* sequence number */ > > cmd[1] = 0; /* cmd type */ > > cmd[2] = 0; /* data length msb */ > > cmd[3] = 1; /* data length lsb */ > > diff --git a/drivers/hwmon/occ/common.h b/drivers/hwmon/occ/common.h > > index fc13f3c..67e6968 100644 > > --- a/drivers/hwmon/occ/common.h > > +++ b/drivers/hwmon/occ/common.h > > @@ -95,6 +95,7 @@ struct occ { > > struct occ_sensors sensors; > > > > int powr_sample_time_us; /* average power sample time */ > > + u8 seq_no; > > u8 poll_cmd_data; /* to perform OCC poll command */ > > int (*send_cmd)(struct occ *occ, u8 *cmd); > > > > -- > > 1.8.3.1 > >
diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c index a2301ce..7da9c81 100644 --- a/drivers/fsi/fsi-occ.c +++ b/drivers/fsi/fsi-occ.c @@ -412,6 +412,7 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, msecs_to_jiffies(OCC_CMD_IN_PRG_WAIT_MS); struct occ *occ = dev_get_drvdata(dev); struct occ_response *resp = response; + u8 seq_no; u16 resp_data_length; unsigned long start; int rc; @@ -426,6 +427,8 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, mutex_lock(&occ->occ_lock); + /* Extract the seq_no from the command (first byte) */ + seq_no = *(const u8 *)request; rc = occ_putsram(occ, OCC_SRAM_CMD_ADDR, request, req_len); if (rc) goto done; @@ -441,11 +444,17 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, if (rc) goto done; - if (resp->return_status == OCC_RESP_CMD_IN_PRG) { + if (resp->return_status == OCC_RESP_CMD_IN_PRG || + resp->seq_no != seq_no) { rc = -ETIMEDOUT; - if (time_after(jiffies, start + timeout)) - break; + if (time_after(jiffies, start + timeout)) { + dev_err(occ->dev, "resp timeout status=%02x " + "resp seq_no=%d our seq_no=%d\n", + resp->return_status, resp->seq_no, + seq_no); + goto done; + } set_current_state(TASK_UNINTERRUPTIBLE); schedule_timeout(wait_time); diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c index d593517..a7d2b16 100644 --- a/drivers/hwmon/occ/common.c +++ b/drivers/hwmon/occ/common.c @@ -124,12 +124,12 @@ struct extended_sensor { static int occ_poll(struct occ *occ) { int rc; - u16 checksum = occ->poll_cmd_data + 1; + u16 checksum = occ->poll_cmd_data + occ->seq_no + 1; u8 cmd[8]; struct occ_poll_response_header *header; /* big endian */ - cmd[0] = 0; /* sequence number */ + cmd[0] = occ->seq_no++; /* sequence number */ cmd[1] = 0; /* cmd type */ cmd[2] = 0; /* data length msb */ cmd[3] = 1; /* data length lsb */ diff --git a/drivers/hwmon/occ/common.h b/drivers/hwmon/occ/common.h index fc13f3c..67e6968 100644 --- a/drivers/hwmon/occ/common.h +++ b/drivers/hwmon/occ/common.h @@ -95,6 +95,7 @@ struct occ { struct occ_sensors sensors; int powr_sample_time_us; /* average power sample time */ + u8 seq_no; u8 poll_cmd_data; /* to perform OCC poll command */ int (*send_cmd)(struct occ *occ, u8 *cmd);
Sequence numbering of the commands submitted to the OCC is required by the OCC interface specification. Add sequence numbering and check for the correct sequence number on the response. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- drivers/fsi/fsi-occ.c | 15 ++++++++++++--- drivers/hwmon/occ/common.c | 4 ++-- drivers/hwmon/occ/common.h | 1 + 3 files changed, 15 insertions(+), 5 deletions(-)