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

["Heiko Stübner" <heiko@sntech.de>]

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