diff mbox

IB/hfi1: Fetch monitor values on-demand for CableInfo query

Message ID 1470755838-14117-1-git-send-email-ira.weiny@intel.com (mailing list archive)
State Accepted
Headers show

Commit Message

Ira Weiny Aug. 9, 2016, 3:17 p.m. UTC 
From: Easwar Hariharan <easwar.hariharan@intel.com>

The monitor values from bytes 22 through 81 of the QSFP memory space
(SFF 8636) are dynamic and serving them out of the QSFP memory cache
maintained by the driver provides stale data to the CableInfo SMA query.
This patch refreshes the dynamic values from the QSFP memory on request
and overwrites the stale data from the cache for the overlap between the
requested range and the monitor range.


Reviewed-by: Jubin John <jubin.john@intel.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Easwar Hariharan <easwar.hariharan@intel.com>
---
 drivers/infiniband/hw/hfi1/qsfp.c | 32 ++++++++++++++++++++++++++++++--
 drivers/infiniband/hw/hfi1/qsfp.h |  3 +++
 2 files changed, 33 insertions(+), 2 deletions(-)

Comments

Leon Romanovsky Aug. 9, 2016, 5:58 p.m. UTC | #1 
On Tue, Aug 09, 2016 at 11:17:18AM -0400, ira.weiny@intel.com wrote:
> From: Easwar Hariharan <easwar.hariharan@intel.com>
> 
> The monitor values from bytes 22 through 81 of the QSFP memory space
> (SFF 8636) are dynamic and serving them out of the QSFP memory cache
> maintained by the driver provides stale data to the CableInfo SMA query.
> This patch refreshes the dynamic values from the QSFP memory on request
> and overwrites the stale data from the cache for the overlap between the
> requested range and the monitor range.
> 
> 

Ira,
On all your patches, you have double line between commit message and
tags.

> Reviewed-by: Jubin John <jubin.john@intel.com>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Easwar Hariharan <easwar.hariharan@intel.com>
Ira Weiny Aug. 10, 2016, 5:35 a.m. UTC | #2 
On Tue, Aug 09, 2016 at 08:58:01PM +0300, Leon Romanovsky wrote:
> On Tue, Aug 09, 2016 at 11:17:18AM -0400, ira.weiny@intel.com wrote:
> > From: Easwar Hariharan <easwar.hariharan@intel.com>
> > 
> > The monitor values from bytes 22 through 81 of the QSFP memory space
> > (SFF 8636) are dynamic and serving them out of the QSFP memory cache
> > maintained by the driver provides stale data to the CableInfo SMA query.
> > This patch refreshes the dynamic values from the QSFP memory on request
> > and overwrites the stale data from the cache for the overlap between the
> > requested range and the monitor range.
> > 
> > 
> 
> Ira,
> On all your patches, you have double line between commit message and
> tags.

Does this cause a problem?

Ira

> 
> > Reviewed-by: Jubin John <jubin.john@intel.com>
> > Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Easwar Hariharan <easwar.hariharan@intel.com>


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Aug. 10, 2016, 5:51 a.m. UTC | #3 
On Wed, Aug 10, 2016 at 01:35:50AM -0400, ira.weiny wrote:
> On Tue, Aug 09, 2016 at 08:58:01PM +0300, Leon Romanovsky wrote:
> > On Tue, Aug 09, 2016 at 11:17:18AM -0400, ira.weiny@intel.com wrote:
> > > From: Easwar Hariharan <easwar.hariharan@intel.com>
> > > 
> > > The monitor values from bytes 22 through 81 of the QSFP memory space
> > > (SFF 8636) are dynamic and serving them out of the QSFP memory cache
> > > maintained by the driver provides stale data to the CableInfo SMA query.
> > > This patch refreshes the dynamic values from the QSFP memory on request
> > > and overwrites the stale data from the cache for the overlap between the
> > > requested range and the monitor range.
> > > 
> > > 
> > 
> > Ira,
> > On all your patches, you have double line between commit message and
> > tags.
> 
> Does this cause a problem?

Problem - no, inconvenience - yes.
It doesn't look pretty in git history.

> 
> Ira
> 
> > 
> > > Reviewed-by: Jubin John <jubin.john@intel.com>
> > > Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> > > Signed-off-by: Easwar Hariharan <easwar.hariharan@intel.com>
> 
>
Leon Romanovsky Aug. 10, 2016, 10:19 a.m. UTC | #4 
On Wed, Aug 10, 2016 at 01:35:50AM -0400, ira.weiny wrote:
> On Tue, Aug 09, 2016 at 08:58:01PM +0300, Leon Romanovsky wrote:
> > On Tue, Aug 09, 2016 at 11:17:18AM -0400, ira.weiny@intel.com wrote:
> > > From: Easwar Hariharan <easwar.hariharan@intel.com>
> > > 
> > > The monitor values from bytes 22 through 81 of the QSFP memory space
> > > (SFF 8636) are dynamic and serving them out of the QSFP memory cache
> > > maintained by the driver provides stale data to the CableInfo SMA query.
> > > This patch refreshes the dynamic values from the QSFP memory on request
> > > and overwrites the stale data from the cache for the overlap between the
> > > requested range and the monitor range.
> > > 
> > > 
> > 
> > Ira,
> > On all your patches, you have double line between commit message and
> > tags.
> 
> Does this cause a problem?

Problem - no, inconvenience - yes.
It doesn't look good in git history.

> 
> Ira
> 
> > 
> > > Reviewed-by: Jubin John <jubin.john@intel.com>
> > > Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> > > Signed-off-by: Easwar Hariharan <easwar.hariharan@intel.com>
> 
>
Dennis Dalessandro Aug. 10, 2016, 7:07 p.m. UTC | #5 
On Wed, 2016-08-10 at 08:51 +0300, Leon Romanovsky wrote:
> On Wed, Aug 10, 2016 at 01:35:50AM -0400, ira.weiny wrote:

> > On Tue, Aug 09, 2016 at 08:58:01PM +0300, Leon Romanovsky wrote:

> > > On Tue, Aug 09, 2016 at 11:17:18AM -0400, ira.weiny@intel.com

> > > wrote:

> > > > From: Easwar Hariharan <easwar.hariharan@intel.com>

> > > > 

> > > > The monitor values from bytes 22 through 81 of the QSFP memory

> > > > space

> > > > (SFF 8636) are dynamic and serving them out of the QSFP memory

> > > > cache

> > > > maintained by the driver provides stale data to the CableInfo

> > > > SMA query.

> > > > This patch refreshes the dynamic values from the QSFP memory on

> > > > request

> > > > and overwrites the stale data from the cache for the overlap

> > > > between the

> > > > requested range and the monitor range.

> > > > 

> > > > 

> > > 

> > > Ira,

> > > On all your patches, you have double line between commit message

> > > and

> > > tags.

> > 

> > Does this cause a problem?

> 

> Problem - no, inconvenience - yes.

> It doesn't look pretty in git history.


We'll catch it next time? Not worth resubmitting I don't think.

-Denny
Leon Romanovsky Aug. 11, 2016, 5:07 a.m. UTC | #6 
On Wed, Aug 10, 2016 at 07:07:12PM +0000, Dalessandro, Dennis wrote:
> On Wed, 2016-08-10 at 08:51 +0300, Leon Romanovsky wrote:
> > On Wed, Aug 10, 2016 at 01:35:50AM -0400, ira.weiny wrote:
> > > On Tue, Aug 09, 2016 at 08:58:01PM +0300, Leon Romanovsky wrote:
> > > > On Tue, Aug 09, 2016 at 11:17:18AM -0400, ira.weiny@intel.com
> > > > wrote:
> > > > > From: Easwar Hariharan <easwar.hariharan@intel.com>
> > > > > 
> > > > > The monitor values from bytes 22 through 81 of the QSFP memory
> > > > > space
> > > > > (SFF 8636) are dynamic and serving them out of the QSFP memory
> > > > > cache
> > > > > maintained by the driver provides stale data to the CableInfo
> > > > > SMA query.
> > > > > This patch refreshes the dynamic values from the QSFP memory on
> > > > > request
> > > > > and overwrites the stale data from the cache for the overlap
> > > > > between the
> > > > > requested range and the monitor range.
> > > > > 
> > > > > 
> > > > 
> > > > Ira,
> > > > On all your patches, you have double line between commit message
> > > > and
> > > > tags.
> > > 
> > > Does this cause a problem?
> > 
> > Problem - no, inconvenience - yes.
> > It doesn't look pretty in git history.
> 
> We'll catch it next time? Not worth resubmitting I don't think.

Sure, no problems.

> 
> -Denny
Doug Ledford Aug. 22, 2016, 6:22 p.m. UTC | #7 
On 8/9/2016 11:17 AM, ira.weiny@intel.com wrote:
> From: Easwar Hariharan <easwar.hariharan@intel.com>
> 
> The monitor values from bytes 22 through 81 of the QSFP memory space
> (SFF 8636) are dynamic and serving them out of the QSFP memory cache
> maintained by the driver provides stale data to the CableInfo SMA query.
> This patch refreshes the dynamic values from the QSFP memory on request
> and overwrites the stale data from the cache for the overlap between the
> requested range and the monitor range.
> 
> 
> Reviewed-by: Jubin John <jubin.john@intel.com>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Easwar Hariharan <easwar.hariharan@intel.com>

Thanks, applied.
diff mbox

Patch

diff --git a/drivers/infiniband/hw/hfi1/qsfp.c b/drivers/infiniband/hw/hfi1/qsfp.c
index a207717ade2a..4e95ad810847 100644
--- a/drivers/infiniband/hw/hfi1/qsfp.c
+++ b/drivers/infiniband/hw/hfi1/qsfp.c
@@ -706,8 +706,8 @@  int get_cable_info(struct hfi1_devdata *dd, u32 port_num, u32 addr, u32 len,
 		   u8 *data)
 {
 	struct hfi1_pportdata *ppd;
-	u32 excess_len = 0;
-	int ret = 0;
+	u32 excess_len = len;
+	int ret = 0, offset = 0;
 
 	if (port_num > dd->num_pports || port_num < 1) {
 		dd_dev_info(dd, "%s: Invalid port number %d\n",
@@ -740,6 +740,34 @@  int get_cable_info(struct hfi1_devdata *dd, u32 port_num, u32 addr, u32 len,
 	}
 
 	memcpy(data, &ppd->qsfp_info.cache[addr], len);
+
+	if (addr <= QSFP_MONITOR_VAL_END &&
+	    (addr + len) >= QSFP_MONITOR_VAL_START) {
+		/* Overlap with the dynamic channel monitor range */
+		if (addr < QSFP_MONITOR_VAL_START) {
+			if (addr + len <= QSFP_MONITOR_VAL_END)
+				len = addr + len - QSFP_MONITOR_VAL_START;
+			else
+				len = QSFP_MONITOR_RANGE;
+			offset = QSFP_MONITOR_VAL_START - addr;
+			addr = QSFP_MONITOR_VAL_START;
+		} else if (addr == QSFP_MONITOR_VAL_START) {
+			offset = 0;
+			if (addr + len > QSFP_MONITOR_VAL_END)
+				len = QSFP_MONITOR_RANGE;
+		} else {
+			offset = 0;
+			if (addr + len > QSFP_MONITOR_VAL_END)
+				len = QSFP_MONITOR_VAL_END - addr + 1;
+		}
+		/* Refresh the values of the dynamic monitors from the cable */
+		ret = one_qsfp_read(ppd, dd->hfi1_id, addr, data + offset, len);
+		if (ret != len) {
+			ret = -EAGAIN;
+			goto set_zeroes;
+		}
+	}
+
 	return 0;
 
 set_zeroes:
diff --git a/drivers/infiniband/hw/hfi1/qsfp.h b/drivers/infiniband/hw/hfi1/qsfp.h
index 69275ebd9597..36cf52359848 100644
--- a/drivers/infiniband/hw/hfi1/qsfp.h
+++ b/drivers/infiniband/hw/hfi1/qsfp.h
@@ -74,6 +74,9 @@ 
 /* Defined fields that Intel requires of qualified cables */
 /* Byte 0 is Identifier, not checked */
 /* Byte 1 is reserved "status MSB" */
+#define QSFP_MONITOR_VAL_START 22
+#define QSFP_MONITOR_VAL_END 81
+#define QSFP_MONITOR_RANGE (QSFP_MONITOR_VAL_END - QSFP_MONITOR_VAL_START + 1)
 #define QSFP_TX_CTRL_BYTE_OFFS 86
 #define QSFP_PWR_CTRL_BYTE_OFFS 93
 #define QSFP_CDR_CTRL_BYTE_OFFS 98