[PATCH] switchtec: Remove immediate status check after submit a MRPC command

[PATCH] switchtec: Remove immediate status check after submit a MRPC command

[Wesley.Sheng]

From: Wesley Sheng <Wesley.Sheng@microchip.com>

After submit a Firmware Download (Download sub-command)
MRPC command, switchtec firmware refuses to response
any management EP's BAR access until current flash
programming finished.

During this time, a READ TLP to the gas area in the BAR of
the management EP will complete with significant delay like
more than 10ms.

It's a switchtec firmware limitation that READ requests
cannot get prompt service during firmware download.

The delayed completion of READ TLP would be a problem on
some system which is sensitive to READ timeout.

Current driver check status immediately after submit a
MRPC command, which triggers READ TLP to the gas area in
the BAR of the management EP. Also, other processes or
functions, like NTB, would also trigger READ TLP by accessing
the GAS.

To avoid this, the immediate check of status is removed
in this patch, and driver delays the status check to the
occurrence of MSIx or MRPC timeout. In the meanwhile, user
must not initiate any gas access during a firmware download.

Also, any process that issues MRPC command will be affected
by the delay in this patch.

However, this is only a software workaround to the READ
issue in firmware download. A complete fix of this should
happen in firmware.

Note: For NTB function, the memory window access is handled
by switchtec hardware. So it's not affected by this firmware
limitation. But the GAS accesses, like for doorbell registers,
in the NTB function are affected by this firmware limitation.

Signed-off-by: Kelvin Cao <Kelvin.Cao@microchip.com>
---
 drivers/pci/switch/switchtec.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 4591f15..b759228 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -142,10 +142,6 @@ static void mrpc_cmd_submit(struct switchtec_dev *stdev)
 		    stuser->data, stuser->data_len);
 	iowrite32(stuser->cmd, &stdev->mmio_mrpc->cmd);
 
-	stuser->status = ioread32(&stdev->mmio_mrpc->status);
-	if (stuser->status != SWITCHTEC_MRPC_STATUS_INPROGRESS)
-		mrpc_complete_cmd(stdev);
-
 	schedule_delayed_work(&stdev->mrpc_timeout,
 			      msecs_to_jiffies(500));
 }
-- 
2.7.4

Re: [PATCH] switchtec: Remove immediate status check after submit a MRPC command

[Logan Gunthorpe]

On 13/09/18 12:43 AM, Wesley.Sheng@microchip.com wrote:
> From: Wesley Sheng <Wesley.Sheng@microchip.com>

You've done something wrong here. The sign-off line below says its
signed off by Kelvin, who was the original author (on the github repo)
but the From: line indicates you were the author. Also, please start
using 'git send-email' as your patches are being mangled by your email
client.

> To avoid this, the immediate check of status is removed
> in this patch, and driver delays the status check to the
> occurrence of MSIx or MRPC timeout. In the meanwhile, user
> must not initiate any gas access during a firmware download.

s/meanwhile/meantime/

> Note: For NTB function, the memory window access is handled
> by switchtec hardware. So it's not affected by this firmware
> limitation. But the GAS accesses, like for doorbell registers,
> in the NTB function are affected by this firmware limitation.
> 
> Signed-off-by: Kelvin Cao <Kelvin.Cao@microchip.com>

We also need a Signed-off-by from you. Anyone who handled the patch
needs to sign off on it.

At least for the contents of the changes, modulo the above fixes:

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

> ---
>  drivers/pci/switch/switchtec.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 4591f15..b759228 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -142,10 +142,6 @@ static void mrpc_cmd_submit(struct switchtec_dev *stdev)
>  		    stuser->data, stuser->data_len);
>  	iowrite32(stuser->cmd, &stdev->mmio_mrpc->cmd);
>  
> -	stuser->status = ioread32(&stdev->mmio_mrpc->status);
> -	if (stuser->status != SWITCHTEC_MRPC_STATUS_INPROGRESS)
> -		mrpc_complete_cmd(stdev);
> -
>  	schedule_delayed_work(&stdev->mrpc_timeout,
>  			      msecs_to_jiffies(500));
>  }
>