On Wed, 30 Oct 2024 at 03:07, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > > of_i2c_register_devices() adds all child nodes of a given i2c bus > however in certain device trees of_alias_from_compatible() and > of_property_read_u32() can fail as the child nodes of the device > might not be valid i2c client devices. One such example is the > i2c aux device for the DRM MST toplogy manager which uses the > display controller device node to add the i2c adaptor [1] leading > to an error spam like below > > i2c i2c-20: of_i2c: register /soc@0/display-subsystem@ae00000/display-controller@ae01000/ports > i2c i2c-20: of_i2c: modalias failure on /soc@0/display-subsystem@ae00000/display-controller@ae01000/ports > i2c i2c-20: Failed to create I2C device for /soc@0/display-subsystem@ae00000/display-controller@ae01000/ports > i2c i2c-20: of_i2c: register /soc@0/display-subsystem@ae00000/display-controller@ae01000/opp-table > i2c i2c-20: of_i2c: invalid reg on /soc@0/display-subsystem@ae00000/display-controller@ae01000/opp-table > i2c i2c-20: Failed to create I2C device for /soc@0/display-subsystem@ae00000/display-controller@ae01000/opp-table > > Add protection against invalid child nodes before trying to register > i2c devices for all child nodes. > > [1] : https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/display/drm_dp_mst_topology.c#L5985 > > Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> > --- > drivers/i2c/i2c-core-of.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c > index a6c407d36800..62a2603c3092 100644 > --- a/drivers/i2c/i2c-core-of.c > +++ b/drivers/i2c/i2c-core-of.c > @@ -86,6 +86,8 @@ void of_i2c_register_devices(struct i2c_adapter *adap) > { > struct device_node *bus, *node; > struct i2c_client *client; > + u32 addr; > + char temp[16]; > > /* Only register child devices if the adapter has a node pointer set */ > if (!adap->dev.of_node) > @@ -101,6 +103,10 @@ void of_i2c_register_devices(struct i2c_adapter *adap) > if (of_node_test_and_set_flag(node, OF_POPULATED)) > continue; > > + if (of_property_read_u32(node, "reg", &addr) || > + of_alias_from_compatible(node, temp, sizeof(temp))) > + continue; I think just of_property_read_u32() should be enough to skip non-I2C-device children. If of_alias_from_compatible() fails, it is a legit error. > + > client = of_i2c_register_device(adap, node); > if (IS_ERR(client)) { > dev_err(&adap->dev, > -- > 2.34.1 > -- With best wishes Dmitry
