linux-kernel - Re: [PATCH 07/14] clk: qcom: clk-branch: Add support for SREG branch ops

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

Message-ID: <we4stuv7td5jmvicsvsjowqg76merg5lmlgqj6dvqvqecsw7xk@bfz2kdjnt6kb>
Date: Fri, 18 Oct 2024 01:00:03 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Stephen Boyd <sboyd@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Konrad Dybcio <konradybcio@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Michael Turquette <mturquette@...libre.com>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Philipp Zabel <p.zabel@...gutronix.de>, 
	Rob Herring <robh@...nel.org>, Taniya Das <quic_tdas@...cinc.com>, 
	linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Kalpak Kawadkar <quic_kkawadka@...cinc.com>
Subject: Re: [PATCH 07/14] clk: qcom: clk-branch: Add support for SREG branch
 ops

On Thu, Oct 17, 2024 at 11:10:20AM -0700, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2024-10-17 09:56:57)
> > From: Kalpak Kawadkar <quic_kkawadka@...cinc.com>
> > 
> > Add support for SREG branch ops. This is for the clocks which require
> 
> What is SREG? Can you spell it out?

Unfortunately, no idea. This is the only register name I know.

> 
> > additional register operations with the SREG register as a part of
> > enable / disable operations.
> > 
> > Signed-off-by: Kalpak Kawadkar <quic_kkawadka@...cinc.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> [...]
> > diff --git a/drivers/clk/qcom/clk-branch.h b/drivers/clk/qcom/clk-branch.h
> > index 47bf59a671c3c8516a57c283fce548a6e5f16619..149d04bae25d1a54999e0f938c4fce175a7c3e42 100644
> > --- a/drivers/clk/qcom/clk-branch.h
> > +++ b/drivers/clk/qcom/clk-branch.h
> > @@ -24,8 +24,11 @@
> >  struct clk_branch {
> >         u32     hwcg_reg;
> >         u32     halt_reg;
> > +       u32     sreg_enable_reg;
> >         u8      hwcg_bit;
> >         u8      halt_bit;
> > +       u32     sreg_core_ack_bit;
> > +       u32     sreg_periph_ack_bit;
> 
> Are these bits? Should be u8 then. Or are they a mask?

masks, will rename.

> 
> >         u8      halt_check;
> 
> Instead of adding these new members can you wrap the struct in another
> struct? There are usually a lot of branches in the system and this
> bloats those structures when the members are never used.
> 
> 	struct clk_sreg_branch {
> 		u32 sreg_enable_reg;
> 		u32 sreg_core_ack_bit;
> 		u32 sreg_periph_ack_bit;
> 		struct clk_branch branch;
> 	};
> 
> But I'm not even sure that is needed vs. just putting a clk_regmap
> inside because the clk_ops don't seem to use any of these other members?

Yes, nice idea. Is it ok to keep the _branch suffix or we'd better
rename it dropping the _branch (and move to another source file while we
are at it)?


-- 
With best wishes
Dmitry