Hi Marek

On 4/14/21 4:04 PM, Marek Vasut wrote:

On 4/14/21 3:03 PM, gabriel.fernandez@foss.st.com wrote:

quoted

Hi Marek,

Hello Gabriel,

quoted

Thanks for the patchset

On 4/8/21 8:57 PM, Marek Vasut wrote:

quoted

The ETHCK_K are modeled as composite clock of MUX and GATE, however per
STM32MP1 Reference Manual RM0436 Rev 3, Page 574, Figure 83. Peripheral
clock distribution for Ethernet, ETHPTPDIV divider is attached past the
ETHCK_K mux, and ETH_CLK/eth_clk_fb clock are output past ETHCKEN gate.
Therefore, in case ETH_CLK/eth_clk_fb are not in use AND PTP clock are
in use, ETHCKEN gate can be turned off. Current driver does not permit
that, fix it.

I don"t understand, it's already the case.

ETHCK_K it's a composite with a MUX and a GATE.

But ETHCK_K is _not_ a composite clock, look at the Figure 83 in the 
datasheet again and schematic below.

quoted

ETHPTP_K (ETHPTPDIV) it's a composite with the same MUX and a DIV (no 
gate)

But ETHPTP_K shouldn't control any mux, it is only a divider.

quoted

If you use only ETHPTPDIV,  ETHCKEN gate can be turned off.

Look, this is what you have today:

            .------------ ETHCK_K -----------.
            |_______               _______   |
pll4_p_ck--|M_ETHCK\             |G_ETHCK\  |
            | MUX    |------+-----| GATE   |-------------[x] ETH_CLK
pll3_q_ck--|_______/       |     |_______/                  eth_clk_fb
            |               |
            |               '--(ETHCKSELR[7:4] divider)--[x] clk_ptp_ref
            |                                          |
            '------------ ETHPTP_K --------------------'

And this is what you should have, to avoid having two composite clock 
which control the same mux using the same register bit, i.e. what this 
patch implements:

            .- ck_ker_eth -.  .--- ETHCK_K --.
            |_______       |  |    _______   |
pll4_p_ck--|M_ETHCK\      |  |   |G_ETHCK\  |
            | MUX    |------+-----| GATE   |-------------[x] ETH_CLK
pll3_q_ck--|_______/       |     |_______/                  eth_clk_fb
                            |
                            '--(ETHCKSELR[7:4] divider)--[x] clk_ptp_ref
                             |                         |
                             '---- ETHPTP_K -----------'

These 2 solutions are valid. I made the choice to implement the first 
one to be able to change parent with the kernel clock of the IP (no need 
to add an intermediate binding). It's the same principle for all kernel 
of this soc.
I can ask to Alexandre to comeback of this principle, but i 'm not 
favorable.

quoted

quoted

This patch converts ETHCK_K from composite clock into a ETHCKEN gate,
ETHPTP_K from composite clock into ETHPTPDIV divider, and adds another
NO_ID clock "ck_ker_eth" which models the ETHSRC mux and is parent clock
to both ETHCK_K and ETHPTP_K. Therefore, all references to ETHCK_K and
ETHPTP_K remain functional as before.

[1] STM32MP1 Reference Manual RM0436 Rev 3, Page 574,
     Figure 83. Peripheral clock distribution for Ethernet
https://www.st.com/resource/en/reference_manual/dm00327659-stm32mp157-advanced-armbased-32bit-mpus-stmicroelectronics.pdf 

[...]

quoted

quoted

diff --git a/drivers/clk/clk-stm32mp1.c b/drivers/clk/clk-stm32mp1.c
index a875649df8b8..a7c7f544ee5d 100644
--- a/drivers/clk/clk-stm32mp1.c
+++ b/drivers/clk/clk-stm32mp1.c

@@ -1949,7 +1949,6 @@ static const struct clock_config 

stm32mp1_clock_cfg[] = {
      KCLK(DSI_K, "dsi_k", dsi_src, 0, G_DSI, M_DSI),
      KCLK(ADFSDM_K, "adfsdm_k", sai_src, 0, G_ADFSDM, M_SAI1),
      KCLK(USBO_K, "usbo_k", usbo_src, 0, G_USBO, M_USBO),
-    KCLK(ETHCK_K, "ethck_k", eth_src, 0, G_ETHCK, M_ETHCK),
      /* Particulary Kernel Clocks (no mux or no gate) */
      MGATE_MP1(DFSDM_K, "dfsdm_k", "ck_mcu", 0, G_DFSDM),

@@ -1958,11 +1957,16 @@ static const struct clock_config 

stm32mp1_clock_cfg[] = {
      MGATE_MP1(GPU_K, "gpu_k", "pll2_q", 0, G_GPU),
      MGATE_MP1(DAC12_K, "dac12_k", "ck_lsi", 0, G_DAC12),
-    COMPOSITE(ETHPTP_K, "ethptp_k", eth_src, CLK_OPS_PARENT_ENABLE |
+    COMPOSITE(NO_ID, "ck_ker_eth", eth_src, CLK_OPS_PARENT_ENABLE |
            CLK_SET_RATE_NO_REPARENT,
            _NO_GATE,
            _MMUX(M_ETHCK),
-          _DIV(RCC_ETHCKSELR, 4, 4, 0, NULL)),
+          _NO_DIV),
+
+    MGATE_MP1(ETHCK_K, "ethck_k", "ck_ker_eth", 0, G_ETHCK),

assigned parent with ETHCK_K will not work

quoted

+
+    DIV(ETHPTP_K, "ethptp_k", "ck_ker_eth", CLK_OPS_PARENT_ENABLE |

CLK_OPS_PARENT_ENABLE flags not useful with a divider.

How so ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel