| Message ID | 20240803-qps615-v2-7-9560b7c71369@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | PCI: Enable Power and configure the QPS615 PCIe switch | expand |
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 >
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 >>
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 --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)
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(+)