Hi Geert,
On Thursday, 5 April 2018 10:33:55 EEST Geert Uytterhoeven wrote:
> On Wed, Apr 4, 2018 at 5:26 PM, Laurent Pinchart 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;
If Niklas is fine with that, I like it too.
--
Regards,
Laurent Pinchart
