Re: [RFC PATCH] i2c: skip of_i2c_register_device() for invalid child nodes

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On Wed, 30 Oct 2024 at 03:07, Abhinav Kumar <quic_abhinavk@quicinc.com> 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@quicinc.com>
> ---
>  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