Hi Prabhakar,
On Mon, Jul 15, 2024 at 2:56 PM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Add family-specific clock driver for RZ/V2H(P) SoCs.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> ---
> v3->v4
> - Dropped masking of parent clks with 0xffff
> - Dropped storing mod clk id and now calculating it
> based on index and bit.
> - Made parent to u16 in struct rzv2h_mod_clk
> - Made a copy of resets array in struct rzv2h_cpg_priv
Thanks for the update!
> --- /dev/null
> +++ b/drivers/clk/renesas/rzv2h-cpg.c
> +/**
> + * struct rzv2h_cpg_priv - Clock Pulse Generator Private Data
> + *
> + * @info: Pointer to platform data
There is no longer an info member.
Hint: W=1 would have told you.
> + * @dev: CPG device
> + * @base: CPG register block base address
> + * @clks: Array containing all Core and Module Clocks
> + * @num_core_clks: Number of Core Clocks in clks[]
> + * @num_mod_clks: Number of Module Clocks in clks[]
> + * @resets: Array of resets
> + * @num_resets: Number of Module Resets in info->resets[]
> + * @last_dt_core_clk: ID of the last Core Clock exported to DT
> + * @rcdev: Reset controller entity
> + */
> +struct rzv2h_cpg_priv {
> + struct device *dev;
> + void __iomem *base;
> +
> + struct clk **clks;
> + unsigned int num_core_clks;
> + unsigned int num_mod_clks;
> + struct rzv2h_reset *resets;
> + unsigned int num_resets;
> + unsigned int last_dt_core_clk;
> +
> + struct reset_controller_dev rcdev;
> +};
> index 000000000000..33631c101541
> --- /dev/null
> +++ b/drivers/clk/renesas/rzv2h-cpg.h
> +#define DEF_RST_BASE(_id, _resindex, _resbit, _monindex, _monbit) \
> + [_id] = { \
Indexing by _id means the reset array will be very sparse. E.g. the
innocent-looking r9a09g057_resets[] with only a single entry takes
600 bytes:
$ nm -S drivers/clk/renesas/r9a09g057-cpg.o | grep r9a09g057_resets
0000000000000038 0000000000000258 r r9a09g057_resets
So please pack the array here, and either unpack it while making the
priv->resets copy, or implement translation ("look-up") from ID to
packed index in rzv2h_cpg_reset_xlate().
> + .reset_index = (_resindex), \
> + .reset_bit = (_resbit), \
> + .mon_index = (_monindex), \
> + .mon_bit = (_monbit), \
> + }
The rest LGTM.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
