[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

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

[Wesley.Sheng]

RnJvbTogV2VzbGV5IFNoZW5nIDxXZXNsZXkuU2hlbmdAbWljcm9jaGlwLmNvbT4NCg0KQWZ0ZXIg
c3VibWl0IGEgRmlybXdhcmUgRG93bmxvYWQgKERvd25sb2FkIHN1Yi1jb21tYW5kKQ0KTVJQQyBj
b21tYW5kLCBzd2l0Y2h0ZWMgZmlybXdhcmUgcmVmdXNlcyB0byByZXNwb25zZQ0KYW55IG1hbmFn
ZW1lbnQgRVAncyBCQVIgYWNjZXNzIHVudGlsIGN1cnJlbnQgZmxhc2gNCnByb2dyYW1taW5nIGZp
bmlzaGVkLg0KDQpEdXJpbmcgdGhpcyB0aW1lLCBhIFJFQUQgVExQIHRvIHRoZSBnYXMgYXJlYSBp
biB0aGUgQkFSIG9mDQp0aGUgbWFuYWdlbWVudCBFUCB3aWxsIGNvbXBsZXRlIHdpdGggc2lnbmlm
aWNhbnQgZGVsYXkgbGlrZQ0KbW9yZSB0aGFuIDEwbXMuDQoNCkl0J3MgYSBzd2l0Y2h0ZWMgZmly
bXdhcmUgbGltaXRhdGlvbiB0aGF0IFJFQUQgcmVxdWVzdHMNCmNhbm5vdCBnZXQgcHJvbXB0IHNl
cnZpY2UgZHVyaW5nIGZpcm13YXJlIGRvd25sb2FkLg0KDQpUaGUgZGVsYXllZCBjb21wbGV0aW9u
IG9mIFJFQUQgVExQIHdvdWxkIGJlIGEgcHJvYmxlbSBvbg0Kc29tZSBzeXN0ZW0gd2hpY2ggaXMg
c2Vuc2l0aXZlIHRvIFJFQUQgdGltZW91dC4NCg0KQ3VycmVudCBkcml2ZXIgY2hlY2sgc3RhdHVz
IGltbWVkaWF0ZWx5IGFmdGVyIHN1Ym1pdCBhDQpNUlBDIGNvbW1hbmQsIHdoaWNoIHRyaWdnZXJz
IFJFQUQgVExQIHRvIHRoZSBnYXMgYXJlYSBpbg0KdGhlIEJBUiBvZiB0aGUgbWFuYWdlbWVudCBF
UC4gQWxzbywgb3RoZXIgcHJvY2Vzc2VzIG9yDQpmdW5jdGlvbnMsIGxpa2UgTlRCLCB3b3VsZCBh
bHNvIHRyaWdnZXIgUkVBRCBUTFAgYnkgYWNjZXNzaW5nDQp0aGUgR0FTLg0KDQpUbyBhdm9pZCB0
aGlzLCB0aGUgaW1tZWRpYXRlIGNoZWNrIG9mIHN0YXR1cyBpcyByZW1vdmVkDQppbiB0aGlzIHBh
dGNoLCBhbmQgZHJpdmVyIGRlbGF5cyB0aGUgc3RhdHVzIGNoZWNrIHRvIHRoZQ0Kb2NjdXJyZW5j
ZSBvZiBNU0l4IG9yIE1SUEMgdGltZW91dC4gSW4gdGhlIG1lYW53aGlsZSwgdXNlcg0KbXVzdCBu
b3QgaW5pdGlhdGUgYW55IGdhcyBhY2Nlc3MgZHVyaW5nIGEgZmlybXdhcmUgZG93bmxvYWQuDQoN
CkFsc28sIGFueSBwcm9jZXNzIHRoYXQgaXNzdWVzIE1SUEMgY29tbWFuZCB3aWxsIGJlIGFmZmVj
dGVkDQpieSB0aGUgZGVsYXkgaW4gdGhpcyBwYXRjaC4NCg0KSG93ZXZlciwgdGhpcyBpcyBvbmx5
IGEgc29mdHdhcmUgd29ya2Fyb3VuZCB0byB0aGUgUkVBRA0KaXNzdWUgaW4gZmlybXdhcmUgZG93
bmxvYWQuIEEgY29tcGxldGUgZml4IG9mIHRoaXMgc2hvdWxkDQpoYXBwZW4gaW4gZmlybXdhcmUu
DQoNCk5vdGU6IEZvciBOVEIgZnVuY3Rpb24sIHRoZSBtZW1vcnkgd2luZG93IGFjY2VzcyBpcyBo
YW5kbGVkDQpieSBzd2l0Y2h0ZWMgaGFyZHdhcmUuIFNvIGl0J3Mgbm90IGFmZmVjdGVkIGJ5IHRo
aXMgZmlybXdhcmUNCmxpbWl0YXRpb24uIEJ1dCB0aGUgR0FTIGFjY2Vzc2VzLCBsaWtlIGZvciBk
b29yYmVsbCByZWdpc3RlcnMsDQppbiB0aGUgTlRCIGZ1bmN0aW9uIGFyZSBhZmZlY3RlZCBieSB0
aGlzIGZpcm13YXJlIGxpbWl0YXRpb24uDQoNClNpZ25lZC1vZmYtYnk6IEtlbHZpbiBDYW8gPEtl
bHZpbi5DYW9AbWljcm9jaGlwLmNvbT4NCi0tLQ0KIGRyaXZlcnMvcGNpL3N3aXRjaC9zd2l0Y2h0
ZWMuYyB8IDQgLS0tLQ0KIDEgZmlsZSBjaGFuZ2VkLCA0IGRlbGV0aW9ucygtKQ0KDQpkaWZmIC0t
Z2l0IGEvZHJpdmVycy9wY2kvc3dpdGNoL3N3aXRjaHRlYy5jIGIvZHJpdmVycy9wY2kvc3dpdGNo
L3N3aXRjaHRlYy5jDQppbmRleCA0NTkxZjE1Li5iNzU5MjI4IDEwMDY0NA0KLS0tIGEvZHJpdmVy
cy9wY2kvc3dpdGNoL3N3aXRjaHRlYy5jDQorKysgYi9kcml2ZXJzL3BjaS9zd2l0Y2gvc3dpdGNo
dGVjLmMNCkBAIC0xNDIsMTAgKzE0Miw2IEBAIHN0YXRpYyB2b2lkIG1ycGNfY21kX3N1Ym1pdChz
dHJ1Y3Qgc3dpdGNodGVjX2RldiAqc3RkZXYpDQogCQkgICAgc3R1c2VyLT5kYXRhLCBzdHVzZXIt
PmRhdGFfbGVuKTsNCiAJaW93cml0ZTMyKHN0dXNlci0+Y21kLCAmc3RkZXYtPm1taW9fbXJwYy0+
Y21kKTsNCiANCi0Jc3R1c2VyLT5zdGF0dXMgPSBpb3JlYWQzMigmc3RkZXYtPm1taW9fbXJwYy0+
c3RhdHVzKTsNCi0JaWYgKHN0dXNlci0+c3RhdHVzICE9IFNXSVRDSFRFQ19NUlBDX1NUQVRVU19J
TlBST0dSRVNTKQ0KLQkJbXJwY19jb21wbGV0ZV9jbWQoc3RkZXYpOw0KLQ0KIAlzY2hlZHVsZV9k
ZWxheWVkX3dvcmsoJnN0ZGV2LT5tcnBjX3RpbWVvdXQsDQogCQkJICAgICAgbXNlY3NfdG9famlm
Zmllcyg1MDApKTsNCiB9DQotLSANCjIuNy40DQo=

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));
>  }
>