* [PATCH] PCI: Don't try to read CLS from PCIe devices in pci_apply_final_quirks @ 2020-12-08 13:26 Heiner Kallweit 2021-03-31 21:00 ` Bjorn Helgaas 0 siblings, 1 reply; 4+ messages in thread From: Heiner Kallweit @ 2020-12-08 13:26 UTC (permalink / raw) To: Bjorn Helgaas; +Cc: linux-pci@vger.kernel.org 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; /* * If arch hasn't set it explicitly yet, use the CLS * value shared by all PCI devices. If there's a -- 2.29.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: Don't try to read CLS from PCIe devices in pci_apply_final_quirks 2020-12-08 13:26 [PATCH] PCI: Don't try to read CLS from PCIe devices in pci_apply_final_quirks Heiner Kallweit @ 2021-03-31 21:00 ` Bjorn Helgaas 2021-04-01 15:26 ` Heiner Kallweit 0 siblings, 1 reply; 4+ messages in thread From: Bjorn Helgaas @ 2021-03-31 21:00 UTC (permalink / raw) To: Heiner Kallweit; +Cc: Bjorn Helgaas, linux-pci@vger.kernel.org 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 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: Don't try to read CLS from PCIe devices in pci_apply_final_quirks 2021-03-31 21:00 ` Bjorn Helgaas @ 2021-04-01 15:26 ` Heiner Kallweit 2021-04-01 16:20 ` Bjorn Helgaas 0 siblings, 1 reply; 4+ messages in thread From: Heiner Kallweit @ 2021-04-01 15:26 UTC (permalink / raw) To: Bjorn Helgaas; +Cc: Bjorn Helgaas, linux-pci@vger.kernel.org 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 >> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: Don't try to read CLS from PCIe devices in pci_apply_final_quirks 2021-04-01 15:26 ` Heiner Kallweit @ 2021-04-01 16:20 ` Bjorn Helgaas 0 siblings, 0 replies; 4+ messages in thread From: Bjorn Helgaas @ 2021-04-01 16:20 UTC (permalink / raw) To: Heiner Kallweit; +Cc: Bjorn Helgaas, linux-pci@vger.kernel.org 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@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. 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 > >> > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-01 17:48 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-12-08 13:26 [PATCH] PCI: Don't try to read CLS from PCIe devices in pci_apply_final_quirks Heiner Kallweit 2021-03-31 21:00 ` Bjorn Helgaas 2021-04-01 15:26 ` Heiner Kallweit 2021-04-01 16:20 ` Bjorn Helgaas
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).