linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper
@ 2022-06-03  7:18 Krishna chaitanya chundru
  2022-06-08 22:17 ` Stephen Boyd
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Krishna chaitanya chundru @ 2022-06-03  7:18 UTC (permalink / raw)
  To: helgaas
  Cc: linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
	quic_hemantk, quic_ramkri, manivannan.sadhasivam, swboyd,
	Krishna chaitanya chundru, Andy Gross, Bjorn Andersson,
	Stanimir Varbanov, Lorenzo Pieralisi, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas

Allow L1 and its sub-states in the qcom dwc pcie wrapper.
By default its disabled. So enable it explicitly.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 6ab9089..f60645c 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -41,6 +41,9 @@
 #define L23_CLK_RMV_DIS				BIT(2)
 #define L1_CLK_RMV_DIS				BIT(1)
 
+#define PCIE20_PARF_PM_CTRL			0x20
+#define REQ_NOT_ENTR_L1				BIT(5)
+
 #define PCIE20_PARF_PHY_CTRL			0x40
 #define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	GENMASK(20, 16)
 #define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		((x) << 16)
@@ -1267,6 +1270,11 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 	val |= BIT(4);
 	writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
 
+	/* Clear PARF PM REQ_NOT_ENTR_L1 bit to allow L1 states */
+	val = readl(pcie->parf + PCIE20_PARF_PM_CTRL);
+	val &= ~REQ_NOT_ENTR_L1;
+	writel(val, pcie->parf + PCIE20_PARF_PM_CTRL);
+
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
 		val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
 		val |= BIT(31);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper
  2022-06-03  7:18 [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper Krishna chaitanya chundru
@ 2022-06-08 22:17 ` Stephen Boyd
  2022-06-15 13:14   ` Krishna Chaitanya Chundru
  2022-06-09 11:26 ` Manivannan Sadhasivam
  2022-06-15 13:15 ` [PATCH v2] PCI: qcom: Allow L1 and its sub states Krishna chaitanya chundru
  2 siblings, 1 reply; 12+ messages in thread
From: Stephen Boyd @ 2022-06-08 22:17 UTC (permalink / raw)
  To: Krishna chaitanya chundru, helgaas
  Cc: linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
	quic_hemantk, quic_ramkri, manivannan.sadhasivam, Andy Gross,
	Bjorn Andersson, Stanimir Varbanov, Lorenzo Pieralisi,
	Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas

Quoting Krishna chaitanya chundru (2022-06-03 00:18:50)
> Allow L1 and its sub-states in the qcom dwc pcie wrapper.
> By default its disabled. So enable it explicitly.
>

Would be good to add some more details about why it's disabled by
default. I guess it's disabled by default in the hardware and enabling
it is OK to do unconditionally for all qcom dwc pcie devices?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper
  2022-06-03  7:18 [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper Krishna chaitanya chundru
  2022-06-08 22:17 ` Stephen Boyd
@ 2022-06-09 11:26 ` Manivannan Sadhasivam
  2022-06-15 13:14   ` Krishna Chaitanya Chundru
  2022-06-15 13:15 ` [PATCH v2] PCI: qcom: Allow L1 and its sub states Krishna chaitanya chundru
  2 siblings, 1 reply; 12+ messages in thread
From: Manivannan Sadhasivam @ 2022-06-09 11:26 UTC (permalink / raw)
  To: Krishna chaitanya chundru
  Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
	quic_hemantk, quic_ramkri, swboyd, Andy Gross, Bjorn Andersson,
	Stanimir Varbanov, Lorenzo Pieralisi, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas

On Fri, Jun 03, 2022 at 12:48:50PM +0530, Krishna chaitanya chundru wrote:
> Allow L1 and its sub-states in the qcom dwc pcie wrapper.

s/wrapper/driver

Also there is no need to use "qcom dwc" in subject. Prefix makes it explicit.

> By default its disabled. So enable it explicitly.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6ab9089..f60645c 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -41,6 +41,9 @@
>  #define L23_CLK_RMV_DIS				BIT(2)
>  #define L1_CLK_RMV_DIS				BIT(1)
>  
> +#define PCIE20_PARF_PM_CTRL			0x20
> +#define REQ_NOT_ENTR_L1				BIT(5)
> +
>  #define PCIE20_PARF_PHY_CTRL			0x40
>  #define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	GENMASK(20, 16)
>  #define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		((x) << 16)
> @@ -1267,6 +1270,11 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>  	val |= BIT(4);
>  	writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
>  
> +	/* Clear PARF PM REQ_NOT_ENTR_L1 bit to allow L1 states */

Mentioning the field in comment is redundant. Just say "Enable L1 and L1ss"

Thanks,
Mani

> +	val = readl(pcie->parf + PCIE20_PARF_PM_CTRL);
> +	val &= ~REQ_NOT_ENTR_L1;
> +	writel(val, pcie->parf + PCIE20_PARF_PM_CTRL);
> +
>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
>  		val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
>  		val |= BIT(31);
> -- 
> 2.7.4
> 

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper
  2022-06-08 22:17 ` Stephen Boyd
@ 2022-06-15 13:14   ` Krishna Chaitanya Chundru
  2022-06-15 15:44     ` Bjorn Helgaas
  0 siblings, 1 reply; 12+ messages in thread
From: Krishna Chaitanya Chundru @ 2022-06-15 13:14 UTC (permalink / raw)
  To: Stephen Boyd, helgaas
  Cc: linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
	quic_hemantk, quic_ramkri, manivannan.sadhasivam, Andy Gross,
	Bjorn Andersson, Stanimir Varbanov, Lorenzo Pieralisi,
	Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas


On 6/9/2022 3:47 AM, Stephen Boyd wrote:
> Quoting Krishna chaitanya chundru (2022-06-03 00:18:50)
>> Allow L1 and its sub-states in the qcom dwc pcie wrapper.
>> By default its disabled. So enable it explicitly.
>>
> Would be good to add some more details about why it's disabled by
> default. I guess it's disabled by default in the hardware and enabling
> it is OK to do unconditionally for all qcom dwc pcie devices?

This is disabled by default in the hardware. We can enable this for all 
qcom devices unconditionally because

Adding this patch alone will not allow aspm transitions we need to 
enable aspm configs. If particular devices doesn't want aspm

they can disable using aspm configs.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper
  2022-06-09 11:26 ` Manivannan Sadhasivam
@ 2022-06-15 13:14   ` Krishna Chaitanya Chundru
  0 siblings, 0 replies; 12+ messages in thread
From: Krishna Chaitanya Chundru @ 2022-06-15 13:14 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
	quic_hemantk, quic_ramkri, swboyd, Andy Gross, Bjorn Andersson,
	Stanimir Varbanov, Lorenzo Pieralisi, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas


On 6/9/2022 4:56 PM, Manivannan Sadhasivam wrote:
> On Fri, Jun 03, 2022 at 12:48:50PM +0530, Krishna chaitanya chundru wrote:
>> Allow L1 and its sub-states in the qcom dwc pcie wrapper.
> s/wrapper/driver
>
> Also there is no need to use "qcom dwc" in subject. Prefix makes it explicit.
Sure will update in the next patch.
>> By default its disabled. So enable it explicitly.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 6ab9089..f60645c 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -41,6 +41,9 @@
>>   #define L23_CLK_RMV_DIS				BIT(2)
>>   #define L1_CLK_RMV_DIS				BIT(1)
>>   
>> +#define PCIE20_PARF_PM_CTRL			0x20
>> +#define REQ_NOT_ENTR_L1				BIT(5)
>> +
>>   #define PCIE20_PARF_PHY_CTRL			0x40
>>   #define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	GENMASK(20, 16)
>>   #define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		((x) << 16)
>> @@ -1267,6 +1270,11 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>>   	val |= BIT(4);
>>   	writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
>>   
>> +	/* Clear PARF PM REQ_NOT_ENTR_L1 bit to allow L1 states */
> Mentioning the field in comment is redundant. Just say "Enable L1 and L1ss"
sure will update in the next patch.
> Thanks,
> Mani
>
>> +	val = readl(pcie->parf + PCIE20_PARF_PM_CTRL);
>> +	val &= ~REQ_NOT_ENTR_L1;
>> +	writel(val, pcie->parf + PCIE20_PARF_PM_CTRL);
>> +
>>   	if (IS_ENABLED(CONFIG_PCI_MSI)) {
>>   		val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
>>   		val |= BIT(31);
>> -- 
>> 2.7.4
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] PCI: qcom: Allow L1 and its sub states
  2022-06-03  7:18 [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper Krishna chaitanya chundru
  2022-06-08 22:17 ` Stephen Boyd
  2022-06-09 11:26 ` Manivannan Sadhasivam
@ 2022-06-15 13:15 ` Krishna chaitanya chundru
  2022-06-24  8:02   ` Krishna Chaitanya Chundru
                     ` (3 more replies)
  2 siblings, 4 replies; 12+ messages in thread
From: Krishna chaitanya chundru @ 2022-06-15 13:15 UTC (permalink / raw)
  To: helgaas
  Cc: linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
	quic_hemantk, quic_nitegupt, quic_skananth, quic_ramkri,
	manivannan.sadhasivam, swboyd, Krishna chaitanya chundru,
	Andy Gross, Bjorn Andersson, Stanimir Varbanov,
	Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
	Bjorn Helgaas

Allow L1 and its sub-states in the qcom pcie driver.
By default this is disabled in the hardware. So enabling it explicitly.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 6ab9089..0d8efcc 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -41,6 +41,9 @@
 #define L23_CLK_RMV_DIS				BIT(2)
 #define L1_CLK_RMV_DIS				BIT(1)
 
+#define PCIE20_PARF_PM_CTRL			0x20
+#define REQ_NOT_ENTR_L1				BIT(5)
+
 #define PCIE20_PARF_PHY_CTRL			0x40
 #define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	GENMASK(20, 16)
 #define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		((x) << 16)
@@ -1267,6 +1270,11 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 	val |= BIT(4);
 	writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
 
+	/* Enable L1 and L1ss */
+	val = readl(pcie->parf + PCIE20_PARF_PM_CTRL);
+	val &= ~REQ_NOT_ENTR_L1;
+	writel(val, pcie->parf + PCIE20_PARF_PM_CTRL);
+
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
 		val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
 		val |= BIT(31);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper
  2022-06-15 13:14   ` Krishna Chaitanya Chundru
@ 2022-06-15 15:44     ` Bjorn Helgaas
  0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2022-06-15 15:44 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Stephen Boyd, linux-pci, linux-arm-msm, linux-kernel,
	quic_vbadigan, quic_hemantk, quic_ramkri, manivannan.sadhasivam,
	Andy Gross, Bjorn Andersson, Stanimir Varbanov,
	Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
	Bjorn Helgaas

On Wed, Jun 15, 2022 at 06:44:19PM +0530, Krishna Chaitanya Chundru wrote:
> On 6/9/2022 3:47 AM, Stephen Boyd wrote:
> > Quoting Krishna chaitanya chundru (2022-06-03 00:18:50)
> > > Allow L1 and its sub-states in the qcom dwc pcie wrapper.
> > > By default its disabled. So enable it explicitly.
> > > 
> > Would be good to add some more details about why it's disabled by
> > default. I guess it's disabled by default in the hardware and enabling
> > it is OK to do unconditionally for all qcom dwc pcie devices?
> 
> This is disabled by default in the hardware. We can enable this for all qcom
> devices unconditionally because
> 
> Adding this patch alone will not allow aspm transitions we need to enable
> aspm configs. If particular devices doesn't want aspm
> they can disable using aspm configs.

This patch only affects qcom.  Is PCIE20_PARF_PM_CTRL qcom-specific?
Or is this something that should be done for all dwc-based drivers?

In fact, it only affects Qcom IP rev 2.7.0 and 1.9.0 (the only users
of qcom_pcie_init_2_7_0()).  I guess the other revisions don't support
ASPM L1 at all?

Does this patch affect the Link Capabilities register?  Before this
patch, does Link Cap advertise L1 support but enabling it doesn't
work?  Or does it not even advertise L1 support?

After this patch, I assume Link Cap advertises L1 support and enabling
L1 and L1 substates via PCI_EXP_LNKCTL_ASPM_L1,
PCI_L1SS_CTL1_ASPM_L1_1, and PCI_L1SS_CTL1_ASPM_L1_2 works per spec,
right?

Bjorn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] PCI: qcom: Allow L1 and its sub states
  2022-06-15 13:15 ` [PATCH v2] PCI: qcom: Allow L1 and its sub states Krishna chaitanya chundru
@ 2022-06-24  8:02   ` Krishna Chaitanya Chundru
  2022-06-24  9:02   ` Manivannan Sadhasivam
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Krishna Chaitanya Chundru @ 2022-06-24  8:02 UTC (permalink / raw)
  To: helgaas
  Cc: linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
	quic_hemantk, quic_nitegupt, quic_skananth, quic_ramkri,
	manivannan.sadhasivam, swboyd, Andy Gross, Bjorn Andersson,
	Stanimir Varbanov, Lorenzo Pieralisi, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas

A gentle remainder.

On 6/15/2022 6:45 PM, Krishna chaitanya chundru wrote:
> Allow L1 and its sub-states in the qcom pcie driver.
> By default this is disabled in the hardware. So enabling it explicitly.
>
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>   drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6ab9089..0d8efcc 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -41,6 +41,9 @@
>   #define L23_CLK_RMV_DIS				BIT(2)
>   #define L1_CLK_RMV_DIS				BIT(1)
>   
> +#define PCIE20_PARF_PM_CTRL			0x20
> +#define REQ_NOT_ENTR_L1				BIT(5)
> +
>   #define PCIE20_PARF_PHY_CTRL			0x40
>   #define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	GENMASK(20, 16)
>   #define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		((x) << 16)
> @@ -1267,6 +1270,11 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>   	val |= BIT(4);
>   	writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
>   
> +	/* Enable L1 and L1ss */
> +	val = readl(pcie->parf + PCIE20_PARF_PM_CTRL);
> +	val &= ~REQ_NOT_ENTR_L1;
> +	writel(val, pcie->parf + PCIE20_PARF_PM_CTRL);
> +
>   	if (IS_ENABLED(CONFIG_PCI_MSI)) {
>   		val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
>   		val |= BIT(31);

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] PCI: qcom: Allow L1 and its sub states
  2022-06-15 13:15 ` [PATCH v2] PCI: qcom: Allow L1 and its sub states Krishna chaitanya chundru
  2022-06-24  8:02   ` Krishna Chaitanya Chundru
@ 2022-06-24  9:02   ` Manivannan Sadhasivam
  2022-06-24 17:11   ` Bjorn Helgaas
  2022-07-15  8:24   ` Manivannan Sadhasivam
  3 siblings, 0 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2022-06-24  9:02 UTC (permalink / raw)
  To: Krishna chaitanya chundru
  Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
	quic_hemantk, quic_nitegupt, quic_skananth, quic_ramkri, swboyd,
	Andy Gross, Bjorn Andersson, Stanimir Varbanov,
	Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
	Bjorn Helgaas

On Wed, Jun 15, 2022 at 06:45:39PM +0530, Krishna chaitanya chundru wrote:
> Allow L1 and its sub-states in the qcom pcie driver.
> By default this is disabled in the hardware. So enabling it explicitly.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6ab9089..0d8efcc 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -41,6 +41,9 @@
>  #define L23_CLK_RMV_DIS				BIT(2)
>  #define L1_CLK_RMV_DIS				BIT(1)
>  
> +#define PCIE20_PARF_PM_CTRL			0x20
> +#define REQ_NOT_ENTR_L1				BIT(5)
> +
>  #define PCIE20_PARF_PHY_CTRL			0x40
>  #define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	GENMASK(20, 16)
>  #define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		((x) << 16)
> @@ -1267,6 +1270,11 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>  	val |= BIT(4);
>  	writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
>  
> +	/* Enable L1 and L1ss */
> +	val = readl(pcie->parf + PCIE20_PARF_PM_CTRL);
> +	val &= ~REQ_NOT_ENTR_L1;
> +	writel(val, pcie->parf + PCIE20_PARF_PM_CTRL);
> +
>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
>  		val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
>  		val |= BIT(31);
> -- 
> 2.7.4
> 

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] PCI: qcom: Allow L1 and its sub states
  2022-06-15 13:15 ` [PATCH v2] PCI: qcom: Allow L1 and its sub states Krishna chaitanya chundru
  2022-06-24  8:02   ` Krishna Chaitanya Chundru
  2022-06-24  9:02   ` Manivannan Sadhasivam
@ 2022-06-24 17:11   ` Bjorn Helgaas
  2022-07-15  8:24   ` Manivannan Sadhasivam
  3 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2022-06-24 17:11 UTC (permalink / raw)
  To: Krishna chaitanya chundru
  Cc: linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
	quic_hemantk, quic_nitegupt, quic_skananth, quic_ramkri,
	manivannan.sadhasivam, swboyd, Andy Gross, Bjorn Andersson,
	Stanimir Varbanov, Lorenzo Pieralisi, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas

On Wed, Jun 15, 2022 at 06:45:39PM +0530, Krishna chaitanya chundru wrote:
> Allow L1 and its sub-states in the qcom pcie driver.
> By default this is disabled in the hardware. So enabling it explicitly.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>

I have a vague memory of my questions at [1] being answered, but I
don't see the answers on the mailing list.  Maybe I missed it?

We should expand the commit log a bit with those details.

I'm also hoping for an ack from Stanimir, Andy, or Bjorn A., since
they're listed as maintainers of this driver.

[1] https://lore.kernel.org/r/20220615154422.GA941075@bhelgaas

> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6ab9089..0d8efcc 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -41,6 +41,9 @@
>  #define L23_CLK_RMV_DIS				BIT(2)
>  #define L1_CLK_RMV_DIS				BIT(1)
>  
> +#define PCIE20_PARF_PM_CTRL			0x20
> +#define REQ_NOT_ENTR_L1				BIT(5)
> +
>  #define PCIE20_PARF_PHY_CTRL			0x40
>  #define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	GENMASK(20, 16)
>  #define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		((x) << 16)
> @@ -1267,6 +1270,11 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>  	val |= BIT(4);
>  	writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
>  
> +	/* Enable L1 and L1ss */
> +	val = readl(pcie->parf + PCIE20_PARF_PM_CTRL);
> +	val &= ~REQ_NOT_ENTR_L1;
> +	writel(val, pcie->parf + PCIE20_PARF_PM_CTRL);
> +
>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
>  		val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
>  		val |= BIT(31);
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] PCI: qcom: Allow L1 and its sub states
  2022-06-15 13:15 ` [PATCH v2] PCI: qcom: Allow L1 and its sub states Krishna chaitanya chundru
                     ` (2 preceding siblings ...)
  2022-06-24 17:11   ` Bjorn Helgaas
@ 2022-07-15  8:24   ` Manivannan Sadhasivam
  2022-07-15 11:29     ` Krishna Chaitanya Chundru
  3 siblings, 1 reply; 12+ messages in thread
From: Manivannan Sadhasivam @ 2022-07-15  8:24 UTC (permalink / raw)
  To: Krishna chaitanya chundru
  Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
	quic_hemantk, quic_nitegupt, quic_skananth, quic_ramkri, swboyd,
	Andy Gross, Bjorn Andersson, Stanimir Varbanov,
	Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
	Bjorn Helgaas

On Wed, Jun 15, 2022 at 06:45:39PM +0530, Krishna chaitanya chundru wrote:
> Allow L1 and its sub-states in the qcom pcie driver.
> By default this is disabled in the hardware. So enabling it explicitly.
> 

You are enabling L1ss for controllers belonging to 2_7_0, so this should
be mentioned in the commit message. Otherwise, it will imply that the
L1ss is added for all controller versions.

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---

Change log should be added here for versions > 1.

Thanks,
Mani

>  drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6ab9089..0d8efcc 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -41,6 +41,9 @@
>  #define L23_CLK_RMV_DIS				BIT(2)
>  #define L1_CLK_RMV_DIS				BIT(1)
>  
> +#define PCIE20_PARF_PM_CTRL			0x20
> +#define REQ_NOT_ENTR_L1				BIT(5)
> +
>  #define PCIE20_PARF_PHY_CTRL			0x40
>  #define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	GENMASK(20, 16)
>  #define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		((x) << 16)
> @@ -1267,6 +1270,11 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>  	val |= BIT(4);
>  	writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
>  
> +	/* Enable L1 and L1ss */
> +	val = readl(pcie->parf + PCIE20_PARF_PM_CTRL);
> +	val &= ~REQ_NOT_ENTR_L1;
> +	writel(val, pcie->parf + PCIE20_PARF_PM_CTRL);
> +
>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
>  		val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
>  		val |= BIT(31);
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] PCI: qcom: Allow L1 and its sub states
  2022-07-15  8:24   ` Manivannan Sadhasivam
@ 2022-07-15 11:29     ` Krishna Chaitanya Chundru
  0 siblings, 0 replies; 12+ messages in thread
From: Krishna Chaitanya Chundru @ 2022-07-15 11:29 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
	quic_hemantk, quic_nitegupt, quic_skananth, quic_ramkri, swboyd,
	Andy Gross, Bjorn Andersson, Stanimir Varbanov,
	Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
	Bjorn Helgaas


On 7/15/2022 1:54 PM, Manivannan Sadhasivam wrote:
> On Wed, Jun 15, 2022 at 06:45:39PM +0530, Krishna chaitanya chundru wrote:
>> Allow L1 and its sub-states in the qcom pcie driver.
>> By default this is disabled in the hardware. So enabling it explicitly.
>>
> You are enabling L1ss for controllers belonging to 2_7_0, so this should
> be mentioned in the commit message. Otherwise, it will imply that the
> L1ss is added for all controller versions.
>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
> Change log should be added here for versions > 1.
>
> Thanks,
> Mani

I will update new patch with your inputs.


Thanks,

Krishna Chaitanya.

>
>>   drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 6ab9089..0d8efcc 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -41,6 +41,9 @@
>>   #define L23_CLK_RMV_DIS				BIT(2)
>>   #define L1_CLK_RMV_DIS				BIT(1)
>>   
>> +#define PCIE20_PARF_PM_CTRL			0x20
>> +#define REQ_NOT_ENTR_L1				BIT(5)
>> +
>>   #define PCIE20_PARF_PHY_CTRL			0x40
>>   #define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	GENMASK(20, 16)
>>   #define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		((x) << 16)
>> @@ -1267,6 +1270,11 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>>   	val |= BIT(4);
>>   	writel(val, pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
>>   
>> +	/* Enable L1 and L1ss */
>> +	val = readl(pcie->parf + PCIE20_PARF_PM_CTRL);
>> +	val &= ~REQ_NOT_ENTR_L1;
>> +	writel(val, pcie->parf + PCIE20_PARF_PM_CTRL);
>> +
>>   	if (IS_ENABLED(CONFIG_PCI_MSI)) {
>>   		val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
>>   		val |= BIT(31);
>> -- 
>> 2.7.4
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-07-15 11:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-03  7:18 [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper Krishna chaitanya chundru
2022-06-08 22:17 ` Stephen Boyd
2022-06-15 13:14   ` Krishna Chaitanya Chundru
2022-06-15 15:44     ` Bjorn Helgaas
2022-06-09 11:26 ` Manivannan Sadhasivam
2022-06-15 13:14   ` Krishna Chaitanya Chundru
2022-06-15 13:15 ` [PATCH v2] PCI: qcom: Allow L1 and its sub states Krishna chaitanya chundru
2022-06-24  8:02   ` Krishna Chaitanya Chundru
2022-06-24  9:02   ` Manivannan Sadhasivam
2022-06-24 17:11   ` Bjorn Helgaas
2022-07-15  8:24   ` Manivannan Sadhasivam
2022-07-15 11:29     ` Krishna Chaitanya Chundru

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).