linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] OCC: FSI and hwmon: Add sequence numbering
@ 2019-06-26 19:13 Eddie James
  2019-06-26 19:40 ` Guenter Roeck
  0 siblings, 1 reply; 4+ messages in thread
From: Eddie James @ 2019-06-26 19:13 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-hwmon, linux, jdelvare, mine260309, Eddie James

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

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] OCC: FSI and hwmon: Add sequence numbering
  2019-06-26 19:13 [PATCH] OCC: FSI and hwmon: Add sequence numbering Eddie James
@ 2019-06-26 19:40 ` Guenter Roeck
  2019-07-02  3:44   ` Joel Stanley
  0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2019-06-26 19:40 UTC (permalink / raw)
  To: Eddie James; +Cc: linux-kernel, linux-hwmon, jdelvare, mine260309

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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] OCC: FSI and hwmon: Add sequence numbering
  2019-06-26 19:40 ` Guenter Roeck
@ 2019-07-02  3:44   ` Joel Stanley
  0 siblings, 0 replies; 4+ messages in thread
From: Joel Stanley @ 2019-07-02  3:44 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Eddie James, Linux Kernel Mailing List, linux-hwmon,
	Jean Delvare, Lei YU

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH] OCC: FSI and hwmon: Add sequence numbering
@ 2019-07-02 15:47 Eddie James
  0 siblings, 0 replies; 4+ messages in thread
From: Eddie James @ 2019-07-02 15:47 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, joel, linux, mine260309, Eddie James

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>
Acked-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Lei YU <mine260309@gmail.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(-)

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-02 15:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-26 19:13 [PATCH] OCC: FSI and hwmon: Add sequence numbering Eddie James
2019-06-26 19:40 ` Guenter Roeck
2019-07-02  3:44   ` Joel Stanley
2019-07-02 15:47 Eddie James

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).