| Message ID | 20220325113148.588163-7-clement.leger@bootlin.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | introduce fwnode in the I2C subsystem | expand |
On Fri, Mar 25, 2022 at 12:31:45PM +0100, Clément Léger wrote: > Add fwnode_find_i2c_adapter_by_node() which allows to retrieve a i2c > adapter using a fwnode. Since dev_fwnode() uses the fwnode provided by > the of_node member of the device, this will also work for devices were > the of_node has been set and not the fwnode field. > For acpi nodes, the check for parent node is skipped since > i2c_acpi_find_adapter_by_handle() does not check it and we don't want > to change this behavior. ... > +#include <linux/device.h> > +#include <linux/i2c.h> Missed headers so far: acpi.h ... > +static int fwnode_dev_or_parent_node_match(struct device *dev, const void *data) > +{ > + if (device_match_fwnode(dev, data)) > + return 1; > + > + /* > + * For ACPI device node, the behavior is to not match the parent (see > + * i2c_acpi_find_adapter_by_handle()) > + */ Would it be harmful to drop this check? > + if (!is_acpi_device_node(dev_fwnode(dev)) && dev->parent) > + return device_match_fwnode(dev->parent, data); > + > + return 0; > +}
Le Fri, 25 Mar 2022 16:35:52 +0200, Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : > On Fri, Mar 25, 2022 at 12:31:45PM +0100, Clément Léger wrote: > > Add fwnode_find_i2c_adapter_by_node() which allows to retrieve a i2c > > adapter using a fwnode. Since dev_fwnode() uses the fwnode provided by > > the of_node member of the device, this will also work for devices were > > the of_node has been set and not the fwnode field. > > For acpi nodes, the check for parent node is skipped since > > i2c_acpi_find_adapter_by_handle() does not check it and we don't want > > to change this behavior. > > ... > > > +#include <linux/device.h> > > +#include <linux/i2c.h> > > Missed headers so far: > acpi.h Indeed, will check that. > > ... > > > +static int fwnode_dev_or_parent_node_match(struct device *dev, const void *data) > > +{ > > + if (device_match_fwnode(dev, data)) > > + return 1; > > + > > + /* > > + * For ACPI device node, the behavior is to not match the parent (see > > + * did not checked the ) > > + */ > > Would it be harmful to drop this check? Can't tell, I would not want to introduce some behavior wrt to parent node for ACPI since it was not done this way. Might works in 99% of the case though. If ok with that, I can drop it.
On Fri, Mar 25, 2022 at 04:09:27PM +0100, Clément Léger wrote: > Le Fri, 25 Mar 2022 16:35:52 +0200, > Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : > > On Fri, Mar 25, 2022 at 12:31:45PM +0100, Clément Léger wrote: ... > > > + * For ACPI device node, the behavior is to not match the parent (see > > > + * did not checked the ) > > > > Would it be harmful to drop this check? > > Can't tell, I would not want to introduce some behavior wrt to parent > node for ACPI since it was not done this way. Might works in 99% of the > case though. > > If ok with that, I can drop it. Let's ask Mika and Jarkko if they know more on this. I think Mika was the one who introduced that (sorry, if I'm mistaken, haven't looked at the history carefully). P.S. Interesting enough that Mika is listed as I2C ACPI maintainer and his email is not in the Cc. Please, check how you form Cc list for this series and include all parties next time.
Le Fri, 25 Mar 2022 17:56:43 +0200, Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : > > > > Can't tell, I would not want to introduce some behavior wrt to parent > > node for ACPI since it was not done this way. Might works in 99% of the > > case though. > > > > If ok with that, I can drop it. > > Let's ask Mika and Jarkko if they know more on this. I think Mika was the > one who introduced that (sorry, if I'm mistaken, haven't looked at the history > carefully). > > P.S. Interesting enough that Mika is listed as I2C ACPI maintainer and his > email is not in the Cc. Please, check how you form Cc list for this series > and include all parties next time. I'm using get_maintainers.pl which does not return Mika nor Jarkko. I was using --nogit which probably ruled out some contributors but not them apparently.
On Fri, Mar 25, 2022 at 05:04:39PM +0100, Clément Léger wrote: > Le Fri, 25 Mar 2022 17:56:43 +0200, > Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : ... > > P.S. Interesting enough that Mika is listed as I2C ACPI maintainer and his > > email is not in the Cc. Please, check how you form Cc list for this series > > and include all parties next time. > > I'm using get_maintainers.pl which does not return Mika nor Jarkko. I > was using --nogit which probably ruled out some contributors but not > them apparently. Yeah, in this case it seems some heuristics has to be used...
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile index c1d493dc9bac..c9c97675163e 100644 --- a/drivers/i2c/Makefile +++ b/drivers/i2c/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_I2C_BOARDINFO) += i2c-boardinfo.o obj-$(CONFIG_I2C) += i2c-core.o i2c-core-objs := i2c-core-base.o i2c-core-smbus.o +i2c-core-objs += i2c-core-fwnode.o i2c-core-$(CONFIG_ACPI) += i2c-core-acpi.o i2c-core-$(CONFIG_I2C_SLAVE) += i2c-core-slave.o i2c-core-$(CONFIG_OF) += i2c-core-of.o diff --git a/drivers/i2c/i2c-core-fwnode.c b/drivers/i2c/i2c-core-fwnode.c new file mode 100644 index 000000000000..fb1c820b686e --- /dev/null +++ b/drivers/i2c/i2c-core-fwnode.c @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Linux I2C core fwnode support code + * + * Copyright (C) 2022 Microchip + */ + +#include <linux/device.h> +#include <linux/i2c.h> + +#include "i2c-core.h" + +static int fwnode_dev_or_parent_node_match(struct device *dev, const void *data) +{ + if (device_match_fwnode(dev, data)) + return 1; + + /* + * For ACPI device node, the behavior is to not match the parent (see + * i2c_acpi_find_adapter_by_handle()) + */ + if (!is_acpi_device_node(dev_fwnode(dev)) && dev->parent) + return device_match_fwnode(dev->parent, data); + + return 0; +} + +struct i2c_adapter * +fwnode_find_i2c_adapter_by_node(struct fwnode_handle *fwnode) +{ + struct device *dev; + struct i2c_adapter *adapter; + + dev = bus_find_device(&i2c_bus_type, NULL, fwnode, + fwnode_dev_or_parent_node_match); + if (!dev) + return NULL; + + adapter = i2c_verify_adapter(dev); + if (!adapter) + put_device(dev); + + return adapter; +} +EXPORT_SYMBOL(fwnode_find_i2c_adapter_by_node); diff --git a/include/linux/i2c.h b/include/linux/i2c.h index 7d4f52ceb7b5..8dadf8c89fd9 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -967,6 +967,9 @@ int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr); #endif /* I2C */ +struct i2c_adapter * +fwnode_find_i2c_adapter_by_node(struct fwnode_handle *fwnode); + #if IS_ENABLED(CONFIG_OF) /* must call put_device() when done with returned i2c_client device */ struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
Add fwnode_find_i2c_adapter_by_node() which allows to retrieve a i2c adapter using a fwnode. Since dev_fwnode() uses the fwnode provided by the of_node member of the device, this will also work for devices were the of_node has been set and not the fwnode field. For acpi nodes, the check for parent node is skipped since i2c_acpi_find_adapter_by_handle() does not check it and we don't want to change this behavior. Signed-off-by: Clément Léger <clement.leger@bootlin.com> --- drivers/i2c/Makefile | 1 + drivers/i2c/i2c-core-fwnode.c | 45 +++++++++++++++++++++++++++++++++++ include/linux/i2c.h | 3 +++ 3 files changed, 49 insertions(+) create mode 100644 drivers/i2c/i2c-core-fwnode.c