linux-kernel - Re: [PATCH] clk: rockchip: add clock ID for CIF0/1 on RK3066

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: <1944322.g5d078U9FE@diego>
Date: Thu, 05 Dec 2024 13:29:55 +0100
From: Heiko Stübner <heiko@...ech.de>
To: Val Packett <val@...kett.cool>, Krzysztof Kozlowski <krzk@...nel.org>
Cc: Michael Turquette <mturquette@...libre.com>,
 Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org
Subject: Re: [PATCH] clk: rockchip: add clock ID for CIF0/1 on RK3066

Hi Krzysztof,

Am Donnerstag, 5. Dezember 2024, 11:25:28 CET schrieb Krzysztof Kozlowski:
> On Thu, Dec 05, 2024 at 02:50:46AM -0300, Val Packett wrote:
> > RK3066 does have two "CIF" video capture interface blocks, add their
> > corresponding clock IDs so that they could be used.
> > 
> > Signed-off-by: Val Packett <val@...kett.cool>
> > ---
> >  drivers/clk/rockchip/clk-rk3188.c             | 4 ++--
> >  include/dt-bindings/clock/rk3188-cru-common.h | 2 ++
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> Please run scripts/checkpatch.pl and fix reported warnings. Then please
> run 'scripts/checkpatch.pl --strict' and (probably) fix more warnings.
> Some warnings can be ignored, especially from --strict run, but the code
> here looks like it needs a fix. Feel free to get in touch if the warning
> is not clear.

I guess you're taking a shot of the indentation?

Though that is an intentional deviation, for the long lists of
clock-declarations, that is more a spreadsheet than actual code.

To give this some context, an excerpt from the rk3188 clock driver:

        COMPOSITE(0, "aclk_lcdc1_pre", mux_pll_src_cpll_gpll_p, 0,
                        RK2928_CLKSEL_CON(31), 15, 1, MFLAGS, 8, 5, DFLAGS,
                        RK2928_CLKGATE_CON(1), 4, GFLAGS),
        GATE(ACLK_PERI, "aclk_peri", "aclk_peri_pre", 0,
                        RK2928_CLKGATE_CON(2), 1, GFLAGS),
        COMPOSITE_NOMUX(HCLK_PERI, "hclk_peri", "aclk_peri_pre", 0,
                        RK2928_CLKSEL_CON(10), 8, 2, DFLAGS | CLK_DIVIDER_POWER_OF_TWO,
                        RK2928_CLKGATE_CON(2), 2, GFLAGS),
        MUX(0, "cif_src", mux_pll_src_cpll_gpll_p, 0,
                        RK2928_CLKSEL_CON(29), 0, 1, MFLAGS),

Rockchip clocks most of the time consist of a mux+divider in one register
and a gate-bit in another. Most clocks are of the COMPOSITE-type above.
Surprisingly this is true since before 2013 - and even today :-) .

So the notation is
      TYPE(id, name, parent-name(s), main-clk-flags,
	MUXDIV-reg, muxoffset, muxwidth, muxflags, divoffset, divwidth, divflags,
	GATE-reg, gatebit, gateflags)

Having all these elements keep their relative position makes it way easier
on the eyes, compared to if they followed that opening parenthesis of each
individual line or maybe reflowing of the elements.

The only real change the clock definitions will see are fixes to wrong
register numbers or wrong bits (or missing clock-ids), so being able to
see check those easily is just nice to have.


Heiko