Re: [PATCH] OCC: FSI and hwmon: Add sequence numbering

[Joel Stanley <joel@jms.id.au>]

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
> >