linux-kernel - Re: [PATCH v6 1/6] clk: sifive: Add pcie_aux clock in prci driver for PCIe driver

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: <YJIrmjsYLWhIuevI@unreal>
Date:   Wed, 5 May 2021 08:22:34 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Greentime Hu <greentime.hu@...ive.com>, paul.walmsley@...ive.com,
        hes@...ive.com, erik.danie@...ive.com, zong.li@...ive.com,
        bhelgaas@...gle.com, robh+dt@...nel.org, aou@...s.berkeley.edu,
        mturquette@...libre.com, sboyd@...nel.org,
        lorenzo.pieralisi@....com, p.zabel@...gutronix.de,
        alex.dewar90@...il.com, khilman@...libre.com,
        hayashi.kunihiko@...ionext.com, vidyas@...dia.com,
        jh80.chung@...sung.com, linux-pci@...r.kernel.org,
        devicetree@...r.kernel.org, linux-riscv@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH v6 1/6] clk: sifive: Add pcie_aux clock in prci driver
 for PCIe driver

On Tue, May 04, 2021 at 01:45:55PM -0500, Bjorn Helgaas wrote:
> On Tue, May 04, 2021 at 09:12:57PM +0300, Leon Romanovsky wrote:
> > On Tue, May 04, 2021 at 11:23:31AM -0500, Bjorn Helgaas wrote:
> 
> > > There are some weird/interesting bool vs int usages nearby, though:
> > > 
> > >   "bool __is_clk_gate_enabled()" goes to some trouble to convert
> > >   int to bool ("return (reg_val & bit_mask) != 0;"), and then
> > >   kona_peri_clk_is_enabled() converts the bool back to int ("return
> > >   is_clk_gate_enabled(bcm_clk->ccu, gate) ? 1 : 0;").
> > > 
> > >   "int lpc32xx_clk_gate_is_enabled()" actually returns a bool that is
> > >   implicitly converted to int.
> > > 
> > >   Many *_is_enabled() functions return !!(...) where !! is an
> > >   int-to-bool conversion that is arguably unnecessary and again
> > >   results in an implicit conversion to int.
> > > 
> > > I don't see any *problems* with any of these; it just seems like a
> > > little more mental effort to think about all the explicit and implicit
> > > conversions going on.
> > 
> > The code is written once but read many times and I can't agree with
> > your that examples given by you are not the *problems*. They clearly
> > says "the API is not great and easily can be improved".
> 
> I certainly agree that it's easier for readers if the style is
> consistent.  I just meant I didn't see anything that's an actual bug.  

No one said that it is a bug. My comment is better seen as s suggestion
to the maintainers on how other subsystems keep their code base clean
and up-to date.

Once "the problem" is spotted, the submitter is asked to fix it globally
including fixing other drivers if needed, before "new feature" can be merged.

Of course, there are exceptions from this rule, but they are rare and
usually given to the people who has proven record.

Thanks

> 
> Bjorn