| Message ID | 20230113142718.3038265-1-o.rempel@pengutronix.de |
|---|---|
| Headers | show |
| Series | ARM: imx: make Ethernet refclock configurable | expand |
Quoting Oleksij Rempel (2023-01-13 06:27:02) > It is not clear from the code what clock should be taken. So, make sure it > is readable and no other clock will be taken by accident. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > arch/arm/mach-imx/mach-imx6q.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c > index 7f6200925752..4885d3dfcf7f 100644 > --- a/arch/arm/mach-imx/mach-imx6q.c > +++ b/arch/arm/mach-imx/mach-imx6q.c > @@ -98,7 +98,7 @@ static void __init imx6q_1588_init(void) > if (!IS_ERR(fec_enet_ref)) > goto put_node; > > - ptp_clk = of_clk_get(np, 2); > + ptp_clk = of_clk_get_by_name(np, "ptp"); The 'clocks' property in DTS should not be reordered. Order matters in the binding. This patch makes the code do a string comparison (or a few?) in the name of readability. Perhaps make a #define for '2' like CLOCKS_PTP_INDEX, or just don't change it because it ain't broke.
Hi Oleksij, On Fri, Jan 13, 2023 at 11:27 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > Configure Ethernet reference clock parent in an obvious way instead of > using cryptic ptp way. Could you please improve the commit log? The "obvious way" is not obvious for people that don't have the board schematics. I like better the way you described the 20/20 patch: "On this board the PHY is the ref clock provider. So, configure ethernet reference clock as input." Please use this format globally in the series, as it becomes clear who is providing the ref clock. Thanks
Hi Oleksij, On 1/13/2023 10:27 PM, Oleksij Rempel wrote: > Old imx6q machine code makes RGMII/RMII clock direction decision based on > configuration of "ptp" clock. "enet_out" is not used and make no real > sense, since we can't configure it as output or use it as clock > provider. > > Instead of "enet_out" use "enet_clk_ref" which is actual selector to > choose between internal and external clock source: > > FEC MAC <---------- enet_clk_ref <--------- SoC PLL > \ > ^------<-> refclock PAD (bi directional) > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > arch/arm/boot/dts/imx6qdl.dtsi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi > index ff1e0173b39b..71522263031a 100644 > --- a/arch/arm/boot/dts/imx6qdl.dtsi > +++ b/arch/arm/boot/dts/imx6qdl.dtsi > @@ -1050,8 +1050,8 @@ fec: ethernet@2188000 { > clocks = <&clks IMX6QDL_CLK_ENET>, > <&clks IMX6QDL_CLK_ENET>, > <&clks IMX6QDL_CLK_ENET_REF>, > - <&clks IMX6QDL_CLK_ENET_REF>; > - clock-names = "ipg", "ahb", "ptp", "enet_out"; > + <&clks IMX6QDL_CLK_ENET_REF_SEL>; > + clock-names = "ipg", "ahb", "ptp", "enet_clk_ref"; Please also update fec binding, otherwise there will be dtbs check error. Thanks, Peng. > fsl,stop-mode = <&gpr 0x34 27>; > status = "disabled"; > };
On Mon, Jan 16, 2023 at 09:01:08AM +0800, Peng Fan wrote: > Hi Oleksij, > > On 1/13/2023 10:27 PM, Oleksij Rempel wrote: > > Old imx6q machine code makes RGMII/RMII clock direction decision based on > > configuration of "ptp" clock. "enet_out" is not used and make no real > > sense, since we can't configure it as output or use it as clock > > provider. > > > > Instead of "enet_out" use "enet_clk_ref" which is actual selector to > > choose between internal and external clock source: > > > > FEC MAC <---------- enet_clk_ref <--------- SoC PLL > > \ > > ^------<-> refclock PAD (bi directional) > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > arch/arm/boot/dts/imx6qdl.dtsi | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi > > index ff1e0173b39b..71522263031a 100644 > > --- a/arch/arm/boot/dts/imx6qdl.dtsi > > +++ b/arch/arm/boot/dts/imx6qdl.dtsi > > @@ -1050,8 +1050,8 @@ fec: ethernet@2188000 { > > clocks = <&clks IMX6QDL_CLK_ENET>, > > <&clks IMX6QDL_CLK_ENET>, > > <&clks IMX6QDL_CLK_ENET_REF>, > > - <&clks IMX6QDL_CLK_ENET_REF>; > > - clock-names = "ipg", "ahb", "ptp", "enet_out"; > > + <&clks IMX6QDL_CLK_ENET_REF_SEL>; > > + clock-names = "ipg", "ahb", "ptp", "enet_clk_ref"; > > > Please also update fec binding, otherwise there will be dtbs check error. Hm, there is no restriction on enet_clk_ref use or requirements to use enet_out in Documentation/devicetree/bindings/net/fsl,fec.yaml Do I missing something? Regards, Oleksij
On 1/16/2023 1:26 PM, Oleksij Rempel wrote: > On Mon, Jan 16, 2023 at 09:01:08AM +0800, Peng Fan wrote: >> Hi Oleksij, >> >> On 1/13/2023 10:27 PM, Oleksij Rempel wrote: >>> Old imx6q machine code makes RGMII/RMII clock direction decision based on >>> configuration of "ptp" clock. "enet_out" is not used and make no real >>> sense, since we can't configure it as output or use it as clock >>> provider. >>> >>> Instead of "enet_out" use "enet_clk_ref" which is actual selector to >>> choose between internal and external clock source: >>> >>> FEC MAC <---------- enet_clk_ref <--------- SoC PLL >>> \ >>> ^------<-> refclock PAD (bi directional) >>> >>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>> --- >>> arch/arm/boot/dts/imx6qdl.dtsi | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi >>> index ff1e0173b39b..71522263031a 100644 >>> --- a/arch/arm/boot/dts/imx6qdl.dtsi >>> +++ b/arch/arm/boot/dts/imx6qdl.dtsi >>> @@ -1050,8 +1050,8 @@ fec: ethernet@2188000 { >>> clocks = <&clks IMX6QDL_CLK_ENET>, >>> <&clks IMX6QDL_CLK_ENET>, >>> <&clks IMX6QDL_CLK_ENET_REF>, >>> - <&clks IMX6QDL_CLK_ENET_REF>; >>> - clock-names = "ipg", "ahb", "ptp", "enet_out"; >>> + <&clks IMX6QDL_CLK_ENET_REF_SEL>; >>> + clock-names = "ipg", "ahb", "ptp", "enet_clk_ref"; >> >> >> Please also update fec binding, otherwise there will be dtbs check error. > > Hm, there is no restriction on enet_clk_ref use or requirements to use > enet_out in Documentation/devicetree/bindings/net/fsl,fec.yaml > > Do I missing something? After check, seems using enet_out would trigger dtbs_check error, using enet_clk_ref would not as what you did in this patch. So your patch is fine. clock-names: minItems: 2 maxItems: 5 items: enum: - ipg - ahb - ptp - enet_clk_ref - enet_out - enet_2x_txclk Regards, Peng. > > Regards, > Oleksij
Most of i.MX SoC variants have configurable FEC/Ethernet reference clock used by RMII specification. This functionality is located in the general purpose registers (GRPx) and till now was not implemented as part of SoC clock tree. With this patch set, we move forward and add this missing functionality to some of i.MX clk drivers. So, we will be able to configure clock topology by using devicetree and be able to troubleshoot clock dependencies by using clk_summary etc. Currently implemented and tested i.MX6Q, i.MX6DL and i.MX6UL variants. Oleksij Rempel (20): clk: imx: add clk-gpr-mux driver clk: imx6q: add ethernet refclock mux support ARM: imx6q: skip ethernet refclock reconfiguration if enet_clk_ref is present ARM: imx6q: use of_clk_get_by_name() instead of_clk_get() to get ptp clock ARM: dts: imx6qdl: use enet_clk_ref instead of enet_out for the FEC node ARM: dts: imx6dl-lanmcu: configure ethernet reference clock parent ARM: dts: imx6dl-alti6p: configure ethernet reference clock parent ARM: dts: imx6dl-plybas: configure ethernet reference clock parent ARM: dts: imx6dl-plym2m: configure ethernet reference clock parent ARM: dts: imx6dl-prtmvt: configure ethernet reference clock parent ARM: dts: imx6dl-victgo: configure ethernet reference clock parent ARM: dts: imx6q-prtwd2: configure ethernet reference clock parent ARM: dts: imx6qdl-skov-cpu: configure ethernet reference clock parent ARM: dts: imx6dl-eckelmann-ci4x10: configure ethernet reference clock parent clk: imx: add imx_obtain_fixed_of_clock() clk: imx6ul: fix enet1 gate configuration clk: imx6ul: add ethernet refclock mux support ARM: dts: imx6ul: set enet_clk_ref to CLK_ENETx_REF_SEL ARM: mach-imx: imx6ul: remove not optional ethernet refclock overwrite ARM: dts: imx6ul-prti6g: configure ethernet reference clock parent arch/arm/boot/dts/imx6dl-alti6p.dts | 12 +- arch/arm/boot/dts/imx6dl-eckelmann-ci4x10.dts | 13 +- arch/arm/boot/dts/imx6dl-lanmcu.dts | 12 +- arch/arm/boot/dts/imx6dl-plybas.dts | 12 +- arch/arm/boot/dts/imx6dl-plym2m.dts | 12 +- arch/arm/boot/dts/imx6dl-prtmvt.dts | 11 +- arch/arm/boot/dts/imx6dl-victgo.dts | 12 +- arch/arm/boot/dts/imx6q-prtwd2.dts | 17 ++- arch/arm/boot/dts/imx6qdl-skov-cpu.dtsi | 12 +- arch/arm/boot/dts/imx6qdl.dtsi | 4 +- arch/arm/boot/dts/imx6ul-prti6g.dts | 14 ++- arch/arm/boot/dts/imx6ul.dtsi | 10 +- arch/arm/mach-imx/mach-imx6q.c | 12 +- arch/arm/mach-imx/mach-imx6ul.c | 20 --- drivers/clk/imx/Makefile | 1 + drivers/clk/imx/clk-gpr-mux.c | 119 ++++++++++++++++++ drivers/clk/imx/clk-imx6q.c | 13 ++ drivers/clk/imx/clk-imx6ul.c | 33 ++++- drivers/clk/imx/clk.c | 14 +++ drivers/clk/imx/clk.h | 8 ++ include/dt-bindings/clock/imx6qdl-clock.h | 4 +- include/dt-bindings/clock/imx6ul-clock.h | 7 +- include/linux/mfd/syscon/imx6q-iomuxc-gpr.h | 6 +- 23 files changed, 297 insertions(+), 81 deletions(-) create mode 100644 drivers/clk/imx/clk-gpr-mux.c