Re: [PATCH v7 5/7] media: i2c: add DS90UB960 driver

On 27/01/2023 11:15, Andy Shevchenko wrote:

On Fri, Jan 27, 2023 at 10:24:04AM +0200, Tomi Valkeinen wrote:

quoted

On 26/01/2023 12:51, Laurent Pinchart wrote:

quoted

On Thu, Jan 26, 2023 at 12:21:06PM +0200, Andy Shevchenko wrote:

quoted

On Thu, Jan 26, 2023 at 10:41:47AM +0200, Tomi Valkeinen wrote:

quoted

On 25/01/2023 17:27, Andy Shevchenko wrote:

...

quoted

quoted

quoted

quoted

quoted

But I probably don't understand the ATR structure and what exactly we need to
pass to it, perhaps it also can be replaced with properties (note, that we have
some interesting ones that called references, which is an alternative to DT
phandle).

Well, maybe this needs a Linux bus implementation. I'm not that familiar
with implementing a bus, but I think that would make it easier to share data
between the deserializer and the serializer. A bus sounds a bit like an
overkill for a 1-to-1 connection, used by a few drivers, but maybe it
wouldn't be too much code.

Have you looked at auxiliary bus (appeared a few releases ago in kernel)?

As far as I understand, the auxiliary bus infrastructure is meant for
use cases where a single hardware device needs to be split into multiple
logical devices (as in struct device). Platform devices were
historically (ab)used for this, and the auxiliary bus is meant as a
cleaner solution. I'm not sure if it would be a good match here, or if
it would be considered an abuse of the auxiliary bus API.

The aux bus docs say "A key requirement for utilizing the auxiliary bus is
that there is no dependency on a physical bus, device, register accesses or
regmap support. These individual devices split from the core cannot live on
the platform bus as they are not physical devices that are controlled by
DT/ACPI.", which doesn't sound like a good fit.

Thanks for checking!

quoted

The deserializer and serializers are currently independent devices and
drivers (the pdata is the only shared thing), but I think we may need
something better here. The devices are more tightly tied together than
"normal" video devices, in my opinion, as the serializer is fully controlled
by the deserializer (including power).

And if we ever want to implement something like power management, we
probably need something more than what we have now. Although I don't know
how that would be done, as all the peripherals behind the serializer would
also lose power...

I believe you have to create a power domain for them and when such device
is added, the power domain of it should belong to the serialized.

I was testing this, and got something working.

I have the deserializer introducing a separate power-domain for each RX 
port, and the serializer and the sensor both refer to their port's 
domain. I can see that the deserializer gets power on/off callbacks 
correctly when either serializer or sensor resumes.

The problem I have now is that while the power comes from the 
deserializer and is thus covered with the power domain, the sensor uses 
services from the serializer (gpios, clocks, i2c bus), and the 
serializer is not woken up when the sensor does runtime-pm resume (the 
power domain is powered up correctly when the sensor resumes).

The serializer creates the i2c adapter to which the sensor is added, so, 
afaics, there should be a child-parent relationship there. But maybe I 
have something wrong there, or it just doesn't work as I imagine it 
would work.

  Tomi