On Wed, Apr 4, 2018 at 5:26 PM, Laurent Pinchart
<laurent.pinch...@ideasonboard.com> wrote:
> On Thursday, 29 March 2018 14:30:39 EEST Maxime Ripard wrote:
>> On Tue, Feb 13, 2018 at 12:01:32AM +0100, Niklas Söderlund wrote:
>> > +   switch (priv->lanes) {
>> > +   case 1:
>> > +           phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_0;
>> > +           break;
>> > +   case 2:
>> > +           phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_1 | PHYCNT_ENABLE_0;
>> > +           break;
>> > +   case 4:
>> > +           phycnt = PHYCNT_ENABLECLK | PHYCNT_ENABLE_3 | PHYCNT_ENABLE_2 |
>> > +                   PHYCNT_ENABLE_1 | PHYCNT_ENABLE_0;
>> > +           break;
>> > +   default:
>> > +           return -EINVAL;
>> > +   }
>>
>> I guess you could have a simpler construct here using this:
>>
>> phycnt = PHYCNT_ENABLECLK;
>>
>> switch (priv->lanes) {
>> case 4:
>>       phycnt |= PHYCNT_ENABLE_3 | PHYCNT_ENABLE_2;
>> case 2:
>>       phycnt |= PHYCNT_ENABLE_1;
>> case 1:
>>       phycnt |= PHYCNT_ENABLE_0;
>>       break;
>>
>> default:
>>       return -EINVAL;
>> }
>>
>> But that's really up to you.
>
> Wouldn't Niklas' version generate simpler code as it uses direct assignments ?

Alternatively, you could check for a valid number of lanes, and use knowledge
about the internal lane bits:

    phycnt = PHYCNT_ENABLECLK;
    phycnt |= (1 << priv->lanes) - 1;

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

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

Reply via email to