[PATCH 0/2] "pcie_aux" clock for i.MX8MQ

[PATCH 0/2] "pcie_aux" clock for i.MX8MQ

[Andrey Smirnov]

Lorenzo:

This small series adds code to control "pcie_aux" clock. This is an
oversight from original submission [pcie-imx8mq-v7], which was only
discovered once I submitted an RFC for corresponding DT changes going
via i.MX tree [imx-dt-rfc].

Thanks,
Andrey Smirnov

[imx-dt-rfc] https://lore.kernel.org/lkml/20190131204333.31846-1-andrew.smirnov@gmail.com
[pcie-imx8mq-v7] https://lore.kernel.org/lkml/20190202001523.12517-1-andrew.smirnov@gmail.com

Andrey Smirnov (2):
  dt-bindings: imx6q-pcie: Add "pcie_aux" clock for imx8mq
  PCI: imx6: Add code to request/control "pcie_aux" clock for i.MX8MQ

 .../devicetree/bindings/pci/fsl,imx6q-pcie.txt   |  4 ++++
 drivers/pci/controller/dwc/pci-imx6.c            | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

-- 
2.20.1

[PATCH 1/2] dt-bindings: imx6q-pcie: Add "pcie_aux" clock for imx8mq

[Andrey Smirnov]

Add a binding for an extra clock required on i.MX8MQ.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@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@nxp.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
index 920ca93870a8..933d98328e07 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
@@ -55,6 +55,10 @@ Additional required properties for imx7d-pcie and imx8mq-pcie:
 	       - "apps"
 	       - "turnoff"
 
+Additional required properties for imx8mq-pcie:
+- clock-names: Must include the following additional entries:
+	- "pcie_aux"
+
 Example:
 
 	pcie@01000000 {
-- 
2.20.1

Re: [PATCH 1/2] dt-bindings: imx6q-pcie: Add "pcie_aux" clock for imx8mq

[Lucas Stach]

Am Montag, den 11.02.2019, 17:51 -0800 schrieb Andrey Smirnov:
> Add a binding for an extra clock required on i.MX8MQ.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@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@nxp.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt 
> b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index 920ca93870a8..933d98328e07 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -55,6 +55,10 @@ Additional required properties for imx7d-pcie and
> imx8mq-pcie:
>  	       - "apps"
>  	       - "turnoff"
>  
> +Additional required properties for imx8mq-pcie:
> +- clock-names: Must include the following additional entries:
> +	- "pcie_aux"
> +
>  Example:
>  
>  	pcie@01000000 {

Re: [PATCH 1/2] dt-bindings: imx6q-pcie: Add "pcie_aux" clock for imx8mq

[Rob Herring]

On Mon, 11 Feb 2019 17:51:07 -0800, Andrey Smirnov wrote:
> Add a binding for an extra clock required on i.MX8MQ.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@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@nxp.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 2/2] PCI: imx6: Add code to request/control "pcie_aux" clock for i.MX8MQ

[Andrey Smirnov]

PCIe IP block has additional clock, "pcie_aux", that needs to be
controlled by the driver. Add code to support that.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@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@nxp.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
---
 drivers/pci/controller/dwc/pci-imx6.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 7cdf8f9ab244..1a7031782846 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -65,6 +65,7 @@ struct imx6_pcie {
 	struct clk		*pcie_phy;
 	struct clk		*pcie_inbound_axi;
 	struct clk		*pcie;
+	struct clk		*pcie_aux;
 	struct regmap		*iomuxc_gpr;
 	u32			controller_id;
 	struct reset_control	*pciephy_reset;
@@ -421,6 +422,12 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 	case IMX7D:
 		break;
 	case IMX8MQ:
+		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
+		if (ret) {
+			dev_err(dev, "unable to enable pcie_aux clock\n");
+			break;
+		}
+
 		offset = imx6_pcie_grp_offset(imx6_pcie);
 		/*
 		 * Set the over ride low and enabled
@@ -904,6 +911,9 @@ static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
 				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
 				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
 		break;
+	case IMX8MQ:
+		clk_disable_unprepare(imx6_pcie->pcie_aux);
+		break;
 	default:
 		break;
 	}
@@ -1049,6 +1059,12 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 			dev_err(dev, "Failed to get PCIE APPS reset control\n");
 			return PTR_ERR(imx6_pcie->apps_reset);
 		}
+
+		imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux");
+		if (IS_ERR(imx6_pcie->pcie_aux)) {
+			dev_err(dev, "pcie_aux clock source missing or invalid\n");
+			return PTR_ERR(imx6_pcie->pcie_aux);
+		}
 		break;
 	default:
 		break;
-- 
2.20.1

Re: [PATCH 2/2] PCI: imx6: Add code to request/control "pcie_aux" clock for i.MX8MQ

[Lucas Stach]

Am Montag, den 11.02.2019, 17:51 -0800 schrieb Andrey Smirnov:
> PCIe IP block has additional clock, "pcie_aux", that needs to be
> controlled by the driver. Add code to support that.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@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@nxp.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 7cdf8f9ab244..1a7031782846 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -65,6 +65,7 @@ struct imx6_pcie {
>  	struct clk		*pcie_phy;
>  	struct clk		*pcie_inbound_axi;
>  	struct clk		*pcie;
> +	struct clk		*pcie_aux;
>  	struct regmap		*iomuxc_gpr;
>  	u32			controller_id;
>  	struct reset_control	*pciephy_reset;
> @@ -421,6 +422,12 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  	case IMX7D:
>  		break;
>  	case IMX8MQ:
> +		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
> +		if (ret) {
> +			dev_err(dev, "unable to enable pcie_aux clock\n");
> +			break;
> +		}
> +
>  		offset = imx6_pcie_grp_offset(imx6_pcie);
>  		/*
>  		 * Set the over ride low and enabled
> @@ -904,6 +911,9 @@ static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
>  				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
>  				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
>  		break;
> +	case IMX8MQ:
> +		clk_disable_unprepare(imx6_pcie->pcie_aux);
> +		break;
>  	default:
>  		break;
>  	}
> @@ -1049,6 +1059,12 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  			dev_err(dev, "Failed to get PCIE APPS reset control\n");
>  			return PTR_ERR(imx6_pcie->apps_reset);
>  		}
> +
> +		imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux");
> +		if (IS_ERR(imx6_pcie->pcie_aux)) {
> +			dev_err(dev, "pcie_aux clock source missing or invalid\n");
> +			return PTR_ERR(imx6_pcie->pcie_aux);
> +		}
>  		break;
>  	default:
>  		break;

Re: [PATCH 2/2] PCI: imx6: Add code to request/control "pcie_aux" clock for i.MX8MQ

[Trent Piepho]

On Tue, 2019-02-12 at 10:36 +0100, Lucas Stach wrote:
> Am Montag, den 11.02.2019, 17:51 -0800 schrieb Andrey Smirnov:
> > PCIe IP block has additional clock, "pcie_aux", that needs to be
> > controlled by the driver. Add code to support that.

This breaks iMX7d.

> > 
> > @@ -1049,6 +1059,12 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> >  			dev_err(dev, "Failed to get PCIE APPS reset control\n");
> >  			return PTR_ERR(imx6_pcie->apps_reset);
> >  		}
> > +
> > +		imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux");
> > +		if (IS_ERR(imx6_pcie->pcie_aux)) {
> > +			dev_err(dev, "pcie_aux clock source missing or invalid\n");
> > +			return PTR_ERR(imx6_pcie->pcie_aux);
> > +		}
> >  		break;
> >  	default:
> >  		break;

One can't see enough context in the patch above, but in linux-next this
section is under

         case IMX7D:
         case IMX8MQ:

It's being applied to imx7d and not just imx8mq and so breaks because
imx7d dts files don't have this clock.  Not sure if this is a bug in
this commit or some kind of merge/rebase mistake.

Re: [PATCH 2/2] PCI: imx6: Add code to request/control "pcie_aux" clock for i.MX8MQ

[Andrey Smirnov]

On Thu, Feb 28, 2019 at 1:24 PM Trent Piepho <tpiepho@impinj.com> wrote:
>
> On Tue, 2019-02-12 at 10:36 +0100, Lucas Stach wrote:
> > Am Montag, den 11.02.2019, 17:51 -0800 schrieb Andrey Smirnov:
> > > PCIe IP block has additional clock, "pcie_aux", that needs to be
> > > controlled by the driver. Add code to support that.
>
> This breaks iMX7d.
>

Ugh, my bad, sorry about that.

> > >
> > > @@ -1049,6 +1059,12 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > >                     dev_err(dev, "Failed to get PCIE APPS reset control\n");
> > >                     return PTR_ERR(imx6_pcie->apps_reset);
> > >             }
> > > +
> > > +           imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux");
> > > +           if (IS_ERR(imx6_pcie->pcie_aux)) {
> > > +                   dev_err(dev, "pcie_aux clock source missing or invalid\n");
> > > +                   return PTR_ERR(imx6_pcie->pcie_aux);
> > > +           }
> > >             break;
> > >     default:
> > >             break;
>
> One can't see enough context in the patch above, but in linux-next this
> section is under
>
>          case IMX7D:
>          case IMX8MQ:
>
> It's being applied to imx7d and not just imx8mq and so breaks because
> imx7d dts files don't have this clock.  Not sure if this is a bug in
> this commit or some kind of merge/rebase mistake.
>

This is just a regular bug, I spaced out and missed the fact that this
path is shared between the two. I'll submit a patch moving "pci_aux"
clock request into a i.MX8MQ specific patch shortly.

Thanks,
Andrey Smirnov

Re: [PATCH 0/2] "pcie_aux" clock for i.MX8MQ

[Lorenzo Pieralisi]

On Mon, Feb 11, 2019 at 05:51:06PM -0800, Andrey Smirnov wrote:
> Lorenzo:
> 
> This small series adds code to control "pcie_aux" clock. This is an
> oversight from original submission [pcie-imx8mq-v7], which was only
> discovered once I submitted an RFC for corresponding DT changes going
> via i.MX tree [imx-dt-rfc].
> 
> Thanks,
> Andrey Smirnov
> 
> [imx-dt-rfc] https://lore.kernel.org/lkml/20190131204333.31846-1-andrew.smirnov@gmail.com
> [pcie-imx8mq-v7] https://lore.kernel.org/lkml/20190202001523.12517-1-andrew.smirnov@gmail.com
> 
> Andrey Smirnov (2):
>   dt-bindings: imx6q-pcie: Add "pcie_aux" clock for imx8mq
>   PCI: imx6: Add code to request/control "pcie_aux" clock for i.MX8MQ
> 
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt   |  4 ++++
>  drivers/pci/controller/dwc/pci-imx6.c            | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+)

Hi Andrey,

I have applied it to pci/dwc for v5.1, however it looks like it would
break the driver with an old dts - I assume that's expected but let
me know if there is a better way to handle this.

Lorenzo

Re: [PATCH 0/2] "pcie_aux" clock for i.MX8MQ

[Lucas Stach]

Hi Lorenzo,

Am Dienstag, den 19.02.2019, 12:42 +0000 schrieb Lorenzo Pieralisi:
> On Mon, Feb 11, 2019 at 05:51:06PM -0800, Andrey Smirnov wrote:
> > Lorenzo:
> > 
> > This small series adds code to control "pcie_aux" clock. This is an
> > oversight from original submission [pcie-imx8mq-v7], which was only
> > discovered once I submitted an RFC for corresponding DT changes going
> > via i.MX tree [imx-dt-rfc].
> > 
> > Thanks,
> > Andrey Smirnov
> > 
> > [imx-dt-rfc] https://lore.kernel.org/lkml/20190131204333.31846-1-andrew.smirnov@gmail.com
> > [pcie-imx8mq-v7] https://lore.kernel.org/lkml/20190202001523.12517-1-andrew.smirnov@gmail.com
> > 
> > Andrey Smirnov (2):
> >   dt-bindings: imx6q-pcie: Add "pcie_aux" clock for imx8mq
> >   PCI: imx6: Add code to request/control "pcie_aux" clock for i.MX8MQ
> > 
> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt   |  4 ++++
> >  drivers/pci/controller/dwc/pci-imx6.c            | 16 ++++++++++++++++
> >  2 files changed, 20 insertions(+)
> 
> Hi Andrey,
> 
> I have applied it to pci/dwc for v5.1, however it looks like it would
> break the driver with an old dts - I assume that's expected but let
> me know if there is a better way to handle this.

There is no upstream DT using the imx8mq binding, yet. We've actually
noticed the issue due to the DT patches adding the PCIe nodes and those
2 patches fix the driver _before_ we introduce any DT using it. So I
think it's okay not to worry about backwards compatibility here.

Regards,
Lucas