linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: imx6: Add L1SS support for i.MX8MQ
@ 2020-01-14 17:02 Andrey Smirnov
  2020-01-15  3:25 ` [EXT] " Richard Zhu
  2020-02-20 16:39 ` Andrey Smirnov
  0 siblings, 2 replies; 5+ messages in thread
From: Andrey Smirnov @ 2020-01-14 17:02 UTC (permalink / raw)
  To: linux-pci
  Cc: linux-arm-kernel, Lorenzo Pieralisi, Richard Zhu, Andrey Smirnov,
	linux-kernel, linux-imx, Bjorn Helgaas, Chris Healy, Lucas Stach

Add code to configure PCI IP block to utilize supported ASPM features.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: linux-imx@nxp.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
---
 drivers/pci/controller/dwc/pci-imx6.c | 72 ++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index acfbd34032a8..3cc94ab7d22b 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -40,6 +40,9 @@
 #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE	GENMASK(11, 8)
 #define IMX8MQ_PCIE2_BASE_ADDR			0x33c00000

+#define IMX8MQ_PCIE_LINK_CAP_L1EL_64US		(0x6 << 15)
+#define IMX8MQ_PCIE_CTRL_APPS_CLK_REQ		BIT(4)
+
 #define to_imx6_pcie(x)	dev_get_drvdata((x)->dev)

 enum imx6_pcie_variants {
@@ -64,12 +67,14 @@ struct imx6_pcie {
 	struct dw_pcie		*pci;
 	int			reset_gpio;
 	bool			gpio_active_high;
+	bool			supports_clkreq;
 	struct clk		*pcie_bus;
 	struct clk		*pcie_phy;
 	struct clk		*pcie_inbound_axi;
 	struct clk		*pcie;
 	struct clk		*pcie_aux;
 	struct regmap		*iomuxc_gpr;
+	struct regmap		*src;
 	u32			controller_id;
 	struct reset_control	*pciephy_reset;
 	struct reset_control	*apps_reset;
@@ -421,11 +426,17 @@ static unsigned int imx6_pcie_grp_offset(const struct imx6_pcie *imx6_pcie)
 	return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
 }

+static unsigned int
+imx6_pcie_pciphy_rcr_offset(const struct imx6_pcie *imx6_pcie)
+{
+	WARN_ON(imx6_pcie->drvdata->variant != IMX8MQ);
+	return imx6_pcie->controller_id == 1 ? 0x48 : 0x2C;
+}
+
 static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 {
 	struct dw_pcie *pci = imx6_pcie->pci;
 	struct device *dev = pci->dev;
-	unsigned int offset;
 	int ret = 0;

 	switch (imx6_pcie->drvdata->variant) {
@@ -463,17 +474,19 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 			break;
 		}

-		offset = imx6_pcie_grp_offset(imx6_pcie);
-		/*
-		 * Set the over ride low and enabled
-		 * make sure that REF_CLK is turned on.
-		 */
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, offset,
-				   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE,
-				   0);
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, offset,
-				   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
-				   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN);
+		if (!imx6_pcie->supports_clkreq) {
+			unsigned int offset = imx6_pcie_grp_offset(imx6_pcie);
+			/*
+			 * Set the over ride low and enabled
+			 * make sure that REF_CLK is turned on.
+			 */
+			regmap_update_bits(imx6_pcie->iomuxc_gpr, offset,
+					   IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE,
+					   0);
+			regmap_update_bits(imx6_pcie->iomuxc_gpr, offset,
+					 IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
+					 IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN);
+		}
 		break;
 	}

@@ -547,6 +560,27 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 	switch (imx6_pcie->drvdata->variant) {
 	case IMX8MQ:
 		reset_control_deassert(imx6_pcie->pciephy_reset);
+		if (imx6_pcie->supports_clkreq) {
+			u32 lcr;
+
+			regmap_update_bits(imx6_pcie->src,
+				imx6_pcie_pciphy_rcr_offset(imx6_pcie),
+				IMX8MQ_PCIE_CTRL_APPS_CLK_REQ,
+				IMX8MQ_PCIE_CTRL_APPS_CLK_REQ);
+			/*
+			 * Configure the L1 latency of rc to less than
+			 * 64us Otherwise, the L1/L1SUB wouldn't be
+			 * enable by ASPM.
+			 */
+			dw_pcie_dbi_ro_wr_en(pci);
+
+			lcr  = dw_pcie_readl_dbi2(pci, PCIE_RC_LCR);
+			lcr &= ~PCI_EXP_LNKCAP_L1EL;
+			lcr |= IMX8MQ_PCIE_LINK_CAP_L1EL_64US;
+			dw_pcie_writel_dbi2(pci, PCIE_RC_LCR, lcr);
+
+			dw_pcie_dbi_ro_wr_dis(pci);
+		}
 		break;
 	case IMX7D:
 		reset_control_deassert(imx6_pcie->pciephy_reset);
@@ -1054,6 +1088,11 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	pci->dbi_base = devm_ioremap_resource(dev, dbi_base);
 	if (IS_ERR(pci->dbi_base))
 		return PTR_ERR(pci->dbi_base);
+	/*
+	 * Configure dbi_base2 to access DBI space with CS2
+	 * asserted
+	 */
+	pci->dbi_base2 = pci->dbi_base + SZ_1M;

 	/* Fetch GPIOs */
 	imx6_pcie->reset_gpio = of_get_named_gpio(node, "reset-gpio", 0);
@@ -1107,6 +1146,13 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 			dev_err(dev, "pcie_aux clock source missing or invalid\n");
 			return PTR_ERR(imx6_pcie->pcie_aux);
 		}
+		imx6_pcie->src =
+			syscon_regmap_lookup_by_compatible("fsl,imx8mq-src");
+		if (IS_ERR(imx6_pcie->src)) {
+			dev_err(dev, "SRC regmap is missing or invalid\n");
+			return PTR_ERR(imx6_pcie->src);
+		}
+
 		/* fall through */
 	case IMX7D:
 		if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
@@ -1179,6 +1225,8 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 		imx6_pcie->vpcie = NULL;
 	}

+	imx6_pcie->supports_clkreq = of_property_read_bool(node,
+							   "supports-clkreq");
 	platform_set_drvdata(pdev, imx6_pcie);

 	ret = imx6_pcie_attach_pd(dev);
--
2.21.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [EXT] [PATCH] PCI: imx6: Add L1SS support for i.MX8MQ
  2020-01-14 17:02 [PATCH] PCI: imx6: Add L1SS support for i.MX8MQ Andrey Smirnov
@ 2020-01-15  3:25 ` Richard Zhu
  2020-01-16 14:37   ` Andrey Smirnov
  2020-02-20 16:39 ` Andrey Smirnov
  1 sibling, 1 reply; 5+ messages in thread
From: Richard Zhu @ 2020-01-15  3:25 UTC (permalink / raw)
  To: Andrey Smirnov, linux-pci
  Cc: Lorenzo Pieralisi, linux-kernel, dl-linux-imx, Bjorn Helgaas,
	Chris Healy, linux-arm-kernel, Lucas Stach


> -----Original Message-----
> From: Andrey Smirnov <andrew.smirnov@gmail.com>
> Sent: 2020年1月15日 1:03
> To: linux-pci@vger.kernel.org
> Cc: Andrey Smirnov <andrew.smirnov@gmail.com>; Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com>; Bjorn Helgaas <bhelgaas@google.com>; Chris
> Healy <cphealy@gmail.com>; Lucas Stach <l.stach@pengutronix.de>; Richard
> Zhu <hongxing.zhu@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] [PATCH] PCI: imx6: Add L1SS support for i.MX8MQ
> 
> Caution: EXT Email
> 
> Add code to configure PCI IP block to utilize supported ASPM features.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
[Richard Zhu]  HI Andrey:
This patch does the regmap to the src region, right?
How about to add another reset to manipulate the *_OVERRIDE bit.
Just like the following bits.
                        resets = <&src IMX8MQ_RESET_PCIEPHY>,
                                 <&src IMX8MQ_RESET_PCIE_CTRL_APPS_EN>,
                                 <&src IMX8MQ_RESET_PCIE_CTRL_APPS_TURNOFF>;
                        reset-names = "pciephy", "apps", "turnoff";

Best Regards
Richard Zhu

> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: linux-imx@nxp.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 72 ++++++++++++++++++++++-----
>  1 file changed, 60 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> b/drivers/pci/controller/dwc/pci-imx6.c
> index acfbd34032a8..3cc94ab7d22b 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -40,6 +40,9 @@
>  #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE    GENMASK(11, 8)
>  #define IMX8MQ_PCIE2_BASE_ADDR                 0x33c00000
> 
> +#define IMX8MQ_PCIE_LINK_CAP_L1EL_64US         (0x6 << 15)
> +#define IMX8MQ_PCIE_CTRL_APPS_CLK_REQ          BIT(4)
> +
>  #define to_imx6_pcie(x)        dev_get_drvdata((x)->dev)
> 
>  enum imx6_pcie_variants {
> @@ -64,12 +67,14 @@ struct imx6_pcie {
>         struct dw_pcie          *pci;
>         int                     reset_gpio;
>         bool                    gpio_active_high;
> +       bool                    supports_clkreq;
>         struct clk              *pcie_bus;
>         struct clk              *pcie_phy;
>         struct clk              *pcie_inbound_axi;
>         struct clk              *pcie;
>         struct clk              *pcie_aux;
>         struct regmap           *iomuxc_gpr;
> +       struct regmap           *src;
>         u32                     controller_id;
>         struct reset_control    *pciephy_reset;
>         struct reset_control    *apps_reset;
> @@ -421,11 +426,17 @@ static unsigned int imx6_pcie_grp_offset(const
> struct imx6_pcie *imx6_pcie)
>         return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 :
> IOMUXC_GPR14;  }
> 
> +static unsigned int
> +imx6_pcie_pciphy_rcr_offset(const struct imx6_pcie *imx6_pcie) {
> +       WARN_ON(imx6_pcie->drvdata->variant != IMX8MQ);
> +       return imx6_pcie->controller_id == 1 ? 0x48 : 0x2C; }
> +
>  static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)  {
>         struct dw_pcie *pci = imx6_pcie->pci;
>         struct device *dev = pci->dev;
> -       unsigned int offset;
>         int ret = 0;
> 
>         switch (imx6_pcie->drvdata->variant) { @@ -463,17 +474,19 @@
> static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>                         break;
>                 }
> 
> -               offset = imx6_pcie_grp_offset(imx6_pcie);
> -               /*
> -                * Set the over ride low and enabled
> -                * make sure that REF_CLK is turned on.
> -                */
> -               regmap_update_bits(imx6_pcie->iomuxc_gpr, offset,
> -
> IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE,
> -                                  0);
> -               regmap_update_bits(imx6_pcie->iomuxc_gpr, offset,
> -
> IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
> -
> IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN);
> +               if (!imx6_pcie->supports_clkreq) {
> +                       unsigned int offset =
> imx6_pcie_grp_offset(imx6_pcie);
> +                       /*
> +                        * Set the over ride low and enabled
> +                        * make sure that REF_CLK is turned on.
> +                        */
> +                       regmap_update_bits(imx6_pcie->iomuxc_gpr,
> offset,
> +
> IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE,
> +                                          0);
> +                       regmap_update_bits(imx6_pcie->iomuxc_gpr,
> offset,
> +
> IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
> +
> IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN);
> +               }
>                 break;
>         }
> 
> @@ -547,6 +560,27 @@ static void imx6_pcie_deassert_core_reset(struct
> imx6_pcie *imx6_pcie)
>         switch (imx6_pcie->drvdata->variant) {
>         case IMX8MQ:
>                 reset_control_deassert(imx6_pcie->pciephy_reset);
> +               if (imx6_pcie->supports_clkreq) {
> +                       u32 lcr;
> +
> +                       regmap_update_bits(imx6_pcie->src,
> +
> imx6_pcie_pciphy_rcr_offset(imx6_pcie),
> +
> IMX8MQ_PCIE_CTRL_APPS_CLK_REQ,
> +
> IMX8MQ_PCIE_CTRL_APPS_CLK_REQ);
> +                       /*
> +                        * Configure the L1 latency of rc to less than
> +                        * 64us Otherwise, the L1/L1SUB wouldn't be
> +                        * enable by ASPM.
> +                        */
> +                       dw_pcie_dbi_ro_wr_en(pci);
> +
> +                       lcr  = dw_pcie_readl_dbi2(pci, PCIE_RC_LCR);
> +                       lcr &= ~PCI_EXP_LNKCAP_L1EL;
> +                       lcr |= IMX8MQ_PCIE_LINK_CAP_L1EL_64US;
> +                       dw_pcie_writel_dbi2(pci, PCIE_RC_LCR, lcr);
> +
> +                       dw_pcie_dbi_ro_wr_dis(pci);
> +               }
>                 break;
>         case IMX7D:
>                 reset_control_deassert(imx6_pcie->pciephy_reset);
> @@ -1054,6 +1088,11 @@ static int imx6_pcie_probe(struct platform_device
> *pdev)
>         pci->dbi_base = devm_ioremap_resource(dev, dbi_base);
>         if (IS_ERR(pci->dbi_base))
>                 return PTR_ERR(pci->dbi_base);
> +       /*
> +        * Configure dbi_base2 to access DBI space with CS2
> +        * asserted
> +        */
> +       pci->dbi_base2 = pci->dbi_base + SZ_1M;
> 
>         /* Fetch GPIOs */
>         imx6_pcie->reset_gpio = of_get_named_gpio(node, "reset-gpio", 0);
> @@ -1107,6 +1146,13 @@ static int imx6_pcie_probe(struct platform_device
> *pdev)
>                         dev_err(dev, "pcie_aux clock source missing or
> invalid\n");
>                         return PTR_ERR(imx6_pcie->pcie_aux);
>                 }
> +               imx6_pcie->src =
> +
> syscon_regmap_lookup_by_compatible("fsl,imx8mq-src");
> +               if (IS_ERR(imx6_pcie->src)) {
> +                       dev_err(dev, "SRC regmap is missing or
> invalid\n");
> +                       return PTR_ERR(imx6_pcie->src);
> +               }
> +
>                 /* fall through */
>         case IMX7D:
>                 if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) @@
> -1179,6 +1225,8 @@ static int imx6_pcie_probe(struct platform_device
> *pdev)
>                 imx6_pcie->vpcie = NULL;
>         }
> 
> +       imx6_pcie->supports_clkreq = of_property_read_bool(node,
> +
> + "supports-clkreq");
>         platform_set_drvdata(pdev, imx6_pcie);
> 
>         ret = imx6_pcie_attach_pd(dev);
> --
> 2.21.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [EXT] [PATCH] PCI: imx6: Add L1SS support for i.MX8MQ
  2020-01-15  3:25 ` [EXT] " Richard Zhu
@ 2020-01-16 14:37   ` Andrey Smirnov
  2020-01-17 13:52     ` Lucas Stach
  0 siblings, 1 reply; 5+ messages in thread
From: Andrey Smirnov @ 2020-01-16 14:37 UTC (permalink / raw)
  To: Richard Zhu, Lucas Stach
  Cc: linux-arm-kernel, Lorenzo Pieralisi, linux-pci, linux-kernel,
	dl-linux-imx, Bjorn Helgaas, Chris Healy

On Tue, Jan 14, 2020 at 7:26 PM Richard Zhu <hongxing.zhu@nxp.com> wrote:
>
>
> > -----Original Message-----
> > From: Andrey Smirnov <andrew.smirnov@gmail.com>
> > Sent: 2020年1月15日 1:03
> > To: linux-pci@vger.kernel.org
> > Cc: Andrey Smirnov <andrew.smirnov@gmail.com>; Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com>; Bjorn Helgaas <bhelgaas@google.com>; Chris
> > Healy <cphealy@gmail.com>; Lucas Stach <l.stach@pengutronix.de>; Richard
> > Zhu <hongxing.zhu@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> > Subject: [EXT] [PATCH] PCI: imx6: Add L1SS support for i.MX8MQ
> >
> > Caution: EXT Email
> >
> > Add code to configure PCI IP block to utilize supported ASPM features.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> [Richard Zhu]  HI Andrey:
> This patch does the regmap to the src region, right?

Indeed.

> How about to add another reset to manipulate the *_OVERRIDE bit.
> Just like the following bits.
>                         resets = <&src IMX8MQ_RESET_PCIEPHY>,
>                                  <&src IMX8MQ_RESET_PCIE_CTRL_APPS_EN>,
>                                  <&src IMX8MQ_RESET_PCIE_CTRL_APPS_TURNOFF>;
>                         reset-names = "pciephy", "apps", "turnoff";
>

Last time I talked to Philipp Zabel (maintainer of reset subsystem) he
made it pretty clear that he though that exposing those PCIe related
bits via reset subsystem (for both imx7 and imx8mq) was a mistake and
going forward he'd like to see only true reset functionality to be
exposed that way. IMX8MQ_PCIE_CTRL_APPS_CLK_REQ is definitely not a
reset line, so the case for adding it to reset driver is even weaker.

Lucas, do you mind sharing your thoughts on this?

Thanks,
Andrey Smirnov

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [EXT] [PATCH] PCI: imx6: Add L1SS support for i.MX8MQ
  2020-01-16 14:37   ` Andrey Smirnov
@ 2020-01-17 13:52     ` Lucas Stach
  0 siblings, 0 replies; 5+ messages in thread
From: Lucas Stach @ 2020-01-17 13:52 UTC (permalink / raw)
  To: Andrey Smirnov, Richard Zhu
  Cc: linux-arm-kernel, Lorenzo Pieralisi, linux-pci, linux-kernel,
	dl-linux-imx, Bjorn Helgaas, Chris Healy

On Do, 2020-01-16 at 06:37 -0800, Andrey Smirnov wrote:
> On Tue, Jan 14, 2020 at 7:26 PM Richard Zhu <hongxing.zhu@nxp.com> wrote:
> > 
> > > -----Original Message-----
> > > From: Andrey Smirnov <andrew.smirnov@gmail.com>
> > > Sent: 2020年1月15日 1:03
> > > To: linux-pci@vger.kernel.org
> > > Cc: Andrey Smirnov <andrew.smirnov@gmail.com>; Lorenzo Pieralisi
> > > <lorenzo.pieralisi@arm.com>; Bjorn Helgaas <bhelgaas@google.com>; Chris
> > > Healy <cphealy@gmail.com>; Lucas Stach <l.stach@pengutronix.de>; Richard
> > > Zhu <hongxing.zhu@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> > > Subject: [EXT] [PATCH] PCI: imx6: Add L1SS support for i.MX8MQ
> > > 
> > > Caution: EXT Email
> > > 
> > > Add code to configure PCI IP block to utilize supported ASPM features.
> > > 
> > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > [Richard Zhu]  HI Andrey:
> > This patch does the regmap to the src region, right?
> 
> Indeed.
> 
> > How about to add another reset to manipulate the *_OVERRIDE bit.
> > Just like the following bits.
> >                         resets = <&src IMX8MQ_RESET_PCIEPHY>,
> >                                  <&src IMX8MQ_RESET_PCIE_CTRL_APPS_EN>,
> >                                  <&src IMX8MQ_RESET_PCIE_CTRL_APPS_TURNOFF>;
> >                         reset-names = "pciephy", "apps", "turnoff";
> > 
> 
> Last time I talked to Philipp Zabel (maintainer of reset subsystem) he
> made it pretty clear that he though that exposing those PCIe related
> bits via reset subsystem (for both imx7 and imx8mq) was a mistake and
> going forward he'd like to see only true reset functionality to be
> exposed that way. IMX8MQ_PCIE_CTRL_APPS_CLK_REQ is definitely not a
> reset line, so the case for adding it to reset driver is even weaker.
> 
> Lucas, do you mind sharing your thoughts on this?

While I'm not too happy that we are now going to have multiple paths to
those PCIe related control bits in the driver, I totally agree that we
should stop abusing the reset API for things that aren't a reset.

Maybe we should even go all the way and switch the APPS_EN bit
manipulation to use the regmap instead of the reset. This would be a DT
compatible change, as we would just ignore the apps reset specified in
old DTs and don't require any DT changes for this to work if the regmap
is looked up by compatible.

Regards,
Lucas


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] PCI: imx6: Add L1SS support for i.MX8MQ
  2020-01-14 17:02 [PATCH] PCI: imx6: Add L1SS support for i.MX8MQ Andrey Smirnov
  2020-01-15  3:25 ` [EXT] " Richard Zhu
@ 2020-02-20 16:39 ` Andrey Smirnov
  1 sibling, 0 replies; 5+ messages in thread
From: Andrey Smirnov @ 2020-02-20 16:39 UTC (permalink / raw)
  To: linux-pci
  Cc: linux-arm-kernel, Lorenzo Pieralisi, Richard Zhu, linux-kernel,
	dl-linux-imx, Bjorn Helgaas, Chris Healy, Lucas Stach

On Tue, Jan 14, 2020 at 9:02 AM Andrey Smirnov <andrew.smirnov@gmail.com> wrote:
>
> Add code to configure PCI IP block to utilize supported ASPM features.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: linux-imx@nxp.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 72 ++++++++++++++++++++++-----
>  1 file changed, 60 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index acfbd34032a8..3cc94ab7d22b 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -40,6 +40,9 @@
>  #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE    GENMASK(11, 8)
>  #define IMX8MQ_PCIE2_BASE_ADDR                 0x33c00000
>
> +#define IMX8MQ_PCIE_LINK_CAP_L1EL_64US         (0x6 << 15)
> +#define IMX8MQ_PCIE_CTRL_APPS_CLK_REQ          BIT(4)
> +
>  #define to_imx6_pcie(x)        dev_get_drvdata((x)->dev)
>
>  enum imx6_pcie_variants {
> @@ -64,12 +67,14 @@ struct imx6_pcie {
>         struct dw_pcie          *pci;
>         int                     reset_gpio;
>         bool                    gpio_active_high;
> +       bool                    supports_clkreq;
>         struct clk              *pcie_bus;
>         struct clk              *pcie_phy;
>         struct clk              *pcie_inbound_axi;
>         struct clk              *pcie;
>         struct clk              *pcie_aux;
>         struct regmap           *iomuxc_gpr;
> +       struct regmap           *src;
>         u32                     controller_id;
>         struct reset_control    *pciephy_reset;
>         struct reset_control    *apps_reset;
> @@ -421,11 +426,17 @@ static unsigned int imx6_pcie_grp_offset(const struct imx6_pcie *imx6_pcie)
>         return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
>  }
>
> +static unsigned int
> +imx6_pcie_pciphy_rcr_offset(const struct imx6_pcie *imx6_pcie)
> +{
> +       WARN_ON(imx6_pcie->drvdata->variant != IMX8MQ);
> +       return imx6_pcie->controller_id == 1 ? 0x48 : 0x2C;
> +}
> +
>  static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  {
>         struct dw_pcie *pci = imx6_pcie->pci;
>         struct device *dev = pci->dev;
> -       unsigned int offset;
>         int ret = 0;
>
>         switch (imx6_pcie->drvdata->variant) {
> @@ -463,17 +474,19 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>                         break;
>                 }
>
> -               offset = imx6_pcie_grp_offset(imx6_pcie);
> -               /*
> -                * Set the over ride low and enabled
> -                * make sure that REF_CLK is turned on.
> -                */
> -               regmap_update_bits(imx6_pcie->iomuxc_gpr, offset,
> -                                  IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE,
> -                                  0);
> -               regmap_update_bits(imx6_pcie->iomuxc_gpr, offset,
> -                                  IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
> -                                  IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN);
> +               if (!imx6_pcie->supports_clkreq) {
> +                       unsigned int offset = imx6_pcie_grp_offset(imx6_pcie);
> +                       /*
> +                        * Set the over ride low and enabled
> +                        * make sure that REF_CLK is turned on.
> +                        */
> +                       regmap_update_bits(imx6_pcie->iomuxc_gpr, offset,
> +                                          IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE,
> +                                          0);
> +                       regmap_update_bits(imx6_pcie->iomuxc_gpr, offset,
> +                                        IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
> +                                        IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN);
> +               }

Ugh, I just realized all of my testing was implicitly relying on
bootloader configuring those CLKREQ overrides bits, so all of the code
related to in in this patch is bogus and broken. Glad it didn't get
applied. Will submit corrected v2 once I work out the right way to do
this.

>                 break;
>         }
>
> @@ -547,6 +560,27 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>         switch (imx6_pcie->drvdata->variant) {
>         case IMX8MQ:
>                 reset_control_deassert(imx6_pcie->pciephy_reset);
> +               if (imx6_pcie->supports_clkreq) {
> +                       u32 lcr;
> +
> +                       regmap_update_bits(imx6_pcie->src,
> +                               imx6_pcie_pciphy_rcr_offset(imx6_pcie),
> +                               IMX8MQ_PCIE_CTRL_APPS_CLK_REQ,
> +                               IMX8MQ_PCIE_CTRL_APPS_CLK_REQ);
> +                       /*
> +                        * Configure the L1 latency of rc to less than
> +                        * 64us Otherwise, the L1/L1SUB wouldn't be
> +                        * enable by ASPM.
> +                        */
> +                       dw_pcie_dbi_ro_wr_en(pci);
> +
> +                       lcr  = dw_pcie_readl_dbi2(pci, PCIE_RC_LCR);
> +                       lcr &= ~PCI_EXP_LNKCAP_L1EL;
> +                       lcr |= IMX8MQ_PCIE_LINK_CAP_L1EL_64US;
> +                       dw_pcie_writel_dbi2(pci, PCIE_RC_LCR, lcr);
> +
> +                       dw_pcie_dbi_ro_wr_dis(pci);
> +               }
>                 break;
>         case IMX7D:
>                 reset_control_deassert(imx6_pcie->pciephy_reset);
> @@ -1054,6 +1088,11 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>         pci->dbi_base = devm_ioremap_resource(dev, dbi_base);
>         if (IS_ERR(pci->dbi_base))
>                 return PTR_ERR(pci->dbi_base);
> +       /*
> +        * Configure dbi_base2 to access DBI space with CS2
> +        * asserted
> +        */
> +       pci->dbi_base2 = pci->dbi_base + SZ_1M;
>
>         /* Fetch GPIOs */
>         imx6_pcie->reset_gpio = of_get_named_gpio(node, "reset-gpio", 0);
> @@ -1107,6 +1146,13 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>                         dev_err(dev, "pcie_aux clock source missing or invalid\n");
>                         return PTR_ERR(imx6_pcie->pcie_aux);
>                 }
> +               imx6_pcie->src =
> +                       syscon_regmap_lookup_by_compatible("fsl,imx8mq-src");
> +               if (IS_ERR(imx6_pcie->src)) {
> +                       dev_err(dev, "SRC regmap is missing or invalid\n");
> +                       return PTR_ERR(imx6_pcie->src);
> +               }
> +
>                 /* fall through */
>         case IMX7D:
>                 if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> @@ -1179,6 +1225,8 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>                 imx6_pcie->vpcie = NULL;
>         }
>
> +       imx6_pcie->supports_clkreq = of_property_read_bool(node,
> +                                                          "supports-clkreq");
>         platform_set_drvdata(pdev, imx6_pcie);
>
>         ret = imx6_pcie_attach_pd(dev);
> --
> 2.21.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2020-02-20 16:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 17:02 [PATCH] PCI: imx6: Add L1SS support for i.MX8MQ Andrey Smirnov
2020-01-15  3:25 ` [EXT] " Richard Zhu
2020-01-16 14:37   ` Andrey Smirnov
2020-01-17 13:52     ` Lucas Stach
2020-02-20 16:39 ` Andrey Smirnov

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).