linux-kernel - Re: [PATCH v2 1/3] Revert "clk: rockchip: Set parent rate for DCLK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e0bef56d-0777-4467-8ddc-3b8b23d7ac21@kwiboo.se>
Date: Sat, 25 Jan 2025 22:28:39 +0100
From: Jonas Karlman <jonas@...boo.se>
To: Elaine Zhang <zhangqing@...k-chips.com>, Alex Bee <knaerzche@...il.com>
Cc: mturquette@...libre.com, sboyd@...nel.org, kever.yang@...k-chips.com,
 heiko@...ech.de, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 linux-clk@...r.kernel.org, linux-rockchip@...ts.infradead.org,
 linux-kernel@...r.kernel.org, huangtao@...k-chips.com,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 1/3] Revert "clk: rockchip: Set parent rate for
 DCLK_VOP clock on RK3228"

Hi Elaine,

On 2025-01-25 06:38, Alex Bee wrote:
> Hi Elaine,
> 
> Am 25.01.25 um 02:15 schrieb Elaine Zhang:
>> This reverts commit 1d34b9757523c1ad547bd6d040381f62d74a3189.
>>
>> RK3228 Only GPLL and CPLL, GPLL is a common clock, does not allow
>> dclk_vop to change its frequency, CPLL is used by GMAC,
>> if dclk_vop use CLK_SET_RATE_PARENT and CLK_SET_RATE_NO_REPARENT flags will
>> affect the GMAC function.
>>
> how do you come to this conclusion?
> 
> On the RK3228, hdmiphy is the default parent of dclk_vop, a clock that is
> not even generated by the CRU but is the output of an external HDMI PHY.
> The CLK_SET_RATE_NO_REPARENT flag ensures that the parent of dclk_vop never
> changes to sclk_vop_pre (and thus never uses CPLL or GPLL). With
> CLK_SET_RATE_PARENT we only ensure that we can use all rates of [0] since
> there is no divider between dclk_vop and hdmiphy. Overall it is pretty much
> the same situation as for RK3328, which handles these clocks in the same
> way (see dclk_lcdc for RK3328).

I came to similar conclusion for v1 of this patch, see [2].

Maybe we should assign clk parent in DT similar to rk3328.dtsi to make
it extra clear that hdmiphy should be used as parent?

  assigned-clocks = <&cru DCLK_VOP>;
  assigned-clock-parents = <&cru SCLK_HDMI_PHY>;

Also for next revert patch you send, please include the patch author in
the recipient list :-)

[2] https://lore.kernel.org/all/d95d317c-5f6e-42bd-94a9-e1a6c7685e2f@kwiboo.se/

Regards,
Jonas

> 
> Regards,
> Alex
> 
> [0] [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c?h=v6.13#n293
>> If the client application does not use GMAC and CPLL is free, make this
>> change on the local branch.
>>
>> Signed-off-by: Elaine Zhang <zhangqing@...k-chips.com>
>> ---
>>   drivers/clk/rockchip/clk-rk3228.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/rockchip/clk-rk3228.c b/drivers/clk/rockchip/clk-rk3228.c
>> index ed602c27b624..9c0284607766 100644
>> --- a/drivers/clk/rockchip/clk-rk3228.c
>> +++ b/drivers/clk/rockchip/clk-rk3228.c
>> @@ -409,7 +409,7 @@ static struct rockchip_clk_branch rk3228_clk_branches[] __initdata = {
>>   			RK2928_CLKSEL_CON(29), 0, 3, DFLAGS),
>>   	DIV(0, "sclk_vop_pre", "sclk_vop_src", 0,
>>   			RK2928_CLKSEL_CON(27), 8, 8, DFLAGS),
>> -	MUX(DCLK_VOP, "dclk_vop", mux_dclk_vop_p, CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT,
>> +	MUX(DCLK_VOP, "dclk_vop", mux_dclk_vop_p, 0,
>>   			RK2928_CLKSEL_CON(27), 1, 1, MFLAGS),
>>   
>>   	FACTOR(0, "xin12m", "xin24m", 0, 1, 2),
> 
>