Re: [PATCH] PCI: Don't try to read CLS from PCIe devices in pci_apply_final_quirks

[Heiner Kallweit <hkallweit1@gmail.com>]

On 31.03.2021 23:00, Bjorn Helgaas wrote:
> On Tue, Dec 08, 2020 at 02:26:46PM +0100, Heiner Kallweit wrote:
>> Don't try to read CLS from PCIe devices in pci_apply_final_quirks().
>> This value has no meaning for PCIe.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/pci/quirks.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index d9cbe69b8..ac8ce9118 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -163,6 +163,9 @@ static int __init pci_apply_final_quirks(void)
>>  	pci_apply_fixup_final_quirks = true;
>>  	for_each_pci_dev(dev) {
>>  		pci_fixup_device(pci_fixup_final, dev);
>> +
>> +		if (pci_is_pcie(dev))
>> +			continue;
> 
> This loop tries to deduce the platform's cache line size by looking at
> the CLS of every PCI device.  It doesn't *write* CLS for any devices.
> 
> IIUC skipping PCIe devices would only make a difference if a PCIe
> device had a non-zero CLS different from the CLS of other devices.
> In that case we would print a "CLS mismatch" message and fall back to
> pci_dfl_cache_line_size.
> 
> The power-up value is zero, so if we read a non-zero CLS, it means
> firmware set it to something.  It would be strange if firmware set it
> to something other than the platform's cache line size.
> 
> Skipping PCIe devices probably doesn't hurt anything, but I don't
> really see a benefit either.  What do you think?  In general I think
> we should add code to check PCI vs PCIe only if it makes a difference.
> 
There is no functional change, right. The benefit is just that we
avoid some unnecessary traffic on the PCI bus.
If you think that this is not really worth a patch, then this is also
fine with me.


>>  		/*
>>  		 * If arch hasn't set it explicitly yet, use the CLS
>>  		 * value shared by all PCI devices.  If there's a
>> -- 
>> 2.29.2
>>