Re: [PATCH v2 2/2] PCI: dwc: ep: Mirror the max link width and speed fields to all functions

[Aksh Garg <a-garg7@ti.com>]

On 24/02/26 00:25, Bjorn Helgaas wrote:
> On Thu, Feb 19, 2026 at 12:15:11PM +0530, Aksh Garg wrote:
>> PCIe r6.0, section 7.5.3.6 states that for multi-function devices, the
>> Max Link Width and Max Link Speed fields in the Link Capabilities
>> Register must report the same values for all functions.
> 
> Please update the citations here and in the comment below to r7.0.
> 
>> Currently, dw_pcie_setup() programs these fields only for physical
>> function 0 (PF0) via dw_pcie_link_set_max_speed() and
>> dw_pcie_link_set_max_link_width(). For multi-function endpoint
>> configurations, PF1 and beyond retain their default values, violating
>> the PCIe specification.
>>
>> Fix this by reading the Max Link Width and Max Link Speed fields from
>> PF0's Link Capabilities Register after dw_pcie_setup() completes,
>> then mirroring these values to all other physical functions.
> 
> "PF" and "physical function" are terms that apply to SR-IOV, not to
> ordinary functions of a multi-function device, so it's confusing to
> use them here.  Instead of "physical function 0" and "PF0", just refer
> to "Function 0" as the spec does.
> 
>> Fixes: 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC")
> 
> Sorry I missed it earlier; it's also the wrong term here in the
> subject of 24ede430fa49, but it's too late to fix that.
> 
>> Fixes: 89db0793c9f2 ("PCI: dwc: Add missing PCI_EXP_LNKCAP_MLW handling")
>> Signed-off-by: Aksh Garg <a-garg7@ti.com>
>> ---
>>
>> The link speed and width would be negotiated through PF0 during
>> initialization that controls the link behaviour, hence it didn't broke
>> the driver. However, the change is proposed just to make the driver
>> compatible with the PCIe base specifications.
>>
>> The fix is implemented in pcie-designware-ep.c rather than modifying
>> dw_pcie_setup() directly to keep pcie-designware.c independent of RC/EP
>> specifics and maintain it as common code.
>>
>> Changes from v1 to v2:
>> - Used FIELD_* macros
>>
>> v1: https://lore.kernel.org/all/20260202072758.101845-3-a-garg7@ti.com/
>>
>>   .../pci/controller/dwc/pcie-designware-ep.c   | 23 ++++++++++++++++++-
>>   1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
>> index 771241e1a2c9..6b9f90810fec 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
>> @@ -1094,7 +1094,8 @@ static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
>>   {
>>   	struct dw_pcie_ep *ep = &pci->ep;
>>   	u8 funcs = ep->epc->max_functions;
>> -	u8 func_no;
>> +	u32 ref_lnkcap, lnkcap;
>> +	u8 func_no, offset;
>>   
>>   	dw_pcie_dbi_ro_wr_en(pci);
>>   
>> @@ -1102,6 +1103,26 @@ static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci)
>>   		dw_pcie_ep_init_rebar_registers(ep, func_no);
>>   
>>   	dw_pcie_setup(pci);
>> +
>> +	/*
>> +	 * PCIe r6.0, section 7.5.3.6 states that for multi-function endpoints,
>> +	 * max link width and speed fields must report same values for all functions.
>> +	 * However, dw_pcie_setup() programs these fields only for physical function 0.
>> +	 * Hence, mirror these fields to all other physical functions as well.
> 
> s/physical function 0/function 0/
> 
> Wrap this to fit in 80 columns like the rest of the file.
> 
>> +	 */
>> +	if (funcs > 1) {
>> +		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>> +		ref_lnkcap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
>> +		ref_lnkcap = FIELD_GET(PCI_EXP_LNKCAP_MLW | PCI_EXP_LNKCAP_SLS, ref_lnkcap);
>> +
>> +		for (func_no = 1; func_no < funcs; func_no++) {
>> +			offset = dw_pcie_ep_find_capability(ep, func_no, PCI_CAP_ID_EXP);
>> +			lnkcap = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_EXP_LNKCAP);
>> +			FIELD_MODIFY(PCI_EXP_LNKCAP_MLW | PCI_EXP_LNKCAP_SLS, &lnkcap, ref_lnkcap);
>> +			dw_pcie_ep_writel_dbi(ep, func_no, offset + PCI_EXP_LNKCAP, lnkcap);
> 
> Please wrap these too, it's not too hard to make these fit in 80
> columns like (most of) the rest of the file.
> 

I will work on these feedbacks and post v3 series.
Thanks!

>> +		}
>> +	}
>> +
>>   	dw_pcie_dbi_ro_wr_dis(pci);
>>   }
>>   
>> -- 
>> 2.34.1
>>