Re: [PATCH v13 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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