From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> Sent: Thursday, October 20, 2016 5:11 PM
> To: Andy Duan <fugang.duan@xxxxxxx>; Fabio Estevam Estevam
> <fabio.estevam@xxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>;
> devicetree@xxxxxxxxxxxxxxx
> Cc: kernel@xxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Subject: Handling of enet_out on i.MX28
>
> Hello,
>
> The i.MX28 reference manual specifies for HW_CLKCTRL_ENET.CLK_OUT_EN:
> "NOTE: This bit must be configured before ENET PLL is enabled.".
>
> Currently this is not implemented: ENET PLL (aka pll2) is the parent of
> enet_out and so common clk enables the PLL first and only then sets
> HW_CLKCTRL_ENET.CLK_OUT_EN.
>
> For now this is a theoretical problem because I don't see any issues. I only
> notice the discrepancy between manual and reality.
>
> Do you think this is a problem?
>
I don't know the history. But I think this is a problem. Maybe it bring some clock glitch that may cause PHY (with RMII) abnormal.
> Apart from that I'm not happy with the handling of this clk. IMHO it should
> better be called "enet-ref" or similar (for the fec). imx28.dtsi specifies that
> the clk is in use[1] and if my machine doesn't I have to
> do:
>
> &mac0 {
> ...
> /* overwrite clocks and clock-names to remove enet_out */
> clocks = <&clks 57>, <&clks 57>;
> clock-names = "ipg", "ahb";
> ...
> };
>
> in my board.dts. This is ugly because I have to repeat stuff that is already in
> imx28.dtsi and it's not understandable without the comment.
>
> It would be nice if dtc allowed to modify an array, then we could do:
>
> clocks += <&clks 64>;
> clock-names = += "enet_out";
>
> (assuming the included dtsi doesn't specify this clock).
>
> I first though it would be a good idea to specify the enet-ref clk as
> follows:
>
> &mac0 {
> ...
> mdio {
> clocks = <&clks 64>;
> clock-names = "enet-ref";
> ...
> };
> };
>
In RMII mode, this clock is for phy and MAC reference clock.
It is better to specify this clock as "enet_clk_ref" and "enet_out" that is easy to understand.
In dtsi file, we can just define ipg and ahb clock.
clocks = <&clks 57>, <&clks 57>;
clock-names = "ipg","ahb";
In dts file, different boards may have design mode like RMII/MII, RMII mode needs to define enet_out and enet_clk_ref, MII mode doesn't need this clocks.
So it depends on board design, we can overwrite the clock in dts board file.
Like in RMII mode:
clocks = <&clks 57>, <&clks 57>, <&clks 64>, <&clks 64>;
clock-names = "ipg","ahb", "enet_clk_ref","enet_out";
In MII mode: no necessary to overwrite them.
> but while this makes it easier for the board.dts to add (or remove) it, it's not
> really right this way because the reference clock is needed for data RX and TX,
> not the mdio bus. Technically these are two different buses even though the
> "passengers" are often the same. (Even if two MACs are in use, the enet-ref
> signal is shared.)
>
> What do you think?
>
> Best regards
> Uwe
>
> [1] clocks = <&clks 57>, <&clks 57>, <&clks 64>; clock-names = "ipg",
> "ahb", "enet_out"; in &mac0.
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
