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

[Guenter Roeck <linux@roeck-us.net>]

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
>