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

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

 



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@70 {
            compatible = "nxp,pca9548";
            reg = <0x70>;
            #address-cells = <1>;
            #size-cells = <0>;

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

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

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

"mux core" here would refer to i2c-mux@70, "mux device"/"mux" i2c@3 or i2c@4. E.g. when I'm saying "in mux 3", I'm talking about i2c@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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux