Re: [PATCH V2 07/10] clk: imx: add mux ops for i.MX8M composite clk

[Leonard Crestez]

On 2020-04-27 12:15 PM, Peng Fan wrote:
>> Subject: Re: [PATCH V2 07/10] clk: imx: add mux ops for i.MX8M composite
>> clk
>>
>> On 2020-03-12 12:27 PM, Peng Fan wrote:
>>> From: Peng Fan <peng.fan@xxxxxxx>
>>>
>>> The CORE/BUS root slice has following design, simplied graph:
>>> The difference is core not have pre_div block.
>>> A composite core/bus clk has 8 inputs for mux to select, saying clk[0-7].
>>>
>>>               SEL_A  GA
>>>               +--+  +-+
>>>               |  +->+ +------+
>>> CLK[0-7]--->+  |  +-+      |
>>>          |    |  |      +----v---+    +----+
>>>          |    +--+      |pre_diva+---->    |  +---------+
>>>          |              +--------+    |mux +--+post_div |
>>>          |    +--+      |pre_divb+--->+    |  +---------+
>>>          |    |  |      +----^---+    +----+
>>>          +--->+  |  +-+      |
>>>               |  +->+ +------+
>>>               +--+  +-+
>>>               SEL_B  GB
>>>
>>> There will be system hang, when doing the following steps:
>>> 1. switch mux from clk0 to clk1
>>> 2. gate off clk0
>>> 3. swtich from clk1 to clk2, or gate off clk1
>>>
>>> Step 3 triggers system hang.
>>>
>>> If we skip step2, keep clk0 on, step 3 will not trigger system hang.
>>> However we have CLK_OPS_PARENT_ENABLE flag, which will unprepare
>>> disable the clk0 which will not be used.
>>
>> As far as I understand when switching from clk1 to clk2 this is done by
>> temporarily switching the rightmost SELECT mux to whatever was in the
>> spare SEL, which is essentially arbitrary from linux POV.
> 
> No. The fixes in this patches has been confirmed by IC design owner
> 
>>
>> This is quite unexpected but in theory it might be desirable to use a third
>> parent as a fallback.
> 
> No. this will make things complicated. To CCM SEL_A and SEL_B,
> it is controlled by a hardware counter. Saying you write n times to
> the target interface.
> 
> The mux will use n % 2 to choose SEL_A or SEL_B. write twice
> to make sure SEL_A and SEL_B has the same value.

What if SEL_A and SEL_B have different values on boot? The first time 
linux does set_parent it will switch to the other SEL_X which might be off.