* [PATCH RFC 0/2] Add I2C fwnode lookup/get interfaces @ 2022-12-07 11:21 Russell King (Oracle) 2022-12-07 11:22 ` [PATCH RFC 1/2] i2c: add fwnode APIs Russell King (Oracle) 2022-12-07 11:22 ` [PATCH RFC 2/2] net: sfp: use i2c_get_adapter_by_fwnode() Russell King (Oracle) 0 siblings, 2 replies; 7+ messages in thread From: Russell King (Oracle) @ 2022-12-07 11:21 UTC (permalink / raw) To: linux-acpi, linux-i2c, netdev Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Heiner Kallweit, Jakub Kicinski, Mika Westerberg, Paolo Abeni, Wolfram Sang Hi, This RFC series is not intended for the coming merge window, and we will need to decide how to merge it as it is split across two subsystems. These patches have been generated against the net-next, since patch 2 depends on a recently merged patch in that tree. Currently, the SFP code attempts to work out what kind of fwnode we found when looking up the I2C bus for the SFP cage, converts the fwnode to the appropriate firmware specific representation to then call the appropriate I2C layer function. This is inefficient, since the device model provides a way to locate items on a bus_type by fwnode. In order to reduce this complexity, this series adds fwnode interfaces to the I2C subsystem to allow I2C adapters to be looked up. I also accidentally also converted the I2C clients to also be looked up, so I've left that in patch 1 if people think that could be useful - if not, I'll remove it. We could also convert the of_* functions to be inline in i2c.h and remove the stub of_* functions and exports. Do we want these to live in i2c-core-fwnode.c ? I don't see a Kconfig symbol that indicates whether we want fwnode support, and I know there are people looking to use software nodes to lookup the SFP I2C bus (which is why the manual firmware-specific code in sfp.c is a problem.) Thanks! drivers/i2c/i2c-core-acpi.c | 13 +------- drivers/i2c/i2c-core-base.c | 72 +++++++++++++++++++++++++++++++++++++++++++++ drivers/i2c/i2c-core-of.c | 51 ++------------------------------ drivers/net/phy/sfp.c | 13 +------- include/linux/i2c.h | 9 ++++++ 5 files changed, 86 insertions(+), 72 deletions(-) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH RFC 1/2] i2c: add fwnode APIs 2022-12-07 11:21 [PATCH RFC 0/2] Add I2C fwnode lookup/get interfaces Russell King (Oracle) @ 2022-12-07 11:22 ` Russell King (Oracle) 2022-12-08 10:04 ` Mika Westerberg 2022-12-07 11:22 ` [PATCH RFC 2/2] net: sfp: use i2c_get_adapter_by_fwnode() Russell King (Oracle) 1 sibling, 1 reply; 7+ messages in thread From: Russell King (Oracle) @ 2022-12-07 11:22 UTC (permalink / raw) To: linux-acpi, linux-i2c, netdev Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Heiner Kallweit, Jakub Kicinski, Mika Westerberg, Paolo Abeni, Wolfram Sang Add fwnode APIs for finding and getting I2C adapters, which will be used by the SFP code. These are passed the fwnode corresponding to the adapter, and return the I2C adapter. It is the responsibility of the caller to find the appropriate fwnode. We keep the DT and ACPI interfaces, but where appropriate, recode them to use the fwnode interfaces internally. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/i2c/i2c-core-acpi.c | 13 +------ drivers/i2c/i2c-core-base.c | 72 +++++++++++++++++++++++++++++++++++++ drivers/i2c/i2c-core-of.c | 51 ++------------------------ include/linux/i2c.h | 9 +++++ 4 files changed, 85 insertions(+), 60 deletions(-) diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c index 4dd777cc0c89..d6037a328669 100644 --- a/drivers/i2c/i2c-core-acpi.c +++ b/drivers/i2c/i2c-core-acpi.c @@ -442,18 +442,7 @@ EXPORT_SYMBOL_GPL(i2c_acpi_find_adapter_by_handle); static struct i2c_client *i2c_acpi_find_client_by_adev(struct acpi_device *adev) { - struct device *dev; - struct i2c_client *client; - - dev = bus_find_device_by_acpi_dev(&i2c_bus_type, adev); - if (!dev) - return NULL; - - client = i2c_verify_client(dev); - if (!client) - put_device(dev); - - return client; + return i2c_find_device_by_fwnode(acpi_fwnode_handle(adev)); } static int i2c_acpi_notify(struct notifier_block *nb, unsigned long value, diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 9aa7b9d9a485..254ec043ce90 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -1011,6 +1011,27 @@ void i2c_unregister_device(struct i2c_client *client) } EXPORT_SYMBOL_GPL(i2c_unregister_device); +/* must call put_device() when done with returned i2c_client device */ +struct i2c_client *i2c_find_device_by_fwnode(struct fwnode_handle *fwnode) +{ + struct i2c_client *client; + struct device *dev; + + if (!fwnode) + return NULL; + + dev = bus_find_device_by_fwnode(&i2c_bus_type, fwnode); + if (!dev) + return NULL; + + client = i2c_verify_client(dev); + if (!client) + put_device(dev); + + return client; +} +EXPORT_SYMBOL(i2c_find_device_by_fwnode); + static const struct i2c_device_id dummy_id[] = { { "dummy", 0 }, @@ -1761,6 +1782,57 @@ int devm_i2c_add_adapter(struct device *dev, struct i2c_adapter *adapter) } EXPORT_SYMBOL_GPL(devm_i2c_add_adapter); +static int i2c_dev_or_parent_fwnode_match(struct device *dev, const void *data) +{ + if (dev_fwnode(dev) == data) + return 1; + + if (dev->parent && dev_fwnode(dev->parent) == data) + return 1; + + return 0; +} + +/* must call put_device() when done with returned i2c_adapter device */ +struct i2c_adapter *i2c_find_adapter_by_fwnode(struct fwnode_handle *fwnode) +{ + struct i2c_adapter *adapter; + struct device *dev; + + if (!fwnode) + return NULL; + + dev = bus_find_device(&i2c_bus_type, NULL, fwnode, + i2c_dev_or_parent_fwnode_match); + if (!dev) + return NULL; + + adapter = i2c_verify_adapter(dev); + if (!adapter) + put_device(dev); + + return adapter; +} +EXPORT_SYMBOL(i2c_find_adapter_by_fwnode); + +/* must call i2c_put_adapter() when done with returned i2c_adapter device */ +struct i2c_adapter *i2c_get_adapter_by_fwnode(struct fwnode_handle *fwnode) +{ + struct i2c_adapter *adapter; + + adapter = i2c_find_adapter_by_fwnode(fwnode); + if (!adapter) + return NULL; + + if (!try_module_get(adapter->owner)) { + put_device(&adapter->dev); + adapter = NULL; + } + + return adapter; +} +EXPORT_SYMBOL(i2c_get_adapter_by_fwnode); + static void i2c_parse_timing(struct device *dev, char *prop_name, u32 *cur_val_p, u32 def_val, bool use_def) { diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c index 3ed74aa4b44b..c3e565e4bddf 100644 --- a/drivers/i2c/i2c-core-of.c +++ b/drivers/i2c/i2c-core-of.c @@ -113,69 +113,24 @@ void of_i2c_register_devices(struct i2c_adapter *adap) of_node_put(bus); } -static int of_dev_or_parent_node_match(struct device *dev, const void *data) -{ - if (dev->of_node == data) - return 1; - - 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) { - struct device *dev; - struct i2c_client *client; - - dev = bus_find_device_by_of_node(&i2c_bus_type, node); - if (!dev) - return NULL; - - client = i2c_verify_client(dev); - if (!client) - put_device(dev); - - return client; + return i2c_find_device_by_fwnode(of_fwnode_handle(node)); } EXPORT_SYMBOL(of_find_i2c_device_by_node); /* must call put_device() when done with returned i2c_adapter device */ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node) { - struct device *dev; - struct i2c_adapter *adapter; - - dev = bus_find_device(&i2c_bus_type, NULL, node, - of_dev_or_parent_node_match); - if (!dev) - return NULL; - - adapter = i2c_verify_adapter(dev); - if (!adapter) - put_device(dev); - - return adapter; + return i2c_find_adapter_by_fwnode(of_fwnode_handle(node)); } EXPORT_SYMBOL(of_find_i2c_adapter_by_node); /* must call i2c_put_adapter() when done with returned i2c_adapter device */ struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node) { - struct i2c_adapter *adapter; - - adapter = of_find_i2c_adapter_by_node(node); - if (!adapter) - return NULL; - - if (!try_module_get(adapter->owner)) { - put_device(&adapter->dev); - adapter = NULL; - } - - return adapter; + return i2c_get_adapter_by_fwnode(of_fwnode_handle(node)); } EXPORT_SYMBOL(of_get_i2c_adapter_by_node); diff --git a/include/linux/i2c.h b/include/linux/i2c.h index d84e0e99f084..bcee9faaf2e6 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -965,6 +965,15 @@ int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr); #endif /* I2C */ +/* must call put_device() when done with returned i2c_client device */ +struct i2c_client *i2c_find_device_by_fwnode(struct fwnode_handle *fwnode); + +/* must call put_device() when done with returned i2c_adapter device */ +struct i2c_adapter *i2c_find_adapter_by_fwnode(struct fwnode_handle *fwnode); + +/* must call i2c_put_adapter() when done with returned i2c_adapter device */ +struct i2c_adapter *i2c_get_adapter_by_fwnode(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); -- 2.30.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH RFC 1/2] i2c: add fwnode APIs 2022-12-07 11:22 ` [PATCH RFC 1/2] i2c: add fwnode APIs Russell King (Oracle) @ 2022-12-08 10:04 ` Mika Westerberg 2022-12-08 10:16 ` Russell King (Oracle) 0 siblings, 1 reply; 7+ messages in thread From: Mika Westerberg @ 2022-12-08 10:04 UTC (permalink / raw) To: Russell King (Oracle) Cc: linux-acpi, linux-i2c, netdev, Andrew Lunn, David S. Miller, Eric Dumazet, Heiner Kallweit, Jakub Kicinski, Paolo Abeni, Wolfram Sang Hi, On Wed, Dec 07, 2022 at 11:22:24AM +0000, Russell King (Oracle) wrote: > Add fwnode APIs for finding and getting I2C adapters, which will be > used by the SFP code. These are passed the fwnode corresponding to > the adapter, and return the I2C adapter. It is the responsibility of > the caller to find the appropriate fwnode. > > We keep the DT and ACPI interfaces, but where appropriate, recode them > to use the fwnode interfaces internally. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Looks good, just few minor comments below. :) > --- > drivers/i2c/i2c-core-acpi.c | 13 +------ > drivers/i2c/i2c-core-base.c | 72 +++++++++++++++++++++++++++++++++++++ > drivers/i2c/i2c-core-of.c | 51 ++------------------------ > include/linux/i2c.h | 9 +++++ > 4 files changed, 85 insertions(+), 60 deletions(-) > > diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c > index 4dd777cc0c89..d6037a328669 100644 > --- a/drivers/i2c/i2c-core-acpi.c > +++ b/drivers/i2c/i2c-core-acpi.c > @@ -442,18 +442,7 @@ EXPORT_SYMBOL_GPL(i2c_acpi_find_adapter_by_handle); > > static struct i2c_client *i2c_acpi_find_client_by_adev(struct acpi_device *adev) > { > - struct device *dev; > - struct i2c_client *client; > - > - dev = bus_find_device_by_acpi_dev(&i2c_bus_type, adev); > - if (!dev) > - return NULL; > - > - client = i2c_verify_client(dev); > - if (!client) > - put_device(dev); > - > - return client; > + return i2c_find_device_by_fwnode(acpi_fwnode_handle(adev)); > } > > static int i2c_acpi_notify(struct notifier_block *nb, unsigned long value, > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c > index 9aa7b9d9a485..254ec043ce90 100644 > --- a/drivers/i2c/i2c-core-base.c > +++ b/drivers/i2c/i2c-core-base.c > @@ -1011,6 +1011,27 @@ void i2c_unregister_device(struct i2c_client *client) > } > EXPORT_SYMBOL_GPL(i2c_unregister_device); > > +/* must call put_device() when done with returned i2c_client device */ I think proper kernel-doc would be better here and all the exported functions. > +struct i2c_client *i2c_find_device_by_fwnode(struct fwnode_handle *fwnode) > +{ > + struct i2c_client *client; > + struct device *dev; > + > + if (!fwnode) > + return NULL; > + > + dev = bus_find_device_by_fwnode(&i2c_bus_type, fwnode); > + if (!dev) > + return NULL; > + > + client = i2c_verify_client(dev); > + if (!client) > + put_device(dev); > + > + return client; > +} > +EXPORT_SYMBOL(i2c_find_device_by_fwnode); > + Drop this empty line. > > static const struct i2c_device_id dummy_id[] = { > { "dummy", 0 }, > @@ -1761,6 +1782,57 @@ int devm_i2c_add_adapter(struct device *dev, struct i2c_adapter *adapter) > } > EXPORT_SYMBOL_GPL(devm_i2c_add_adapter); > > +static int i2c_dev_or_parent_fwnode_match(struct device *dev, const void *data) > +{ > + if (dev_fwnode(dev) == data) > + return 1; > + > + if (dev->parent && dev_fwnode(dev->parent) == data) > + return 1; > + > + return 0; > +} > + > +/* must call put_device() when done with returned i2c_adapter device */ > +struct i2c_adapter *i2c_find_adapter_by_fwnode(struct fwnode_handle *fwnode) > +{ > + struct i2c_adapter *adapter; > + struct device *dev; > + > + if (!fwnode) > + return NULL; > + > + dev = bus_find_device(&i2c_bus_type, NULL, fwnode, > + i2c_dev_or_parent_fwnode_match); > + if (!dev) > + return NULL; > + > + adapter = i2c_verify_adapter(dev); > + if (!adapter) > + put_device(dev); > + > + return adapter; > +} > +EXPORT_SYMBOL(i2c_find_adapter_by_fwnode); > + > +/* must call i2c_put_adapter() when done with returned i2c_adapter device */ > +struct i2c_adapter *i2c_get_adapter_by_fwnode(struct fwnode_handle *fwnode) > +{ > + struct i2c_adapter *adapter; > + > + adapter = i2c_find_adapter_by_fwnode(fwnode); > + if (!adapter) > + return NULL; > + > + if (!try_module_get(adapter->owner)) { > + put_device(&adapter->dev); > + adapter = NULL; > + } > + > + return adapter; > +} > +EXPORT_SYMBOL(i2c_get_adapter_by_fwnode); > + > static void i2c_parse_timing(struct device *dev, char *prop_name, u32 *cur_val_p, > u32 def_val, bool use_def) > { > diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c > index 3ed74aa4b44b..c3e565e4bddf 100644 > --- a/drivers/i2c/i2c-core-of.c > +++ b/drivers/i2c/i2c-core-of.c > @@ -113,69 +113,24 @@ void of_i2c_register_devices(struct i2c_adapter *adap) > of_node_put(bus); > } > > -static int of_dev_or_parent_node_match(struct device *dev, const void *data) > -{ > - if (dev->of_node == data) > - return 1; > - > - 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) > { > - struct device *dev; > - struct i2c_client *client; > - > - dev = bus_find_device_by_of_node(&i2c_bus_type, node); > - if (!dev) > - return NULL; > - > - client = i2c_verify_client(dev); > - if (!client) > - put_device(dev); > - > - return client; > + return i2c_find_device_by_fwnode(of_fwnode_handle(node)); > } > EXPORT_SYMBOL(of_find_i2c_device_by_node); > > /* must call put_device() when done with returned i2c_adapter device */ > struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node) > { > - struct device *dev; > - struct i2c_adapter *adapter; > - > - dev = bus_find_device(&i2c_bus_type, NULL, node, > - of_dev_or_parent_node_match); > - if (!dev) > - return NULL; > - > - adapter = i2c_verify_adapter(dev); > - if (!adapter) > - put_device(dev); > - > - return adapter; > + return i2c_find_adapter_by_fwnode(of_fwnode_handle(node)); > } > EXPORT_SYMBOL(of_find_i2c_adapter_by_node); > > /* must call i2c_put_adapter() when done with returned i2c_adapter device */ > struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node) > { > - struct i2c_adapter *adapter; > - > - adapter = of_find_i2c_adapter_by_node(node); > - if (!adapter) > - return NULL; > - > - if (!try_module_get(adapter->owner)) { > - put_device(&adapter->dev); > - adapter = NULL; > - } > - > - return adapter; > + return i2c_get_adapter_by_fwnode(of_fwnode_handle(node)); > } > EXPORT_SYMBOL(of_get_i2c_adapter_by_node); > > diff --git a/include/linux/i2c.h b/include/linux/i2c.h > index d84e0e99f084..bcee9faaf2e6 100644 > --- a/include/linux/i2c.h > +++ b/include/linux/i2c.h > @@ -965,6 +965,15 @@ int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr); > > #endif /* I2C */ > > +/* must call put_device() when done with returned i2c_client device */ > +struct i2c_client *i2c_find_device_by_fwnode(struct fwnode_handle *fwnode); With the kernel-docs in place you probably can drop these comments. > + > +/* must call put_device() when done with returned i2c_adapter device */ > +struct i2c_adapter *i2c_find_adapter_by_fwnode(struct fwnode_handle *fwnode); > + > +/* must call i2c_put_adapter() when done with returned i2c_adapter device */ > +struct i2c_adapter *i2c_get_adapter_by_fwnode(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); > -- > 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC 1/2] i2c: add fwnode APIs 2022-12-08 10:04 ` Mika Westerberg @ 2022-12-08 10:16 ` Russell King (Oracle) 2022-12-19 8:47 ` Russell King (Oracle) 0 siblings, 1 reply; 7+ messages in thread From: Russell King (Oracle) @ 2022-12-08 10:16 UTC (permalink / raw) To: Mika Westerberg Cc: linux-acpi, linux-i2c, netdev, Andrew Lunn, David S. Miller, Eric Dumazet, Heiner Kallweit, Jakub Kicinski, Paolo Abeni, Wolfram Sang Hi Mika, On Thu, Dec 08, 2022 at 12:04:02PM +0200, Mika Westerberg wrote: > Hi, > > On Wed, Dec 07, 2022 at 11:22:24AM +0000, Russell King (Oracle) wrote: > > +struct i2c_client *i2c_find_device_by_fwnode(struct fwnode_handle *fwnode) > > +{ > > + struct i2c_client *client; > > + struct device *dev; > > + > > + if (!fwnode) > > + return NULL; > > + > > + dev = bus_find_device_by_fwnode(&i2c_bus_type, fwnode); > > + if (!dev) > > + return NULL; > > + > > + client = i2c_verify_client(dev); > > + if (!client) > > + put_device(dev); > > + > > + return client; > > +} > > +EXPORT_SYMBOL(i2c_find_device_by_fwnode); > > + > > Drop this empty line. The additional empty line was there before, and I guess is something the I2C maintainer wants to logically separate the i2c device stuff from the rest of the file. > > +/* must call put_device() when done with returned i2c_client device */ > > +struct i2c_client *i2c_find_device_by_fwnode(struct fwnode_handle *fwnode); > > With the kernel-docs in place you probably can drop these comments. It's what is there against the other prototypes - and is very easy to get wrong, as I've recently noticed in the sfp.c code as a result of creating this series. I find the whole _find_ vs _get_ thing a tad confusing, and there probably should be just one interface with one way of putting afterwards to avoid subtle long-standing bugs like this. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC 1/2] i2c: add fwnode APIs 2022-12-08 10:16 ` Russell King (Oracle) @ 2022-12-19 8:47 ` Russell King (Oracle) 2022-12-19 9:23 ` Mika Westerberg 0 siblings, 1 reply; 7+ messages in thread From: Russell King (Oracle) @ 2022-12-19 8:47 UTC (permalink / raw) To: Mika Westerberg Cc: linux-acpi, linux-i2c, netdev, Andrew Lunn, David S. Miller, Eric Dumazet, Heiner Kallweit, Jakub Kicinski, Paolo Abeni, Wolfram Sang Hi Mika, On Thu, Dec 08, 2022 at 10:16:07AM +0000, Russell King (Oracle) wrote: > Hi Mika, > > On Thu, Dec 08, 2022 at 12:04:02PM +0200, Mika Westerberg wrote: > > Hi, > > > > On Wed, Dec 07, 2022 at 11:22:24AM +0000, Russell King (Oracle) wrote: > > > +EXPORT_SYMBOL(i2c_find_device_by_fwnode); > > > + > > > > Drop this empty line. > > The additional empty line was there before, and I guess is something the > I2C maintainer wants to logically separate the i2c device stuff from > the rest of the file. > > > > +/* must call put_device() when done with returned i2c_client device */ > > > +struct i2c_client *i2c_find_device_by_fwnode(struct fwnode_handle *fwnode); > > > > With the kernel-docs in place you probably can drop these comments. > > It's what is there against the other prototypes - and is very easy to > get wrong, as I've recently noticed in the sfp.c code as a result of > creating this series. > > I find the whole _find_ vs _get_ thing a tad confusing, and there > probably should be just one interface with one way of putting > afterwards to avoid subtle long-standing bugs like this. > > Thanks. Do you have any comments on my reply please? Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC 1/2] i2c: add fwnode APIs 2022-12-19 8:47 ` Russell King (Oracle) @ 2022-12-19 9:23 ` Mika Westerberg 0 siblings, 0 replies; 7+ messages in thread From: Mika Westerberg @ 2022-12-19 9:23 UTC (permalink / raw) To: Russell King (Oracle) Cc: linux-acpi, linux-i2c, netdev, Andrew Lunn, David S. Miller, Eric Dumazet, Heiner Kallweit, Jakub Kicinski, Paolo Abeni, Wolfram Sang Hi, On Mon, Dec 19, 2022 at 08:47:07AM +0000, Russell King (Oracle) wrote: > Hi Mika, > > On Thu, Dec 08, 2022 at 10:16:07AM +0000, Russell King (Oracle) wrote: > > Hi Mika, > > > > On Thu, Dec 08, 2022 at 12:04:02PM +0200, Mika Westerberg wrote: > > > Hi, > > > > > > On Wed, Dec 07, 2022 at 11:22:24AM +0000, Russell King (Oracle) wrote: > > > > +EXPORT_SYMBOL(i2c_find_device_by_fwnode); > > > > + > > > > > > Drop this empty line. > > > > The additional empty line was there before, and I guess is something the > > I2C maintainer wants to logically separate the i2c device stuff from > > the rest of the file. > > > > > > +/* must call put_device() when done with returned i2c_client device */ > > > > +struct i2c_client *i2c_find_device_by_fwnode(struct fwnode_handle *fwnode); > > > > > > With the kernel-docs in place you probably can drop these comments. > > > > It's what is there against the other prototypes - and is very easy to > > get wrong, as I've recently noticed in the sfp.c code as a result of > > creating this series. > > > > I find the whole _find_ vs _get_ thing a tad confusing, and there > > probably should be just one interface with one way of putting > > afterwards to avoid subtle long-standing bugs like this. > > > > Thanks. > > Do you have any comments on my reply please? Sorry, no comments :) Thanks for the clarification. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH RFC 2/2] net: sfp: use i2c_get_adapter_by_fwnode() 2022-12-07 11:21 [PATCH RFC 0/2] Add I2C fwnode lookup/get interfaces Russell King (Oracle) 2022-12-07 11:22 ` [PATCH RFC 1/2] i2c: add fwnode APIs Russell King (Oracle) @ 2022-12-07 11:22 ` Russell King (Oracle) 1 sibling, 0 replies; 7+ messages in thread From: Russell King (Oracle) @ 2022-12-07 11:22 UTC (permalink / raw) To: linux-acpi, linux-i2c, netdev Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Heiner Kallweit, Jakub Kicinski, Mika Westerberg, Paolo Abeni, Wolfram Sang Use the newly introduced i2c_get_adapter_by_fwnode() API, so that we can retrieve the I2C adapter in a firmware independent manner once we have the fwnode handle for the adapter. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/net/phy/sfp.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c index 83b99d95b278..aa2f7ebbdebc 100644 --- a/drivers/net/phy/sfp.c +++ b/drivers/net/phy/sfp.c @@ -2644,10 +2644,8 @@ static void sfp_cleanup(void *data) static int sfp_i2c_get(struct sfp *sfp) { - struct acpi_handle *acpi_handle; struct fwnode_handle *h; struct i2c_adapter *i2c; - struct device_node *np; int err; h = fwnode_find_reference(dev_fwnode(sfp->dev), "i2c-bus", 0); @@ -2656,16 +2654,7 @@ static int sfp_i2c_get(struct sfp *sfp) return -ENODEV; } - if (is_acpi_device_node(h)) { - acpi_handle = ACPI_HANDLE_FWNODE(h); - i2c = i2c_acpi_find_adapter_by_handle(acpi_handle); - } else if ((np = to_of_node(h)) != NULL) { - i2c = of_find_i2c_adapter_by_node(np); - } else { - err = -EINVAL; - goto put; - } - + i2c = i2c_get_adapter_by_fwnode(h); if (!i2c) { err = -EPROBE_DEFER; goto put; -- 2.30.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-19 9:23 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-12-07 11:21 [PATCH RFC 0/2] Add I2C fwnode lookup/get interfaces Russell King (Oracle) 2022-12-07 11:22 ` [PATCH RFC 1/2] i2c: add fwnode APIs Russell King (Oracle) 2022-12-08 10:04 ` Mika Westerberg 2022-12-08 10:16 ` Russell King (Oracle) 2022-12-19 8:47 ` Russell King (Oracle) 2022-12-19 9:23 ` Mika Westerberg 2022-12-07 11:22 ` [PATCH RFC 2/2] net: sfp: use i2c_get_adapter_by_fwnode() Russell King (Oracle)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).