Re: [PATCH v2 2/3] i2c: atr: Allow unmapped addresses from nested ATRs

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

 





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@xxxxxxxxxxxxxxxx> 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






[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