Re: [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast

[Shawn Lin <shawn.lin@rock-chips.com>]

在 2025/11/21 星期五 14:48, zhangsenchuan 写道:
> 
> 
> 
>> -----Original Messages-----
>> From: "Manivannan Sadhasivam" <mani@kernel.org>
>> Send time:Thursday, 20/11/2025 20:45:38
>> To: zhangsenchuan@eswincomputing.com
>> Cc: bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com, Frank.li@nxp.com
>> Subject: Re: [PATCH v6 3/3] PCI: dwc: Add no_suspport_L2 flag and skip PME_Turn_Off broadcast
>>
>> On Thu, Nov 20, 2025 at 06:12:35PM +0800, zhangsenchuan@eswincomputing.com wrote:
>>> From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
>>>
>>> The ESWIN EIC7700 soc does not support enter L2 link state. Therefore add
>>> no_suspport_L2 flag skip PME_Turn_Off broadcast and link state check code,
>>> other driver can reuse this flag if meet the similar situation.
>>>
>>> Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
>>> Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
>>> Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
>>
>> Does this patch work for you?
>> https://lore.kernel.org/linux-pci/20251119-pci-dwc-suspend-rework-v1-1-aad104828562@oss.qualcomm.com/
> 
> if the PCIe link is not up, this suits me too, but if the PCIe link up,
> our hardware does not support entering the L2 link state. At this point,

Per PCIe spec, 5.2 Link State Power Management:
"L2/L3 Ready transition protocol support is required.
The L2/L3 Ready state entry transition process must begin as soon as
possible following the acknowledgment of a PME_Turn_Off Message, (i.e.,
the injection of a PME_TO_Ack TLP). The Downstream component initiates
L2/L3 Ready entry by sending a PM_Enter_L23 DLLP. "

"L2 support is optional, and dependent upon the presence of auxiliary power"

I interpret it as it is mandatory support for broadcast PME_Turn_Off and
ack PME_TO_Ack, which is what the original code does. Then your IP can't
really go into L2 for whatever reasons? Will your controller be broken 
if acking PME_Turn_Off?  Otherwise we could still let L2/L3 Ready
transstion happen and leave the device in L3 to save power.


> it is also necessary to skip the broadcast PME_Turn_Off message and wait
> for L2 transition.
> 
> Kind regards,
> Senchuan Zhang
> 
>>
>>> ---
>>>   drivers/pci/controller/dwc/pcie-designware-host.c | 4 ++++
>>>   drivers/pci/controller/dwc/pcie-designware.h      | 1 +
>>>   2 files changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>>> index e92513c5bda5..a203577606e5 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>>> @@ -1156,6 +1156,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>>>   	if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1)
>>>   		return 0;
>>>
>>> +	if (pci->no_suspport_L2)
>>> +		goto stop_link;
>>> +
>>>   	if (pci->pp.ops->pme_turn_off) {
>>>   		pci->pp.ops->pme_turn_off(&pci->pp);
>>>   	} else {
>>> @@ -1182,6 +1185,7 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>>>   	 */
>>>   	udelay(1);
>>>
>>> +stop_link:
>>>   	dw_pcie_stop_link(pci);
>>>   	if (pci->pp.ops->deinit)
>>>   		pci->pp.ops->deinit(&pci->pp);
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>>> index e995f692a1ec..170a73299ce5 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>>> @@ -539,6 +539,7 @@ struct dw_pcie {
>>>   	 * use_parent_dt_ranges to true to avoid this warning.
>>>   	 */
>>>   	bool			use_parent_dt_ranges;
>>> +	bool			no_suspport_L2;
>>>   };
>>>
>>>   #define to_dw_pcie_from_pp(port) container_of((port), struct dw_pcie, pp)
>>> --
>>> 2.25.1
>>>
>>
>> -- 
>> மணிவண்ணன் சதாசிவம்