lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250530133820.GC18205@pendragon.ideasonboard.com>
Date: Fri, 30 May 2025 16:38:20 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
Cc: Geert Uytterhoeven <geert@...ux-m68k.org>,
	ALOK TIWARI <alok.a.tiwari@...cle.com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Sakari Ailus <sakari.ailus@...ux.intel.com>,
	Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>,
	linux-media@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/4] media: rcar-csi2: Add D-PHY support for V4H

On Mon, May 12, 2025 at 10:48:43AM +0200, Niklas Söderlund wrote:
> On 2025-05-12 09:37:48 +0200, Geert Uytterhoeven wrote:
> > On Sun, 11 May 2025 at 22:03, Niklas Söderlund 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.

I recommend an array of values. That will significantly reduce the size,
while keeping the code easy to compare with the documentation. We can
try to be more clever than that in a separate patch if desired.

-- 
Regards,

Laurent Pinchart

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ