Re: [PATCH] i2c: of: Try to find an I2C adapter matching the parent

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3647 bytes --]


Any update on this patch? I thought it was interesting.

BTW adding Rob (finally) to CC...

On Mon, Oct 01, 2018 at 01:02:30AM +0200, Wolfram Sang wrote:
> On Tue, Sep 25, 2018 at 06:06:11PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > If an I2C adapter doesn't match the provided device tree node, also try
> > matching the parent's device tree node. This allows finding an adapter
> > based on the device node of the parent device that was used to register
> > it.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Hi Wolfram,
> > 
> > This is a fix for the issue discussed in a long email thread a couple of
> > months ago. In a nutshell the issue is that we want to be able to find a
> > I2C-over-AUX adapter based on the device tree node (this is to hook up a
> > I2C adapter for use as DDC to query EDID from a display panel, for
> > example).
> > 
> > Here's a link to the prior discussion, which ended up getting split into
> > two for some reason, possibly because the initial submission and
> > subsequent discussion took place over an extended period of time:
> > 
> > 	https://patchwork.kernel.org/patch/9516105/
> > 	https://patchwork.kernel.org/patch/10200879/
> 
> Ok, I read that...
> 
> > 
> > I never got around to submitting the patch properly, but here it is. As
> > mentioned in the discussion linked to above, I think the problem here is
> > that the new lookup via the parent device only happens when looking for
> > the adapter from its device tree node. However, this also means that the
> > children for the controller won't be added for these devices, because it
> > only happens for adapters that have the dev->of_node set, not for the
> > node reference by dev->parent->of_node. However, I'm not sure if that's
> > really a problem. A device that doesn't have dev->of_node set would be a
> > "virtual" I2C bus, as in the case of I2C-over-AUX. Proper I2C busses are
> > still going to have to have their dev->of_node set, otherwise any child
> > devices listed in device tree won't be added.
> > 
> > Anyway, let me know what you think of this. Also adding Lucas, Rob and
> > Andrzej to the discussion since they were involved back at the time.
> 
> ... and it looks okay from that discussion point of view. And we could
> stop reusing the parent's of_node for the adapter's of_node. Which is
> better practice. Rob, can you confirm.
> 
> > diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
> > index 6cb7ad608bcd..37d34885ea2d 100644
> > --- a/drivers/i2c/i2c-core-of.c
> > +++ b/drivers/i2c/i2c-core-of.c
> > @@ -121,6 +121,14 @@ static int of_dev_node_match(struct device *dev, void *data)
> >  	return dev->of_node == data;
> >  }
> >  
> > +static int of_parent_node_match(struct device *dev, void *data)
> > +{
> > +	if (dev->parent)
> > +		return dev->parent->of_node == data;
> > +
> > +	return 0;
> > +}
> > +
> >  /* must call put_device() when done with returned i2c_client device */
> >  struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
> >  {
> > @@ -146,6 +154,9 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
> >  	struct i2c_adapter *adapter;
> >  
> >  	dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match);
> > +	if (!dev)
> > +		dev = bus_find_device(&i2c_bus_type, NULL, node, of_parent_node_match);
> > +
> 
> To avoid the double loop iteration, maybe introduce
> 'of_dev_or_parent_node_match' and merge those two functionalities?
> 
> Regards,
> 
>    Wolfram



[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel