On Thu, Oct 31, 2024 at 11:45:53AM -0700, Abhinav Kumar wrote: > > > On 10/31/2024 11:23 AM, Dmitry Baryshkov wrote: > > 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. > > > > Thanks for the review. > > of_alias_from_compatible() looks for a compatible string but all child nodes > such as ports will not have the compatible. Hence below error will still be > seen: > > i2c i2c-20: of_i2c: modalias failure on > /soc@0/display-subsystem@ae00000/display-controller@ae01000/ports But ports node don't have a reg property too, so it should be skipped based on that. > > > > + > > > client = of_i2c_register_device(adap, node); > > > if (IS_ERR(client)) { > > > dev_err(&adap->dev, > > > -- > > > 2.34.1 > > > > > > > -- With best wishes Dmitry
