From: Quentin Schulz <quentin.schulz@cherry.de>
To: Peter Rosin <peda@axentia.se>,
Farouk Bouabid <farouk.bouabid@cherry.de>,
Wolfram Sang <wsa+renesas@sang-engineering.com>,
Andi Shyti <andi.shyti@kernel.org>, Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Conor Dooley <conor+dt@kernel.org>,
Heiko Stuebner <heiko@sntech.de>,
Quentin Schulz <quentin.schulz@theobroma-systems.com>
Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
Subject: Re: [PATCH v2 1/7] i2c: mux: add the ability to share mux core address with child nodes
Date: Tue, 7 May 2024 11:48:41 +0200 [thread overview]
Message-ID: <34968505-f8b1-4c6f-9d9c-5938edcdba68@cherry.de> (raw)
In-Reply-To: <9b12dc9f-054b-fba5-e23d-48d0fe1a00e2@axentia.se>
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
next prev parent reply other threads:[~2024-05-07 9:48 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-06 11:37 [PATCH v2 0/7] Add Mule I2C multiplexer support Farouk Bouabid
2024-05-06 11:37 ` [PATCH v2 1/7] i2c: mux: add the ability to share mux core address with child nodes Farouk Bouabid
2024-05-06 21:26 ` Peter Rosin
2024-05-07 9:48 ` Quentin Schulz [this message]
2024-05-06 11:37 ` [PATCH v2 2/7] dt-bindings: i2c: mux: mule: add dt-bindings for mule i2c multiplexer Farouk Bouabid
2024-05-06 20:26 ` Rob Herring (Arm)
2024-05-06 11:37 ` [PATCH v2 3/7] i2c: muxes: add support " Farouk Bouabid
2024-05-06 11:37 ` [PATCH v2 4/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3399-puma Farouk Bouabid
2024-05-06 11:37 ` [PATCH v2 5/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-tiger Farouk Bouabid
2024-05-06 11:37 ` [PATCH v2 6/7] arm64: dts: rockchip: add mule i2c mux (0x18) on px30-ringneck Farouk Bouabid
2024-05-06 11:37 ` [PATCH v2 7/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-jaguar Farouk Bouabid
2024-05-06 20:40 ` [PATCH v2 0/7] Add Mule I2C multiplexer support Rob Herring (Arm)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=34968505-f8b1-4c6f-9d9c-5938edcdba68@cherry.de \
--to=quentin.schulz@cherry.de \
--cc=andi.shyti@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=farouk.bouabid@cherry.de \
--cc=heiko@sntech.de \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=peda@axentia.se \
--cc=quentin.schulz@theobroma-systems.com \
--cc=robh@kernel.org \
--cc=wsa+renesas@sang-engineering.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).