linux-kernel - Re: [PATCH 1/6] firmware: arm_scmi: Simplify enable/disable Clock operations

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]

Date:   Sat, 26 Aug 2023 13:50:58 +0100
From:   Cristian Marussi <cristian.marussi@....com>
To:     Stephen Boyd <sboyd@...nel.org>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        sudeep.holla@....com, james.quinlan@...adcom.com,
        f.fainelli@...il.com, vincent.guittot@...aro.org,
        etienne.carriere@...aro.org, peng.fan@....nxp.com,
        chuck.cannon@....com, souvik.chakravarty@....com,
        nicola.mazzucato@....com,
        Michael Turquette <mturquette@...libre.com>,
        linux-clk@...r.kernel.org
Subject: Re: [PATCH 1/6] firmware: arm_scmi: Simplify enable/disable Clock
 operations

On Thu, Aug 24, 2023 at 11:43:35AM -0700, Stephen Boyd wrote:
> Quoting Cristian Marussi (2023-08-24 07:25:21)
> > On Wed, Aug 23, 2023 at 11:01:17AM -0700, Stephen Boyd wrote:
> > > 
> > > Perhaps we need a local variable to make it more readable.
> > > 
> > >       static int scmi_clk_enable(struct clk_hw *hw)
> > >       {
> > >              bool can_sleep = false;
> > >              struct scmi_clk *clk = to_scmi_clk(hw);
> > > 
> > >              return scmi_proto_clk_ops->enable(clk->ph, clk->id, can_sleep);
> > >       }
> > > 
> > > This let's the reader quickly understand what the parameter means. I'm
> > > OK with adding the function parameter, but a plain 'true' or 'false'
> > > doesn't help with clarity.
> > 
> > Thanks for the suggestion, it would help definitely making it more
> > readable, maybe a local define or enum could make it without even
> > putting anything on the stack.
> > 
> 
> Surely the compiler can optimize that so there isn't stack local
> storage for a local variable used as an argument to a function call?

Yes indeed the compiler will certainly drop anything at the end, but still
I'd have to fill with local vars definitions all the related functions just
to be able to make them more readable while I can improve the readability
also by just adding a pair descriptive defines to use all over.

I'll send a V2 and then you tell if it is fine for you.

Thanks
Cristian