| Message ID | 20250205173918.600037-4-herve.codina@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: Introduce i2c bus extensions | expand |
On 05/02/2025 18:39, Herve Codina wrote: > > dev_dbg(&adap->dev, "of_i2c: walking child nodes from %pOF\n", bus); > > - /* Register device directly attached to this bus */ > + /* > + * Register device directly described in this bus node before looking > + * at extensions. > + */ > for_each_available_child_of_node(bus, node) { > + /* Filter out extension node */ > + if (of_node_name_eq(node, "i2c-bus-extension")) Where is the ABI documented? > + continue; > + > if (of_node_test_and_set_flag(node, OF_POPULATED)) > continue; > > @@ -103,6 +110,23 @@ static void of_i2c_register_children(struct i2c_adapter *adap, > of_node_clear_flag(node, OF_POPULATED); > } > } > + > + /* Look at extensions */ > + for_each_available_child_of_node(bus, node) { > + if (!of_node_name_eq(node, "i2c-bus-extension")) > + continue; > + > + extension = of_parse_phandle(node, "i2c-bus", 0); And this? > + if (!extension) > + continue; > + Best regards, Krzysztof
Hi Krzysztof, On Wed, 12 Feb 2025 06:54:19 +0100 Krzysztof Kozlowski <krzk@kernel.org> wrote: > On 05/02/2025 18:39, Herve Codina wrote: > > > > dev_dbg(&adap->dev, "of_i2c: walking child nodes from %pOF\n", bus); > > > > - /* Register device directly attached to this bus */ > > + /* > > + * Register device directly described in this bus node before looking > > + * at extensions. > > + */ > > for_each_available_child_of_node(bus, node) { > > + /* Filter out extension node */ > > + if (of_node_name_eq(node, "i2c-bus-extension")) > > Where is the ABI documented? > > > + continue; > > + > > if (of_node_test_and_set_flag(node, OF_POPULATED)) > > continue; > > > > @@ -103,6 +110,23 @@ static void of_i2c_register_children(struct i2c_adapter *adap, > > of_node_clear_flag(node, OF_POPULATED); > > } > > } > > + > > + /* Look at extensions */ > > + for_each_available_child_of_node(bus, node) { > > + if (!of_node_name_eq(node, "i2c-bus-extension")) > > + continue; > > + > > + extension = of_parse_phandle(node, "i2c-bus", 0); > > And this? > > > + if (!extension) > > + continue; > > + I know the binding is not present in this RFC series. As I mentioned in my cover letter, the binding that needs to be updated is available in dt-schema repo [0]. When the binding is be accepted in dt-schema repo, I will not be able to change it and because two repos are involved, I cannot send the binding and the implementation in the same series. Before sending a patch to update the binding in dt-schema repo, I would like first to discuss the proposed i2c bus extension idea in terms of: 1) DT properties naming and purpose 2) implementation [0] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/i2c/i2c-controller.yaml Best regards, Hervé
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c index b4c9db137f5a..406565beabbe 100644 --- a/drivers/i2c/i2c-core-of.c +++ b/drivers/i2c/i2c-core-of.c @@ -85,13 +85,20 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap, static void of_i2c_register_children(struct i2c_adapter *adap, struct device_node *bus) { + struct device_node *node, *extension; struct i2c_client *client; - struct device_node *node; dev_dbg(&adap->dev, "of_i2c: walking child nodes from %pOF\n", bus); - /* Register device directly attached to this bus */ + /* + * Register device directly described in this bus node before looking + * at extensions. + */ for_each_available_child_of_node(bus, node) { + /* Filter out extension node */ + if (of_node_name_eq(node, "i2c-bus-extension")) + continue; + if (of_node_test_and_set_flag(node, OF_POPULATED)) continue; @@ -103,6 +110,23 @@ static void of_i2c_register_children(struct i2c_adapter *adap, of_node_clear_flag(node, OF_POPULATED); } } + + /* Look at extensions */ + for_each_available_child_of_node(bus, node) { + if (!of_node_name_eq(node, "i2c-bus-extension")) + continue; + + extension = of_parse_phandle(node, "i2c-bus", 0); + if (!extension) + continue; + + /* + * Register children available at this extension possibly + * walking other chained extensions. + */ + of_i2c_register_children(adap, extension); + of_node_put(extension); + } } void of_i2c_register_devices(struct i2c_adapter *adap)
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 processing done when a i2c adapter is registered registers i2c clients described at the adapter node level. With i2c bus extensions, the process needs to look also at extensions to register devices described in those extensions and so connected to the adapter. Extend of_i2c_register_children() to look recursively at those i2c bus extensions. Signed-off-by: Herve Codina <herve.codina@bootlin.com> --- drivers/i2c/i2c-core-of.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)