| Message ID | 20241205055122.11613-1-val@packett.cool (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | clk: rockchip: add clock ID for CIF0/1 on RK3066 | expand |
Hi Val, Am Donnerstag, 5. Dezember 2024, 06:50:46 CET schrieb Val Packett: > 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> please split this into two patches - addition of the clock-ids to the dt-binding header - setting the clock-ids in the clock driver Thanks Heiko > --- > drivers/clk/rockchip/clk-rk3188.c | 4 ++-- > include/dt-bindings/clock/rk3188-cru-common.h | 2 ++ > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c > index 684233e72105..81e94b338d0f 100644 > --- a/drivers/clk/rockchip/clk-rk3188.c > +++ b/drivers/clk/rockchip/clk-rk3188.c > @@ -344,7 +344,7 @@ static struct rockchip_clk_branch common_clk_branches[] __initdata = { > > GATE(0, "pclkin_cif0", "ext_cif0", 0, > RK2928_CLKGATE_CON(3), 3, GFLAGS), > - INVERTER(0, "pclk_cif0", "pclkin_cif0", > + INVERTER(PCLK_CIF0, "pclk_cif0", "pclkin_cif0", > RK2928_CLKSEL_CON(30), 8, IFLAGS), > > FACTOR(0, "xin12m", "xin24m", 0, 1, 2), > @@ -602,7 +602,7 @@ static struct rockchip_clk_branch rk3066a_clk_branches[] __initdata = { > > GATE(0, "pclkin_cif1", "ext_cif1", 0, > RK2928_CLKGATE_CON(3), 4, GFLAGS), > - INVERTER(0, "pclk_cif1", "pclkin_cif1", > + INVERTER(PCLK_CIF1, "pclk_cif1", "pclkin_cif1", > RK2928_CLKSEL_CON(30), 12, IFLAGS), > > COMPOSITE(0, "aclk_gpu_src", mux_pll_src_cpll_gpll_p, 0, > diff --git a/include/dt-bindings/clock/rk3188-cru-common.h b/include/dt-bindings/clock/rk3188-cru-common.h > index 01e14ab252a7..dd988cc9d582 100644 > --- a/include/dt-bindings/clock/rk3188-cru-common.h > +++ b/include/dt-bindings/clock/rk3188-cru-common.h > @@ -103,6 +103,8 @@ > #define PCLK_PERI 351 > #define PCLK_DDRUPCTL 352 > #define PCLK_PUBL 353 > +#define PCLK_CIF0 354 > +#define PCLK_CIF1 355 > > /* hclk gates */ > #define HCLK_SDMMC 448 >
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. Best regards, Krzysztof
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
On 05/12/2024 13:29, Heiko Stübner wrote: > 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? No, there is a big, fat warning about bindings. Best regards, Krzysztof
diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c index 684233e72105..81e94b338d0f 100644 --- a/drivers/clk/rockchip/clk-rk3188.c +++ b/drivers/clk/rockchip/clk-rk3188.c @@ -344,7 +344,7 @@ static struct rockchip_clk_branch common_clk_branches[] __initdata = { GATE(0, "pclkin_cif0", "ext_cif0", 0, RK2928_CLKGATE_CON(3), 3, GFLAGS), - INVERTER(0, "pclk_cif0", "pclkin_cif0", + INVERTER(PCLK_CIF0, "pclk_cif0", "pclkin_cif0", RK2928_CLKSEL_CON(30), 8, IFLAGS), FACTOR(0, "xin12m", "xin24m", 0, 1, 2), @@ -602,7 +602,7 @@ static struct rockchip_clk_branch rk3066a_clk_branches[] __initdata = { GATE(0, "pclkin_cif1", "ext_cif1", 0, RK2928_CLKGATE_CON(3), 4, GFLAGS), - INVERTER(0, "pclk_cif1", "pclkin_cif1", + INVERTER(PCLK_CIF1, "pclk_cif1", "pclkin_cif1", RK2928_CLKSEL_CON(30), 12, IFLAGS), COMPOSITE(0, "aclk_gpu_src", mux_pll_src_cpll_gpll_p, 0, diff --git a/include/dt-bindings/clock/rk3188-cru-common.h b/include/dt-bindings/clock/rk3188-cru-common.h index 01e14ab252a7..dd988cc9d582 100644 --- a/include/dt-bindings/clock/rk3188-cru-common.h +++ b/include/dt-bindings/clock/rk3188-cru-common.h @@ -103,6 +103,8 @@ #define PCLK_PERI 351 #define PCLK_DDRUPCTL 352 #define PCLK_PUBL 353 +#define PCLK_CIF0 354 +#define PCLK_CIF1 355 /* hclk gates */ #define HCLK_SDMMC 448
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(-)