Re: [PATCH v5 7/8] media: i2c: add DS90UB913 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/01/2023 17:32, Laurent Pinchart wrote:
On Wed, Jan 04, 2023 at 04:13:17PM +0200, Tomi Valkeinen wrote:
On 04/01/2023 15:55, Laurent Pinchart wrote:
Hi Tomi,

On Mon, Dec 26, 2022 at 09:25:34PM +0200, Tomi Valkeinen wrote:
On 26/12/2022 18:56, Laurent Pinchart wrote:
On Wed, Dec 14, 2022 at 08:36:47AM +0200, Tomi Valkeinen wrote:
On 14/12/2022 08:29, Tomi Valkeinen wrote:

wondering if the struct device of the DS90UB913 could be passed instead
of the port, to avoid passing the port throught
ds90ub9xx_platform_data.

Interesting thought. That would limit the number of remote i2c busses to
one, though. Not a problem for FPD-Link, but I wonder if that's assuming
too much for the future users. Then again, this is an in-kernel API so
we could extend it later if needed. So I'll try this out and see if I
hit any issues.

Right, so the issue with this one would be that it would prevent a
single device uses. E.g. a single chip which acts as an ATR (similar to
i2c-mux chips), i.e. it contains both the main and the remote i2c busses.

I don't think I understand this, sorry.

What you are suggesting above means that we'd have a separate device for
each port of the ATR. Which is fine in our current case, as the i2c
master busses are behind separate remote devices.

But if you consider a case similar to i2c-mux, where we have a single
chip with the slave bus and, say, 4 master busses. We would probably
have only a single device for that.

Hmmm... Yes you're right, it won't work in that case. Maybe we could
have two functions, the existing i2c_atr_add_adapter(), and another one
that wraps it ? It would be nice if we could get rid of the platform
data for the UB913 and UB953 drivers.

I wouldn't mind that at all, but we already have the bc_rate there. And
I have a feeling that we might need more if we implement more features.

Indeed. I feel that platform data is a bit of a hack here, but maybe
it's not that bad.

And we also have the atr pointer there. Or do you think that could be
dropped also? In your mail above you only mention the port, but maybe
the deser could register the serializer device and port to the ATR, and
then the ser could just use its device pointer instead of atr & port.

I was wondering if we could drop the atr pointer too, yes. I'm not sure
how, and there's no urgency to fix this. My main concern is that new
drivers should ideally not be forced to use platform data just for ATR
support, if they don't use it already for something else.

Good point. However, we don't know who will use ATR or how ATR is going to be used. Using it the same way i2c-mux is used, there's no problem and platform data is not needed. Using it in this split manner we do with FPDLink does bring up the problem. And using i2c-mux in the split manner would also bring up the same problem. So maybe there's some neat solution out there that would solve the issue for both i2c-atr and i2c-mux, but it escapes me at the moment.

 Tomi




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux