* Re: [PATCH 2/3] perf/dwc_pcie: Load DesignWare PCIe PMU driver automatically on Ampere SoCs [not found] ` <20241008231824.5102-3-ilkka@os.amperecomputing.com> @ 2024-10-24 11:32 ` Will Deacon 2024-10-24 22:19 ` Ilkka Koskinen 0 siblings, 1 reply; 7+ messages in thread From: Will Deacon @ 2024-10-24 11:32 UTC (permalink / raw) To: Ilkka Koskinen Cc: Shuai Xue, Jing Zhang, Mark Rutland, linux-arm-kernel, linux-kernel On Tue, Oct 08, 2024 at 11:18:23PM +0000, Ilkka Koskinen wrote: > Load DesignWare PCIe PMU driver automatically if the system has a PCI > bridge by Ampere. > > Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> > --- > drivers/perf/dwc_pcie_pmu.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c > index 3581d916d851..d752168733cf 100644 > --- a/drivers/perf/dwc_pcie_pmu.c > +++ b/drivers/perf/dwc_pcie_pmu.c > @@ -782,6 +782,16 @@ static void __exit dwc_pcie_pmu_exit(void) > module_init(dwc_pcie_pmu_init); > module_exit(dwc_pcie_pmu_exit); > > +static const struct pci_device_id dwc_pcie_pmu_table[] = { > + { > + PCI_DEVICE(PCI_VENDOR_ID_AMPERE, PCI_ANY_ID), > + .class = PCI_CLASS_BRIDGE_PCI_NORMAL, > + .class_mask = ~0, > + }, > + { } > +}; > +MODULE_DEVICE_TABLE(pci, dwc_pcie_pmu_table); Hmm, won't this only work if the driver is modular? Should we be calling pci_register_driver() for the builtin case? Will ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] perf/dwc_pcie: Load DesignWare PCIe PMU driver automatically on Ampere SoCs 2024-10-24 11:32 ` [PATCH 2/3] perf/dwc_pcie: Load DesignWare PCIe PMU driver automatically on Ampere SoCs Will Deacon @ 2024-10-24 22:19 ` Ilkka Koskinen 2024-10-28 17:31 ` Will Deacon 0 siblings, 1 reply; 7+ messages in thread From: Ilkka Koskinen @ 2024-10-24 22:19 UTC (permalink / raw) To: Will Deacon Cc: Ilkka Koskinen, Shuai Xue, Jing Zhang, Mark Rutland, linux-arm-kernel, linux-kernel Hi Will, On Thu, 24 Oct 2024, Will Deacon wrote: > On Tue, Oct 08, 2024 at 11:18:23PM +0000, Ilkka Koskinen wrote: >> Load DesignWare PCIe PMU driver automatically if the system has a PCI >> bridge by Ampere. >> >> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> >> --- >> drivers/perf/dwc_pcie_pmu.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c >> index 3581d916d851..d752168733cf 100644 >> --- a/drivers/perf/dwc_pcie_pmu.c >> +++ b/drivers/perf/dwc_pcie_pmu.c >> @@ -782,6 +782,16 @@ static void __exit dwc_pcie_pmu_exit(void) >> module_init(dwc_pcie_pmu_init); >> module_exit(dwc_pcie_pmu_exit); >> >> +static const struct pci_device_id dwc_pcie_pmu_table[] = { >> + { >> + PCI_DEVICE(PCI_VENDOR_ID_AMPERE, PCI_ANY_ID), >> + .class = PCI_CLASS_BRIDGE_PCI_NORMAL, >> + .class_mask = ~0, >> + }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(pci, dwc_pcie_pmu_table); > > Hmm, won't this only work if the driver is modular? Should we be calling > pci_register_driver() for the builtin case? That would be the normal case indeed. However, this driver is quite different: dwc_pcie_pmu_init() goes through all the pci devices looking for root ports with the pmu capabilities. Moreover, the probe function isn't bound to any specific vendor/class/device IDs. This patch simply makes sure the driver is loaded and the init function gets called, if the driver was built as module and ran on Ampere system. Cheers, Ilkka > > Will > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] perf/dwc_pcie: Load DesignWare PCIe PMU driver automatically on Ampere SoCs 2024-10-24 22:19 ` Ilkka Koskinen @ 2024-10-28 17:31 ` Will Deacon 2024-10-29 3:27 ` Ilkka Koskinen 0 siblings, 1 reply; 7+ messages in thread From: Will Deacon @ 2024-10-28 17:31 UTC (permalink / raw) To: Ilkka Koskinen Cc: Shuai Xue, Jing Zhang, Mark Rutland, linux-arm-kernel, linux-kernel On Thu, Oct 24, 2024 at 03:19:17PM -0700, Ilkka Koskinen wrote: > > Hi Will, > > On Thu, 24 Oct 2024, Will Deacon wrote: > > On Tue, Oct 08, 2024 at 11:18:23PM +0000, Ilkka Koskinen wrote: > > > Load DesignWare PCIe PMU driver automatically if the system has a PCI > > > bridge by Ampere. > > > > > > Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> > > > --- > > > drivers/perf/dwc_pcie_pmu.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c > > > index 3581d916d851..d752168733cf 100644 > > > --- a/drivers/perf/dwc_pcie_pmu.c > > > +++ b/drivers/perf/dwc_pcie_pmu.c > > > @@ -782,6 +782,16 @@ static void __exit dwc_pcie_pmu_exit(void) > > > module_init(dwc_pcie_pmu_init); > > > module_exit(dwc_pcie_pmu_exit); > > > > > > +static const struct pci_device_id dwc_pcie_pmu_table[] = { > > > + { > > > + PCI_DEVICE(PCI_VENDOR_ID_AMPERE, PCI_ANY_ID), > > > + .class = PCI_CLASS_BRIDGE_PCI_NORMAL, > > > + .class_mask = ~0, > > > + }, > > > + { } > > > +}; > > > +MODULE_DEVICE_TABLE(pci, dwc_pcie_pmu_table); > > > > Hmm, won't this only work if the driver is modular? Should we be calling > > pci_register_driver() for the builtin case? > > That would be the normal case indeed. However, this driver is quite > different: dwc_pcie_pmu_init() goes through all the pci devices looking for > root ports with the pmu capabilities. Moreover, the probe function isn't > bound to any specific vendor/class/device IDs. This patch simply makes sure > the driver is loaded and the init function gets called, if the driver was > built as module and ran on Ampere system. Ok, but that seems like the wrong approach, no? We end up with a weird list of vendors who want the thing to probe on their SoCs and, by omission, everybody not on the list doesn't want that behaviour. Will ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] perf/dwc_pcie: Load DesignWare PCIe PMU driver automatically on Ampere SoCs 2024-10-28 17:31 ` Will Deacon @ 2024-10-29 3:27 ` Ilkka Koskinen 2024-10-29 13:00 ` Will Deacon 0 siblings, 1 reply; 7+ messages in thread From: Ilkka Koskinen @ 2024-10-29 3:27 UTC (permalink / raw) To: Will Deacon Cc: Ilkka Koskinen, Shuai Xue, Jing Zhang, Mark Rutland, linux-arm-kernel, linux-kernel On Mon, 28 Oct 2024, Will Deacon wrote: > On Thu, Oct 24, 2024 at 03:19:17PM -0700, Ilkka Koskinen wrote: >> >> Hi Will, >> >> On Thu, 24 Oct 2024, Will Deacon wrote: >>> On Tue, Oct 08, 2024 at 11:18:23PM +0000, Ilkka Koskinen wrote: >>>> Load DesignWare PCIe PMU driver automatically if the system has a PCI >>>> bridge by Ampere. >>>> >>>> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> >>>> --- >>>> drivers/perf/dwc_pcie_pmu.c | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c >>>> index 3581d916d851..d752168733cf 100644 >>>> --- a/drivers/perf/dwc_pcie_pmu.c >>>> +++ b/drivers/perf/dwc_pcie_pmu.c >>>> @@ -782,6 +782,16 @@ static void __exit dwc_pcie_pmu_exit(void) >>>> module_init(dwc_pcie_pmu_init); >>>> module_exit(dwc_pcie_pmu_exit); >>>> >>>> +static const struct pci_device_id dwc_pcie_pmu_table[] = { >>>> + { >>>> + PCI_DEVICE(PCI_VENDOR_ID_AMPERE, PCI_ANY_ID), >>>> + .class = PCI_CLASS_BRIDGE_PCI_NORMAL, >>>> + .class_mask = ~0, >>>> + }, >>>> + { } >>>> +}; >>>> +MODULE_DEVICE_TABLE(pci, dwc_pcie_pmu_table); >>> >>> Hmm, won't this only work if the driver is modular? Should we be calling >>> pci_register_driver() for the builtin case? >> >> That would be the normal case indeed. However, this driver is quite >> different: dwc_pcie_pmu_init() goes through all the pci devices looking for >> root ports with the pmu capabilities. Moreover, the probe function isn't >> bound to any specific vendor/class/device IDs. This patch simply makes sure >> the driver is loaded and the init function gets called, if the driver was >> built as module and ran on Ampere system. > > Ok, but that seems like the wrong approach, no? We end up with a weird > list of vendors who want the thing to probe on their SoCs and, by > omission, everybody not on the list doesn't want that behaviour. Ideally, dwc pmu driver would claim the supported root ports but I think the PCIe driver is doing that. How about if we simply drop the auto loading patch and let users to manually load the driver as they have been doing so far? Cheers, Ilkka > > Will > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] perf/dwc_pcie: Load DesignWare PCIe PMU driver automatically on Ampere SoCs 2024-10-29 3:27 ` Ilkka Koskinen @ 2024-10-29 13:00 ` Will Deacon 2024-10-29 15:39 ` Jonathan Cameron 0 siblings, 1 reply; 7+ messages in thread From: Will Deacon @ 2024-10-29 13:00 UTC (permalink / raw) To: Ilkka Koskinen Cc: Shuai Xue, Jing Zhang, Mark Rutland, linux-arm-kernel, linux-kernel On Mon, Oct 28, 2024 at 08:27:27PM -0700, Ilkka Koskinen wrote: > > On Mon, 28 Oct 2024, Will Deacon wrote: > > On Thu, Oct 24, 2024 at 03:19:17PM -0700, Ilkka Koskinen wrote: > > > > > > Hi Will, > > > > > > On Thu, 24 Oct 2024, Will Deacon wrote: > > > > On Tue, Oct 08, 2024 at 11:18:23PM +0000, Ilkka Koskinen wrote: > > > > > Load DesignWare PCIe PMU driver automatically if the system has a PCI > > > > > bridge by Ampere. > > > > > > > > > > Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> > > > > > --- > > > > > drivers/perf/dwc_pcie_pmu.c | 10 ++++++++++ > > > > > 1 file changed, 10 insertions(+) > > > > > > > > > > diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c > > > > > index 3581d916d851..d752168733cf 100644 > > > > > --- a/drivers/perf/dwc_pcie_pmu.c > > > > > +++ b/drivers/perf/dwc_pcie_pmu.c > > > > > @@ -782,6 +782,16 @@ static void __exit dwc_pcie_pmu_exit(void) > > > > > module_init(dwc_pcie_pmu_init); > > > > > module_exit(dwc_pcie_pmu_exit); > > > > > > > > > > +static const struct pci_device_id dwc_pcie_pmu_table[] = { > > > > > + { > > > > > + PCI_DEVICE(PCI_VENDOR_ID_AMPERE, PCI_ANY_ID), > > > > > + .class = PCI_CLASS_BRIDGE_PCI_NORMAL, > > > > > + .class_mask = ~0, > > > > > + }, > > > > > + { } > > > > > +}; > > > > > +MODULE_DEVICE_TABLE(pci, dwc_pcie_pmu_table); > > > > > > > > Hmm, won't this only work if the driver is modular? Should we be calling > > > > pci_register_driver() for the builtin case? > > > > > > That would be the normal case indeed. However, this driver is quite > > > different: dwc_pcie_pmu_init() goes through all the pci devices looking for > > > root ports with the pmu capabilities. Moreover, the probe function isn't > > > bound to any specific vendor/class/device IDs. This patch simply makes sure > > > the driver is loaded and the init function gets called, if the driver was > > > built as module and ran on Ampere system. > > > > Ok, but that seems like the wrong approach, no? We end up with a weird > > list of vendors who want the thing to probe on their SoCs and, by > > omission, everybody not on the list doesn't want that behaviour. > > Ideally, dwc pmu driver would claim the supported root ports but I think the > PCIe driver is doing that. How about if we simply drop the auto loading > patch and let users to manually load the driver as they have been doing so > far? Sure, I'll pick the other two up. Thanks! Will ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] perf/dwc_pcie: Load DesignWare PCIe PMU driver automatically on Ampere SoCs 2024-10-29 13:00 ` Will Deacon @ 2024-10-29 15:39 ` Jonathan Cameron 0 siblings, 0 replies; 7+ messages in thread From: Jonathan Cameron @ 2024-10-29 15:39 UTC (permalink / raw) To: Will Deacon Cc: Ilkka Koskinen, Shuai Xue, Jing Zhang, Mark Rutland, linux-arm-kernel, linux-kernel On Tue, 29 Oct 2024 13:00:15 +0000 Will Deacon <will@kernel.org> wrote: > On Mon, Oct 28, 2024 at 08:27:27PM -0700, Ilkka Koskinen wrote: > > > > On Mon, 28 Oct 2024, Will Deacon wrote: > > > On Thu, Oct 24, 2024 at 03:19:17PM -0700, Ilkka Koskinen wrote: > > > > > > > > Hi Will, > > > > > > > > On Thu, 24 Oct 2024, Will Deacon wrote: > > > > > On Tue, Oct 08, 2024 at 11:18:23PM +0000, Ilkka Koskinen wrote: > > > > > > Load DesignWare PCIe PMU driver automatically if the system has a PCI > > > > > > bridge by Ampere. > > > > > > > > > > > > Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> > > > > > > --- > > > > > > drivers/perf/dwc_pcie_pmu.c | 10 ++++++++++ > > > > > > 1 file changed, 10 insertions(+) > > > > > > > > > > > > diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c > > > > > > index 3581d916d851..d752168733cf 100644 > > > > > > --- a/drivers/perf/dwc_pcie_pmu.c > > > > > > +++ b/drivers/perf/dwc_pcie_pmu.c > > > > > > @@ -782,6 +782,16 @@ static void __exit dwc_pcie_pmu_exit(void) > > > > > > module_init(dwc_pcie_pmu_init); > > > > > > module_exit(dwc_pcie_pmu_exit); > > > > > > > > > > > > +static const struct pci_device_id dwc_pcie_pmu_table[] = { > > > > > > + { > > > > > > + PCI_DEVICE(PCI_VENDOR_ID_AMPERE, PCI_ANY_ID), > > > > > > + .class = PCI_CLASS_BRIDGE_PCI_NORMAL, > > > > > > + .class_mask = ~0, > > > > > > + }, > > > > > > + { } > > > > > > +}; > > > > > > +MODULE_DEVICE_TABLE(pci, dwc_pcie_pmu_table); > > > > > > > > > > Hmm, won't this only work if the driver is modular? Should we be calling > > > > > pci_register_driver() for the builtin case? > > > > > > > > That would be the normal case indeed. However, this driver is quite > > > > different: dwc_pcie_pmu_init() goes through all the pci devices looking for > > > > root ports with the pmu capabilities. Moreover, the probe function isn't > > > > bound to any specific vendor/class/device IDs. This patch simply makes sure > > > > the driver is loaded and the init function gets called, if the driver was > > > > built as module and ran on Ampere system. > > > > > > Ok, but that seems like the wrong approach, no? We end up with a weird > > > list of vendors who want the thing to probe on their SoCs and, by > > > omission, everybody not on the list doesn't want that behaviour. > > > > Ideally, dwc pmu driver would claim the supported root ports but I think the > > PCIe driver is doing that. How about if we simply drop the auto loading > > patch and let users to manually load the driver as they have been doing so > > far? Yup. The PCIe portdrv binds to the port. Lots of work needed to clean that up and make it extensible. Maybe we can then kick this PMU driver off from there once it's done. Jonathan > > Sure, I'll pick the other two up. Thanks! > > Will > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] perf/dwc_pcie: Enable DesignWare PCIe PMU on Ampere SoCs [not found] <20241008231824.5102-1-ilkka@os.amperecomputing.com> [not found] ` <20241008231824.5102-3-ilkka@os.amperecomputing.com> @ 2024-10-29 16:15 ` Will Deacon 1 sibling, 0 replies; 7+ messages in thread From: Will Deacon @ 2024-10-29 16:15 UTC (permalink / raw) To: Shuai Xue, Jing Zhang, Ilkka Koskinen Cc: catalin.marinas, kernel-team, Will Deacon, Mark Rutland, linux-arm-kernel, linux-kernel On Tue, 08 Oct 2024 23:18:21 +0000, Ilkka Koskinen wrote: > Enable DesignWare PCIe PMU driver on Ampere SoC. In addition, load the > driver automatically, if the system has a PCI bridge by Ampere > > Ilkka Koskinen (3): > perf/dwc_pcie: Add support for Ampere SoCs > perf/dwc_pcie: Load DesignWare PCIe PMU driver automatically on Ampere > SoCs > perf/dwc_pcie: Fix typos in event names > > [...] Applied 1 and 3 to will (for-next/perf), thanks! [1/3] perf/dwc_pcie: Add support for Ampere SoCs https://git.kernel.org/will/c/83d511c3ca0c [3/3] perf/dwc_pcie: Fix typos in event names https://git.kernel.org/will/c/94b3ad10c2e1 Cheers, -- Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-10-29 17:40 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20241008231824.5102-1-ilkka@os.amperecomputing.com>
[not found] ` <20241008231824.5102-3-ilkka@os.amperecomputing.com>
2024-10-24 11:32 ` [PATCH 2/3] perf/dwc_pcie: Load DesignWare PCIe PMU driver automatically on Ampere SoCs Will Deacon
2024-10-24 22:19 ` Ilkka Koskinen
2024-10-28 17:31 ` Will Deacon
2024-10-29 3:27 ` Ilkka Koskinen
2024-10-29 13:00 ` Will Deacon
2024-10-29 15:39 ` Jonathan Cameron
2024-10-29 16:15 ` [PATCH 0/3] perf/dwc_pcie: Enable DesignWare PCIe PMU " Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox