[PATCH 1/2] benet: Wait f/w POST until timeout

[PATCH 1/2] benet: Wait f/w POST until timeout

[Gavin Shan]

While PCI card faces EEH errors, reset (usually hot reset) is
expected to recover from the EEH errors. After EEH core finishes
the reset, the driver callback (be_eeh_reset) is called and wait
the firmware to complete POST successfully. The original code would
return with error once detecting failure during POST stage. That
seems not enough.

The patch forces the driver (be_eeh_reset) to wait the firmware
completes POST until timeout, instead of returning error upon
detection POST failure immediately. Also, it would improve the
reliability of the EEH funtionality of the driver.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c |   27 ++++++++++-----------------
 1 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 071aea7..813407f 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -473,7 +473,7 @@ static int be_mbox_notify_wait(struct be_adapter *adapter)
 	return 0;
 }
 
-static int be_POST_stage_get(struct be_adapter *adapter, u16 *stage)
+static void be_POST_stage_get(struct be_adapter *adapter, u16 *stage)
 {
 	u32 sem;
 	u32 reg = skyhawk_chip(adapter) ? SLIPORT_SEMAPHORE_OFFSET_SH :
@@ -481,11 +481,6 @@ static int be_POST_stage_get(struct be_adapter *adapter, u16 *stage)
 
 	pci_read_config_dword(adapter->pdev, reg, &sem);
 	*stage = sem & POST_STAGE_MASK;
-
-	if ((sem >> POST_ERR_SHIFT) & POST_ERR_MASK)
-		return -1;
-	else
-		return 0;
 }
 
 int lancer_wait_ready(struct be_adapter *adapter)
@@ -579,19 +574,17 @@ int be_fw_wait_ready(struct be_adapter *adapter)
 	}
 
 	do {
-		status = be_POST_stage_get(adapter, &stage);
-		if (status) {
-			dev_err(dev, "POST error; stage=0x%x\n", stage);
-			return -1;
-		} else if (stage != POST_STAGE_ARMFW_RDY) {
-			if (msleep_interruptible(2000)) {
-				dev_err(dev, "Waiting for POST aborted\n");
-				return -EINTR;
-			}
-			timeout += 2;
-		} else {
+		be_POST_stage_get(adapter, &stage);
+		if (stage == POST_STAGE_ARMFW_RDY)
 			return 0;
+
+		dev_info(dev, "Waiting for POST, %ds elapsed\n",
+			 timeout);
+		if (msleep_interruptible(2000)) {
+			dev_err(dev, "Waiting for POST aborted\n");
+			return -EINTR;
 		}
+		timeout += 2;
 	} while (timeout < 60);
 
 	dev_err(dev, "POST timeout; stage=0x%x\n", stage);
-- 
1.7.5.4

[PATCH 2/2] benet: Wait I/O while resuming device

[Gavin Shan]

After resetting the adapter, the config space register (0x7c) might
give fake information to indicate the f/w is ready. In turn, 0xFF's
is always returned while accessing on I/O space registers. The patch
adds more check to make sure the I/O space is ready for access before
accessing that region.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c |   29 +++++++++++++++++++++++++++
 drivers/net/ethernet/emulex/benet/be_cmds.h |    1 +
 drivers/net/ethernet/emulex/benet/be_main.c |    5 ++++
 3 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 813407f..3e8824e 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -696,6 +696,35 @@ static struct be_mcc_wrb *wrb_from_mccq(struct be_adapter *adapter)
 	return wrb;
 }
 
+int be_cmd_fw_wait_io(struct be_adapter *adapter)
+{
+	void __iomem *db = adapter->db + MPU_MAILBOX_DB_OFFSET;
+	int timeout = 0;
+	u32 val;
+
+	if (lancer_chip(adapter))
+		return 0;
+
+	do {
+		val = ioread32(db);
+		if (val != 0xffffffff)
+			return 0;
+
+		dev_info(&adapter->pdev->dev,
+			"Wating for I/O (0x%08x), %ds elapsed\n",
+			val, timeout);
+		if (msleep_interruptible(2000)) {
+			dev_err(&adapter->pdev->dev,
+				"Waiting for I/O aborted\n");
+			return -EIO;
+		}
+		timeout += 2;
+	} while (timeout < 60);
+
+	dev_err(&adapter->pdev->dev, "Timeout waiting for I/O (%d)\n", timeout);
+	return -1;
+}
+
 /* Tell fw we're about to start firing cmds by writing a
  * special pattern across the wrb hdr; uses mbox
  */
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index 9697086..9d03cb1 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -1888,6 +1888,7 @@ int be_cmd_get_flash_crc(struct be_adapter *adapter, u8 *flashed_crc,
 				int offset);
 extern int be_cmd_enable_magic_wol(struct be_adapter *adapter, u8 *mac,
 				struct be_dma_mem *nonemb_cmd);
+extern int be_cmd_fw_wait_io(struct be_adapter *adapter);
 extern int be_cmd_fw_init(struct be_adapter *adapter);
 extern int be_cmd_fw_clean(struct be_adapter *adapter);
 extern void be_async_mcc_enable(struct be_adapter *adapter);
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 3860888..7609424 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4347,6 +4347,11 @@ static void be_eeh_resume(struct pci_dev *pdev)
 
 	pci_save_state(pdev);
 
+	/* Wating for I/O ready */
+	status = be_cmd_fw_wait_io(adapter);
+	if (status)
+		goto err;
+
 	/* tell fw we're ready to fire cmds */
 	status = be_cmd_fw_init(adapter);
 	if (status)
-- 
1.7.5.4

RE: [PATCH 2/2] benet: Wait I/O while resuming device

[Perla, Sathya]

> -----Original Message-----
> From: Gavin Shan [mailto:shangw@linux.vnet.ibm.com]
> 
> After resetting the adapter, the config space register (0x7c) might give fake
> information to indicate the f/w is ready. In turn, 0xFF's is always returned while
> accessing on I/O space registers. The patch adds more check to make sure the
> I/O space is ready for access before accessing that region.
> 
...
> 
> +int be_cmd_fw_wait_io(struct be_adapter *adapter) {
> +	void __iomem *db = adapter->db + MPU_MAILBOX_DB_OFFSET;
> +	int timeout = 0;
> +	u32 val;
> +
> +	if (lancer_chip(adapter))
> +		return 0;
> +
> +	do {
> +		val = ioread32(db);
> +		if (val != 0xffffffff)
> +			return 0;
Wouldn't reading 0xffffffff from a register cause a new EEH flow to be triggered? 

Anyway, the SLIPORT_SEMAPHORE register that returns a valid POST
state *always* is available in the CSR BAR space. This will be for BE2 and BE3
chips. For Skyhawk-R chips config-space/0x7c will work fine.
I can give a patch with these changes in a few days if that works for you....

> +
> +		dev_info(&adapter->pdev->dev,
> +			"Wating for I/O (0x%08x), %ds elapsed\n",
> +			val, timeout);
> +		if (msleep_interruptible(2000)) {
> +			dev_err(&adapter->pdev->dev,
> +				"Waiting for I/O aborted\n");
> +			return -EIO;
> +		}
> +		timeout += 2;
> +	} while (timeout < 60);
> +
> +	dev_err(&adapter->pdev->dev, "Timeout waiting for I/O (%d)\n",
> timeout);
> +	return -1;
> +}
> +

RE: [PATCH 2/2] benet: Wait I/O while resuming device

[Perla, Sathya]

> -----Original Message-----
> From: Gavin Shan [mailto:shangw@linux.vnet.ibm.com]
> 
> On Mon, Mar 04, 2013 at 09:17:22AM +0000, Perla, Sathya wrote:
> >> -----Original Message-----
> >> From: Gavin Shan [mailto:shangw@linux.vnet.ibm.com]
> >>
> >> After resetting the adapter, the config space register (0x7c) might
> >> give fake information to indicate the f/w is ready. In turn, 0xFF's
> >> is always returned while accessing on I/O space registers. The patch
> >> adds more check to make sure the I/O space is ready for access before
> accessing that region.
> >>
> >...
> >>
> >> +int be_cmd_fw_wait_io(struct be_adapter *adapter) {
> >> +	void __iomem *db = adapter->db + MPU_MAILBOX_DB_OFFSET;
> >> +	int timeout = 0;
> >> +	u32 val;
> >> +
> >> +	if (lancer_chip(adapter))
> >> +		return 0;
> >> +
> >> +	do {
> >> +		val = ioread32(db);
> >> +		if (val != 0xffffffff)
> >> +			return 0;
> >Wouldn't reading 0xffffffff from a register cause a new EEH flow to be
> triggered?
> >
> 
> I don't think 0xffffffff here indicates a new EEH error and the adapter was
> recovering from EEH error, which already happened before.

> 
> Actually, I'm developing new feature to support EEH for PowerNV (PPC)
> platform. While the benet adapter detected EEH error and tried to resume, the
> I/O access always returned 0xffffffff even through I could get correct values
> from the config space. Since I don't have the specification, I would guess the
> config space becomes ready prior to I/O space after resetting the adapter and
> its firmware.

The root-cause here is that the config-space/0x7c register is incorrectly
reflecting the POST state to be FW_READY. This causes the driver to go ahead
and read I/O space "prematurely" causing a 0xffffffff read. 
The config-space/0x7c register reflects the correct
value a little later. This happens only for BE2/3 chips.

I propose to fix this by polling instead on CSR-BAR/0xac register for POST status
for BE2/3 chips.

thanks,
-Sathya

RE: [PATCH 2/2] benet: Wait I/O while resuming device

[Perla, Sathya]

> -----Original Message-----
> From: Gavin Shan [mailto:shangw@linux.vnet.ibm.com]
> 
> On Mon, Mar 04, 2013 at 11:11:21AM +0000, Perla, Sathya wrote:
> >> >> After resetting the adapter, the config space register (0x7c)
> >> >> might give fake information to indicate the f/w is ready. In turn,
> >> >> 0xFF's is always returned while accessing on I/O space registers.
> >> >> The patch adds more check to make sure the I/O space is ready for
> >> >> access before
> >> accessing that region.
> >> >>
> >> >...
> >> >>
....
> >> >
> >>
> >> I don't think 0xffffffff here indicates a new EEH error and the
> >> adapter was recovering from EEH error, which already happened before.
> >
> >>
> >> Actually, I'm developing new feature to support EEH for PowerNV (PPC)
> >> platform. While the benet adapter detected EEH error and tried to
> >> resume, the I/O access always returned 0xffffffff even through I
> >> could get correct values from the config space. Since I don't have
> >> the specification, I would guess the config space becomes ready prior
> >> to I/O space after resetting the adapter and its firmware.
> >
> >The root-cause here is that the config-space/0x7c register is
> >incorrectly reflecting the POST state to be FW_READY. This causes the
> >driver to go ahead and read I/O space "prematurely" causing a 0xffffffff read.
> >The config-space/0x7c register reflects the correct value a little
> >later. This happens only for BE2/3 chips.
> >
> 
> Yes, Sathya. It's exactly the case happened on BE3.
> 
> >I propose to fix this by polling instead on CSR-BAR/0xac register for
> >POST status for BE2/3 chips.
> >
> 
> Do you mean to poll CSR-BAR/0xac (adapter->db + 0xac) for POST status? I just
> had one experiment. It seems that CSR-BAR/0xac always returns 0x0 even
> though the firmware have been ready completely. Could you please double-
> check on it?

Gavin, The CSR-BAR is not being mapped in the current code.
That should be done as a part of the fix....

Would it be possible for you to test this patch. 
I have it working on my setup but I feel it still needs some regression testing...

---
 drivers/net/ethernet/emulex/benet/be.h      |    1 +
 drivers/net/ethernet/emulex/benet/be_cmds.c |   10 ++++++----
 drivers/net/ethernet/emulex/benet/be_hw.h   |    4 ++--
 drivers/net/ethernet/emulex/benet/be_main.c |    9 +++++++++
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index 28ceb84..03c638f 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -349,6 +349,7 @@ struct be_adapter {
 	struct pci_dev *pdev;
 	struct net_device *netdev;
 
+	u8 __iomem *csr;
 	u8 __iomem *db;		/* Door Bell */
 
 	struct mutex mbox_lock; /* For serializing mbox cmds to BE card */
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 071aea7..91b4f88 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -476,12 +476,14 @@ static int be_mbox_notify_wait(struct be_adapter *adapter)
 static int be_POST_stage_get(struct be_adapter *adapter, u16 *stage)
 {
 	u32 sem;
-	u32 reg = skyhawk_chip(adapter) ? SLIPORT_SEMAPHORE_OFFSET_SH :
-					  SLIPORT_SEMAPHORE_OFFSET_BE;
 
-	pci_read_config_dword(adapter->pdev, reg, &sem);
-	*stage = sem & POST_STAGE_MASK;
+	if (BEx_chip(adapter))
+		sem  = ioread32(adapter->csr + SLIPORT_SEMAPHORE_OFFSET_BEx);
+	else
+		pci_read_config_dword(adapter->pdev,
+				      SLIPORT_SEMAPHORE_OFFSET_SH, &sem);
 
+	*stage = sem & POST_STAGE_MASK;
 	if ((sem >> POST_ERR_SHIFT) & POST_ERR_MASK)
 		return -1;
 	else
diff --git a/drivers/net/ethernet/emulex/benet/be_hw.h b/drivers/net/ethernet/emulex/benet/be_hw.h
index 541d453..62dc220 100644
--- a/drivers/net/ethernet/emulex/benet/be_hw.h
+++ b/drivers/net/ethernet/emulex/benet/be_hw.h
@@ -32,8 +32,8 @@
 #define MPU_EP_CONTROL 		0
 
 /********** MPU semphore: used for SH & BE  *************/
-#define SLIPORT_SEMAPHORE_OFFSET_BE		0x7c
-#define SLIPORT_SEMAPHORE_OFFSET_SH		0x94
+#define SLIPORT_SEMAPHORE_OFFSET_BEx		0xac  /* CSR BAR offset */
+#define SLIPORT_SEMAPHORE_OFFSET_SH		0x94  /* PCI-CFG offset */
 #define POST_STAGE_MASK				0x0000FFFF
 #define POST_ERR_MASK				0x1
 #define POST_ERR_SHIFT				31
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 3860888..bd0d56f 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -3688,6 +3688,8 @@ static void be_netdev_init(struct net_device *netdev)
 
 static void be_unmap_pci_bars(struct be_adapter *adapter)
 {
+	if (adapter->csr)
+		pci_iounmap(adapter->pdev, adapter->csr);
 	if (adapter->db)
 		pci_iounmap(adapter->pdev, adapter->db);
 }
@@ -3721,6 +3723,13 @@ static int be_map_pci_bars(struct be_adapter *adapter)
 	adapter->if_type = (sli_intf & SLI_INTF_IF_TYPE_MASK) >>
 				SLI_INTF_IF_TYPE_SHIFT;
 
+	if (be_physfn(adapter) && BEx_chip(adapter)) {
+		addr = pci_iomap(adapter->pdev, 2, 0);
+		if (addr == NULL)
+			return -ENOMEM;
+		adapter->csr = addr;
+	}
+
 	addr = pci_iomap(adapter->pdev, db_bar(adapter), 0);
 	if (addr == NULL)
 		goto pci_map_err;
-- 
1.7.1

Re: [PATCH 1/2] benet: Wait f/w POST until timeout

[David Miller]

From: Gavin Shan <shangw@linux.vnet.ibm.com>
Date: Mon,  4 Mar 2013 15:48:46 +0800

> While PCI card faces EEH errors, reset (usually hot reset) is
> expected to recover from the EEH errors. After EEH core finishes
> the reset, the driver callback (be_eeh_reset) is called and wait
> the firmware to complete POST successfully. The original code would
> return with error once detecting failure during POST stage. That
> seems not enough.
> 
> The patch forces the driver (be_eeh_reset) to wait the firmware
> completes POST until timeout, instead of returning error upon
> detection POST failure immediately. Also, it would improve the
> reliability of the EEH funtionality of the driver.
> 
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>

I know that patch #2 of this series needs to be implemented differently,
but this patch seems fine.

So can I get an ACK from one of the benet driver folks?

Thanks.

RE: [PATCH 1/2] benet: Wait f/w POST until timeout

[Perla, Sathya]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> 
> From: Gavin Shan <shangw@linux.vnet.ibm.com>
> Date: Mon,  4 Mar 2013 15:48:46 +0800
> 
> > While PCI card faces EEH errors, reset (usually hot reset) is expected
> > to recover from the EEH errors. After EEH core finishes the reset, the
> > driver callback (be_eeh_reset) is called and wait the firmware to
> > complete POST successfully. The original code would return with error
> > once detecting failure during POST stage. That seems not enough.
> >
> > The patch forces the driver (be_eeh_reset) to wait the firmware
> > completes POST until timeout, instead of returning error upon
> > detection POST failure immediately. Also, it would improve the
> > reliability of the EEH funtionality of the driver.
> >
> > Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> 
> I know that patch #2 of this series needs to be implemented differently, but this
> patch seems fine.
> 
> So can I get an ACK from one of the benet driver folks?

Acked-by: Sathya Perla <sathya.perla@emulex.com>

thanks!

Re: [PATCH 1/2] benet: Wait f/w POST until timeout

[David Miller]

From: "Perla, Sathya" <Sathya.Perla@Emulex.Com>
Date: Wed, 6 Mar 2013 04:45:03 +0000

>> -----Original Message-----
>> From: David Miller [mailto:davem@davemloft.net]
>> 
>> From: Gavin Shan <shangw@linux.vnet.ibm.com>
>> Date: Mon,  4 Mar 2013 15:48:46 +0800
>> 
>> > While PCI card faces EEH errors, reset (usually hot reset) is expected
>> > to recover from the EEH errors. After EEH core finishes the reset, the
>> > driver callback (be_eeh_reset) is called and wait the firmware to
>> > complete POST successfully. The original code would return with error
>> > once detecting failure during POST stage. That seems not enough.
>> >
>> > The patch forces the driver (be_eeh_reset) to wait the firmware
>> > completes POST until timeout, instead of returning error upon
>> > detection POST failure immediately. Also, it would improve the
>> > reliability of the EEH funtionality of the driver.
>> >
>> > Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>> 
>> I know that patch #2 of this series needs to be implemented differently, but this
>> patch seems fine.
>> 
>> So can I get an ACK from one of the benet driver folks?
> 
> Acked-by: Sathya Perla <sathya.perla@emulex.com>

Applied, thanks for reviewing.