linux-kernel - Re: [RFC PATCH v2 3/4] 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+V-a8vFjZdprmxFyHn1v4Uc7X7ZGhhxTMxXXgrZQTikO50=6w@mail.gmail.com>
Date: Thu, 27 Jun 2024 14:24:45 +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>, 
	Magnus Damm <magnus.damm@...il.com>, linux-renesas-soc@...r.kernel.org, 
	linux-clk@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, 
	Fabrizio Castro <fabrizio.castro.jz@...esas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [RFC PATCH v2 3/4] clk: renesas: Add family-specific clock driver
 for RZ/V2H(P)

Hi Geert,

On Thu, Jun 27, 2024 at 8:01 AM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Wed, Jun 26, 2024 at 7:36 PM Lad, Prabhakar
> <prabhakar.csengg@...il.com> wrote:
> > On Wed, Jun 26, 2024 at 11:07 AM Geert Uytterhoeven
> > <geert@...ux-m68k.org> wrote:
> > > On Tue, Jun 11, 2024 at 1:32 AM Prabhakar <prabhakar.csengg@...il.com> wrote:
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > > >
> > > > Add family-specific clock driver for RZ/V2H(P) SoCs.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
>
> > > > +/**
> > > > + * struct mod_clock - Module clock
> > > > + *
> > > > + * @hw: handle between common and hardware-specific interfaces
> > > > + * @off: register offset
> > > > + * @bit: ON/MON bit
> > > > + * @monoff: monitor register offset
> > > > + * @monbit: montor bit
> > > > + * @priv: CPG private data
> > > > + */
> > > > +struct mod_clock {
> > > > +       struct clk_hw hw;
> > > > +       u8 on_index;
> > > > +       u8 on_bit;
> > > > +       u16 mon_index;
>
> BTW, why is this u16? The corresponding member in rzv2h_mod_clk is u8.
>
Oops, I will fix this.

> > > > +       u8 mon_bit;
>
> > > > +static int rzv2h_mod_clock_endisable(struct clk_hw *hw, bool enable)
> > > > +{
> > > > +       struct mod_clock *clock = to_mod_clock(hw);
> > > > +       unsigned int reg = GET_CLK_ON_OFFSET(clock->on_index);
> > > > +       struct rzv2h_cpg_priv *priv = clock->priv;
> > > > +       u32 bitmask = BIT(clock->on_bit);
> > > > +       struct device *dev = priv->dev;
> > > > +       u32 value;
> > > > +       int error;
> > > > +
> > > > +       dev_dbg(dev, "CLK_ON 0x%x/%pC %s\n", reg, hw->clk,
> > > > +               enable ? "ON" : "OFF");
> > > > +
> > > > +       value = bitmask << 16;
> > > > +       if (enable)
> > > > +               value |= bitmask;
> > > > +
> > > > +       writel(value, priv->base + reg);
> > > > +
> > > > +       if (!enable)
> > > > +               return 0;
> > > > +
> > > > +       reg = GET_CLK_MON_OFFSET(clock->mon_index);
> > >
> > > What if a clock does not have a clock monitor bit?
> > > Clock bits in registers CPG_CLKON_22 and later do not have corresponding
> > > clock monitor bits.
> > >
> > Oops I had missed this case.
> >
> > I'll introduce a macro (NO_MON_REG_INDEX) for clocks which do not have
> > monitor support and add a check above to skip clk monitor operation if
> > clock->mon_index == NO_MON_REG_INDEX.
> >
> > /* monitor index for clocks which do not have CLKMON support */
> > #define NO_MON_REG_INDEX    0xff
> >
> > Does this sound OK?
>
> Either that, or make mon_index signed (which would reduce its
> effective range by one bit).
>
Ok I'll make it to s8 instead and add a negative check monitor index.

Cheers,
Prabhakar