On Tue, Jun 11, 2013 at 3:06 PM, Heiko Stübner [off-list ref] wrote:

Am Dienstag, 11. Juni 2013, 13:51:56 schrieb Andy Shevchenko:

quoted

On Tue, Jun 11, 2013 at 2:29 PM, Heiko Stübner [off-list ref] wrote:

[]

quoted

quoted

@@ -141,6 +149,8 @@ static bool _is_valid_div(struct clk_divider

*divider, unsigned int div)

                return is_power_of_2(div);

        if (divider->table)

                return _is_valid_table_div(divider->table, div);

+       if (divider->flags & CLK_DIVIDER_EVEN && div != 1 && (div % 2) !=
0) +               return false;

        return true;

 }

What if rewrite like

 if (divider->flags & CLK_DIVIDER_EVEN == 0)
 return true;

return div < 2 || div % 2 == 0;

hmm, the current structure is of the form of testing for each feature and
doing a applicable action if the flag is set. So it also is extensible for
future flags and checking for the absence of an attribute while the rest of
the conditionals check for the presence also might make the code harder to
read.

So for me the current variant somehow looks more intuitive.

This variant I think fits:

if (!(divider->flags & CLK_DIVIDER_EVEN))
  return div < 2 || div % 2 == 0;

You check for feature and do accordingly.

--
With Best Regards,
Andy Shevchenko