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/[email protected]
[pcie-imx8mq-v7] https://lore.kernel.org/lkml/[email protected]
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
Add a binding for an extra clock required on i.MX8MQ.
Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Rob Herring <[email protected]>
Cc: [email protected]
---
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
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 <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Rob Herring <[email protected]>
Cc: [email protected]
---
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
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 <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Chris Healy <[email protected]>
> Cc: Lucas Stach <[email protected]>
> Cc: Leonard Crestez <[email protected]>
> Cc: "A.s. Dong" <[email protected]>
> Cc: Richard Zhu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Rob Herring <[email protected]>
> Cc: [email protected]
Reviewed-by: Lucas Stach <[email protected]>
> ---
> 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 {
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 <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Chris Healy <[email protected]>
> Cc: Lucas Stach <[email protected]>
> Cc: Leonard Crestez <[email protected]>
> Cc: "A.s. Dong" <[email protected]>
> Cc: Richard Zhu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Rob Herring <[email protected]>
> Cc: [email protected]
Reviewed-by: Lucas Stach <[email protected]>
> ---
> 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;
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/[email protected]
> [pcie-imx8mq-v7] https://lore.kernel.org/lkml/[email protected]
>
> 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
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/[email protected]
> > [pcie-imx8mq-v7] https://lore.kernel.org/lkml/[email protected]
> >
> > 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
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 <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Chris Healy <[email protected]>
> Cc: Lucas Stach <[email protected]>
> Cc: Leonard Crestez <[email protected]>
> Cc: "A.s. Dong" <[email protected]>
> Cc: Richard Zhu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Rob Herring <[email protected]>
> Cc: [email protected]
> ---
> Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
Reviewed-by: Rob Herring <[email protected]>
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.
On Thu, Feb 28, 2019 at 1:24 PM Trent Piepho <[email protected]> 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