[PATCH v5 1/3] ARM: imx: mach-imx6q: Search for fsl,imx6q-iomuxc-gpr earlier

[PATCH v5 1/3] ARM: imx: mach-imx6q: Search for fsl,imx6q-iomuxc-gpr earlier

[Sven Van Asbroeck]

From: Fabio Estevam <festevam@gmail.com>

Check the presence of fsl,imx6q-iomuxc-gpr earlier and exit in case
of failure.

This is done in preparation for adding support for configuring the
GPR5 register for i.MX6QP a bit easier.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---

Tree: v5.8-rc3

Patch history: see [PATCH v5 3/3]

To: Shawn Guo <shawnguo@kernel.org>
To: Andy Duan <fugang.duan@nxp.com>
To: Rob Herring <robh+dt@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

 arch/arm/mach-imx/mach-imx6q.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 85c084a716ab..ae89ad93ca83 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -169,6 +169,12 @@ static void __init imx6q_1588_init(void)
 	struct regmap *gpr;
 	u32 clksel;
 
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+	if (IS_ERR(gpr)) {
+		pr_err("failed to find fsl,imx6q-iomuxc-gpr regmap\n");
+		return;
+	}
+
 	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-fec");
 	if (!np) {
 		pr_warn("%s: failed to find fec node\n", __func__);
@@ -195,13 +201,8 @@ static void __init imx6q_1588_init(void)
 	clksel = clk_is_match(ptp_clk, enet_ref) ?
 				IMX6Q_GPR1_ENET_CLK_SEL_ANATOP :
 				IMX6Q_GPR1_ENET_CLK_SEL_PAD;
-	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
-	if (!IS_ERR(gpr))
-		regmap_update_bits(gpr, IOMUXC_GPR1,
-				IMX6Q_GPR1_ENET_CLK_SEL_MASK,
-				clksel);
-	else
-		pr_err("failed to find fsl,imx6q-iomuxc-gpr regmap\n");
+	regmap_update_bits(gpr, IOMUXC_GPR1, IMX6Q_GPR1_ENET_CLK_SEL_MASK,
+			   clksel);
 
 	clk_put(enet_ref);
 put_ptp_clk:
-- 
2.17.1

[PATCH v5 2/3] dt-bindings: fec: add fsl,ptpclk-bypass-pad boolean property

[Sven Van Asbroeck]

If present, sources the fec's PTP clock straight from
the enet PLL, instead of having to be routed via a SoC pad.

This is only possible on certain SoCs, notably the imx6 (quad) plus.

Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---

Tree: v5.8-rc3

Patch history: see [PATCH v5 3/3]

To: Shawn Guo <shawnguo@kernel.org>
To: Andy Duan <fugang.duan@nxp.com>
To: Rob Herring <robh+dt@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

 Documentation/devicetree/bindings/net/fsl-fec.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 9b543789cd52..e34df25a38f6 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -39,6 +39,9 @@ Optional properties:
   tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII interrupts.
   For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for the pulse
   per second interrupt associated with 1588 precision time protocol(PTP).
+- fsl,ptpclk-bypass-pad: If present, sources the fec's PTP clock straight from
+  the enet PLL, instead of having to be routed via a SoC pad. This is only
+  possible on certain SoCs, notably the imx6 (quad) plus.
 
 Optional subnodes:
 - mdio : specifies the mdio bus in the FEC, used as a container for phy nodes
-- 
2.17.1

[PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Sven Van Asbroeck]

On imx6, the ethernet reference clock (clk_enet_ref) can be generated
by either the imx6, or an external source (e.g. an oscillator or the
PHY). When generated by the imx6, the clock source (from ANATOP)
must be routed to the input of clk_enet_ref via two pads on the SoC,
typically via a dedicated track on the PCB.

On an imx6 plus however, there is a new setting which enables this
clock to be routed internally on the SoC, from its ANATOP clock
source, straight to clk_enet_ref, without having to go through
the SoC pads.

Enable internal routing if the fsl,ptpclk-bypass-pad boolean
property is present in the "fsl,imx6q-fec" devicetree node.

Link: https://lore.kernel.org/lkml/CAOMZO5BYC3DmE_G0XRwRH9vSJiVVvKbtznODyntsAuorF=HoqA@mail.gmail.com/
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---

Tree: v5.8-rc3

v4 -> v5:
  - identified that existing imx6q-plus boards could break ethernet if v4
    patch is applied.
    reached consensus: prevent breakage by requiring an explicit devicetree
    property for internal ptp clk routing.
    Link: https://lore.kernel.org/lkml/CAOMZO5BYC3DmE_G0XRwRH9vSJiVVvKbtznODyntsAuorF=HoqA@mail.gmail.com/
v3 -> v4:
  - avoid double-check for IS_ERR(gpr) by including Fabio Estevam's
    patch.
v2 -> v3:
  - remove check for imx6q, which is already implied when
    of_machine_is_compatible("fsl,imx6qp")
v1 -> v2:
  - Fabio Estevam: use of_machine_is_compatible() to determine if we
    are running on an imx6 plus.

To: Shawn Guo <shawnguo@kernel.org>
To: Andy Duan <fugang.duan@nxp.com>
To: Rob Herring <robh+dt@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

 arch/arm/mach-imx/mach-imx6q.c              | 14 ++++++++++++++
 include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index ae89ad93ca83..ac62994eb7ba 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -204,6 +204,20 @@ static void __init imx6q_1588_init(void)
 	regmap_update_bits(gpr, IOMUXC_GPR1, IMX6Q_GPR1_ENET_CLK_SEL_MASK,
 			   clksel);
 
+	/*
+	 * On imx6 plus, enet_ref from ANATOP/CCM can be internally routed to
+	 * be the PTP clock source, instead of having to be routed through
+	 * pads.
+	 */
+	if (of_machine_is_compatible("fsl,imx6qp")) {
+		clksel = of_property_read_bool(np, "fsl,ptpclk-bypass-pad") ?
+				IMX6Q_GPR5_ENET_TXCLK_SEL_PLL :
+				IMX6Q_GPR5_ENET_TXCLK_SEL_PAD;
+		regmap_update_bits(gpr, IOMUXC_GPR5,
+				   IMX6Q_GPR5_ENET_TXCLK_SEL_MASK, clksel);
+	}
+
+
 	clk_put(enet_ref);
 put_ptp_clk:
 	clk_put(ptp_clk);
diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
index d4b5e527a7a3..58377002427f 100644
--- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
@@ -240,6 +240,9 @@
 #define IMX6Q_GPR4_IPU_RD_CACHE_CTL		BIT(0)
 
 #define IMX6Q_GPR5_L2_CLK_STOP			BIT(8)
+#define IMX6Q_GPR5_ENET_TXCLK_SEL_MASK		BIT(9)
+#define IMX6Q_GPR5_ENET_TXCLK_SEL_PAD		0
+#define IMX6Q_GPR5_ENET_TXCLK_SEL_PLL		BIT(9)
 #define IMX6Q_GPR5_SATA_SW_PD			BIT(10)
 #define IMX6Q_GPR5_SATA_SW_RST			BIT(11)
 
-- 
2.17.1

Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Fabio Estevam]

Hi Sven,

On Thu, Jul 2, 2020 at 2:53 PM Sven Van Asbroeck <thesven73@gmail.com> wrote:

> +       /*
> +        * On imx6 plus, enet_ref from ANATOP/CCM can be internally routed to
> +        * be the PTP clock source, instead of having to be routed through
> +        * pads.
> +        */
> +       if (of_machine_is_compatible("fsl,imx6qp")) {
> +               clksel = of_property_read_bool(np, "fsl,ptpclk-bypass-pad") ?
> +                               IMX6Q_GPR5_ENET_TXCLK_SEL_PLL :
> +                               IMX6Q_GPR5_ENET_TXCLK_SEL_PAD;
> +               regmap_update_bits(gpr, IOMUXC_GPR5,
> +                                  IMX6Q_GPR5_ENET_TXCLK_SEL_MASK, clksel);
> +       }

With the device tree approach, I think that a better place to touch
GPR5 would be inside the fec driver.

You can refer to drivers/pci/controller/dwc/pci-imx6.c and follow the
same approach for accessing the GPR register:
...
/* Grab GPR config register range */
imx6_pcie->iomuxc_gpr =
syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr")

For the property name, what about fsl,txclk-from-pll?

Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Sven Van Asbroeck]

Hi Fabio,

On Thu, Jul 2, 2020 at 6:29 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> With the device tree approach, I think that a better place to touch
> GPR5 would be inside the fec driver.
>

Cool idea. I notice that the latest FEC driver (v5.8-rc3) accesses individual
bits inside the gpr (via fsl,stop-mode). So perhaps I can do the same here,
and populate that gpr node in imx6qp.dtsi - because it doesn't exist on other
SoCs.

> For the property name, what about fsl,txclk-from-pll?

Sounds good. Does anyone have more suggestions?

RE: [EXT] Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Andy Duan]

From: Sven Van Asbroeck <thesven73@gmail.com> Sent: Friday, July 3, 2020 8:51 AM
> Hi Fabio,
> 
> On Thu, Jul 2, 2020 at 6:29 PM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > With the device tree approach, I think that a better place to touch
> > GPR5 would be inside the fec driver.
> >
> 
> Cool idea. I notice that the latest FEC driver (v5.8-rc3) accesses individual bits
> inside the gpr (via fsl,stop-mode). So perhaps I can do the same here, and
> populate that gpr node in imx6qp.dtsi - because it doesn't exist on other SoCs.
> 
> > For the property name, what about fsl,txclk-from-pll?
> 
> Sounds good. Does anyone have more suggestions?

The property seems good, don't let the property confuse somebody for other
platforms, binding doc describe the property is limited to use for imx6qp only.

Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Sven Van Asbroeck]

Hi Fabio, Andy,

On Thu, Jul 2, 2020 at 6:29 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> With the device tree approach, I think that a better place to touch
> GPR5 would be inside the fec driver.
>

Are we 100% sure this is the best way forward, though?

All the FEC driver should care about is the FEC logic block
inside the SoC. It should not concern itself with the way a SoC
happens to bring a clock (PTP clock) to the input of the FEC
logic block - that is purely a SoC implementation detail.

It makes sense to add fsl,stop-mode (= a GPR bit) to the FEC driver,
as this bit is a logic input to the FEC block, which the driver needs
to dynamically flip.

But the PTP clock is different, because it's statically routed by
the SoC.

Maybe this problem needs a clock routing solution?
Isn't there an imx6q plus clock mux we're not modelling?

  enet_ref-o------>ext>---pad_clk--| \
           |                       |M |----fec_ptp_clk
           o-----------------------|_/

Where M = mux controlled by GPR5[9]

The issue here is that pad_clk is routed externally, so its parent
could be any internal or external clock. I have no idea how to
model this in the clock framework.

RE: [EXT] Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Andy Duan]

From: Sven Van Asbroeck <thesven73@gmail.com>
> Hi Fabio, Andy,
> 
> On Thu, Jul 2, 2020 at 6:29 PM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > With the device tree approach, I think that a better place to touch
> > GPR5 would be inside the fec driver.
> >
> 
> Are we 100% sure this is the best way forward, though?
> 
> All the FEC driver should care about is the FEC logic block inside the SoC. It
> should not concern itself with the way a SoC happens to bring a clock (PTP
> clock) to the input of the FEC logic block - that is purely a SoC implementation
> detail.

I also agree with that relates to SOC integration. 
> 
> It makes sense to add fsl,stop-mode (= a GPR bit) to the FEC driver, as this bit
> is a logic input to the FEC block, which the driver needs to dynamically flip.
> 
> But the PTP clock is different, because it's statically routed by the SoC.
> 
> Maybe this problem needs a clock routing solution?
> Isn't there an imx6q plus clock mux we're not modelling?
> 
>   enet_ref-o------>ext>---pad_clk--| \
>            |                       |M |----fec_ptp_clk
>            o-----------------------|_/
> 
> Where M = mux controlled by GPR5[9]
> 
> The issue here is that pad_clk is routed externally, so its parent could be any
> internal or external clock. I have no idea how to model this in the clock
> framework.
Don't consider it complex, GPR5[9] just select the rgmii gtx source from PAD or internal
Like：
GPR5[9] is cleared: PAD -> MAC gtx
GPR5[9] is set: Pll_enet -> MAC gtx
As you said, register one clock mux for the selection, assign the clock parent by board dts
file, but now current clock driver doesn't support GPR clock. 

Re: [EXT] Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Sven Van Asbroeck]

Hi Andy, thank you so much for your time and attention. See below.

On Sun, Jul 5, 2020 at 10:45 AM Andy Duan <fugang.duan@nxp.com> wrote:
>
> Don't consider it complex, GPR5[9] just select the rgmii gtx source from PAD or internal
> Like：
> GPR5[9] is cleared: PAD -> MAC gtx
> GPR5[9] is set: Pll_enet -> MAC gtx
> As you said, register one clock mux for the selection, assign the clock parent by board dts
> file, but now current clock driver doesn't support GPR clock.

Ok, so for imx6q plus only, we create two new clocks (MAC_GTX and PAD)
and a new clock mux, controlled by GPR5[9]:

  enet_ref-o------>ext>---pad------| \
           |                       |M |----mac_gtx
           o-----------------------|_/

Where M = mux controlled by GPR5[9]

clk_mac_gtx -> clk_pad -> clk_enet_ref is the default. when a board
wants internal routing, it can just do:

&fec {
assigned-clocks = <&clks IMX6QDL_CLK_MAC_GTX>;
assigned-clock-parents = <&clks IMX6QDL_CLK_ENET_REF>;
};

But, how do we manage clk_pad? It is routed externally, and can be
connected to:
- enet_ref (typically via GPIO_16)
- an external oscillator
- an external PHY clock

  ext phy---------| \
                  |  |
  enet_ref-o------|M |----pad------| \
           |      |_/              |  |
           |                       |M |----mac_gtx
           |                       |  |
           o-----------------------|_/


How do we tell the clock framework that clk_pad has a mux that can
be connected to _any_ external clock? and also enet_ref?

RE: [EXT] Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Andy Duan]

From: Sven Van Asbroeck <thesven73@gmail.com> Sent: Sunday, July 5, 2020 11:34 PM
> 
>   ext phy---------| \
>                   |  |
>   enet_ref-o------|M |----pad------| \
>            |      |_/              |  |
>            |                       |M |----mac_gtx
>            |                       |  |
>            o-----------------------|_/
> 
> 
> How do we tell the clock framework that clk_pad has a mux that can be
> connected to _any_ external clock? and also enet_ref?

The clock depends on board design, HW refer guide can describe the clk
usage in detail and customer select one clk path as their board design.

To make thing simple, we can just control the second "M" that is controlled
by gpr bit.

Re: [EXT] Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Fabio Estevam]

On Mon, Jul 6, 2020 at 2:30 AM Andy Duan <fugang.duan@nxp.com> wrote:
>
> From: Sven Van Asbroeck <thesven73@gmail.com> Sent: Sunday, July 5, 2020 11:34 PM
> >
> >   ext phy---------| \
> >                   |  |
> >   enet_ref-o------|M |----pad------| \
> >            |      |_/              |  |
> >            |                       |M |----mac_gtx
> >            |                       |  |
> >            o-----------------------|_/
> >
> >
> > How do we tell the clock framework that clk_pad has a mux that can be
> > connected to _any_ external clock? and also enet_ref?
>
> The clock depends on board design, HW refer guide can describe the clk
> usage in detail and customer select one clk path as their board design.
>
> To make thing simple, we can just control the second "M" that is controlled
> by gpr bit.

Would it make sense to use compatible = "mmio-mux"; like we do on
imx7s, imx6qdl.dtsi and imx8mq.dtsi?

Re: [EXT] Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Sven Van Asbroeck]

Hi Fabio,

On Mon, Jul 6, 2020 at 9:46 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Would it make sense to use compatible = "mmio-mux"; like we do on
> imx7s, imx6qdl.dtsi and imx8mq.dtsi?

Maybe "fixed-mmio-clock" is a better candidate ?

Re: [EXT] Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Sven Van Asbroeck]

On Mon, Jul 6, 2020 at 10:58 AM Sven Van Asbroeck <thesven73@gmail.com> wrote:
>
> Hi Fabio,
>
> On Mon, Jul 6, 2020 at 9:46 AM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Would it make sense to use compatible = "mmio-mux"; like we do on
> > imx7s, imx6qdl.dtsi and imx8mq.dtsi?
>
> Maybe "fixed-mmio-clock" is a better candidate ?

Actually no, the mmio memory there only controls the frequency...

I don't think the clock framework has a ready-made abstraction
suitable for a GPR clock mux yet?

RE: [EXT] Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Andy Duan]

From: Sven Van Asbroeck <thesven73@gmail.com> Sent: Monday, July 6, 2020 11:00 PM
> On Mon, Jul 6, 2020 at 10:58 AM Sven Van Asbroeck <thesven73@gmail.com>
> wrote:
> >
> > Hi Fabio,
> >
> > On Mon, Jul 6, 2020 at 9:46 AM Fabio Estevam <festevam@gmail.com>
> wrote:
> > >
> > > Would it make sense to use compatible = "mmio-mux"; like we do on
> > > imx7s, imx6qdl.dtsi and imx8mq.dtsi?
> >
> > Maybe "fixed-mmio-clock" is a better candidate ?
> 
> Actually no, the mmio memory there only controls the frequency...
> 
> I don't think the clock framework has a ready-made abstraction suitable for a
> GPR clock mux yet?

That needs to add GPR clock API support.

Re: [EXT] Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Sven Van Asbroeck]

Andy, Fabio,

Sounds like we now have a solution which makes logical sense, although it
requires changes and additions to drivers/clk/imx/. Before I create a patch,
can you read the plan below and check that it makes sense, please?

Problem
=======
On the imx6q plus, the NXP hw designers introduced an alternative way to clock
the built-in ethernet (FEC). One single customer (archronix) seems to be
currently using this functionality, and would like to add mainline support.

Changing the ethernet (FEC) driver appears illogical, as this change is
unrelated to the FEC itself: fundamentally, it's a clock tree change.

We also need to prevent breaking existing boards with mis-configured FEC
clocking:

Some board designers indicate in the devicetree that ENET_REF_CLK is connected
to the FEC ptp clock, but in reality, they are providing an unrelated external
clock through the pad. This will work in many cases, because the FEC driver
does not really care where its PTP clock comes from, and the enet ref clock is
set up to mirror the external clk frequency by the NXP U-Boot fork.

Proposed changes must not break these cases.

Proposed Solution
=================
On non-plus imx6q there are no changes. On imx6q plus however:

Create two new clocks (mac_gtx and pad) and a new clock mux:

  enet_ref-o--------->pad----->| \
           |                   |  |
           |                   |M1|----mac_gtx --- to FEC
           |                   |  |
           o-------------------|_/

The defaults (indicated with ">") are carefully chosen, so these changes will
not break any existing plus designs.

We then tie the gtx clock to the FEC driver ptp clock, like so:

in imx6qp.dtsi:
&fec {
    clocks = <&clks IMX6QDL_CLK_ENET>,
        <&clks IMX6QDL_CLK_ENET>,
        <&clks IMX6QDL_CLK_MAC_GTX>;
};

Mux M1 is controlled by GPR5[9]. This mux can just write to the GPR5 memory
address. However, the GPR registers are already exposed as a mux controller
compatible = "mmio-mux". This mux does currently not use GPR5.

So we have to make a choice here: write straight to the memory address (easy),
or create a new clock mux type, which uses an underlying mux controller.

When that is done, board designers can choose between:

1. internal clocking (GTX clock routed internally)

&fec {
    assigned-clocks = <&clks IMX6QDL_CLK_MAC_GTX>;
    assigned-clock-parents = <&clks IMX6QDL_CLK_ENET_REF>;
};

2. external clocking (GTX clock from pad, pad connected to enet_ref,
   typically via GPIO_16). This is the default and does not need to be present.

&fec {
    assigned-clocks = <&clks IMX6QDL_CLK_MAC_GTX>;
    assigned-clock-parents = <&clks IMX6QDL_CLK_PAD>;
};


3. external clocking (GTX clock from pad, pad connected to external PHY
   clock or external oscillator).

phy_osc: oscillator {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <50000000>;
};

&fec {
    clocks = <&clks IMX6QDL_CLK_ENET>,
        <&clks IMX6QDL_CLK_ENET>,
        <&phy_osc>;
};

RE: [EXT] Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Andy Duan]

From: Sven Van Asbroeck <thesven73@gmail.com> Sent: Tuesday, July 7, 2020 11:21 PM
> Andy, Fabio,
> 
> Sounds like we now have a solution which makes logical sense, although it
> requires changes and additions to drivers/clk/imx/. Before I create a patch,
> can you read the plan below and check that it makes sense, please?
> 
> Problem
> =======
> On the imx6q plus, the NXP hw designers introduced an alternative way to
> clock the built-in ethernet (FEC). One single customer (archronix) seems to be
> currently using this functionality, and would like to add mainline support.
> 
> Changing the ethernet (FEC) driver appears illogical, as this change is
> unrelated to the FEC itself: fundamentally, it's a clock tree change.
> 
> We also need to prevent breaking existing boards with mis-configured FEC
> clocking:
> 
> Some board designers indicate in the devicetree that ENET_REF_CLK is
> connected to the FEC ptp clock, but in reality, they are providing an unrelated
> external clock through the pad. This will work in many cases, because the FEC
> driver does not really care where its PTP clock comes from, and the enet ref
> clock is set up to mirror the external clk frequency by the NXP U-Boot fork.
> 
> Proposed changes must not break these cases.
> 
> Proposed Solution
> =================
> On non-plus imx6q there are no changes. On imx6q plus however:
> 
> Create two new clocks (mac_gtx and pad) and a new clock mux:
> 
>   enet_ref-o--------->pad----->| \
>            |                   |  |
>            |                   |M1|----mac_gtx --- to FEC
>            |                   |  |
>            o-------------------|_/
> 
> The defaults (indicated with ">") are carefully chosen, so these changes will
> not break any existing plus designs.
> 
> We then tie the gtx clock to the FEC driver ptp clock, like so:
> 
> in imx6qp.dtsi:
> &fec {
>     clocks = <&clks IMX6QDL_CLK_ENET>,
>         <&clks IMX6QDL_CLK_ENET>,
>         <&clks IMX6QDL_CLK_MAC_GTX>;
> };
> 
> Mux M1 is controlled by GPR5[9]. This mux can just write to the GPR5
> memory address. However, the GPR registers are already exposed as a mux
> controller compatible = "mmio-mux". This mux does currently not use GPR5.
> 
> So we have to make a choice here: write straight to the memory address
> (easy), or create a new clock mux type, which uses an underlying mux
> controller.
> 
> When that is done, board designers can choose between:
> 
> 1. internal clocking (GTX clock routed internally)
> 
> &fec {
>     assigned-clocks = <&clks IMX6QDL_CLK_MAC_GTX>;
>     assigned-clock-parents = <&clks IMX6QDL_CLK_ENET_REF>; };

The one is our requirement that gtx is from internal pll and only for
6qp boards. It is required to set in board dts file due to depends on board
design.

Here also need to assign enet_ref clk rate to 125Mhz.
> 
> 2. external clocking (GTX clock from pad, pad connected to enet_ref,
>    typically via GPIO_16). This is the default and does not need to be
> present.
> 
> &fec {
>     assigned-clocks = <&clks IMX6QDL_CLK_MAC_GTX>;
>     assigned-clock-parents = <&clks IMX6QDL_CLK_PAD>; };
> 
As above, here also need to assign the enet_ref clk rate to 125Mhz.
The clk tree:
Pll6 -> enet_ref -> clk_pad -> mac_gtx
Pll6 -> enet_ref -> clk_pad -> route back to supply clk for ptp

> 
> 3. external clocking (GTX clock from pad, pad connected to external PHY
>    clock or external oscillator).
> 
> phy_osc: oscillator {
> #clock-cells = <0>;
> compatible = "fixed-clock";
> clock-frequency = <50000000>;
> };
> 
> &fec {
>     clocks = <&clks IMX6QDL_CLK_ENET>,
>         <&clks IMX6QDL_CLK_ENET>,
>         <&phy_osc>;
> };

Re: [EXT] Re: [PATCH v5 3/3] ARM: imx6plus: optionally enable internal routing of clk_enet_ref

[Sven Van Asbroeck]

Hi Andy,

On Mon, Jul 6, 2020 at 1:30 AM Andy Duan <fugang.duan@nxp.com> wrote:
>
> The clock depends on board design, HW refer guide can describe the clk
> usage in detail and customer select one clk path as their board design.
>

Yes, I agree. But how does a board designer practically describe this
in the devicetree?

Example: mac_gtx -> clk_pad (the default)

Imagine I have a fixed oscillator on the board:

phy_ref_clk: oscillator {
    compatible = "fixed-clock";
    #clock-cells = <0>;
    clock-frequency = <50000000>;
};

How can I now make <&phy_ref_clk> the parent of
<&clks CLK_PAD> ?