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:

quoted

On Tue, May 04, 2021 at 11:23:31AM -0500, Bjorn Helgaas wrote:

quoted

quoted

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