PCI: dwc: Program device-id

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

Posted by Sushrut Shree Trivedi 4 days, 6 hours ago

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@oss.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;
+	u32			device_id;
 };
 
 struct dw_pcie_ep_ops {

-- 
2.25.1

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

Posted by Manivannan Sadhasivam 2 days, 14 hours ago

On Thu, Nov 27, 2025 at 09:00:51PM +0530, Sushrut Shree Trivedi wrote:
> 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.

The Device ID you are programming is for the Root Port, not for the
controller/Host bridge.

> This would
> cause issues while applying controller specific quirks.
> 

This statement is misleading and wrong. Controller specific quirks cannot be
applied using Root Port IDs. We have controller specific DT compatible propery
for that purpose.

> So, program the correct device-id after reading it from the
> devicetree.
> 

Even though the dtschema allows having these Root Port IDs in the controller
node, it is deprecated (odd that we don't mark it as such). These properties are
supposed to be added to the Root Port binding and the DWC core should parse them
and program the IDs if available.

But the DWC driver doesn't parse Root Port nodes atm. OTOH, your colleague is
working on a series that does that and once that gets submitted, please rebase
this series on top and resend.

- Mani

> Signed-off-by: Sushrut Shree Trivedi <sushrut.trivedi@oss.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;
> +	u32			device_id;
>  };
>  
>  struct dw_pcie_ep_ops {
> 
> -- 
> 2.25.1
> 

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

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

Posted by Shawn Lin 3 days, 21 hours ago

在 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@oss.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?

How about do it in the defective HW drivers?


> +	u32			device_id;
>   };
>   
>   struct dw_pcie_ep_ops {
>

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

Posted by Manivannan Sadhasivam 2 days, 14 hours ago

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@oss.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.

- Mani

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

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

Posted by Shawn Lin 1 day, 19 hours ago

在 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@oss.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. Could dwc core provide a common helper for 
them(this qcom instance inclueded) to call without bothering the dwc core?

> - Mani
>

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

Posted by Manivannan Sadhasivam 1 day, 7 hours ago

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@oss.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

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