Re: [PATCH v2 4/4] media: rcar-csi2: Add D-PHY support for V4H

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

On 2025-05-12 09:37:48 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Sun, 11 May 2025 at 22:03, Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > On 2025-05-12 00:37:09 +0530, ALOK TIWARI wrote:
> > > On 11-05-2025 23:17, Niklas Söderlund wrote:
> > > > +   rcsi2_write16(priv, V4H_CORE_DIG_COMMON_RW_DESKEW_FINE_MEM_REG, 0x0404);
> > > > +   rcsi2_write16(priv, V4H_CORE_DIG_COMMON_RW_DESKEW_FINE_MEM_REG, 0x040c);
> > > > +   rcsi2_write16(priv, V4H_CORE_DIG_COMMON_RW_DESKEW_FINE_MEM_REG, 0x0414);
> > > > +   rcsi2_write16(priv, V4H_CORE_DIG_COMMON_RW_DESKEW_FINE_MEM_REG, 0x041c);
> 
>  [...]
> 
> > > Instead of manually writing each call, it could use a loop ?
> > >
> > > for (int i = 0x0404; i <= 0x07fc; i += 0x08) {
> > >     rcsi2_write16(priv, V4H_CORE_DIG_COMMON_RW_DESKEW_FINE_MEM_REG, i);
> >
> > Unfortunately the values are not all sequential, see the progression
> > 0x061c -> 0x0623 and 0x071c -> 0x0723 for example.
> >
> > > or if values are not strictly sequential, iterating over the array.
> > > static const u16 register_values[]= {0x0404, 0x040c, 0x0414 etc,,}
> > > rcsi2_write16(priv, V4H_CORE_DIG_COMMON_RW_DESKEW_FINE_MEM_REG,
> > > register_values[i]);
> >
> > I agree with you, a array of values would make this look a tad less
> > silly and would reduce the number of lines. I considered this while
> > writing it but opted for this. My reason was as most of the register
> > writes needed to setup the PHY are not documented in the docs I have and
> > I wanted to keep the driver as close to the table of magic values I have
> > to make it easy to compare driver and the limited documentation.
> >
> > I guess it's really a matter of style. I have no real strong opinion, if
> > people think an array would be nicer I have no issue switching to that.
> 
> Have you looked at the impact on kernel size?

That is a good point, I'm sure an array would reduce the kernel size. I 
could possibly even craft a few clever loops to to generate the values 
as they are almost sequential. As these are magic values I had opted to 
keep it close to the docs, but seems people prefere something else. Will 
change this and send new version.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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

-- 
Kind Regards,
Niklas Söderlund