Hi Wolfram,
On Wed, Mar 23, 2016 at 4:26 PM, Wolfram Sang <wsa@xxxxxxxxxxxxx> wrote:
>> > - if (core->type == CLK_TYPE_FF) {
>> > - clk = clk_register_fixed_factor(NULL, core->name,
>> > - parent_name, 0,
>> > - core->mult, core->div);
>> > - } else {
>> > +
>> > + if (core->type == CLK_TYPE_DIV6_RO)
>> > + /* Multiply with the DIV6 register value */
>> > + div *= (readl(priv->base + core->offset) & 0x3f) + 1;
>> > +
>> > + if (core->type == CLK_TYPE_DIV6P1) {
>> > clk = cpg_div6_register(core->name, 1, &parent_name,
>> > priv->base + core->offset);
>> > + } else {
>> > + clk = clk_register_fixed_factor(NULL, core->name,
>> > + parent_name, 0,
>> > + core->mult, div);
>> > }
>>
>> Please use a switch() instead of if / else. It avoids people wondering where
>> the "core->type == CLK_TYPE_FF" case went to, until they realize it's covered
>> by the else.
>
> Are you sure? It is as you say when reading the diff. But looking at the
> source file after the patch, I think this is easily understandable?
Sorry, you're right again.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
