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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 25, 2018 at 06:06:11PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---
> 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: signature.asc
Description: PGP signature

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

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux