On 7/28/21 4:58 PM, Shanker R Donthineni wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Bjorn,
>
> On 7/28/21 3:23 PM, Bjorn Helgaas wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On Wed, Jul 28, 2021 at 01:54:16PM -0500, Shanker R Donthineni wrote:
>>> On 7/27/21 5:12 PM, Bjorn Helgaas wrote:
>>>> On Fri, Jul 09, 2021 at 06:08:06PM +0530, Amey Narkhede wrote:
>>>>> Add has_pcie_flr bitfield in struct pci_dev to indicate support for PCIe
>>>>> FLR to avoid reading PCI_EXP_DEVCAP multiple times.
>>>>>
>>>>> Currently there is separate function pcie_has_flr() to probe if PCIe FLR
>>>>> is supported by the device which does not match the calling convention
>>>>> followed by reset methods which use second function argument to decide
>>>>> whether to probe or not. Add new function pcie_reset_flr() that follows
>>>>> the calling convention of reset methods.
>>>>>
>>>>> Signed-off-by: Amey Narkhede <ameynarkhede03@xxxxxxxxx>
>>>>> ---
>>>>> drivers/crypto/cavium/nitrox/nitrox_main.c | 4 +-
>>>>> drivers/pci/pci.c | 59 +++++++++++-----------
>>>>> drivers/pci/pcie/aer.c | 12 ++---
>>>>> drivers/pci/probe.c | 6 ++-
>>>>> drivers/pci/quirks.c | 9 ++--
>>>>> include/linux/pci.h | 3 +-
>>>>> 6 files changed, 45 insertions(+), 48 deletions(-)
>>>>>
>>>>> diff --git a/drivers/crypto/cavium/nitrox/nitrox_main.c b/drivers/crypto/cavium/nitrox/nitrox_main.c
>>>>> index facc8e6bc..15d6c8452 100644
>>>>> --- a/drivers/crypto/cavium/nitrox/nitrox_main.c
>>>>> +++ b/drivers/crypto/cavium/nitrox/nitrox_main.c
>>>>> @@ -306,9 +306,7 @@ static int nitrox_device_flr(struct pci_dev *pdev)
>>>>> return -ENOMEM;
>>>>> }
>>>>>
>>>>> - /* check flr support */
>>>>> - if (pcie_has_flr(pdev))
>>>>> - pcie_flr(pdev);
>>>>> + pcie_reset_flr(pdev, 0);
>>>> I'm not really a fan of exposing the "probe" argument outside
>>>> drivers/pci/. I think this would be the only occurrence. Is there a
>>>> way to avoid that?
>>>>
>>>> Can we just make pcie_flr() do the equivalent of pcie_has_flr()
>>>> internally?
>>>>
>>> I like your suggestion don't change the existing definition of
>>> pcie_has_flr()/pcie_flr() and define a new function pcie_reset_flr()
>>> to fit into the reset framework. This way no need to modify
>>> logic/drivers outside of driver/pci/xxx.
>>>
>>> int pcie_reset_flr(struct pci_dev *dev, int probe)
>>> {
>>> if (!pcie_has_flr(dev))
>>> return -ENOTTY;
>>>
>>> if (probe)
>>> return 0;
>>>
>>> return pcie_flr(dev);
>>> }
>> Can't remember the whole context of this in the series, but I assume
>> this would be static?
> It should be static since it's referenced in driver/pci/qrirk.c and aer.c.
>
Sorry it should not be static since it's referenced in driver/pci/quirk.c and aer.c
