[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <mlpiuko7n6rp3x55z4qterdns2wzqnfwgjxikbshrvakrscsak@antl2vzla5bd>
Date: Thu, 31 Oct 2024 21:30:07 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Abhinav Kumar <quic_abhinavk@...cinc.com>
Cc: Wolfram Sang <wsa+renesas@...g-engineering.com>,
freedreno@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org, robdclark@...il.com,
swboyd@...omium.org, airlied@...il.com, quic_jesszhan@...cinc.com, lyude@...hat.com,
simona@...ll.ch, linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] i2c: skip of_i2c_register_device() for invalid child
nodes
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@...cinc.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@...isplay-subsystem@...0000/display-controller@...1000/ports
> > > i2c i2c-20: of_i2c: modalias failure on /soc@...isplay-subsystem@...0000/display-controller@...1000/ports
> > > i2c i2c-20: Failed to create I2C device for /soc@...isplay-subsystem@...0000/display-controller@...1000/ports
> > > i2c i2c-20: of_i2c: register /soc@...isplay-subsystem@...0000/display-controller@...1000/opp-table
> > > i2c i2c-20: of_i2c: invalid reg on /soc@...isplay-subsystem@...0000/display-controller@...1000/opp-table
> > > i2c i2c-20: Failed to create I2C device for /soc@...isplay-subsystem@...0000/display-controller@...1000/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@...cinc.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.
> >
>
> 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@...isplay-subsystem@...0000/display-controller@...1000/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
Powered by blists - more mailing lists