linux-kernel - Re: [RFC PATCH v2 12/16] media: i2c: add Maxim GMSL2/3 serializer and deserializer drivers

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPY8ntCtycm+fha9yuJyr2_9obq8cq6xjYJT7acnFPgh_sCi8Q@mail.gmail.com>
Date: Wed, 7 May 2025 12:22:50 +0100
From: Dave Stevenson <dave.stevenson@...pberrypi.com>
To: Jakub Kostiw <jakub.kostiw@...etronic.com>
Cc: Cosmin Tanislav <demonsingur@...il.com>, Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, 
	Cosmin Tanislav <cosmin.tanislav@...log.com>, Mauro Carvalho Chehab <mchehab@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Niklas Söderlund <niklas.soderlund@...natech.se>, 
	Julien Massot <julien.massot@...labora.com>, Catalin Marinas <catalin.marinas@....com>, 
	Will Deacon <will@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, 
	Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>, 
	Bjorn Andersson <quic_bjorande@...cinc.com>, Geert Uytterhoeven <geert+renesas@...der.be>, 
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Arnd Bergmann <arnd@...db.de>, 
	Taniya Das <quic_tdas@...cinc.com>, Biju Das <biju.das.jz@...renesas.com>, 
	Nícolas F . R . A . Prado <nfraprado@...labora.com>, 
	Eric Biggers <ebiggers@...gle.com>, Javier Carrasco <javier.carrasco@...fvision.net>, 
	Ross Burton <ross.burton@....com>, Hans Verkuil <hverkuil@...all.nl>, 
	Sakari Ailus <sakari.ailus@...ux.intel.com>, 
	Laurent Pinchart <laurent.pinchart@...asonboard.com>, Zhi Mao <zhi.mao@...iatek.com>, 
	Kieran Bingham <kieran.bingham@...asonboard.com>, Dongcheng Yan <dongcheng.yan@...el.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, 
	Benjamin Mugnier <benjamin.mugnier@...s.st.com>, Tommaso Merciai <tomm.merciai@...il.com>, 
	Dan Carpenter <dan.carpenter@...aro.org>, Ihor Matushchak <ihor.matushchak@...box.net>, 
	Laurentiu Palcu <laurentiu.palcu@....nxp.com>, linux-media@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-staging@...ts.linux.dev, 
	linux-gpio@...r.kernel.org
Subject: Re: [RFC PATCH v2 12/16] media: i2c: add Maxim GMSL2/3 serializer and
 deserializer drivers

Hi Jakub and Cosmin

On Tue, 6 May 2025 at 19:34, Jakub Kostiw <jakub.kostiw@...etronic.com> wrote:
>
> Hi Cosmin
>
> Awesome work. The initiative to establish a common framework for GMSL
> devices is a great idea.
>
> I believe that we have found few bugs:
>
> > +#define MAX9296A_BACKTOP22(x)                        (0x31d * (x) * 0x3)
>
> The first multiplication is wrong and should be replaced with addition:
>
> +#define MAX9296A_BACKTOP22(x)                  (0x31d + (x) * 0x3)
>
> The same goes for MAX96724 equivalent macro:
>
> > +#define MAX96724_BACKTOP22(x)                        (0x415 * (x) * 0x3)
>
> In MAX96714 driver there is an issue with setting up lane-polarities.
>
> > +static const struct max9296a_chip_info max96714_info = {
> > +     .max_register = 0x5011,
> > +     .set_pipe_stream_id = max96714_set_pipe_stream_id,
> > +     .set_pipe_enable = max96714_set_pipe_enable,
> > +     .set_pipe_tunnel_enable = max96714_set_pipe_tunnel_enable,
> > +     .phys_configs = {
> > +             .num_configs = ARRAY_SIZE(max96714_phys_configs),
> > +             .configs = max96714_phys_configs,
> > +     },
> > +     .polarity_on_physical_lanes = true,
> > +     .supports_phy_log = true,
> > +     .adjust_rlms = true,
> > +     .num_pipes = 1,
> > +     .pipe_hw_ids = { 1 },
> > +     .num_phys = 1,
> > +     .phy_hw_ids = { 1 },
> > +     .num_links = 1,
> > +};
>
> In order to make thing work we had to set
>
> > +     .polarity_on_physical_lanes = true,
>
> To false. So this field is either improperly set for MAX96714, or handling of this case is wrong:
>
> > +             if (priv->info->polarity_on_physical_lanes)
> > +                     map = phy->mipi.data_lanes[i];
> > +             else
> > +                     map = i;
>
> Upon mentioned changes we have successfully tested two GMSL2
> combinations on Raspberry 5 platform:
>
> 1. MAX96724 + MAX96717 (only 2 MIPI-CSI2 lanes with single camera due to
> hardware limitations)
>
> 2. MAX96714 + MAX96717

Feel free to shout if you want help on the Pi side.

Pi5 should be able to extract multiple virtual channels to support
several sensors simultaneously (up to 4 VC/DT combinations). It does
need a rework so the CFE runs from memory rather than being fed data
directly from the CSI-2 receiver, but I believe that is pencilled in
as future work with libcamera already.

Unless things have regressed, libcamera should report all connected
sensors to SerDes setups, and set up Media Controller appropriately to
use them one at a time. I know I've had that working with simple CSI-2
multiplexers and thought I'd had it working with TI FPD-Link III
SerDes (Arducam's IMX219 V3Link kit, modded to remove their MCU). I
don't have any GMSL hardware to test with.

We're also fairly open to merging drivers and overlays for 3rd party
hardware into the downstream Pi kernel. If they've been reviewed and
merged upstream then that is the ideal, but if you're prepared to
support them, then it saves the user the headache of having to build
out-of-tree modules.

Cheers
  Dave