diff mbox series

[v2,7/8] PCI: qcom: Add support for host_stop_link() & host_start_link()

Message ID 20240803-qps615-v2-7-9560b7c71369@quicinc.com (mailing list archive)
State New
Delegated to: Manivannan Sadhasivam
Headers show
Series PCI: Enable Power and configure the QPS615 PCIe switch | expand

Commit Message

Krishna Chaitanya Chundru Aug. 3, 2024, 3:22 a.m. UTC
For the switches like QPS615 which needs to configure it before
the PCIe link is established.

if the link is not up assert the PERST# and disable LTSSM bit so
that PCIe controller will not participate in the link training
as part of host_stop_link().

De-assert the PERST# and enable LTSSM bit back in host_start_link().

Introduce ltssm_disable function op to stop the link training.

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

Comments

Bjorn Helgaas Aug. 6, 2024, 7:12 p.m. UTC | #1
On Sat, Aug 03, 2024 at 08:52:53AM +0530, Krishna chaitanya chundru wrote:
> For the switches like QPS615 which needs to configure it before
> the PCIe link is established.
> 
> if the link is not up assert the PERST# and disable LTSSM bit so
> that PCIe controller will not participate in the link training
> as part of host_stop_link().
> 
> De-assert the PERST# and enable LTSSM bit back in host_start_link().
> 
> Introduce ltssm_disable function op to stop the link training.

pcie-qcom.c is a driver for a PCIe host controller.  Apparently QPS615
is a switch in a hierarchy that could be below any PCIe host
controller, so I'm missing the connection with pcie-qcom.c.

Does this fix a problem that only occurs with pcie-qcom.c?  What
happens if you put a QPS615 below some other controller?

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 39 ++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 0180edf3310e..f4a6df53139c 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -233,6 +233,7 @@ struct qcom_pcie_ops {
>  	void (*host_post_init)(struct qcom_pcie *pcie);
>  	void (*deinit)(struct qcom_pcie *pcie);
>  	void (*ltssm_enable)(struct qcom_pcie *pcie);
> +	void (*ltssm_disable)(struct qcom_pcie *pcie);
>  	int (*config_sid)(struct qcom_pcie *pcie);
>  };
>  
> @@ -555,6 +556,41 @@ static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie)
>  	return 0;
>  }
>  
> +static int qcom_pcie_host_start_link(struct dw_pcie *pci)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> +
> +	if (!dw_pcie_link_up(pcie->pci))  {
> +		qcom_ep_reset_deassert(pcie);
> +
> +		if (pcie->cfg->ops->ltssm_enable)
> +			pcie->cfg->ops->ltssm_enable(pcie);
> +	}
> +
> +	return 0;
> +}
> +
> +static void qcom_pcie_host_stop_link(struct dw_pcie *pci)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> +
> +	if (!dw_pcie_link_up(pcie->pci))  {
> +		qcom_ep_reset_assert(pcie);
> +
> +		if (pcie->cfg->ops->ltssm_disable)
> +			pcie->cfg->ops->ltssm_disable(pcie);
> +	}
> +}
> +
> +static void qcom_pcie_2_3_2_ltssm_disable(struct qcom_pcie *pcie)
> +{
> +	u32 val;
> +
> +	val = readl(pcie->parf + PARF_LTSSM);
> +	val &= ~LTSSM_EN;
> +	writel(val, pcie->parf + PARF_LTSSM);
> +}
> +
>  static void qcom_pcie_2_3_2_ltssm_enable(struct qcom_pcie *pcie)
>  {
>  	u32 val;
> @@ -1306,6 +1342,7 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
>  	.host_post_init = qcom_pcie_host_post_init_2_7_0,
>  	.deinit = qcom_pcie_deinit_2_7_0,
>  	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> +	.ltssm_disable = qcom_pcie_2_3_2_ltssm_disable,
>  	.config_sid = qcom_pcie_config_sid_1_9_0,
>  };
>  
> @@ -1363,6 +1400,8 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
>  static const struct dw_pcie_ops dw_pcie_ops = {
>  	.link_up = qcom_pcie_link_up,
>  	.start_link = qcom_pcie_start_link,
> +	.host_start_link = qcom_pcie_host_start_link,
> +	.host_stop_link = qcom_pcie_host_stop_link,
>  };
>  
>  static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
> 
> -- 
> 2.34.1
>
Krishna Chaitanya Chundru Sept. 2, 2024, 6:51 a.m. UTC | #2
On 8/7/2024 12:42 AM, Bjorn Helgaas wrote:
> On Sat, Aug 03, 2024 at 08:52:53AM +0530, Krishna chaitanya chundru wrote:
>> For the switches like QPS615 which needs to configure it before
>> the PCIe link is established.
>>
>> if the link is not up assert the PERST# and disable LTSSM bit so
>> that PCIe controller will not participate in the link training
>> as part of host_stop_link().
>>
>> De-assert the PERST# and enable LTSSM bit back in host_start_link().
>>
>> Introduce ltssm_disable function op to stop the link training.
> 
> pcie-qcom.c is a driver for a PCIe host controller.  Apparently QPS615
> is a switch in a hierarchy that could be below any PCIe host
> controller, so I'm missing the connection with pcie-qcom.c.
> 
> Does this fix a problem that only occurs with pcie-qcom.c?  What
> happens if you put a QPS615 below some other controller?
> 
Hi Bjorn,

The qps615 is the qualcomm in-house PCIe switch it is not available to
others. so we are trying to add for qualcomm soc's only.

- Krishna chaitanya.
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 39 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 0180edf3310e..f4a6df53139c 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -233,6 +233,7 @@ struct qcom_pcie_ops {
>>   	void (*host_post_init)(struct qcom_pcie *pcie);
>>   	void (*deinit)(struct qcom_pcie *pcie);
>>   	void (*ltssm_enable)(struct qcom_pcie *pcie);
>> +	void (*ltssm_disable)(struct qcom_pcie *pcie);
>>   	int (*config_sid)(struct qcom_pcie *pcie);
>>   };
>>   
>> @@ -555,6 +556,41 @@ static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie)
>>   	return 0;
>>   }
>>   
>> +static int qcom_pcie_host_start_link(struct dw_pcie *pci)
>> +{
>> +	struct qcom_pcie *pcie = to_qcom_pcie(pci);
>> +
>> +	if (!dw_pcie_link_up(pcie->pci))  {
>> +		qcom_ep_reset_deassert(pcie);
>> +
>> +		if (pcie->cfg->ops->ltssm_enable)
>> +			pcie->cfg->ops->ltssm_enable(pcie);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void qcom_pcie_host_stop_link(struct dw_pcie *pci)
>> +{
>> +	struct qcom_pcie *pcie = to_qcom_pcie(pci);
>> +
>> +	if (!dw_pcie_link_up(pcie->pci))  {
>> +		qcom_ep_reset_assert(pcie);
>> +
>> +		if (pcie->cfg->ops->ltssm_disable)
>> +			pcie->cfg->ops->ltssm_disable(pcie);
>> +	}
>> +}
>> +
>> +static void qcom_pcie_2_3_2_ltssm_disable(struct qcom_pcie *pcie)
>> +{
>> +	u32 val;
>> +
>> +	val = readl(pcie->parf + PARF_LTSSM);
>> +	val &= ~LTSSM_EN;
>> +	writel(val, pcie->parf + PARF_LTSSM);
>> +}
>> +
>>   static void qcom_pcie_2_3_2_ltssm_enable(struct qcom_pcie *pcie)
>>   {
>>   	u32 val;
>> @@ -1306,6 +1342,7 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
>>   	.host_post_init = qcom_pcie_host_post_init_2_7_0,
>>   	.deinit = qcom_pcie_deinit_2_7_0,
>>   	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
>> +	.ltssm_disable = qcom_pcie_2_3_2_ltssm_disable,
>>   	.config_sid = qcom_pcie_config_sid_1_9_0,
>>   };
>>   
>> @@ -1363,6 +1400,8 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
>>   static const struct dw_pcie_ops dw_pcie_ops = {
>>   	.link_up = qcom_pcie_link_up,
>>   	.start_link = qcom_pcie_start_link,
>> +	.host_start_link = qcom_pcie_host_start_link,
>> +	.host_stop_link = qcom_pcie_host_stop_link,
>>   };
>>   
>>   static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
>>
>> -- 
>> 2.34.1
>>
Dmitry Baryshkov Sept. 2, 2024, 6:32 p.m. UTC | #3
On Mon, Sep 02, 2024 at 12:21:22PM GMT, Krishna Chaitanya Chundru wrote:
> 
> 
> On 8/7/2024 12:42 AM, Bjorn Helgaas wrote:
> > On Sat, Aug 03, 2024 at 08:52:53AM +0530, Krishna chaitanya chundru wrote:
> > > For the switches like QPS615 which needs to configure it before
> > > the PCIe link is established.
> > > 
> > > if the link is not up assert the PERST# and disable LTSSM bit so
> > > that PCIe controller will not participate in the link training
> > > as part of host_stop_link().
> > > 
> > > De-assert the PERST# and enable LTSSM bit back in host_start_link().
> > > 
> > > Introduce ltssm_disable function op to stop the link training.
> > 
> > pcie-qcom.c is a driver for a PCIe host controller.  Apparently QPS615
> > is a switch in a hierarchy that could be below any PCIe host
> > controller, so I'm missing the connection with pcie-qcom.c.
> > 
> > Does this fix a problem that only occurs with pcie-qcom.c?  What
> > happens if you put a QPS615 below some other controller?
> > 
> Hi Bjorn,
> 
> The qps615 is the qualcomm in-house PCIe switch it is not available to
> others. so we are trying to add for qualcomm soc's only.

Any guarantee that the status quo will stay so in future? Or that it
won't appear on Qualcomm platform with the virtualized PCIe controller?
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 0180edf3310e..f4a6df53139c 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -233,6 +233,7 @@  struct qcom_pcie_ops {
 	void (*host_post_init)(struct qcom_pcie *pcie);
 	void (*deinit)(struct qcom_pcie *pcie);
 	void (*ltssm_enable)(struct qcom_pcie *pcie);
+	void (*ltssm_disable)(struct qcom_pcie *pcie);
 	int (*config_sid)(struct qcom_pcie *pcie);
 };
 
@@ -555,6 +556,41 @@  static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie)
 	return 0;
 }
 
+static int qcom_pcie_host_start_link(struct dw_pcie *pci)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+
+	if (!dw_pcie_link_up(pcie->pci))  {
+		qcom_ep_reset_deassert(pcie);
+
+		if (pcie->cfg->ops->ltssm_enable)
+			pcie->cfg->ops->ltssm_enable(pcie);
+	}
+
+	return 0;
+}
+
+static void qcom_pcie_host_stop_link(struct dw_pcie *pci)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+
+	if (!dw_pcie_link_up(pcie->pci))  {
+		qcom_ep_reset_assert(pcie);
+
+		if (pcie->cfg->ops->ltssm_disable)
+			pcie->cfg->ops->ltssm_disable(pcie);
+	}
+}
+
+static void qcom_pcie_2_3_2_ltssm_disable(struct qcom_pcie *pcie)
+{
+	u32 val;
+
+	val = readl(pcie->parf + PARF_LTSSM);
+	val &= ~LTSSM_EN;
+	writel(val, pcie->parf + PARF_LTSSM);
+}
+
 static void qcom_pcie_2_3_2_ltssm_enable(struct qcom_pcie *pcie)
 {
 	u32 val;
@@ -1306,6 +1342,7 @@  static const struct qcom_pcie_ops ops_1_9_0 = {
 	.host_post_init = qcom_pcie_host_post_init_2_7_0,
 	.deinit = qcom_pcie_deinit_2_7_0,
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
+	.ltssm_disable = qcom_pcie_2_3_2_ltssm_disable,
 	.config_sid = qcom_pcie_config_sid_1_9_0,
 };
 
@@ -1363,6 +1400,8 @@  static const struct qcom_pcie_cfg cfg_sc8280xp = {
 static const struct dw_pcie_ops dw_pcie_ops = {
 	.link_up = qcom_pcie_link_up,
 	.start_link = qcom_pcie_start_link,
+	.host_start_link = qcom_pcie_host_start_link,
+	.host_stop_link = qcom_pcie_host_stop_link,
 };
 
 static int qcom_pcie_icc_init(struct qcom_pcie *pcie)