Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH v2 07/11] drivers: clk: renesas: r9a07g044-cpg: Update
> {GIC,IA55,SCIF} clock/reset entries
>
> Hi Biju,
>
> On Fri, Jun 25, 2021 at 6:08 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > > Subject: Re: [PATCH v2 07/11] drivers: clk: renesas: r9a07g044-cpg:
> > > Update {GIC,IA55,SCIF} clock/reset entries On Thu, Jun 24, 2021 at
> > > 3:03 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > wrote:
> > > > Update {GIC,IA55,SCIF} clock and reset entries to CPG driver to
> > > > match with RZ/G2L clock list hardware
> > > > manual(RZG2L_clock_list_r02_02.xlsx)
> > > > and RZ/G2L HW manual(Rev.0.50).
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > Reviewed-by: Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> > > > --- a/drivers/clk/renesas/renesas-rzg2l-cpg.h
> > > > +++ b/drivers/clk/renesas/renesas-rzg2l-cpg.h
> > > > @@ -78,7 +78,6 @@ enum clk_types {
> > > > * @parent: id of parent clock
> > > > * @off: register offset
> > > > * @onoff: ON/MON bits
> > > > - * @reset: reset bits
> > > > */
> > > > struct rzg2l_mod_clk {
> > > > const char *name;
> > > > @@ -86,17 +85,15 @@ struct rzg2l_mod_clk {
> > > > unsigned int parent;
> > > > u16 off;
> > > > u8 onoff;
> > > > - u8 reset;
> > > > };
> > > >
> > > > -#define DEF_MOD(_name, _id, _parent, _off, _onoff, _reset) \
> > > > +#define DEF_MOD(_name, _id, _parent, _off, _onoff) \
> > > > [_id] = { \
> > >
> > > Hadn't realized this before, but do you need the "[_id] ="?
> > > rzg2l_cpg_info.mod_clks[] is only indexed during initialization.
> > > If there are gaps due to not all clocks being implemented yet, they
> > > are skipped by the .name check in rzg2l_cpg_register_mod_clk().
> > > But I think you can just remove the gaps instead, decreasing kernel
> > > size (for now).
> >
> > That means we need to loop through the array and find out the index
> corresponding to the ID.
> >
> > Current implementation, we don't need to find out ID iterating through
> > LUT. But as you said it is at the expense of kernel memory.
> >
> > Speed vs memory. Long term any way we will fill the holes. The max index
> now is 96.
>
> Unless I'm missing something, this array is only used for initializing the
> clocks? Clock lookup by ID is done using rzg2l_cpg_priv.clks[], which is
> indexed by ID.
Geert, you are correct. _Id can be removed.
Regards,
Biju
