[PATCH v2 1/7] i2c: mux: add the ability to share mux core address with child nodes

[Quentin Schulz]

Hi Peter,

On 5/6/24 11:26 PM, Peter Rosin wrote:
> Hi!
> 
> Regarding the subject (and elsewhere) I think of "mux core" as roughly
> the code in the i2c-mux.c file. So, for me, the "mux core" does not have
> an address, it is a mux "driver instance" or "device" that sits on the
> I2C address that you need to share.
> 

I'm the one who suggested mux core here (privately) :)

The issue is that in my head a mux device is the i2c adapter/bus (from 
i2c-mux.yaml dt binding example):

"""
     i2c {
         #address-cells = <1>;
         #size-cells = <0>;

         i2c-mux at 70 {
             compatible = "nxp,pca9548";
             reg = <0x70>;
             #address-cells = <1>;
             #size-cells = <0>;

             i2c at 3 {
                 #address-cells = <1>;
                 #size-cells = <0>;
                 reg = <3>;

                 gpio at 20 {
                     compatible = "nxp,pca9555";
                     gpio-controller;
                     #gpio-cells = <2>;
                     reg = <0x20>;
                 };
             };
             i2c at 4 {
                 #address-cells = <1>;
                 #size-cells = <0>;
                 reg = <4>;

                 gpio at 20 {
                     compatible = "nxp,pca9555";
                     gpio-controller;
                     #gpio-cells = <2>;
                     reg = <0x20>;
                 };
             };
         };
     };
"""

"mux core" here would refer to i2c-mux at 70, "mux device"/"mux" i2c at 3 or 
i2c at 4. E.g. when I'm saying "in mux 3", I'm talking about i2c at 3 here.

For me a driver instance is a device, so "mux driver instance" would be 
a "mux device". Ah... naming is hard. Anyway, up to you, I just wanted 
to make sure we're talking about the same thing and there's no confusion 
here.

[...]
> I also wonder if that second condition (...->type == &i2c_client_type) should
> be a WARN_ON_ONCE? I don't see how the flag can be set sanely on an adapter
> that is not itself an I2C client. Can it?
> 

Agreed, good suggestion here... Though... 
https://lwn.net/Articles/969923/ it seems new additions of WARN_ON are 
now discouraged? Not looking to start a discussion here about whether 
WARN_ON is good or bad, merely pointing at this if it was missed somehow.

>> +
>> +		if (!quirks)
>> +			return -ENOMEM;
>> +
>> +		if (parent->quirks)
>> +			memcpy(quirks, parent->quirks, sizeof(*quirks));
>> +
>> +		quirks->flags |= I2C_AQ_SKIP_ADDR_CHECK;
>> +		quirks->skip_addr_in_parent = client->addr;
>> +		priv->adap.quirks = quirks;
> 
> The I2C_AQ_SKIP_ADDR_CHECK flag should probably not be propagated?
> 

Oh... you mean if we have a mux on an i2c adapter of a mux and the 
adapters handled by the parent mux have SKIP_ADDR set and we don't want 
the adapters handled by the leaf mux to have this flag as well? Is that 
what you meant?

Cheers,
Quentin