| Message ID | 1560534863-15115-14-git-send-email-suzuki.poulose@arm.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | None | expand |
On 2019-06-14 19:54, Suzuki K Poulose wrote: > Add a wrapper to class_find_device() to search for a device > by the of_node pointer, reusing the generic match function. > Also convert the existing users to make use of the new helper. > > Cc: Alan Tull <atull@kernel.org> > Cc: Moritz Fischer <mdf@kernel.org> > Cc: linux-fpga@vger.kernel.org > Cc: Peter Rosin <peda@axentia.se> > Cc: Mark Brown <broonie@kernel.org> > Cc: Florian Fainelli <f.fainelli@gmail.com> > Cc: Heiner Kallweit <hkallweit1@gmail.com> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Andrew Lunn <andrew@lunn.ch> > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > Cc: Jiri Slaby <jslaby@suse.com> > Acked-by: Mark Brown <broonie@kernel.org> > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > Reviewed-by: Peter Rosin <peda@axentia.se> Whoooa! I reviewed only the drivers/mux/core.c changes when this was done in a series of much smaller patches. This tag makes it seem as if I have reviewed the whole thing, which I had not done when you added this tag out of the blue. Now, this stuff is trivial and by now I have looked at the other files and it all seems simple enough. So, you can keep the tag, but it is NOT ok to handle tags like you have done here. Cheers, Peter > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > drivers/fpga/fpga-bridge.c | 8 +------- > drivers/fpga/fpga-mgr.c | 8 +------- > drivers/mux/core.c | 7 +------ > drivers/net/phy/mdio_bus.c | 9 +-------- > drivers/regulator/of_regulator.c | 7 +------ > drivers/spi/spi.c | 11 ++--------- > include/linux/device.h | 12 ++++++++++++ > 7 files changed, 19 insertions(+), 43 deletions(-)
Hi Peter, On 22/06/2019 06:25, Peter Rosin wrote: > On 2019-06-14 19:54, Suzuki K Poulose wrote: >> Add a wrapper to class_find_device() to search for a device >> by the of_node pointer, reusing the generic match function. >> Also convert the existing users to make use of the new helper. >> >> Cc: Alan Tull <atull@kernel.org> >> Cc: Moritz Fischer <mdf@kernel.org> >> Cc: linux-fpga@vger.kernel.org >> Cc: Peter Rosin <peda@axentia.se> >> Cc: Mark Brown <broonie@kernel.org> >> Cc: Florian Fainelli <f.fainelli@gmail.com> >> Cc: Heiner Kallweit <hkallweit1@gmail.com> >> Cc: "David S. Miller" <davem@davemloft.net> >> Cc: Andrew Lunn <andrew@lunn.ch> >> Cc: Liam Girdwood <lgirdwood@gmail.com> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: "Rafael J. Wysocki" <rafael@kernel.org> >> Cc: Jiri Slaby <jslaby@suse.com> >> Acked-by: Mark Brown <broonie@kernel.org> >> Reviewed-by: Andrew Lunn <andrew@lunn.ch> >> Reviewed-by: Peter Rosin <peda@axentia.se> > > Whoooa! I reviewed only the drivers/mux/core.c changes when this was done > in a series of much smaller patches. This tag makes it seem as if I have > reviewed the whole thing, which I had not done when you added this tag out > of the blue. Apologies for the surprise. The patch was simply squashed with the change that introduced the "helper" to better aid the reviewers, based on suggestions on the list. I kept your tags, only because there were no changes, but some additional context on the core driver. > > Now, this stuff is trivial and by now I have looked at the other files > and it all seems simple enough. So, you can keep the tag, but it is NOT > ok to handle tags like you have done here. Sure, I will keep that in mind. Cheers Suzuki
On 2019-06-24 10:34, Suzuki K Poulose wrote: > Hi Peter, > > On 22/06/2019 06:25, Peter Rosin wrote: >> On 2019-06-14 19:54, Suzuki K Poulose wrote: >>> Add a wrapper to class_find_device() to search for a device >>> by the of_node pointer, reusing the generic match function. >>> Also convert the existing users to make use of the new helper. >>> >>> Cc: Alan Tull <atull@kernel.org> >>> Cc: Moritz Fischer <mdf@kernel.org> >>> Cc: linux-fpga@vger.kernel.org >>> Cc: Peter Rosin <peda@axentia.se> >>> Cc: Mark Brown <broonie@kernel.org> >>> Cc: Florian Fainelli <f.fainelli@gmail.com> >>> Cc: Heiner Kallweit <hkallweit1@gmail.com> >>> Cc: "David S. Miller" <davem@davemloft.net> >>> Cc: Andrew Lunn <andrew@lunn.ch> >>> Cc: Liam Girdwood <lgirdwood@gmail.com> >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> Cc: "Rafael J. Wysocki" <rafael@kernel.org> >>> Cc: Jiri Slaby <jslaby@suse.com> >>> Acked-by: Mark Brown <broonie@kernel.org> >>> Reviewed-by: Andrew Lunn <andrew@lunn.ch> >>> Reviewed-by: Peter Rosin <peda@axentia.se> >> >> Whoooa! I reviewed only the drivers/mux/core.c changes when this was done >> in a series of much smaller patches. This tag makes it seem as if I have >> reviewed the whole thing, which I had not done when you added this tag out >> of the blue. > > Apologies for the surprise. The patch was simply squashed with the change that > introduced the "helper" to better aid the reviewers, based on suggestions on the > list. I kept your tags, only because there were no changes, but some additional > context on the core driver. You could e.g. have written: ... [For the drivers/mux/core.c part] Reviewed-by: Peter Rosin <peda@axentia.se> ... >> >> Now, this stuff is trivial and by now I have looked at the other files >> and it all seems simple enough. So, you can keep the tag, but it is NOT >> ok to handle tags like you have done here. > > Sure, I will keep that in mind. Great! Cheers, Peter
On 24/06/2019 15:07, Peter Rosin wrote: > On 2019-06-24 10:34, Suzuki K Poulose wrote: >> Hi Peter, >> >> On 22/06/2019 06:25, Peter Rosin wrote: >>> On 2019-06-14 19:54, Suzuki K Poulose wrote: >>>> Add a wrapper to class_find_device() to search for a device >>>> by the of_node pointer, reusing the generic match function. >>>> Also convert the existing users to make use of the new helper. >>>> >>>> Cc: Alan Tull <atull@kernel.org> >>>> Cc: Moritz Fischer <mdf@kernel.org> >>>> Cc: linux-fpga@vger.kernel.org >>>> Cc: Peter Rosin <peda@axentia.se> >>>> Cc: Mark Brown <broonie@kernel.org> >>>> Cc: Florian Fainelli <f.fainelli@gmail.com> >>>> Cc: Heiner Kallweit <hkallweit1@gmail.com> >>>> Cc: "David S. Miller" <davem@davemloft.net> >>>> Cc: Andrew Lunn <andrew@lunn.ch> >>>> Cc: Liam Girdwood <lgirdwood@gmail.com> >>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>>> Cc: "Rafael J. Wysocki" <rafael@kernel.org> >>>> Cc: Jiri Slaby <jslaby@suse.com> >>>> Acked-by: Mark Brown <broonie@kernel.org> >>>> Reviewed-by: Andrew Lunn <andrew@lunn.ch> >>>> Reviewed-by: Peter Rosin <peda@axentia.se> >>> >>> Whoooa! I reviewed only the drivers/mux/core.c changes when this was done >>> in a series of much smaller patches. This tag makes it seem as if I have >>> reviewed the whole thing, which I had not done when you added this tag out >>> of the blue. >> >> Apologies for the surprise. The patch was simply squashed with the change that >> introduced the "helper" to better aid the reviewers, based on suggestions on the >> list. I kept your tags, only because there were no changes, but some additional >> context on the core driver. > > You could e.g. have written: > > ... > [For the drivers/mux/core.c part] > Reviewed-by: Peter Rosin <peda@axentia.se> > ... Ok. This might change again, another level of merging, but no changes to the hunk for mux. So I can use the above. Cheers Suzuki
diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c index 80bd8f1..4bab902 100644 --- a/drivers/fpga/fpga-bridge.c +++ b/drivers/fpga/fpga-bridge.c @@ -19,11 +19,6 @@ static struct class *fpga_bridge_class; /* Lock for adding/removing bridges to linked lists*/ static spinlock_t bridge_list_lock; -static int fpga_bridge_of_node_match(struct device *dev, const void *data) -{ - return dev->of_node == data; -} - /** * fpga_bridge_enable - Enable transactions on the bridge * @@ -104,8 +99,7 @@ struct fpga_bridge *of_fpga_bridge_get(struct device_node *np, { struct device *dev; - dev = class_find_device(fpga_bridge_class, NULL, np, - fpga_bridge_of_node_match); + dev = class_find_device_by_of_node(fpga_bridge_class, np); if (!dev) return ERR_PTR(-ENODEV); diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c index c386681..e05104f 100644 --- a/drivers/fpga/fpga-mgr.c +++ b/drivers/fpga/fpga-mgr.c @@ -482,11 +482,6 @@ struct fpga_manager *fpga_mgr_get(struct device *dev) } EXPORT_SYMBOL_GPL(fpga_mgr_get); -static int fpga_mgr_of_node_match(struct device *dev, const void *data) -{ - return dev->of_node == data; -} - /** * of_fpga_mgr_get - Given a device node, get a reference to a fpga mgr. * @@ -498,8 +493,7 @@ struct fpga_manager *of_fpga_mgr_get(struct device_node *node) { struct device *dev; - dev = class_find_device(fpga_mgr_class, NULL, node, - fpga_mgr_of_node_match); + dev = class_find_device_by_of_node(fpga_mgr_class, node); if (!dev) return ERR_PTR(-ENODEV); diff --git a/drivers/mux/core.c b/drivers/mux/core.c index d1271c1..1fb2238 100644 --- a/drivers/mux/core.c +++ b/drivers/mux/core.c @@ -405,17 +405,12 @@ int mux_control_deselect(struct mux_control *mux) } EXPORT_SYMBOL_GPL(mux_control_deselect); -static int of_dev_node_match(struct device *dev, const void *data) -{ - return dev->of_node == data; -} - /* Note this function returns a reference to the mux_chip dev. */ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np) { struct device *dev; - dev = class_find_device(&mux_class, NULL, np, of_dev_node_match); + dev = class_find_device_by_of_node(&mux_class, np); return dev ? to_mux_chip(dev) : NULL; } diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index bd04fe7..ce94087 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -262,11 +262,6 @@ static struct class mdio_bus_class = { }; #if IS_ENABLED(CONFIG_OF_MDIO) -/* Helper function for of_mdio_find_bus */ -static int of_mdio_bus_match(struct device *dev, const void *mdio_bus_np) -{ - return dev->of_node == mdio_bus_np; -} /** * of_mdio_find_bus - Given an mii_bus node, find the mii_bus. * @mdio_bus_np: Pointer to the mii_bus. @@ -287,9 +282,7 @@ struct mii_bus *of_mdio_find_bus(struct device_node *mdio_bus_np) if (!mdio_bus_np) return NULL; - d = class_find_device(&mdio_bus_class, NULL, mdio_bus_np, - of_mdio_bus_match); - + d = class_find_device_by_of_node(&mdio_bus_class, mdio_bus_np); return d ? to_mii_bus(d) : NULL; } EXPORT_SYMBOL(of_mdio_find_bus); diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c index 0ead116..1b40277 100644 --- a/drivers/regulator/of_regulator.c +++ b/drivers/regulator/of_regulator.c @@ -442,16 +442,11 @@ struct regulator_init_data *regulator_of_get_init_data(struct device *dev, return NULL; } -static int of_node_match(struct device *dev, const void *data) -{ - return dev->of_node == data; -} - struct regulator_dev *of_find_regulator_by_node(struct device_node *np) { struct device *dev; - dev = class_find_device(®ulator_class, NULL, np, of_node_match); + dev = class_find_device_by_of_node(®ulator_class, np); return dev ? dev_to_rdev(dev) : NULL; } diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 3da1121..a256be5 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -3554,21 +3554,14 @@ EXPORT_SYMBOL_GPL(of_find_spi_device_by_node); #endif /* IS_ENABLED(CONFIG_OF) */ #if IS_ENABLED(CONFIG_OF_DYNAMIC) -static int __spi_of_controller_match(struct device *dev, const void *data) -{ - return dev->of_node == data; -} - /* the spi controllers are not using spi_bus, so we find it with another way */ static struct spi_controller *of_find_spi_controller_by_node(struct device_node *node) { struct device *dev; - dev = class_find_device(&spi_master_class, NULL, node, - __spi_of_controller_match); + dev = class_find_device_by_of_node(&spi_master_class, node); if (!dev && IS_ENABLED(CONFIG_SPI_SLAVE)) - dev = class_find_device(&spi_slave_class, NULL, node, - __spi_of_controller_match); + dev = class_find_device_by_of_node(&spi_slave_class, node); if (!dev) return NULL; diff --git a/include/linux/device.h b/include/linux/device.h index bb14c7f..9228502 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -497,6 +497,18 @@ static inline struct device *class_find_device_by_name(struct class *class, return class_find_device(class, NULL, name, device_match_name); } +/** + * class_find_device_by_of_node : device iterator for locating a particular device + * matching the of_node. + * @class: class type + * @np: of_node of the device to match. + */ +static inline struct device * +class_find_device_by_of_node(struct class *class, const struct device_node *np) +{ + return class_find_device(class, NULL, np, device_match_of_node); +} + struct class_attribute { struct attribute attr; ssize_t (*show)(struct class *class, struct class_attribute *attr,