linux-kernel - Re: [PATCH v3 2/3] clk: renesas: Add family-specific clock driver for RZ/V2H(P)

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] [day] [month] [year] [list]

Message-ID: <CA+V-a8tpCURbrGAG6NwD5WMz8NH3BfGvQXUj=Ooef2SsbPbnJA@mail.gmail.com>
Date: Mon, 15 Jul 2024 10:55:40 +0100
From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To: Geert Uytterhoeven <geert@...ux-m68k.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>, 
	Philipp Zabel <p.zabel@...gutronix.de>, Magnus Damm <magnus.damm@...il.com>, 
	linux-clk@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org, 
	Biju Das <biju.das.jz@...renesas.com>, 
	Fabrizio Castro <fabrizio.castro.jz@...esas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v3 2/3] clk: renesas: Add family-specific clock driver for RZ/V2H(P)

Hi Geert,

On Mon, Jul 15, 2024 at 10:42 AM Geert Uytterhoeven
<geert@...ux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Jul 15, 2024 at 10:44 AM Lad, Prabhakar
> <prabhakar.csengg@...il.com> wrote:
> > On Fri, Jul 12, 2024 at 6:11 PM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > > On Fri, Jul 12, 2024 at 5:29 PM Lad, Prabhakar
> > > <prabhakar.csengg@...il.com> wrote:
<snip>
> > /**
> >  * struct rzv2h_reset - Reset definitions
> >  *
> >  * @reset_index: reset register index
> >  * @reset_bit: reset bit
> >  * @mon_index: monitor register index
> >  * @mon_bit: monitor bit
> >  */
> > struct rzv2h_reset {
> >     u8 reset_index;
> >     u8 reset_bit;
> >     u8 mon_index;
> >     u8 mon_bit;
> > };
> >
> > #define DEF_RST_BASE(_resindex, _resbit, _monindex, _monbit)    \
> >     { \
> >         .reset_index = (_resindex), \
> >         .reset_bit = (_resbit), \
> >         .mon_index = (_monindex), \
> >         .mon_bit = (_monbit), \
> >     }
> >
> > #define DEF_RST(_resindex, _resbit, _monindex, _monbit)    \
> >     DEF_RST_BASE(_resindex, _resbit, _monindex, _monbit)
> >
> >
> > in rzv2h_cpg_probe() (.num_resets = ARRAY_SIZE(r9a09g057_resets))
> >
> >     resets = devm_kmalloc_array(dev, info->num_resets, sizeof(struct
> > rzv2h_reset), GFP_KERNEL);
> >     if (!resets)
> >         return -ENOMEM;
> >
> >     for (i = 0; i < priv->num_resets; i++)
> >         memcpy(&resets[i], &info->resets[i], sizeof(struct rzv2h_reset));
>
> You can combine both using devm_kmemdup().
>
Thanks for the pointer.

> > And have the below xlate function that will convert id into index ie
> > index into rests array.
> >
> > static int rzv2h_get_reset_index(struct rzv2h_cpg_priv *priv,
> >                  unsigned long id)
> > {
> >     u8 reset_index = id / 16;
> >     u8 reset_bit = id % 16;
> >     unsigned int i;
> >
> >     for (i = 0; i < priv->num_resets; i++) {
> >         if (priv->resets[i].reset_index == reset_index &&
> >             priv->resets[i].reset_bit == reset_bit)
> >             return i;
> >     }
> >
> >     return -EINVAL;
> > }
> >
> > static int rzv2h_cpg_reset_xlate(struct reset_controller_dev *rcdev,
> >                  const struct of_phandle_args *reset_spec)
> > {
> >     struct rzv2h_cpg_priv *priv = rcdev_to_priv(rcdev);
> >     unsigned int id = reset_spec->args[0];
> >     int index = rzv2h_get_reset_index(priv, id);
> >
> >     if (index < 0) {
> >         dev_err(rcdev->dev, "Invalid reset index %u\n", id);
> >         return -EINVAL;
> >     }
> >
> >     return index;
> > }
> >
> >
> > rzv2h_cpg_assert() and rzv2h_cpg_deassert() which will use an id that
> > can directly index into resets[] array.
> >
> > Please let me know if this is OK.
>
> That would work, too, at the expense of needing a loop for look-up
> (traditional trade-off between memory and time ;-)
> But look-up is only done once (per device), so that should be fine.
>
> It all depends on how many resets you will end up using...
> Memory allocation also has a granularity, so once you have more
> than a specific number of resets, you better use a sparse array,
> and simple indexing.
>
Ok got you, I'll update it and send a new version.

Cheers,
Prabhakar