[PATCH] PCI: Make reset warning messages different

Discussion:

Gavin Shan

2014-10-15 23:41:10 UTC

We have same warning message for FLR and AF FLR and users can't
know which type of resets the PCI device is taking when there are
pending transactions. The patch makes them different for FLR and
AF FLR cases.

Signed-off-by: Gavin Shan <***@linux.vnet.ibm.com>
---
drivers/pci/pci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 625a4ac..2d708cd 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3144,7 +3144,7 @@ static int pcie_flr(struct pci_dev *dev, int probe)
return 0;

if (!pci_wait_for_pending_transaction(dev))
- dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
+ dev_err(&dev->dev, "Force FLR with pending transaction\n");

pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);

@@ -3178,7 +3178,7 @@ static int pci_af_flr(struct pci_dev *dev, int probe)
PCI_AF_STATUS_TP << 8))
goto clear;

- dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
+ dev_err(&dev->dev, "Force AF FLR with pending transaction\n");

clear:
pci_write_config_byte(dev, pos + PCI_AF_CTRL, PCI_AF_CTRL_FLR);

--
1.8.3.2

Bjorn Helgaas

2014-10-15 23:48:35 UTC

Permalink

Post by Gavin Shan
We have same warning message for FLR and AF FLR and users can't
know which type of resets the PCI device is taking when there are
pending transactions. The patch makes them different for FLR and
AF FLR cases.
---
drivers/pci/pci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 625a4ac..2d708cd 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3144,7 +3144,7 @@ static int pcie_flr(struct pci_dev *dev, int probe)
return 0;
if (!pci_wait_for_pending_transaction(dev))
- dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
+ dev_err(&dev->dev, "Force FLR with pending transaction\n");
pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
@@ -3178,7 +3178,7 @@ static int pci_af_flr(struct pci_dev *dev, int probe)
PCI_AF_STATUS_TP << 8))
goto clear;
- dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
+ dev_err(&dev->dev, "Force AF FLR with pending transaction\n");

Making the text different is fine, but I don't think "FLR" and "AF
FLR" are meaningful except to extremely technical people. So I think
"reset" needs to stay spelled out in the message.

Post by Gavin Shan
pci_write_config_byte(dev, pos + PCI_AF_CTRL, PCI_AF_CTRL_FLR);
--
1.8.3.2

Gavin Shan

2014-10-16 00:19:21 UTC

Permalink

Post by Bjorn Helgaas

Making the text different is fine, but I don't think "FLR" and "AF
FLR" are meaningful except to extremely technical people. So I think
"reset" needs to stay spelled out in the message.

Agree, it's worthy to keep "reset". How about something like this:

"Force function level reset with pending transaction" - FLR
"Force AF function level reset with pending transaction" - AF FLR

If above messages look good to you, I'll send out another revision.

Thanks,
Gavin

Post by Bjorn Helgaas

Post by Gavin Shan
pci_write_config_byte(dev, pos + PCI_AF_CTRL, PCI_AF_CTRL_FLR);
--
1.8.3.2

Bjorn Helgaas

2014-10-16 03:43:32 UTC

Permalink

Post by Gavin Shan

Post by Bjorn Helgaas

Making the text different is fine, but I don't think "FLR" and "AF
FLR" are meaningful except to extremely technical people. So I think
"reset" needs to stay spelled out in the message.

"Force function level reset with pending transaction" - FLR
"Force AF function level reset with pending transaction" - AF FLR
If above messages look good to you, I'll send out another revision.

How about something like "timed out waiting for pending transaction;
performing function level reset"?

"Force reset with pending transaction" sounds like a pending
transaction might be the mechanism we're using to perform the reset.

Out of curiosity, is there some issue you tripped over where it's
important for users to know this difference?

Just FYI, I'll be on vacation the rest of this week.

Bjorn

Gavin Shan

2014-10-16 04:59:41 UTC

Permalink

Post by Bjorn Helgaas

Post by Gavin Shan

Post by Bjorn Helgaas

Making the text different is fine, but I don't think "FLR" and "AF
FLR" are meaningful except to extremely technical people. So I think
"reset" needs to stay spelled out in the message.

"Force function level reset with pending transaction" - FLR
"Force AF function level reset with pending transaction" - AF FLR
If above messages look good to you, I'll send out another revision.

Yep, I'll change according to your suggestion.

Post by Bjorn Helgaas
Out of curiosity, is there some issue you tripped over where it's
important for users to know this difference?

I saw this message when issuing FLR (not AF case) to (EEH) frozen device.
Since 0xFF's is always returned from the frozen device, this message was
surely printed. However, I didn't know it was FLR or AF-FLR from the
original message.

Post by Bjorn Helgaas
Just FYI, I'll be on vacation the rest of this week.

Ok. Have a good vacation.

Thanks,
Gavin

Post by Bjorn Helgaas
Bjorn