Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: andrew.smirnov@gmail.com (Andrey Smirnov)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/3] PCI: imx: Add support for i.MX8MQ
Date: Mon, 26 Nov 2018 10:09:48 -0800
Message-ID: <CAHQ1cqER2jjZCfFeaMi0T3-N-yNhiLiX5WYztpzuseXhA96JFw@mail.gmail.com> (raw)
In-Reply-To: <AM0PR0402MB35706B20F01340140367FECD8CD80@AM0PR0402MB3570.eurprd04.prod.outlook.com>

On Sun, Nov 18, 2018 at 11:07 PM Richard Zhu <hongxing.zhu@nxp.com> wrote:
>
> Hi Andrey:
> Thanks for your patch-set.
> I have comment about the L1SS implementation below.
> It's better to figure out one method to fix it.
>
> BR
> Richard
>
> > -----Original Message-----
> > From: Andrey Smirnov [mailto:andrew.smirnov at gmail.com]
> > Sent: 2018?11?18? 2:12
> > To: linux-kernel at vger.kernel.org
> > Cc: Andrey Smirnov <andrew.smirnov@gmail.com>; bhelgaas at google.com;
> > Fabio Estevam <fabio.estevam@nxp.com>; cphealy at gmail.com;
> > l.stach at pengutronix.de; Leonard Crestez <leonard.crestez@nxp.com>;
> > Aisheng DONG <aisheng.dong@nxp.com>; Richard Zhu
> > <hongxing.zhu@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> > linux-arm-kernel at lists.infradead.org; linux-pci at vger.kernel.org
> > Subject: [PATCH 3/3] PCI: imx: Add support for i.MX8MQ
> >
> > Cc: bhelgaas at google.com
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: cphealy at gmail.com
> > Cc: l.stach at pengutronix.de
> > Cc: Leonard Crestez <leonard.crestez@nxp.com>
> > Cc: "A.s. Dong" <aisheng.dong@nxp.com>
> > Cc: Richard Zhu <hongxing.zhu@nxp.com>
> > Cc: linux-imx at nxp.com
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: linux-kernel at vger.kernel.org
> > Cc: linux-pci at vger.kernel.org
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  drivers/pci/controller/dwc/Kconfig    |   2 +-
> >  drivers/pci/controller/dwc/pci-imx6.c | 117 ++++++++++++++++++++++++--
> >  2 files changed, 113 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig
> > b/drivers/pci/controller/dwc/Kconfig
> > index 91b0194240a5..2b139acccf32 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -90,7 +90,7 @@ config PCI_EXYNOS
> >
> >  config PCI_IMX6
> >       bool "Freescale i.MX6 PCIe controller"
> > -     depends on SOC_IMX6Q || (ARM && COMPILE_TEST)
> > +     depends on SOC_IMX8MQ || SOC_IMX6Q || (ARM && COMPILE_TEST)
> >       depends on PCI_MSI_IRQ_DOMAIN
> >       select PCIE_DW_HOST
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 3c3002861d25..8d1f310e41a6 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -8,6 +8,7 @@
> >   * Author: Sean Cross <xobs@kosagi.com>
> >   */
> >
> > +#include <linux/bitfield.h>
> >  #include <linux/clk.h>
> >  #include <linux/delay.h>
> >  #include <linux/gpio.h>
> > @@ -30,6 +31,14 @@
> >
> >  #include "pcie-designware.h"
> >
> > +#define IMX8MQ_PCIE_LINK_CAP_REG_OFFSET              0x7C
> > +#define IMX8MQ_PCIE_LINK_CAP_L1EL_64US               (0x6 << 15)
> > +
> > +#define IMX8MQ_GPR_PCIE_REF_USE_PAD          BIT(9)
> > +#define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN  BIT(10)
> > +#define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE     BIT(11)
> > +
> > +
> >  #define to_imx6_pcie(x)      dev_get_drvdata((x)->dev)
> >
> >  enum imx6_pcie_variants {
> > @@ -37,6 +46,7 @@ enum imx6_pcie_variants {
> >       IMX6SX,
> >       IMX6QP,
> >       IMX7D,
> > +     IMX8MQ,
> >  };
> >
> >  struct imx6_pcie {
> > @@ -48,8 +58,10 @@ struct imx6_pcie {
> >       struct clk              *pcie_inbound_axi;
> >       struct clk              *pcie;
> >       struct regmap           *iomuxc_gpr;
> > +     u32                     gpr1x;
> >       struct reset_control    *pciephy_reset;
> >       struct reset_control    *apps_reset;
> > +     struct reset_control    *apps_clk_req;
> >       struct reset_control    *turnoff_reset;
> >       enum imx6_pcie_variants variant;
> >       u32                     tx_deemph_gen1;
> > @@ -59,6 +71,7 @@ struct imx6_pcie {
> >       u32                     tx_swing_low;
> >       int                     link_gen;
> >       struct regulator        *vpcie;
> > +     u32                     device_type[2];
> >  };
> >
> >  /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */ @@
> > -245,7 +258,8 @@ static void imx6_pcie_reset_phy(struct imx6_pcie
> > *imx6_pcie)  {
> >       u32 tmp;
> >
> > -     if (imx6_pcie->variant == IMX7D)
> > +     if (imx6_pcie->variant == IMX7D ||
> > +         imx6_pcie->variant == IMX8MQ)
> >               return;
> >
> >       pcie_phy_read(imx6_pcie, PHY_RX_OVRD_IN_LO, &tmp); @@ -261,6
> > +275,7 @@ static void imx6_pcie_reset_phy(struct imx6_pcie *imx6_pcie)
> >       pcie_phy_write(imx6_pcie, PHY_RX_OVRD_IN_LO, tmp);  }
> >
> > +#ifdef CONFIG_ARM
> >  /*  Added for PCI abort handling */
> >  static int imx6q_pcie_abort_handler(unsigned long addr,
> >               unsigned int fsr, struct pt_regs *regs) @@ -294,6 +309,7 @@ static
> > int imx6q_pcie_abort_handler(unsigned long addr,
> >
> >       return 1;
> >  }
> > +#endif
> >
> >  static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > { @@ -301,6 +317,7 @@ static void imx6_pcie_assert_core_reset(struct
> > imx6_pcie *imx6_pcie)
> >
> >       switch (imx6_pcie->variant) {
> >       case IMX7D:
> > +     case IMX8MQ: /* FALLTHROUGH */
> >               reset_control_assert(imx6_pcie->pciephy_reset);
> >               reset_control_assert(imx6_pcie->apps_reset);
> >               break;
> > @@ -369,6 +386,18 @@ static int imx6_pcie_enable_ref_clk(struct
> > imx6_pcie *imx6_pcie)
> >               break;
> >       case IMX7D:
> >               break;
> > +     case IMX8MQ:
> > +             /*
> > +              * Set the over ride low and enabled
> > +              * make sure that REF_CLK is turned on.
> > +              */
> > +             regmap_update_bits(imx6_pcie->iomuxc_gpr, imx6_pcie->gpr1x,
> > +                                IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE,
> > +                                0);
> > +             regmap_update_bits(imx6_pcie->iomuxc_gpr, imx6_pcie->gpr1x,
> > +                                IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
> > +                                IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN);
> > +             break;
> >       }
> >
> >       return ret;
> > @@ -397,6 +426,7 @@ static void imx6_pcie_deassert_core_reset(struct
> > imx6_pcie *imx6_pcie)  {
> >       struct dw_pcie *pci = imx6_pcie->pci;
> >       struct device *dev = pci->dev;
> > +     unsigned int val;
> >       int ret;
> >
> >       if (imx6_pcie->vpcie && !regulator_is_enabled(imx6_pcie->vpcie)) { @@
> > -445,6 +475,29 @@ static void imx6_pcie_deassert_core_reset(struct
> > imx6_pcie *imx6_pcie)
> >       }
> >
> >       switch (imx6_pcie->variant) {
> > +     case IMX8MQ:
> > +             reset_control_deassert(imx6_pcie->pciephy_reset);
> > +             udelay(100);
> > +             /*
> > +              * Configure the CLK_REQ# high, let the L1SS
> > +              * automatically controlled by HW.
> > +              */
> > +             reset_control_assert(imx6_pcie->apps_clk_req);
> > +
> > +             /*
> > +              * Configure the L1 latency of rc to less than 64us
> > +              * Otherwise, the L1/L1SUB wouldn't be enable by ASPM.
> > +              */
> > +             val = dw_pcie_readl_dbi(imx6_pcie->pci,
> > +                                     SZ_1M +
> > +                                     IMX8MQ_PCIE_LINK_CAP_REG_OFFSET);
> > +             val &= ~PCI_EXP_LNKCAP_L1EL;
> > +             val |= IMX8MQ_PCIE_LINK_CAP_L1EL_64US;
> > +             dw_pcie_writel_dbi(imx6_pcie->pci,
> > +                                SZ_1M +
> > +                                IMX8MQ_PCIE_LINK_CAP_REG_OFFSET,
> > +                                val);
> > +             break;
> >       case IMX7D:
> >               reset_control_deassert(imx6_pcie->pciephy_reset);
> >               imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
> > @@ -483,6 +536,15 @@ static void imx6_pcie_deassert_core_reset(struct
> > imx6_pcie *imx6_pcie)  static void imx6_pcie_init_phy(struct imx6_pcie
> > *imx6_pcie)  {
> >       switch (imx6_pcie->variant) {
> > +     case IMX8MQ:
> > +             /*
> > +              * TODO: Currently this code assumes external
> > +              * oscillator is being used
> > +              */
> > +             regmap_update_bits(imx6_pcie->iomuxc_gpr, imx6_pcie->gpr1x,
> > +                                IMX8MQ_GPR_PCIE_REF_USE_PAD,
> > +                                IMX8MQ_GPR_PCIE_REF_USE_PAD);
> > +             break;
> >       case IMX7D:
> >               regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >                                  IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0); @@
> > -519,7 +581,8 @@ static void imx6_pcie_init_phy(struct imx6_pcie
> > *imx6_pcie)
> >       }
> >
> >       regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > -                     IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT <<
> > 12);
> > +                        imx6_pcie->device_type[0],
> > +                        imx6_pcie->device_type[1]);
> >  }
> >
> >  static int imx6_setup_phy_mpll(struct imx6_pcie *imx6_pcie) @@ -528,7
> > +591,8 @@ static int imx6_setup_phy_mpll(struct imx6_pcie *imx6_pcie)
> >       int mult, div;
> >       u32 val;
> >
> > -     if (imx6_pcie->variant == IMX7D)
> > +     if (imx6_pcie->variant == IMX7D ||
> > +         imx6_pcie->variant == IMX8MQ)
> >               return 0;
> >
> >       switch (phy_rate) {
> > @@ -616,6 +680,7 @@ static void imx6_pcie_ltssm_enable(struct device
> > *dev)
> >                                  IMX6Q_GPR12_PCIE_CTL_2);
> >               break;
> >       case IMX7D:
> > +     case IMX8MQ:            /* FALLTHROUGH */
> >               reset_control_deassert(imx6_pcie->apps_reset);
> >               break;
> >       }
> > @@ -798,10 +863,24 @@ static void imx6_pcie_clk_disable(struct imx6_pcie
> > *imx6_pcie)
> >       clk_disable_unprepare(imx6_pcie->pcie_phy);
> >       clk_disable_unprepare(imx6_pcie->pcie_bus);
> >
> > -     if (imx6_pcie->variant == IMX7D) {
> > +     switch (imx6_pcie->variant) {
> > +     case IMX7D:
> >               regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >                                  IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> >                                  IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > +             break;
> > +     /*
> > +      * Disable the override. Configure the CLK_REQ# high, let the
> > +      * L1SS automatically controlled by HW when link is up.
> > +      * Otherwise, turn off the REF_CLK to save power consumption.
> > +      */
> [Richard Zhu] About the L1SS support, there is a back-compatible issue.
> When one none-L1SS capability EP device is used in one HW design solution.
> In this case, EP device wouldn't manipulated its own CLK_REQ#.
> The CLK_REQ# would be high when the override_en is disabled here.
> That means the REFCLK would be turned off abnormally.
> System would have problem when the REFCLK of PCIe is turned off abnormally after link is up.
>

I don't have a lot of expertise in this area, so please take my
comment with a grain of salt. The way I understand it, is in legacy
systems that do not support L1SS with CLKREQ that feature shouldn't be
enabled/used. Unless I've missed something, I think it should be
possible to do so and disable the feature by configuring corresponding
CLKREQ_B pad as GPIO and adding a GPIO hog node to pull CLKREQ low. Do
you see any problems with this approach?

Regardless though, I wasn't really planning on including PM support in
this series. The code in question shouldn't even be in the patch since
it'll never be executed because imx6_pcie_clk_disable() is only called
by imx6_pcie_suspend_noirq() which is a no-op on all variants except
i.MX7D.

I'll drop all of the unused PM-related bits I missed from the series
and we can add them later in a separate submission.

Thanks,
Andrey Smirnov

  reply index

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-17 18:12 [PATCH 0/3] PCIE " andrew.smirnov
2018-11-17 18:12 ` [PATCH 1/3] PCI: imx: No-op imx6_setup_phy_mpll() on i.MX7D andrew.smirnov
2018-11-17 18:12 ` [PATCH 2/3] PCI: imx: No-op imx6_pcie_reset_phy() " andrew.smirnov
2018-11-20  1:22   ` tpiepho
2018-11-26 18:32     ` andrew.smirnov
2018-11-17 18:12 ` [PATCH 3/3] PCI: imx: Add support for i.MX8MQ andrew.smirnov
2018-11-19  7:07   ` hongxing.zhu
2018-11-26 18:09     ` andrew.smirnov [this message]
2018-11-29 10:52       ` hongxing.zhu
2018-11-20 10:48   ` leonard.crestez
2018-11-26 18:24     ` andrew.smirnov
2018-11-27 10:06       ` l.stach
2018-11-27 10:46         ` leonard.crestez
2018-11-27 21:14           ` andrew.smirnov
2018-11-28 10:55             ` leonard.crestez
2018-11-29 11:15               ` l.stach
2018-11-29 11:12             ` l.stach
2018-11-27 10:16       ` leonard.crestez
2018-11-27 21:03         ` andrew.smirnov
2018-12-03 12:20 ` [PATCH 0/3] PCIE " Lorenzo Pieralisi
2018-12-07 11:37 ` Lorenzo Pieralisi

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHQ1cqER2jjZCfFeaMi0T3-N-yNhiLiX5WYztpzuseXhA96JFw@mail.gmail.com \
    --to=andrew.smirnov@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git