diff mbox series

[v2,2/6] PCI: qcom: add support for IPQ60xx PCIe controller

Message ID e17461407cf4bb79fed5925ec81196a0b84e7827.1620203062.git.baruch@tkos.co.il (mailing list archive)
State Changes Requested
Delegated to: Lorenzo Pieralisi
Headers show
Series arm64: IPQ6018 PCIe support | expand

Commit Message

Baruch Siach May 5, 2021, 9:18 a.m. UTC
From: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>

IPQ60xx series of SoCs have one port of PCIe gen 3. Add support for that
platform.

The code is based on downstream Codeaurora kernel v5.4. Split out the
DBI registers access part from .init into .post_init. DBI registers are
only accessible after phy_power_on().

Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2:
  * Drop ATU configuration; rely on common code instead

  * Use more common register macros

  * Use bulk clk and reset APIs
---
 drivers/pci/controller/dwc/pcie-designware.h |   1 +
 drivers/pci/controller/dwc/pcie-qcom.c       | 150 +++++++++++++++++++
 2 files changed, 151 insertions(+)

Comments

Rob Herring Aug. 6, 2021, 7:59 p.m. UTC | #1
On Wed, May 5, 2021 at 3:18 AM Baruch Siach <baruch@tkos.co.il> wrote:
>
> From: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
>
> IPQ60xx series of SoCs have one port of PCIe gen 3. Add support for that
> platform.
>
> The code is based on downstream Codeaurora kernel v5.4. Split out the
> DBI registers access part from .init into .post_init. DBI registers are
> only accessible after phy_power_on().
>
> Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2:
>   * Drop ATU configuration; rely on common code instead
>
>   * Use more common register macros
>
>   * Use bulk clk and reset APIs
> ---
>  drivers/pci/controller/dwc/pcie-designware.h |   1 +
>  drivers/pci/controller/dwc/pcie-qcom.c       | 150 +++++++++++++++++++
>  2 files changed, 151 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index ceb359b6e6a6..346462c74a3e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -76,6 +76,7 @@
>
>  #define GEN3_RELATED_OFF                       0x890
>  #define GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL   BIT(0)
> +#define GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS    BIT(13)
>  #define GEN3_RELATED_OFF_GEN3_EQ_DISABLE       BIT(16)
>  #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24
>  #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK  GENMASK(25, 24)
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 8a7a300163e5..93766aee3e7c 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -52,6 +52,10 @@
>  #define PCIE20_PARF_DBI_BASE_ADDR              0x168
>  #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE                0x16C
>  #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL       0x174
> +#define AHB_CLK_EN                             BIT(0)
> +#define MSTR_AXI_CLK_EN                                BIT(1)
> +#define BYPASS                                 BIT(4)
> +
>  #define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT      0x178
>  #define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2   0x1A8
>  #define PCIE20_PARF_LTSSM                      0x1B0
> @@ -94,6 +98,12 @@
>  #define SLV_ADDR_SPACE_SZ                      0x10000000
>
>  #define PCIE20_LNK_CONTROL2_LINK_STATUS2       0xa0
> +#define PCIE_CAP_CURR_DEEMPHASIS               BIT(16)

Isn't this a standard register field?

> +#define SPEED_GEN1                             0x1
> +#define SPEED_GEN2                             0x2
> +#define SPEED_GEN3                             0x3

And these?

There's already some common DWC code for setting the link speed.

> +#define AXI_CLK_RATE                           200000000
> +#define RCHNG_CLK_RATE                         100000000
>
>  #define DEVICE_TYPE_RC                         0x4
>
> @@ -168,6 +178,11 @@ struct qcom_pcie_resources_2_7_0 {
>         struct clk *pipe_clk;
>  };
>
> +struct qcom_pcie_resources_2_9_0 {
> +       struct clk_bulk_data clks[5];
> +       struct reset_control *rst;
> +};
> +
>  union qcom_pcie_resources {
>         struct qcom_pcie_resources_1_0_0 v1_0_0;
>         struct qcom_pcie_resources_2_1_0 v2_1_0;
> @@ -175,6 +190,7 @@ union qcom_pcie_resources {
>         struct qcom_pcie_resources_2_3_3 v2_3_3;
>         struct qcom_pcie_resources_2_4_0 v2_4_0;
>         struct qcom_pcie_resources_2_7_0 v2_7_0;
> +       struct qcom_pcie_resources_2_9_0 v2_9_0;
>  };
>
>  struct qcom_pcie;
> @@ -1266,6 +1282,130 @@ static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie)
>         clk_disable_unprepare(res->pipe_clk);
>  }
>
> +static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
> +{
> +       struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> +       struct dw_pcie *pci = pcie->pci;
> +       struct device *dev = pci->dev;
> +       int ret;
> +
> +       res->clks[0].id = "iface";
> +       res->clks[1].id = "axi_m";
> +       res->clks[2].id = "axi_s";
> +       res->clks[3].id = "axi_bridge";
> +       res->clks[4].id = "rchng";
> +
> +       ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
> +       if (ret < 0)
> +               return ret;
> +
> +       res->rst = devm_reset_control_array_get_exclusive(dev);
> +       if (IS_ERR(res->rst))
> +               return PTR_ERR(res->rst);
> +
> +       return 0;
> +}
> +
> +static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
> +{
> +       struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> +
> +       clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
> +}
> +
> +static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
> +{
> +       struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> +       struct device *dev = pcie->pci->dev;
> +       int ret;
> +
> +       ret = reset_control_assert(res->rst);
> +       if (ret) {
> +               dev_err(dev, "reset assert failed (%d)\n", ret);
> +               return ret;
> +       }
> +
> +       usleep_range(2000, 2500);
> +
> +       ret = reset_control_deassert(res->rst);
> +       if (ret) {
> +               dev_err(dev, "reset deassert failed (%d)\n", ret);
> +               return ret;
> +       }
> +
> +       /*
> +        * Don't have a way to see if the reset has completed.
> +        * Wait for some time.
> +        */
> +       usleep_range(2000, 2500);
> +
> +       ret = clk_bulk_prepare_enable(ARRAY_SIZE(res->clks), res->clks);
> +       if (ret)
> +               goto err_reset;
> +
> +       return 0;
> +
> +       /*
> +        * Not checking for failure, will anyway return
> +        * the original failure in 'ret'.
> +        */
> +err_reset:
> +       reset_control_assert(res->rst);
> +
> +       return ret;
> +}
> +
> +static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> +{
> +       struct dw_pcie *pci = pcie->pci;
> +       u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +       u32 val;
> +       int i;
> +
> +       writel(SLV_ADDR_SPACE_SZ,
> +               pcie->parf + PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE);
> +
> +       val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> +       val &= ~BIT(0);
> +       writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> +
> +       writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
> +
> +       writel(DEVICE_TYPE_RC, pcie->parf + PCIE20_PARF_DEVICE_TYPE);
> +       writel(BYPASS | MSTR_AXI_CLK_EN | AHB_CLK_EN,
> +               pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
> +       writel(GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS
> +               | GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL,
> +               pci->dbi_base + GEN3_RELATED_OFF);
> +
> +       writel(MST_WAKEUP_EN | SLV_WAKEUP_EN | MSTR_ACLK_CGC_DIS
> +               | SLV_ACLK_CGC_DIS | CORE_CLK_CGC_DIS |
> +               AUX_PWR_DET | L23_CLK_RMV_DIS | L1_CLK_RMV_DIS,
> +               pcie->parf + PCIE20_PARF_SYS_CTRL);
> +
> +       writel(0, pcie->parf + PCIE20_PARF_Q2A_FLUSH);
> +
> +       dw_pcie_dbi_ro_wr_en(pci);
> +       writel(PCIE_CAP_LINK1_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP);
> +
> +       /* Configure PCIe link capabilities for ASPM */
> +       val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
> +       val &= ~PCI_EXP_LNKCAP_ASPMS;
> +       writel(val, pci->dbi_base + offset + PCI_EXP_LNKCAP);
> +
> +       writel(PCI_EXP_DEVCTL2_COMP_TMOUT_DIS, pci->dbi_base + offset +
> +                       PCI_EXP_DEVCTL2);
> +
> +       writel(PCIE_CAP_CURR_DEEMPHASIS | SPEED_GEN3,
> +                       pci->dbi_base + offset + PCI_EXP_DEVCTL2);

Doesn't this overwrite the prior register write?

> +
> +       for (i = 0;i < 256;i++)
> +               writel(0x0, pcie->parf + PCIE20_PARF_BDF_TO_SID_TABLE_N
> +                               + (4 * i));
> +
> +       return 0;
> +}
> +
>  static int qcom_pcie_link_up(struct dw_pcie *pci)
>  {
>         u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> @@ -1456,6 +1596,15 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
>         .config_sid = qcom_pcie_config_sid_sm8250,
>  };
>
> +/* Qcom IP rev.: 2.9.0  Synopsys IP rev.: 5.00a */
> +static const struct qcom_pcie_ops ops_2_9_0 = {
> +       .get_resources = qcom_pcie_get_resources_2_9_0,
> +       .init = qcom_pcie_init_2_9_0,
> +       .post_init = qcom_pcie_post_init_2_9_0,
> +       .deinit = qcom_pcie_deinit_2_9_0,
> +       .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> +};
> +
>  static const struct dw_pcie_ops dw_pcie_ops = {
>         .link_up = qcom_pcie_link_up,
>         .start_link = qcom_pcie_start_link,
> @@ -1555,6 +1704,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>         { .compatible = "qcom,pcie-qcs404", .data = &ops_2_4_0 },
>         { .compatible = "qcom,pcie-sdm845", .data = &ops_2_7_0 },
>         { .compatible = "qcom,pcie-sm8250", .data = &ops_1_9_0 },
> +       { .compatible = "qcom,pcie-ipq6018", .data = &ops_2_9_0 },
>         { }
>  };
>
> --
> 2.30.2
>
Baruch Siach Aug. 25, 2021, 11:15 a.m. UTC | #2
Hi Rob,

On Fri, Aug 06 2021, Rob Herring wrote:
> On Wed, May 5, 2021 at 3:18 AM Baruch Siach <baruch@tkos.co.il> wrote:
>>
>> From: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
>>
>> IPQ60xx series of SoCs have one port of PCIe gen 3. Add support for that
>> platform.
>>
>> The code is based on downstream Codeaurora kernel v5.4. Split out the
>> DBI registers access part from .init into .post_init. DBI registers are
>> only accessible after phy_power_on().
>>
>> Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> ---
>> v2:
>>   * Drop ATU configuration; rely on common code instead
>>
>>   * Use more common register macros
>>
>>   * Use bulk clk and reset APIs
>> ---
>>  drivers/pci/controller/dwc/pcie-designware.h |   1 +
>>  drivers/pci/controller/dwc/pcie-qcom.c       | 150 +++++++++++++++++++
>>  2 files changed, 151 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index ceb359b6e6a6..346462c74a3e 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -76,6 +76,7 @@
>>
>>  #define GEN3_RELATED_OFF                       0x890
>>  #define GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL   BIT(0)
>> +#define GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS    BIT(13)
>>  #define GEN3_RELATED_OFF_GEN3_EQ_DISABLE       BIT(16)
>>  #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24
>>  #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK  GENMASK(25, 24)
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 8a7a300163e5..93766aee3e7c 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -52,6 +52,10 @@
>>  #define PCIE20_PARF_DBI_BASE_ADDR              0x168
>>  #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE                0x16C
>>  #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL       0x174
>> +#define AHB_CLK_EN                             BIT(0)
>> +#define MSTR_AXI_CLK_EN                                BIT(1)
>> +#define BYPASS                                 BIT(4)
>> +
>>  #define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT      0x178
>>  #define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2   0x1A8
>>  #define PCIE20_PARF_LTSSM                      0x1B0
>> @@ -94,6 +98,12 @@
>>  #define SLV_ADDR_SPACE_SZ                      0x10000000
>>
>>  #define PCIE20_LNK_CONTROL2_LINK_STATUS2       0xa0
>> +#define PCIE_CAP_CURR_DEEMPHASIS               BIT(16)
>
> Isn't this a standard register field?

I don't know. I could not find any reference to this field or the
registers it's part of.
n
>> +#define SPEED_GEN1                             0x1
>> +#define SPEED_GEN2                             0x2
>> +#define SPEED_GEN3                             0x3
>
> And these?
>
> There's already some common DWC code for setting the link speed.

dw_pcie_link_set_max_speed() deals with other registers, as far as I can
see.

>> +#define AXI_CLK_RATE                           200000000
>> +#define RCHNG_CLK_RATE                         100000000
>>
>>  #define DEVICE_TYPE_RC                         0x4

[snip]

>> +static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>> +{
>> +       struct dw_pcie *pci = pcie->pci;
>> +       u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>> +       u32 val;
>> +       int i;
>> +
>> +       writel(SLV_ADDR_SPACE_SZ,
>> +               pcie->parf + PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE);
>> +
>> +       val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
>> +       val &= ~BIT(0);
>> +       writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
>> +
>> +       writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
>> +
>> +       writel(DEVICE_TYPE_RC, pcie->parf + PCIE20_PARF_DEVICE_TYPE);
>> +       writel(BYPASS | MSTR_AXI_CLK_EN | AHB_CLK_EN,
>> +               pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
>> +       writel(GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS
>> +               | GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL,
>> +               pci->dbi_base + GEN3_RELATED_OFF);
>> +
>> +       writel(MST_WAKEUP_EN | SLV_WAKEUP_EN | MSTR_ACLK_CGC_DIS
>> +               | SLV_ACLK_CGC_DIS | CORE_CLK_CGC_DIS |
>> +               AUX_PWR_DET | L23_CLK_RMV_DIS | L1_CLK_RMV_DIS,
>> +               pcie->parf + PCIE20_PARF_SYS_CTRL);
>> +
>> +       writel(0, pcie->parf + PCIE20_PARF_Q2A_FLUSH);
>> +
>> +       dw_pcie_dbi_ro_wr_en(pci);
>> +       writel(PCIE_CAP_LINK1_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP);
>> +
>> +       /* Configure PCIe link capabilities for ASPM */
>> +       val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
>> +       val &= ~PCI_EXP_LNKCAP_ASPMS;
>> +       writel(val, pci->dbi_base + offset + PCI_EXP_LNKCAP);
>> +
>> +       writel(PCI_EXP_DEVCTL2_COMP_TMOUT_DIS, pci->dbi_base + offset +
>> +                       PCI_EXP_DEVCTL2);
>> +
>> +       writel(PCIE_CAP_CURR_DEEMPHASIS | SPEED_GEN3,
>> +                       pci->dbi_base + offset + PCI_EXP_DEVCTL2);
>
> Doesn't this overwrite the prior register write?

It does. There are two mistakes here. The writel() above should set
PCIE20_DEVICE_CONTROL2_STATUS2 (offset 0x98). This writel() should set
PCIE20_LNK_CONTROL2_LINK_STATUS2 (offset 0xa0). So both are wrong.

>> +
>> +       for (i = 0;i < 256;i++)
>> +               writel(0x0, pcie->parf + PCIE20_PARF_BDF_TO_SID_TABLE_N
>> +                               + (4 * i));
>> +
>> +       return 0;
>> +}

Thanks,
baruch
Rob Herring Aug. 25, 2021, 1:38 p.m. UTC | #3
On Wed, Aug 25, 2021 at 6:25 AM Baruch Siach <baruch@tkos.co.il> wrote:
>
> Hi Rob,
>
> On Fri, Aug 06 2021, Rob Herring wrote:
> > On Wed, May 5, 2021 at 3:18 AM Baruch Siach <baruch@tkos.co.il> wrote:
> >>
> >> From: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
> >>
> >> IPQ60xx series of SoCs have one port of PCIe gen 3. Add support for that
> >> platform.
> >>
> >> The code is based on downstream Codeaurora kernel v5.4. Split out the
> >> DBI registers access part from .init into .post_init. DBI registers are
> >> only accessible after phy_power_on().
> >>
> >> Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
> >> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> >> ---
> >> v2:
> >>   * Drop ATU configuration; rely on common code instead
> >>
> >>   * Use more common register macros
> >>
> >>   * Use bulk clk and reset APIs
> >> ---
> >>  drivers/pci/controller/dwc/pcie-designware.h |   1 +
> >>  drivers/pci/controller/dwc/pcie-qcom.c       | 150 +++++++++++++++++++
> >>  2 files changed, 151 insertions(+)
> >>
> >> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> >> index ceb359b6e6a6..346462c74a3e 100644
> >> --- a/drivers/pci/controller/dwc/pcie-designware.h
> >> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> >> @@ -76,6 +76,7 @@
> >>
> >>  #define GEN3_RELATED_OFF                       0x890
> >>  #define GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL   BIT(0)
> >> +#define GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS    BIT(13)
> >>  #define GEN3_RELATED_OFF_GEN3_EQ_DISABLE       BIT(16)
> >>  #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24
> >>  #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK  GENMASK(25, 24)
> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> >> index 8a7a300163e5..93766aee3e7c 100644
> >> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> >> @@ -52,6 +52,10 @@
> >>  #define PCIE20_PARF_DBI_BASE_ADDR              0x168
> >>  #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE                0x16C
> >>  #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL       0x174
> >> +#define AHB_CLK_EN                             BIT(0)
> >> +#define MSTR_AXI_CLK_EN                                BIT(1)
> >> +#define BYPASS                                 BIT(4)
> >> +
> >>  #define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT      0x178
> >>  #define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2   0x1A8
> >>  #define PCIE20_PARF_LTSSM                      0x1B0
> >> @@ -94,6 +98,12 @@
> >>  #define SLV_ADDR_SPACE_SZ                      0x10000000
> >>
> >>  #define PCIE20_LNK_CONTROL2_LINK_STATUS2       0xa0
> >> +#define PCIE_CAP_CURR_DEEMPHASIS               BIT(16)
> >
> > Isn't this a standard register field?
>
> I don't know. I could not find any reference to this field or the
> registers it's part of.

It is. I found it in the spec. But it's not in pci_reg.h and it is a
read-only status bit (though some in DWC can be written).

> n
> >> +#define SPEED_GEN1                             0x1
> >> +#define SPEED_GEN2                             0x2
> >> +#define SPEED_GEN3                             0x3
> >
> > And these?

#define PCI_EXP_LNKCTL2         48      /* Link Control 2 */
#define  PCI_EXP_LNKCTL2_TLS            0x000f
#define  PCI_EXP_LNKCTL2_TLS_2_5GT      0x0001 /* Supported Speed 2.5GT/s */
#define  PCI_EXP_LNKCTL2_TLS_5_0GT      0x0002 /* Supported Speed 5GT/s */
#define  PCI_EXP_LNKCTL2_TLS_8_0GT      0x0003 /* Supported Speed 8GT/s */
#define  PCI_EXP_LNKCTL2_TLS_16_0GT     0x0004 /* Supported Speed 16GT/s */
#define  PCI_EXP_LNKCTL2_TLS_32_0GT     0x0005 /* Supported Speed 32GT/s */
#define  PCI_EXP_LNKCTL2_TLS_64_0GT     0x0006 /* Supported Speed 64GT/s */
#define  PCI_EXP_LNKCTL2_ENTER_COMP     0x0010 /* Enter Compliance */
#define  PCI_EXP_LNKCTL2_TX_MARGIN      0x0380 /* Transmit Margin */
#define  PCI_EXP_LNKCTL2_HASD           0x0020 /* HW Autonomous Speed Disable */
#define PCI_EXP_LNKSTA2         50      /* Link Status 2 */

> >
> > There's already some common DWC code for setting the link speed.
>
> dw_pcie_link_set_max_speed() deals with other registers, as far as I can
> see.

Link Control2 which is what the above function configures appears to
be the same register to me. Your definition is combining 2 registers.

>
> >> +#define AXI_CLK_RATE                           200000000
> >> +#define RCHNG_CLK_RATE                         100000000
> >>
> >>  #define DEVICE_TYPE_RC                         0x4
>
> [snip]
>
> >> +static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> >> +{
> >> +       struct dw_pcie *pci = pcie->pci;
> >> +       u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> >> +       u32 val;
> >> +       int i;
> >> +
> >> +       writel(SLV_ADDR_SPACE_SZ,
> >> +               pcie->parf + PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE);
> >> +
> >> +       val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> >> +       val &= ~BIT(0);
> >> +       writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> >> +
> >> +       writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
> >> +
> >> +       writel(DEVICE_TYPE_RC, pcie->parf + PCIE20_PARF_DEVICE_TYPE);
> >> +       writel(BYPASS | MSTR_AXI_CLK_EN | AHB_CLK_EN,
> >> +               pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
> >> +       writel(GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS
> >> +               | GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL,
> >> +               pci->dbi_base + GEN3_RELATED_OFF);
> >> +
> >> +       writel(MST_WAKEUP_EN | SLV_WAKEUP_EN | MSTR_ACLK_CGC_DIS
> >> +               | SLV_ACLK_CGC_DIS | CORE_CLK_CGC_DIS |
> >> +               AUX_PWR_DET | L23_CLK_RMV_DIS | L1_CLK_RMV_DIS,
> >> +               pcie->parf + PCIE20_PARF_SYS_CTRL);
> >> +
> >> +       writel(0, pcie->parf + PCIE20_PARF_Q2A_FLUSH);
> >> +
> >> +       dw_pcie_dbi_ro_wr_en(pci);
> >> +       writel(PCIE_CAP_LINK1_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP);
> >> +
> >> +       /* Configure PCIe link capabilities for ASPM */
> >> +       val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
> >> +       val &= ~PCI_EXP_LNKCAP_ASPMS;
> >> +       writel(val, pci->dbi_base + offset + PCI_EXP_LNKCAP);
> >> +
> >> +       writel(PCI_EXP_DEVCTL2_COMP_TMOUT_DIS, pci->dbi_base + offset +
> >> +                       PCI_EXP_DEVCTL2);
> >> +
> >> +       writel(PCIE_CAP_CURR_DEEMPHASIS | SPEED_GEN3,
> >> +                       pci->dbi_base + offset + PCI_EXP_DEVCTL2);
> >
> > Doesn't this overwrite the prior register write?
>
> It does. There are two mistakes here. The writel() above should set
> PCIE20_DEVICE_CONTROL2_STATUS2 (offset 0x98).

No. Did you check what 'offset' is? PCIE20_DEVICE_CONTROL2_STATUS2 is
PCI_EXP_DEVCTL2 plus the status reg. What's wrong is it should be a
16-bit write.

> This writel() should set
> PCIE20_LNK_CONTROL2_LINK_STATUS2 (offset 0xa0). So both are wrong.
>
> >> +
> >> +       for (i = 0;i < 256;i++)
> >> +               writel(0x0, pcie->parf + PCIE20_PARF_BDF_TO_SID_TABLE_N
> >> +                               + (4 * i));
> >> +
> >> +       return 0;
> >> +}
>
> Thanks,
> baruch
>
> --
>                                                      ~. .~   Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
>    - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
Baruch Siach Aug. 25, 2021, 2:09 p.m. UTC | #4
Hi Rob,

On Wed, Aug 25 2021, Rob Herring wrote:
> On Wed, Aug 25, 2021 at 6:25 AM Baruch Siach <baruch@tkos.co.il> wrote:
>> On Fri, Aug 06 2021, Rob Herring wrote:
>> > On Wed, May 5, 2021 at 3:18 AM Baruch Siach <baruch@tkos.co.il> wrote:
>> >> +       writel(PCI_EXP_DEVCTL2_COMP_TMOUT_DIS, pci->dbi_base + offset +
>> >> +                       PCI_EXP_DEVCTL2);
>> >> +
>> >> +       writel(PCIE_CAP_CURR_DEEMPHASIS | SPEED_GEN3,
>> >> +                       pci->dbi_base + offset + PCI_EXP_DEVCTL2);
>> >
>> > Doesn't this overwrite the prior register write?
>>
>> It does. There are two mistakes here. The writel() above should set
>> PCIE20_DEVICE_CONTROL2_STATUS2 (offset 0x98).
>
> No. Did you check what 'offset' is? PCIE20_DEVICE_CONTROL2_STATUS2 is
> PCI_EXP_DEVCTL2 plus the status reg. What's wrong is it should be a
> 16-bit write.

Thanks for enlightening me. 'offset' is 0x70 here. So PCI_EXP_DEVCTL2 is
at 0x98, and PCI_EXP_LNKCTL2 is at 0xa0. Only the second writel() is
wrong. But since generic code handles speed, I can drop it entirely.

I see that dw_pcie_link_set_max_speed() uses dw_pcie_writel_dbi() to
write to PCI_EXP_LNKCTL2. Is that 16-bit write?

Why are pci_regs.h register offsets in decimal?

>> This writel() should set
>> PCIE20_LNK_CONTROL2_LINK_STATUS2 (offset 0xa0). So both are wrong.

Thanks,
baruch
Rob Herring Aug. 25, 2021, 3:03 p.m. UTC | #5
On Wed, Aug 25, 2021 at 9:23 AM Baruch Siach <baruch@tkos.co.il> wrote:
>
> Hi Rob,
>
> On Wed, Aug 25 2021, Rob Herring wrote:
> > On Wed, Aug 25, 2021 at 6:25 AM Baruch Siach <baruch@tkos.co.il> wrote:
> >> On Fri, Aug 06 2021, Rob Herring wrote:
> >> > On Wed, May 5, 2021 at 3:18 AM Baruch Siach <baruch@tkos.co.il> wrote:
> >> >> +       writel(PCI_EXP_DEVCTL2_COMP_TMOUT_DIS, pci->dbi_base + offset +
> >> >> +                       PCI_EXP_DEVCTL2);
> >> >> +
> >> >> +       writel(PCIE_CAP_CURR_DEEMPHASIS | SPEED_GEN3,
> >> >> +                       pci->dbi_base + offset + PCI_EXP_DEVCTL2);
> >> >
> >> > Doesn't this overwrite the prior register write?
> >>
> >> It does. There are two mistakes here. The writel() above should set
> >> PCIE20_DEVICE_CONTROL2_STATUS2 (offset 0x98).
> >
> > No. Did you check what 'offset' is? PCIE20_DEVICE_CONTROL2_STATUS2 is
> > PCI_EXP_DEVCTL2 plus the status reg. What's wrong is it should be a
> > 16-bit write.
>
> Thanks for enlightening me. 'offset' is 0x70 here. So PCI_EXP_DEVCTL2 is
> at 0x98, and PCI_EXP_LNKCTL2 is at 0xa0. Only the second writel() is
> wrong. But since generic code handles speed, I can drop it entirely.
>
> I see that dw_pcie_link_set_max_speed() uses dw_pcie_writel_dbi() to
> write to PCI_EXP_LNKCTL2. Is that 16-bit write?

No, that may be because some platforms can only do 32-bit accesses and
dw_pcie_writew_dbi would be a RMW in that case. Or maybe there's some
reliance on clearing the status register.

> Why are pci_regs.h register offsets in decimal?

No idea...

Rob
Bjorn Helgaas Aug. 25, 2021, 4:05 p.m. UTC | #6
On Wed, Aug 25, 2021 at 10:03:51AM -0500, Rob Herring wrote:
> On Wed, Aug 25, 2021 at 9:23 AM Baruch Siach <baruch@tkos.co.il> wrote:

> > Why are pci_regs.h register offsets in decimal?
> 
> No idea...

That does seem confusing and error prone.  I'd merge a patch that made
them hex to match the spec.
Bjorn Helgaas Aug. 25, 2021, 4:37 p.m. UTC | #7
In subject:

s/add support/Add support/ to match previous history.

On Wed, May 05, 2021 at 12:18:30PM +0300, Baruch Siach wrote:
> From: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
> 
> IPQ60xx series of SoCs have one port of PCIe gen 3. Add support for that
> platform.
> 
> The code is based on downstream Codeaurora kernel v5.4. Split out the
> DBI registers access part from .init into .post_init. DBI registers are
> only accessible after phy_power_on().

The "downstream Codeaurora kernel v5.4" reference would be more useful
if there were a URL reference to that driver.

> +#define AXI_CLK_RATE				200000000
> +#define RCHNG_CLK_RATE				100000000

These are unused.

> +	for (i = 0;i < 256;i++)

Add spaces after semicolons.
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index ceb359b6e6a6..346462c74a3e 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -76,6 +76,7 @@ 
 
 #define GEN3_RELATED_OFF			0x890
 #define GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL	BIT(0)
+#define GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS	BIT(13)
 #define GEN3_RELATED_OFF_GEN3_EQ_DISABLE	BIT(16)
 #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT	24
 #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK	GENMASK(25, 24)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 8a7a300163e5..93766aee3e7c 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -52,6 +52,10 @@ 
 #define PCIE20_PARF_DBI_BASE_ADDR		0x168
 #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE		0x16C
 #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL	0x174
+#define AHB_CLK_EN				BIT(0)
+#define MSTR_AXI_CLK_EN				BIT(1)
+#define BYPASS					BIT(4)
+
 #define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT	0x178
 #define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2	0x1A8
 #define PCIE20_PARF_LTSSM			0x1B0
@@ -94,6 +98,12 @@ 
 #define SLV_ADDR_SPACE_SZ			0x10000000
 
 #define PCIE20_LNK_CONTROL2_LINK_STATUS2	0xa0
+#define PCIE_CAP_CURR_DEEMPHASIS		BIT(16)
+#define SPEED_GEN1				0x1
+#define SPEED_GEN2				0x2
+#define SPEED_GEN3				0x3
+#define AXI_CLK_RATE				200000000
+#define RCHNG_CLK_RATE				100000000
 
 #define DEVICE_TYPE_RC				0x4
 
@@ -168,6 +178,11 @@  struct qcom_pcie_resources_2_7_0 {
 	struct clk *pipe_clk;
 };
 
+struct qcom_pcie_resources_2_9_0 {
+	struct clk_bulk_data clks[5];
+	struct reset_control *rst;
+};
+
 union qcom_pcie_resources {
 	struct qcom_pcie_resources_1_0_0 v1_0_0;
 	struct qcom_pcie_resources_2_1_0 v2_1_0;
@@ -175,6 +190,7 @@  union qcom_pcie_resources {
 	struct qcom_pcie_resources_2_3_3 v2_3_3;
 	struct qcom_pcie_resources_2_4_0 v2_4_0;
 	struct qcom_pcie_resources_2_7_0 v2_7_0;
+	struct qcom_pcie_resources_2_9_0 v2_9_0;
 };
 
 struct qcom_pcie;
@@ -1266,6 +1282,130 @@  static void qcom_pcie_post_deinit_2_7_0(struct qcom_pcie *pcie)
 	clk_disable_unprepare(res->pipe_clk);
 }
 
+static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
+	struct dw_pcie *pci = pcie->pci;
+	struct device *dev = pci->dev;
+	int ret;
+
+	res->clks[0].id = "iface";
+	res->clks[1].id = "axi_m";
+	res->clks[2].id = "axi_s";
+	res->clks[3].id = "axi_bridge";
+	res->clks[4].id = "rchng";
+
+	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
+	if (ret < 0)
+		return ret;
+
+	res->rst = devm_reset_control_array_get_exclusive(dev);
+	if (IS_ERR(res->rst))
+		return PTR_ERR(res->rst);
+
+	return 0;
+}
+
+static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
+
+	clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
+}
+
+static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
+	struct device *dev = pcie->pci->dev;
+	int ret;
+
+	ret = reset_control_assert(res->rst);
+	if (ret) {
+		dev_err(dev, "reset assert failed (%d)\n", ret);
+		return ret;
+	}
+
+	usleep_range(2000, 2500);
+
+	ret = reset_control_deassert(res->rst);
+	if (ret) {
+		dev_err(dev, "reset deassert failed (%d)\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Don't have a way to see if the reset has completed.
+	 * Wait for some time.
+	 */
+	usleep_range(2000, 2500);
+
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(res->clks), res->clks);
+	if (ret)
+		goto err_reset;
+
+	return 0;
+
+	/*
+	 * Not checking for failure, will anyway return
+	 * the original failure in 'ret'.
+	 */
+err_reset:
+	reset_control_assert(res->rst);
+
+	return ret;
+}
+
+static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
+{
+	struct dw_pcie *pci = pcie->pci;
+	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u32 val;
+	int i;
+
+	writel(SLV_ADDR_SPACE_SZ,
+		pcie->parf + PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE);
+
+	val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
+	val &= ~BIT(0);
+	writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
+
+	writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
+
+	writel(DEVICE_TYPE_RC, pcie->parf + PCIE20_PARF_DEVICE_TYPE);
+	writel(BYPASS | MSTR_AXI_CLK_EN | AHB_CLK_EN,
+		pcie->parf + PCIE20_PARF_MHI_CLOCK_RESET_CTRL);
+	writel(GEN3_RELATED_OFF_RXEQ_RGRDLESS_RXTS
+		| GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL,
+		pci->dbi_base + GEN3_RELATED_OFF);
+
+	writel(MST_WAKEUP_EN | SLV_WAKEUP_EN | MSTR_ACLK_CGC_DIS
+		| SLV_ACLK_CGC_DIS | CORE_CLK_CGC_DIS |
+		AUX_PWR_DET | L23_CLK_RMV_DIS | L1_CLK_RMV_DIS,
+		pcie->parf + PCIE20_PARF_SYS_CTRL);
+
+	writel(0, pcie->parf + PCIE20_PARF_Q2A_FLUSH);
+
+	dw_pcie_dbi_ro_wr_en(pci);
+	writel(PCIE_CAP_LINK1_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP);
+
+	/* Configure PCIe link capabilities for ASPM */
+	val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
+	val &= ~PCI_EXP_LNKCAP_ASPMS;
+	writel(val, pci->dbi_base + offset + PCI_EXP_LNKCAP);
+
+	writel(PCI_EXP_DEVCTL2_COMP_TMOUT_DIS, pci->dbi_base + offset +
+			PCI_EXP_DEVCTL2);
+
+	writel(PCIE_CAP_CURR_DEEMPHASIS | SPEED_GEN3,
+			pci->dbi_base + offset + PCI_EXP_DEVCTL2);
+
+	for (i = 0;i < 256;i++)
+		writel(0x0, pcie->parf + PCIE20_PARF_BDF_TO_SID_TABLE_N
+				+ (4 * i));
+
+	return 0;
+}
+
 static int qcom_pcie_link_up(struct dw_pcie *pci)
 {
 	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
@@ -1456,6 +1596,15 @@  static const struct qcom_pcie_ops ops_1_9_0 = {
 	.config_sid = qcom_pcie_config_sid_sm8250,
 };
 
+/* Qcom IP rev.: 2.9.0  Synopsys IP rev.: 5.00a */
+static const struct qcom_pcie_ops ops_2_9_0 = {
+	.get_resources = qcom_pcie_get_resources_2_9_0,
+	.init = qcom_pcie_init_2_9_0,
+	.post_init = qcom_pcie_post_init_2_9_0,
+	.deinit = qcom_pcie_deinit_2_9_0,
+	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
+};
+
 static const struct dw_pcie_ops dw_pcie_ops = {
 	.link_up = qcom_pcie_link_up,
 	.start_link = qcom_pcie_start_link,
@@ -1555,6 +1704,7 @@  static const struct of_device_id qcom_pcie_match[] = {
 	{ .compatible = "qcom,pcie-qcs404", .data = &ops_2_4_0 },
 	{ .compatible = "qcom,pcie-sdm845", .data = &ops_2_7_0 },
 	{ .compatible = "qcom,pcie-sm8250", .data = &ops_1_9_0 },
+	{ .compatible = "qcom,pcie-ipq6018", .data = &ops_2_9_0 },
 	{ }
 };