| Message ID | b5b1456c-d5ff-d1d1-ba95-b9a12eda8ae7@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: Don't try to read CLS from PCIe devices in pci_apply_final_quirks | expand |
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. > /* > * If arch hasn't set it explicitly yet, use the CLS > * value shared by all PCI devices. If there's a > -- > 2.29.2 >
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 >>
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; /* * If arch hasn't set it explicitly yet, use the CLS * value shared by all PCI devices. If there's a
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(+)