On Tue, Jun 11, 2013 at 3:06 PM, Heiko Stübner <heiko@xxxxxxxxx> wrote: > Am Dienstag, 11. Juni 2013, 13:51:56 schrieb Andy Shevchenko: >> On Tue, Jun 11, 2013 at 2:29 PM, Heiko Stübner <heiko@xxxxxxxxx> wrote: [] >> > @@ -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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
