[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f9492c14-b07f-43d1-6f78-a895f2d8a2d9@quicinc.com>
Date: Fri, 16 Aug 2024 09:11:52 +0530
From: Krishna Chaitanya Chundru <quic_krichai@...cinc.com>
To: Yicong Yang <yangyicong@...wei.com>
CC: Shuai Xue <xueshuai@...ux.alibaba.com>,
Jing Zhang
<renyu.zj@...ux.alibaba.com>,
Will Deacon <will@...nel.org>, Mark Rutland
<mark.rutland@....com>,
Baolin Wang <baolin.wang@...ux.alibaba.com>,
"Yicong
Yang" <yangyicong@...ilicon.com>,
Jonathan Cameron
<Jonathan.Cameron@...wei.com>,
Jonathan Corbet <corbet@....net>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-doc@...r.kernel.org>, <quic_vbadigan@...cinc.com>,
<quic_nitegupt@...cinc.com>, <quic_skananth@...cinc.com>,
<quic_ramkri@...cinc.com>, <quic_parass@...cinc.com>,
<quic_mrana@...cinc.com>
Subject: Re: [PATCH 1/4] perf/dwc_pcie: Fix registration issue in multi PCIe
controller instances
On 8/15/2024 7:10 PM, Yicong Yang wrote:
> On 2024/7/31 12:23, Krishna chaitanya chundru wrote:
>> When there are multiple of instances of PCIe controllers, registration
>> to perf driver fails with this error.
>> sysfs: cannot create duplicate filename '/devices/platform/dwc_pcie_pmu.0'
>> CPU: 0 PID: 166 Comm: modprobe Not tainted 6.10.0-rc2-next-20240607-dirty
>> Hardware name: Qualcomm SA8775P Ride (DT)
>> Call trace:
>> dump_backtrace.part.8+0x98/0xf0
>> show_stack+0x14/0x1c
>> dump_stack_lvl+0x74/0x88
>> dump_stack+0x14/0x1c
>> sysfs_warn_dup+0x60/0x78
>> sysfs_create_dir_ns+0xe8/0x100
>> kobject_add_internal+0x94/0x224
>> kobject_add+0xa8/0x118
>> device_add+0x298/0x7b4
>> platform_device_add+0x1a0/0x228
>> platform_device_register_full+0x11c/0x148
>> dwc_pcie_register_dev+0x74/0xf0 [dwc_pcie_pmu]
>> dwc_pcie_pmu_init+0x7c/0x1000 [dwc_pcie_pmu]
>> do_one_initcall+0x58/0x1c0
>> do_init_module+0x58/0x208
>> load_module+0x1804/0x188c
>> __do_sys_init_module+0x18c/0x1f0
>> __arm64_sys_init_module+0x14/0x1c
>> invoke_syscall+0x40/0xf8
>> el0_svc_common.constprop.1+0x70/0xf4
>> do_el0_svc+0x18/0x20
>> el0_svc+0x28/0xb0
>> el0t_64_sync_handler+0x9c/0xc0
>> el0t_64_sync+0x160/0x164
>> kobject: kobject_add_internal failed for dwc_pcie_pmu.0 with -EEXIST,
>> don't try to register things with the same name in the same directory.
>>
>> This is because of having same bdf value for devices under two different
>> controllers.
>>
>> Update the logic to use sbdf which is a unique number in case of
>> multi instance also.
>>
>> Fixes: af9597adc2f1 ("drivers/perf: add DesignWare PCIe PMU driver")
>
> Did you run into this on a QCOM platform with Patch 4/4 since there's
> multiple PCIe domains?
>
Yes we ran this in QCOM platform where it has multiple PCIe instances.
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@...cinc.com>
>> ---
>> drivers/perf/dwc_pcie_pmu.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
>> index c5e328f23841..c115348b8d53 100644
>> --- a/drivers/perf/dwc_pcie_pmu.c
>> +++ b/drivers/perf/dwc_pcie_pmu.c
>> @@ -556,10 +556,10 @@ static int dwc_pcie_register_dev(struct pci_dev *pdev)
>> {
>> struct platform_device *plat_dev;
>> struct dwc_pcie_dev_info *dev_info;
>> - u32 bdf;
>> + u32 sbdf;
>>
>> - bdf = PCI_DEVID(pdev->bus->number, pdev->devfn);
>> - plat_dev = platform_device_register_data(NULL, "dwc_pcie_pmu", bdf,
>> + sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);
>> + plat_dev = platform_device_register_data(NULL, "dwc_pcie_pmu", sbdf,
>> pdev, sizeof(*pdev));
>>
>> if (IS_ERR(plat_dev))
>> @@ -611,15 +611,15 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
>> struct pci_dev *pdev = plat_dev->dev.platform_data;
>> struct dwc_pcie_pmu *pcie_pmu;
>> char *name;
>> - u32 bdf, val;
>> + u32 sbdf, val;
>> u16 vsec;
>> int ret;
>>
>> vsec = pci_find_vsec_capability(pdev, pdev->vendor,
>> DWC_PCIE_VSEC_RAS_DES_ID);
>> pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val);
>> - bdf = PCI_DEVID(pdev->bus->number, pdev->devfn);
>> - name = devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x", bdf);
>> + sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);
>
> sbdf is also registerd as the id of the platform device in platform_device_register_data() above,
> can we use it directly here without encoding it again?
>
> Thanks.
>
ack.
- Krishna chaitanya.
>> + name = devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x", sbdf);
>> if (!name)
>> return -ENOMEM;
>>
>> @@ -650,7 +650,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
>> ret = cpuhp_state_add_instance(dwc_pcie_pmu_hp_state,
>> &pcie_pmu->cpuhp_node);
>> if (ret) {
>> - pci_err(pdev, "Error %d registering hotplug @%x\n", ret, bdf);
>> + pci_err(pdev, "Error %d registering hotplug @%x\n", ret, sbdf);
>> return ret;
>> }
>>
>> @@ -663,7 +663,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
>>
>> ret = perf_pmu_register(&pcie_pmu->pmu, name, -1);
>> if (ret) {
>> - pci_err(pdev, "Error %d registering PMU @%x\n", ret, bdf);
>> + pci_err(pdev, "Error %d registering PMU @%x\n", ret, sbdf);
>> return ret;
>> }
>> ret = devm_add_action_or_reset(&plat_dev->dev, dwc_pcie_unregister_pmu,
>>
Powered by blists - more mailing lists