linux-kernel - Re: [PATCH 1/2] PCI: dwc: Program device-id

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2mhc33c5wvaaa7krhfrilz7qkfm47efx7h2esnydyzz4sk5umw@jxixtcmrsq76>
Date: Sun, 30 Nov 2025 20:35:35 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Shawn Lin <shawn.lin@...k-chips.com>
Cc: Sushrut Shree Trivedi <sushrut.trivedi@....qualcomm.com>, 
	Jingoo Han <jingoohan1@...il.com>, Lorenzo Pieralisi <lpieralisi@...nel.org>, 
	Krzysztof Wilczyński <kwilczynski@...nel.org>, Rob Herring <robh@...nel.org>, 
	Bjorn Helgaas <bhelgaas@...gle.com>, cros-qcom-dts-watchers@...omium.org, 
	Bjorn Andersson <andersson@...nel.org>, Konrad Dybcio <konradybcio@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, linux-pci@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 1/2] PCI: dwc: Program device-id

On Sun, Nov 30, 2025 at 10:43:14AM +0800, Shawn Lin wrote:
> 在 2025/11/29 星期六 15:34, Manivannan Sadhasivam 写道:
> > On Fri, Nov 28, 2025 at 09:03:52AM +0800, Shawn Lin wrote:
> > > 在 2025/11/27 星期四 23:30, Sushrut Shree Trivedi 写道:
> > > > For some controllers, HW doesn't program the correct device-id
> > > > leading to incorrect identification in lspci. For ex, QCOM
> > > > controller SC7280 uses same device id as SM8250. This would
> > > > cause issues while applying controller specific quirks.
> > > > 
> > > > So, program the correct device-id after reading it from the
> > > > devicetree.
> > > > 
> > > > Signed-off-by: Sushrut Shree Trivedi <sushrut.trivedi@....qualcomm.com>
> > > > ---
> > > >    drivers/pci/controller/dwc/pcie-designware-host.c | 7 +++++++
> > > >    drivers/pci/controller/dwc/pcie-designware.h      | 2 ++
> > > >    2 files changed, 9 insertions(+)
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > index e92513c5bda5..e8b975044b22 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > @@ -619,6 +619,9 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> > > >    		}
> > > >    	}
> > > > +	pp->device_id = 0xffff;
> > > > +	of_property_read_u32(np, "device-id", &pp->device_id);
> > > > +
> > > >    	dw_pcie_version_detect(pci);
> > > >    	dw_pcie_iatu_detect(pci);
> > > > @@ -1094,6 +1097,10 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
> > > >    	dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0);
> > > > +	/* Program correct device id */
> > > > +	if (pp->device_id != 0xffff)
> > > > +		dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, pp->device_id);
> > > > +
> > > >    	/* Program correct class for RC */
> > > >    	dw_pcie_writew_dbi(pci, PCI_CLASS_DEVICE, PCI_CLASS_BRIDGE_PCI);
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > > index e995f692a1ec..eff6da9438c4 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > > @@ -431,6 +431,8 @@ struct dw_pcie_rp {
> > > >    	struct pci_config_window *cfg;
> > > >    	bool			ecam_enabled;
> > > >    	bool			native_ecam;
> > > > +	u32			vendor_id;
> > > 
> > > I don't see where vendor_id is used.
> > > And why should dwc core take care of per HW bugs, could someone else
> > > will argue their HW doesn't program correct vender id/class code, then
> > > we add more into dw_pcie_rp to fix these?
> > > 
> > 
> > Device ID and Vendor ID are PCI generic properties and many controllers specify
> > them in devicetree due to the default values being wrong or just hardcode them
> > in the driver. There is nothing wrong in DWC core programming these values if
> > they are available in devicetree.
> > 
> > > How about do it in the defective HW drivers?
> > > 
> > 
> > If the issue is a vendor DWC wrapper specific, for sure it should be added to
> > the relevant controller driver. But this issue is pretty common among the DWC
> > wrapper implementations.
> > 
> 
> I think there are already some dwc instances working around this kind of
> defects in their relevant condroller driver.
> 
> drivers/pci/controller/dwc/pci-keystone.c:806:  dw_pcie_writew_dbi(pci,
> PCI_VENDOR_ID, id & PCIE_VENDORID_MASK);
> 
> drivers/pci/controller/dwc/pcie-spacemit-k1.c:146: dw_pcie_writew_dbi(pci,
> PCI_VENDOR_ID, PCI_VENDOR_ID_SPACEMIT);
> 
> drivers/pci/controller/dwc/pcie-spear13xx.c:139: dw_pcie_writew_dbi(pci,
> PCI_VENDOR_ID, 0x104A);
> 
> drivers/pci/controller/dwc/pci-keystone.c:807:  dw_pcie_writew_dbi(pci,
> PCI_DEVICE_ID, id >> PCIE_DEVICEID_SHIFT);
> 
> drivers/pci/controller/dwc/pcie-spacemit-k1.c:147: dw_pcie_writew_dbi(pci,
> PCI_DEVICE_ID, PCI_DEVICE_ID_SPACEMIT_K1);
> 
> drivers/pci/controller/dwc/pcie-spear13xx.c:140: dw_pcie_writew_dbi(pci,
> PCI_DEVICE_ID, 0xCD80);
> 
> If this patch applied, there are two non-consistent apporaches to work
> around this situation.

Nothing inconsistent here. DWC core will set the IDs based on the devicetree
properties if exist and controller drivers can override them or set sane default
if they want.

> Could dwc core provide a common helper for them(this
> qcom instance inclueded) to call without bothering the dwc core?
> 

Then the function will be called something like,

	dw_pcie_set_ids(dwc, 0x0, 0x0); /* for Qcom */
	dw_pcie_set_ids(dwc, PCI_VENDOR_ID_SPACEMIT, PCI_DEVICE_ID_SPACEMIT_K1);

I don't think combining both in the same API is a good idea. But I'm OK with an
API just to set the passed IDs.

- Mani

-- 
மணிவண்ணன் சதாசிவம்