i2c bus extensions were introduced to decouple i2c busses when they are
wired to connectors. Combined with devicetree overlays, they introduce
an additional level of indirection, which is needed to decouple the
overlay (describing the hardware available on addon baord) and the base
tree (describing resources provided to the addon board).
For instance, the following devicetree fragment, available once
overlays are applied, is legit:
i2c1: i2c@abcd0000 {
compatible = "xyz,i2c-ctrl";
i2c-bus-extension@0 {
reg = <0>;
i2c-bus = <&i2c-ctrl>;
};
...
};
connector {
i2c-ctrl {
i2c-parent = <&i2c1>;
#address-cells = <1>;
#size-cells = <0>;
i2c-bus-extension@0 {
reg = <0>;
i2c-bus = <&i2c-other-connector>;
};
device@10 {
compatible = "xyz,foo";
reg = <0x10>;
};
};
devices {
other-connector {
i2c-at-other-connector {
i2c-parent = <&i2c-ctrl>;
#address-cells = <1>;
#size-cells = <0>;
device@20 {
compatible = "xyz,bar";
reg = <0x20>;
};
};
};
};
};
Current implementation of i2c_find_adapter_by_fwnode() supposes that the
fwnode parameter correspond to the adapter.
This assumption is no more valid with i2c bus extensions. Indeed, the
fwnode parameter can reference an i2c bus extension node and not the
related adapter.
In the example, i2c-ctrl and i2c-at-other-connector nodes are chained
bus extensions and those node can be passed to
i2c_find_adapter_by_fwnode() in order to get the related adapter (i.e
the adapter handling the bus and its extensions: i2c@abcd0000).
Extend i2c_find_adapter_by_fwnode() to perform the walking from the
given fwnode through i2c-parent references up to the adapter.
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
drivers/i2c/i2c-core-base.c | 43 ++++++++++++++++++++++++++++++++++++-
1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 5546184df05f..cb7adc5c1285 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1827,14 +1827,55 @@ static int i2c_dev_or_parent_fwnode_match(struct device *dev, const void *data)
*/
struct i2c_adapter *i2c_find_adapter_by_fwnode(struct fwnode_handle *fwnode)
{
+ struct fwnode_reference_args args;
+ struct fwnode_handle *adap_fwnode;
struct i2c_adapter *adapter;
struct device *dev;
+ int err;
if (!fwnode)
return NULL;
- dev = bus_find_device(&i2c_bus_type, NULL, fwnode,
+ adap_fwnode = fwnode_handle_get(fwnode);
+
+ /* Walk extension busses (through i2c-parent) up to the adapter node */
+ while (fwnode_property_present(adap_fwnode, "i2c-parent")) {
+ /*
+ * A specific case exists for the i2c demux pinctrl. The i2c bus
+ * node related this component (the i2c demux pinctrl node
+ * itself) has an i2c-parent property set. This property is used
+ * by the i2c demux pinctrl component for the demuxing purpose
+ * and is not related to the extension bus feature.
+ *
+ * In this current i2c-parent walking, the i2c demux pinctrl
+ * node has to be considered as an adapter node and so, if
+ * the adap_fwnode node is an i2c demux pinctrl node, simply
+ * stop the i2c-parent walking.
+ */
+ if (fwnode_property_match_string(adap_fwnode, "compatible",
+ "i2c-demux-pinctrl") >= 0)
+ break;
+
+ /*
+ * i2c-parent property available in a i2c bus node means that
+ * this node is an extension bus node. In that case,
+ * continue i2c-parent walking up to the adapter node.
+ */
+ err = fwnode_property_get_reference_args(adap_fwnode, "i2c-parent",
+ NULL, 0, 0, &args);
+ if (err)
+ break;
+
+ pr_debug("Find adapter for %pfw, use parent: %pfw\n", fwnode,
+ args.fwnode);
+
+ fwnode_handle_put(adap_fwnode);
+ adap_fwnode = args.fwnode;
+ }
+
+ dev = bus_find_device(&i2c_bus_type, NULL, adap_fwnode,
i2c_dev_or_parent_fwnode_match);
+ fwnode_handle_put(adap_fwnode);
if (!dev)
return NULL;
--
2.47.1
> Extend i2c_find_adapter_by_fwnode() to perform the walking from the
> given fwnode through i2c-parent references up to the adapter.
Even with the review of the schema going on, here are some comments
already.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
> drivers/i2c/i2c-core-base.c | 43 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 5546184df05f..cb7adc5c1285 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1827,14 +1827,55 @@ static int i2c_dev_or_parent_fwnode_match(struct device *dev, const void *data)
> */
> struct i2c_adapter *i2c_find_adapter_by_fwnode(struct fwnode_handle *fwnode)
> {
> + struct fwnode_reference_args args;
> + struct fwnode_handle *adap_fwnode;
> struct i2c_adapter *adapter;
> struct device *dev;
> + int err;
>
> if (!fwnode)
> return NULL;
>
> - dev = bus_find_device(&i2c_bus_type, NULL, fwnode,
> + adap_fwnode = fwnode_handle_get(fwnode);
> +
> + /* Walk extension busses (through i2c-parent) up to the adapter node */
> + while (fwnode_property_present(adap_fwnode, "i2c-parent")) {
> + /*
> + * A specific case exists for the i2c demux pinctrl. The i2c bus
> + * node related this component (the i2c demux pinctrl node
> + * itself) has an i2c-parent property set. This property is used
> + * by the i2c demux pinctrl component for the demuxing purpose
> + * and is not related to the extension bus feature.
> + *
> + * In this current i2c-parent walking, the i2c demux pinctrl
> + * node has to be considered as an adapter node and so, if
> + * the adap_fwnode node is an i2c demux pinctrl node, simply
> + * stop the i2c-parent walking.
> + */
> + if (fwnode_property_match_string(adap_fwnode, "compatible",
> + "i2c-demux-pinctrl") >= 0)
> + break;
I understand the unlikeliness of another demux driver showing up, yet
relying on compatible-values here is too easy to get stale. What about
checking if the i2c-parent property has more than one entry? That should
be only true for demuxers.
> +
> + /*
> + * i2c-parent property available in a i2c bus node means that
> + * this node is an extension bus node. In that case,
> + * continue i2c-parent walking up to the adapter node.
> + */
> + err = fwnode_property_get_reference_args(adap_fwnode, "i2c-parent",
> + NULL, 0, 0, &args);
> + if (err)
> + break;
> +
> + pr_debug("Find adapter for %pfw, use parent: %pfw\n", fwnode,
> + args.fwnode);
Is this useful when creating the overlays? I tend to ask you to remove
it one RFC phase is over. If it is useful, it should be at least
dev_dbg?
> +
> + fwnode_handle_put(adap_fwnode);
> + adap_fwnode = args.fwnode;
> + }
> +
> + dev = bus_find_device(&i2c_bus_type, NULL, adap_fwnode,
> i2c_dev_or_parent_fwnode_match);
> + fwnode_handle_put(adap_fwnode);
> if (!dev)
> return NULL;
>
> --
> 2.47.1
>
Hi Wolfram,
On Thu, 3 Apr 2025 11:03:27 +0200
Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:
> > Extend i2c_find_adapter_by_fwnode() to perform the walking from the
> > given fwnode through i2c-parent references up to the adapter.
>
> Even with the review of the schema going on, here are some comments
> already.
Yes. Of course, depending on this review, things could be changed in the
implementation but every things already discussed here make the topic
moving forward. Thanks for that!
...
> > +
> > + /* Walk extension busses (through i2c-parent) up to the adapter node */
> > + while (fwnode_property_present(adap_fwnode, "i2c-parent")) {
> > + /*
> > + * A specific case exists for the i2c demux pinctrl. The i2c bus
> > + * node related this component (the i2c demux pinctrl node
> > + * itself) has an i2c-parent property set. This property is used
> > + * by the i2c demux pinctrl component for the demuxing purpose
> > + * and is not related to the extension bus feature.
> > + *
> > + * In this current i2c-parent walking, the i2c demux pinctrl
> > + * node has to be considered as an adapter node and so, if
> > + * the adap_fwnode node is an i2c demux pinctrl node, simply
> > + * stop the i2c-parent walking.
> > + */
> > + if (fwnode_property_match_string(adap_fwnode, "compatible",
> > + "i2c-demux-pinctrl") >= 0)
> > + break;
>
> I understand the unlikeliness of another demux driver showing up, yet
> relying on compatible-values here is too easy to get stale. What about
> checking if the i2c-parent property has more than one entry? That should
> be only true for demuxers.
Indeed, this is better.
I will stop the walking based on this number of entries in the i2c-parent
property.
>
> > +
> > + /*
> > + * i2c-parent property available in a i2c bus node means that
> > + * this node is an extension bus node. In that case,
> > + * continue i2c-parent walking up to the adapter node.
> > + */
> > + err = fwnode_property_get_reference_args(adap_fwnode, "i2c-parent",
> > + NULL, 0, 0, &args);
> > + if (err)
> > + break;
> > +
> > + pr_debug("Find adapter for %pfw, use parent: %pfw\n", fwnode,
> > + args.fwnode);
>
> Is this useful when creating the overlays? I tend to ask you to remove
> it one RFC phase is over. If it is useful, it should be at least
> dev_dbg?
Using dev_dbg could lead to issues. Indeed, dev is not given as an argument
of i2c_find_adapter_by_fwnode() and there is no reason to add it (except
for this debug message).
Without a dev given as argument we have to retrieve it from the given
fw_node argument. This given fw_node may have its dev field not already set.
Indeed, the dev instanciation could be done later when the bus this fw_node
is connected to will probe().
For instance
https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/gpu/drm/panel/panel-simple.c#L606
The panel driver can call of_find_i2c_adapter_by_node() which in turn call
i2c_find_adapter_by_fwnode() without having the I2C controller related to the
adapter already present. the panel driver will return a legit EPROBE_DEFER.
So back to our debug message in i2c_find_adapter_by_fwnode(), either I keep
pr_debug() or I fully remove the message but I don't thing I should change
the i2c_find_adapter_by_fwnode() prototype and update all the callers just
for this debug message.
The debug message can be interesting when things went wrong and we want
to investigate potential issue with i2c-parent chain from the last device
up to the adapter.
I don't have a strong opinion about the need of this message and I can
simply remove it.
What is your preference ?
Best regards,
Hervé
> The debug message can be interesting when things went wrong and we want > to investigate potential issue with i2c-parent chain from the last device > up to the adapter. I thought so but couldn't estimate how often this is useful in reality. I agree that introducing 'dev' is too much hazzle, yet I think the message should have some id to disitnguish potential different adapter chains. Either that, or... > I don't have a strong opinion about the need of this message and I can > simply remove it. ... we just remove it and let people add their debug stuff while developing. > What is your preference ? A tad more 'removing'.
Hi Wolfram, On Thu, 3 Apr 2025 13:20:05 +0200 Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > The debug message can be interesting when things went wrong and we want > > to investigate potential issue with i2c-parent chain from the last device > > up to the adapter. > > I thought so but couldn't estimate how often this is useful in reality. > I agree that introducing 'dev' is too much hazzle, yet I think the > message should have some id to disitnguish potential different adapter > chains. Either that, or... > > > I don't have a strong opinion about the need of this message and I can > > simply remove it. > > ... we just remove it and let people add their debug stuff while > developing. I agree. > > > What is your preference ? > > A tad more 'removing'. > Will be removed. Hervé
© 2016 - 2026 Red Hat, Inc.