Received-SPF: pass (google.com: domain of linux-kernel+bounces-134265-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Content-Type: text/plain; charset="utf-8"
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <803b0bd3-e615-41c1-888e-69b6ecca0b8a@luigi311.com>
References: <20240403150355.189229-1-git@luigi311.com> <20240403150355.189229-10-git@luigi311.com> <Zg2jvUDFnY83drlg@duo.ucw.cz> <803b0bd3-e615-41c1-888e-69b6ecca0b8a@luigi311.com>
Subject: Re: [PATCH v3 09/25] media: i2c: imx258: Add support for running on 2 CSI data lanes
From: Kieran Bingham <kieran.bingham@ideasonboard.com>
Cc: linux-media@vger.kernel.org, dave.stevenson@raspberrypi.com, jacopo.mondi@ideasonboard.com, mchehab@kernel.org, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com, sakari.ailus@linux.intel.com, devicetree@vger.kernel.org, imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, phone-devel@vger.kernel.org
To: Luis Garcia <git@luigi311.com>, Pavel Machek <pavel@ucw.cz>
Date: Sun, 07 Apr 2024 09:51:54 +0100
Message-ID: <171247991454.2374960.14379287208228554950@ping.linuxembedded.co.uk>
User-Agent: alot/0.10

Quoting Luis Garcia (2024-04-06 06:25:41)
> On 4/3/24 12:45, Pavel Machek wrote:
> > Hi!
> >=20
> >> +/*
> >> + * 4208x3120 @ 30 fps needs 1267Mbps/lane, 4 lanes.
> >> + * To avoid further computation of clock settings, adopt the same per
> >> + * lane data rate when using 2 lanes, thus allowing a maximum of 15fp=
s.
> >> + */
> >> +static const struct imx258_reg mipi_1267mbps_19_2mhz_2l[] =3D {
> >> +    { 0x0136, 0x13 },
> >> +    { 0x0137, 0x33 },
> >> +    { 0x0301, 0x0A },
> >> +    { 0x0303, 0x02 },
> >> +    { 0x0305, 0x03 },
> >> +    { 0x0306, 0x00 },
> >> +    { 0x0307, 0xC6 },
> >> +    { 0x0309, 0x0A },
> >> +    { 0x030B, 0x01 },
> >> +    { 0x030D, 0x02 },
> >> +    { 0x030E, 0x00 },
> >> +    { 0x030F, 0xD8 },
> >> +    { 0x0310, 0x00 },
> >> +
> >> +    { 0x0114, 0x01 },
> >> +    { 0x0820, 0x09 },
> >> +    { 0x0821, 0xa6 },
> >> +    { 0x0822, 0x66 },
> >> +    { 0x0823, 0x66 },
> >> +};
> >> +
> >> +static const struct imx258_reg mipi_1267mbps_19_2mhz_4l[] =3D {
> >>      { 0x0136, 0x13 },
> >>      { 0x0137, 0x33 },
> >>      { 0x0301, 0x05 },
> >=20
> > I wish we did not have to copy all the magic values like this.
> >=20
> > Best regards,
> >                                                               Pavel
> >                                                              =20
>=20
> no kidding, magic values everywhere.... it makes it annoying
> for me to move things around because they all start to look
> similar. Down the line we added in more defined names so its
> not as bad but still its bad lol.

This series converts the defines to names, which is great. It would have
been nicer if the series converted first, but I know the history here
means you have done the register naming on top of existing patches - so
I don't think there's a requirement to change the ordering now.

But I see new drivers coming in with register tables. I hope we can
start to apply more pressure to driver submitters to use higher quality
named register sets in the future, now that we have a greater precendent
of sensor drivers 'doing the right thing'.

Sets of tables like we have are basically a binary blob stored as ascii
and make maintainance far more difficult IMO.

Maybe I should hit send on my comments on the latest GalaxyCore driver
coming in that I hesitated on ...
--
Kieran