[PATCH 1/7] clk: stm32mp1: Split ETHCK_K into separate MUX and GATE clock

[Marek Vasut]

On 4/14/21 3:03 PM, gabriel.fernandez at foss.st.com wrote:
> Hi Marek,

Hello Gabriel,

> Thanks for the patchset
> 
> On 4/8/21 8:57 PM, Marek Vasut wrote:
>> 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.

> 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.

> 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 -----------'

>> 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 
>>

[...]

>> 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
>> +
>> +    DIV(ETHPTP_K, "ethptp_k", "ck_ker_eth", CLK_OPS_PARENT_ENABLE |
> 
> CLK_OPS_PARENT_ENABLE flags not useful with a divider.

How so ?