* [PATCH] PCI: Make reset warning messages different
@ 2014-10-15 23:41 Gavin Shan
2014-10-15 23:48 ` Bjorn Helgaas
0 siblings, 1 reply; 5+ messages in thread
From: Gavin Shan @ 2014-10-15 23:41 UTC (permalink / raw)
To: linux-pci; +Cc: bhelgaas, weiyang, 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.
Signed-off-by: Gavin Shan <gwshan@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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI: Make reset warning messages different
2014-10-15 23:41 [PATCH] PCI: Make reset warning messages different Gavin Shan
@ 2014-10-15 23:48 ` Bjorn Helgaas
2014-10-16 0:19 ` Gavin Shan
0 siblings, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2014-10-15 23:48 UTC (permalink / raw)
To: Gavin Shan; +Cc: linux-pci, Richard Yang
On Wed, Oct 15, 2014 at 5:41 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> 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 <gwshan@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");
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.
> clear:
> pci_write_config_byte(dev, pos + PCI_AF_CTRL, PCI_AF_CTRL_FLR);
> --
> 1.8.3.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI: Make reset warning messages different
2014-10-15 23:48 ` Bjorn Helgaas
@ 2014-10-16 0:19 ` Gavin Shan
2014-10-16 3:43 ` Bjorn Helgaas
0 siblings, 1 reply; 5+ messages in thread
From: Gavin Shan @ 2014-10-16 0:19 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Gavin Shan, linux-pci, Richard Yang
On Wed, Oct 15, 2014 at 05:48:35PM -0600, Bjorn Helgaas wrote:
>On Wed, Oct 15, 2014 at 5:41 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>> 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 <gwshan@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");
>
>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
>> clear:
>> pci_write_config_byte(dev, pos + PCI_AF_CTRL, PCI_AF_CTRL_FLR);
>> --
>> 1.8.3.2
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI: Make reset warning messages different
2014-10-16 0:19 ` Gavin Shan
@ 2014-10-16 3:43 ` Bjorn Helgaas
2014-10-16 4:59 ` Gavin Shan
0 siblings, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2014-10-16 3:43 UTC (permalink / raw)
To: Gavin Shan; +Cc: linux-pci, Richard Yang
On Wed, Oct 15, 2014 at 6:19 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> On Wed, Oct 15, 2014 at 05:48:35PM -0600, Bjorn Helgaas wrote:
>>On Wed, Oct 15, 2014 at 5:41 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>>> 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 <gwshan@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");
>>
>>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.
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI: Make reset warning messages different
2014-10-16 3:43 ` Bjorn Helgaas
@ 2014-10-16 4:59 ` Gavin Shan
0 siblings, 0 replies; 5+ messages in thread
From: Gavin Shan @ 2014-10-16 4:59 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Gavin Shan, linux-pci, Richard Yang
On Wed, Oct 15, 2014 at 09:43:32PM -0600, Bjorn Helgaas wrote:
>On Wed, Oct 15, 2014 at 6:19 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>> On Wed, Oct 15, 2014 at 05:48:35PM -0600, Bjorn Helgaas wrote:
>>>On Wed, Oct 15, 2014 at 5:41 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>>>> 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 <gwshan@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");
>>>
>>>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.
>
>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.
>
Yep, I'll change according to your suggestion.
>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.
>Just FYI, I'll be on vacation the rest of this week.
>
Ok. Have a good vacation.
Thanks,
Gavin
>Bjorn
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-16 4:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-15 23:41 [PATCH] PCI: Make reset warning messages different Gavin Shan
2014-10-15 23:48 ` Bjorn Helgaas
2014-10-16 0:19 ` Gavin Shan
2014-10-16 3:43 ` Bjorn Helgaas
2014-10-16 4:59 ` Gavin Shan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).