On Wed, Apr 4, 2018 at 5:26 PM, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> 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 -- 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