Message-ID: <fba09c57-cf13-4f50-8fb2-874ae6f6d310@gmail.com>
Date: Tue, 3 Dec 2024 12:35:29 +0200
From: Cosmin Tanislav <demonsingur@...il.com>
To: Luca Ceresoli <luca.ceresoli@...tlin.com>,
 Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: Wolfram Sang <wsa+renesas@...g-engineering.com>,
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>, linux-i2c@...r.kernel.org,
 linux-kernel@...r.kernel.org, Wolfram Sang <wsa@...nel.org>,
 Mauro Carvalho Chehab <mchehab@...nel.org>,
 Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>,
 Romain Gantois <romain.gantois@...tlin.com>,
 Matti Vaittinen <Matti.Vaittinen@...rohmeurope.com>
Subject: Re: [PATCH v2 2/3] i2c: atr: Allow unmapped addresses from nested
 ATRs



On 12/3/24 11:39 AM, Luca Ceresoli wrote:
> Hello Tomi,
> 
> On Fri, 29 Nov 2024 15:31:45 +0200
> Tomi Valkeinen <tomi.valkeinen@...asonboard.com> wrote:
> 
>> On 29/11/2024 13:53, Luca Ceresoli wrote:
>>
>>>> So strictly speaking it's not an ATR, but this achieves the same.
>>>
>>> Thanks for the extensive and very useful explanation. I had completely
>>> missed the GMSL serder and their different I2C handling, apologies.
>>>
>>> So, the "parent ATR" is the GMSL deser, which is not an ATR but
>>> implementing it using i2c-atr makes the implementation cleaner. That
>>> makes sense.
>>
>> Right.
>>
>> But, honestly, I can't make my mind if I like the use of ATR here or not =).
> 
> Hehe, indeed, hardware designers use a lot of fantasy in stretching the
> I2C standard to its limits, perhaps more than actually needed.
> 
>> So it's not an ATR, but I'm not quite sure what it is. It's not just
>> that we need to change the addresses of the serializers, we need to do
>> that in particular way, enabling one port at a time to do the change.
>>
>> If we forget about the init time hurdles, and consider the situation
>> after the serializers are been set up and all ports have been enabled,
>> we have:
>>
>> There's the main i2c bus, on which we have the deserializer. The
>> deserializer acts as a i2c repeater (for any transaction that's not
>> directed to the deser), sending the messages to all serializers. The
>> serializers catch transactions directed at the ser, and everything else
>> goes through ATR and to the remote bus.
>>
>> Do we have something that represents such a "i2c repeater"? I guess we
>> could just have an i2c bus, created by the deser, and all the sers would
>> be on that bus. So we'd somehow do the initial address change first,
>> then set up the i2c bus, and the serializer i2c clients would be added
>> to that bus.
> 
> So you think about another thing, like i2c-repeater, in addition to
> i2c-mux and i2c-atr?
> 

Since most of the functionality needed (besides allowing pass-through
transfers for unmapped I2C addresses) can be achieved already using I2C
ATR, I think we should make use of it.

> Well, I think it would make sense, as it would generalize a feature
> that might be used by other chips. However at the moment we do have a
> working driver for the GMSL deser, and so I don't see the benefit of
> extracting the i2c-repeater functionality to a separate file, unless
> there are drivers for other chips being implemented: this would motivate
> extracting common features to a shared file. IOW, I'd not generalize
> something with a single user.
> 

We have GMSL drivers for 6 new chips that make use of the I2C ATR, and
we want to send these upstream. Adding pass-through support for the I2C
ATR is one of the only things keeping us back, and it's the solution
that makes the most sense to me.

Semantically, indeed, our GMSL deserializers don't have an ATR hardware
block. The serializers do, with the particularity that they pass through
all traffic, even if it is for unmapped addresses.

The current GMSL2 deserializer driver doesn't make use of ATR
functionality because it's a single link deserializer, so it doesn't
need to handle multiple serializers with the same I2C address.

The GMSL deserializers we want to add have more than one link (either
2 or 4), and they need the ATR to reassign the serializer I2C addresses
to ones that can be addresses without conflicts. The address changing
is done in the ATR attach_client() callback.

The ATR driver already exists and allows us to implement this, even if
semantically there's no translation block.

> [Interesting side note: the i2c-atr has been implemented with a single
> user, violating the above principle O:-) but I think that was due to the
> similarity with i2c-mux or something like that. Out of luck, another
> ATR user appeared after some time.]
> 
> Luca
> 

Powered by blists - more mailing lists