Re: [RFCv3 0/6] TI camera serdes and I2C address translation (Was: [RFCv3 0/6] Hi,)

From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Luca Ceresoli <luca@lucaceresoli.net>,
	linux-media@vger.kernel.org, linux-i2c@vger.kernel.org
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Wolfram Sang <wsa@the-dreams.de>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Kieran Bingham <kieran.bingham@ideasonboard.com>,
	Jacopo Mondi <jacopo@jmondi.org>,
	Vladimir Zapolskiy <vz@mleia.com>, Peter Rosin <peda@axentia.se>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	matti.vaittinen@fi.rohmeurope.com
Subject: Re: [RFCv3 0/6] TI camera serdes and I2C address translation (Was: [RFCv3 0/6] Hi,)
Date: Mon, 7 Feb 2022 14:06:53 +0200	[thread overview]
Message-ID: <7e5af144-bd5f-cd0e-2109-49b318449a78@ideasonboard.com> (raw)
In-Reply-To: <20220206115939.3091265-1-luca@lucaceresoli.net>

Hi Luca,

On 06/02/2022 13:59, Luca Ceresoli wrote:
> this RFCv3, codename "FOSDEM Fries", of RFC patches to support the TI
> DS90UB9xx serializer/deserializer chipsets with I2C address translation.
> 
> I sent RFCv2 back in 2019 (!). After that I have applied most of the
> improvements proposed during code review, most notably device tree
> representation and proper use of kernel abstractions for clocks and GPIO. I
> have also done many improvements all over the drivers code.

Thanks for sending this! I'll have a closer look at the code in the near 
future.

> However I still don't consider these drivers "ready", hence the RFC status.
> 
> One reason is that, while the I2C ATR idea has been considered good by
> Wolfram, its implementation requires I2C core changes that have been tried
> but never made it to mainline. I think that discussion needs to be reopened
> and work has to be done on that side. Thus for the time being this code
> still has the alias pool: it is an interim solution until I2C core is
> ready.
> 
> Also be aware that the only hardware where I sould test this code runs a
> v4.19 kernel. I cannot guarantee it will work perfectly on mainline.
> 
> And since my hardware has only one camera connected to each deserializer I
> dropped support. However I wrote the code to be able to easily add support
> for 2 and 4 camera inputs as well as 2 CSI-2 outputs (DS90UB960).
 >
> Finally, I dropped all attempts at supporting hotplug. The goals I had 2+
> years ago are not reasonably doable even with current kernels. Luckily all
> the users that I talked with are happy without hotplug. For this reason I
> simplified the serializer management in the DS90UB954 driver by keeping the
> serializer always instantiated.
> 
> Even with the above limitations I felt I'd send this v3 anyway since
> several people have contacted me since v2 asking whether this
> implementation has made progress towards mainline. Some even improved on
> top of my code it their own forks. As I cannot afford to work on this topic
> in the near future, here is the latest and greatest version I can produce,
> with all the improvements I made so far.

I've discussed with Luca in private emails, but I'll add a short status 
about my work in this thread:

About a year ago I took Luca's then-latest-patches and started working 
on them. The aim was to get full multiplexed streams support to v4l2 so 
that we could support CSI-2 bus with multiple virtual channels and 
embedded data, and after that, add support for fpdlink devices.

Since then I have sent multiple versions of the v4l2 work (no drivers 
yet, only the framework changes) to upstream lists. Some pieces have 
already been merged to upstream (e.g. subdev state), but most of it is 
still under work. Here's a link to v10 of the streams series:

https://lore.kernel.org/all/20211130141536.891878-1-tomi.valkeinen@ideasonboard.com/

It has a link to my (now slightly outdated) git branch which contains 
the driver work too.

The fpdlink drivers have diverged from Luca's version quite a bit. The 
most obvious difference is the support for multiplexed streams, of 
course, but there are lots of other changes too. The drivers support 
DS90UB960 (no UB954 at the moment), DS90UB953 and DS90UB913. UB960 
supports all the inputs and outputs. I have also dropped some code which 
I did not need and which I wasn't sure if it's correctly implemented, to 
make it easier to work on the multiplexed streams version. Some of that 
code may need to be added back.

I have not changed the i2c-atr driver, and my fpdlink driver uses it 
more or less the same way as in Luca's version.

Considering that you're not able to work on this, my suggestion is to 
review the i2c-atr patches here (or perhaps send those patches in a 
separate series?), but afaics the fpdlink drivers without multiplexed 
streams is a dead-end, as they can only support a single camera (and no 
embedded data), so I don't see much point in properly reviewing them.

However, I will go through the fpdlink drivers in this series and 
cherry-pick the changes that make sense. I was about to start working on 
proper fpdlink-clock-rate and clkout support, but I see you've already 
done that work =).

But, of course, I'm open to other ideas on how to proceed.

  Tomi