On Thu, Apr 01, 2021 at 05:26:57PM +0200, Heiner Kallweit wrote:
> 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@xxxxxxxxx>
> >> ---
> >> 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.
OK, I think I'll drop it just to avoid the cost of convincing
ourselves that there's no risk. If we see "CLS mismatch" warnings or
if is a real performance cost, we should do something. But I don't
think the extra PCI traffic will be measurable in this case.
> >> /*
> >> * If arch hasn't set it explicitly yet, use the CLS
> >> * value shared by all PCI devices. If there's a
> >> --
> >> 2.29.2
> >>
>
