2018-10-09 10:18:52

by Silesh C V

[permalink] [raw]
Subject: [PATCH v3 1/2] Driver core: add bus_find_device_by_fwnode

Some drivers need to find the device on a bus having a specific firmware
node. Currently, such drivers have their own implementations to do this.
Provide a helper similar to bus_find_device_by_name so that each driver
does not have to reinvent this.

Signed-off-by: Silesh C V <[email protected]>
---
Changes since v2:
- make use of dev_fwnode in match_fwnode.

drivers/base/bus.c | 20 ++++++++++++++++++++
include/linux/device.h | 3 +++
2 files changed, 23 insertions(+)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 8bfd27e..a2f39db 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -17,6 +17,7 @@
#include <linux/string.h>
#include <linux/mutex.h>
#include <linux/sysfs.h>
+#include <linux/property.h>
#include "base.h"
#include "power/power.h"

@@ -373,6 +374,25 @@ struct device *bus_find_device_by_name(struct bus_type *bus,
}
EXPORT_SYMBOL_GPL(bus_find_device_by_name);

+static int match_fwnode(struct device *dev, void *fwnode)
+{
+ return dev_fwnode(dev) == fwnode;
+}
+
+/**
+ * bus_find_device_by_fwnode - device iterator for locating a particular device
+ * having a specific firmware node
+ * @bus: bus type
+ * @start: Device to begin with
+ * @fwnode: firmware node of the device to match
+ */
+struct device *bus_find_device_by_fwnode(struct bus_type *bus, struct device *start,
+ struct fwnode_handle *fwnode)
+{
+ return bus_find_device(bus, start, (void *)fwnode, match_fwnode);
+}
+EXPORT_SYMBOL_GPL(bus_find_device_by_fwnode);
+
/**
* subsys_find_device_by_id - find a device with a specific enumeration number
* @subsys: subsystem
diff --git a/include/linux/device.h b/include/linux/device.h
index 8f88254..09384f6 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -171,6 +171,9 @@ struct device *bus_find_device(struct bus_type *bus, struct device *start,
struct device *bus_find_device_by_name(struct bus_type *bus,
struct device *start,
const char *name);
+struct device *bus_find_device_by_fwnode(struct bus_type *bus,
+ struct device *start,
+ struct fwnode_handle *fwnode);
struct device *subsys_find_device_by_id(struct bus_type *bus, unsigned int id,
struct device *hint);
int bus_for_each_drv(struct bus_type *bus, struct device_driver *start,
--
1.9.1



2018-10-09 10:19:41

by Silesh C V

[permalink] [raw]
Subject: [PATCH v3 2/2] treewide: use bus_find_device_by_fwnode

Use bus_find_device_by_fwnode helper to find the device having a
specific firmware node on a bus.

Signed-off-by: Silesh C V <[email protected]>
Reviewed-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
Changes since v2:
- Add Reviewed-by tags

drivers/hwtracing/coresight/of_coresight.c | 14 ++++----------
drivers/i2c/i2c-core-of.c | 9 ++-------
drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 8 +-------
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 8 +-------
drivers/nvmem/core.c | 7 +------
drivers/of/of_mdio.c | 8 +-------
drivers/of/platform.c | 7 +------
drivers/spi/spi.c | 10 +++-------
8 files changed, 14 insertions(+), 57 deletions(-)

diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
index 6880bee..8193b50 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -17,12 +17,6 @@
#include <linux/cpumask.h>
#include <asm/smp_plat.h>

-
-static int of_dev_node_match(struct device *dev, void *data)
-{
- return dev->of_node == data;
-}
-
static struct device *
of_coresight_get_endpoint_device(struct device_node *endpoint)
{
@@ -32,8 +26,8 @@ static int of_dev_node_match(struct device *dev, void *data)
* If we have a non-configurable replicator, it will be found on the
* platform bus.
*/
- dev = bus_find_device(&platform_bus_type, NULL,
- endpoint, of_dev_node_match);
+ dev = bus_find_device_by_fwnode(&platform_bus_type, NULL,
+ &endpoint->fwnode);
if (dev)
return dev;

@@ -41,8 +35,8 @@ static int of_dev_node_match(struct device *dev, void *data)
* We have a configurable component - circle through the AMBA bus
* looking for the device that matches the endpoint node.
*/
- return bus_find_device(&amba_bustype, NULL,
- endpoint, of_dev_node_match);
+ return bus_find_device_by_fwnode(&amba_bustype, NULL,
+ &endpoint->fwnode);
}

static void of_coresight_get_ports(const struct device_node *node,
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index 6cb7ad6..2b8ef8d 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -116,18 +116,13 @@ void of_i2c_register_devices(struct i2c_adapter *adap)
of_node_put(bus);
}

-static int of_dev_node_match(struct device *dev, void *data)
-{
- return dev->of_node == data;
-}
-
/* 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(&i2c_bus_type, NULL, node, of_dev_node_match);
+ dev = bus_find_device_by_fwnode(&i2c_bus_type, NULL, &node->fwnode);
if (!dev)
return NULL;

@@ -145,7 +140,7 @@ 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_node_match);
+ dev = bus_find_device_by_fwnode(&i2c_bus_type, NULL, &node->fwnode);
if (!dev)
return NULL;

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 081aa91..b0d418e 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -4832,19 +4832,13 @@ static void hns_roce_v1_cleanup_eq_table(struct hns_roce_dev *hr_dev)
};
MODULE_DEVICE_TABLE(acpi, hns_roce_acpi_match);

-static int hns_roce_node_match(struct device *dev, void *fwnode)
-{
- return dev->fwnode == fwnode;
-}
-
static struct
platform_device *hns_roce_find_pdev(struct fwnode_handle *fwnode)
{
struct device *dev;

/* get the 'device' corresponding to the matching 'fwnode' */
- dev = bus_find_device(&platform_bus_type, NULL,
- fwnode, hns_roce_node_match);
+ dev = bus_find_device_by_fwnode(&platform_bus_type, NULL, fwnode);
/* get the platform device */
return dev ? to_platform_device(dev) : NULL;
}
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
index 16294cd..d5d7c88 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
@@ -758,17 +758,11 @@ struct dsaf_misc_op *hns_misc_op_get(struct dsaf_device *dsaf_dev)
return (void *)misc_op;
}

-static int hns_dsaf_dev_match(struct device *dev, void *fwnode)
-{
- return dev->fwnode == fwnode;
-}
-
struct
platform_device *hns_dsaf_find_platform_device(struct fwnode_handle *fwnode)
{
struct device *dev;

- dev = bus_find_device(&platform_bus_type, NULL,
- fwnode, hns_dsaf_dev_match);
+ dev = bus_find_device_by_fwnode(&platform_bus_type, NULL, fwnode);
return dev ? to_platform_device(dev) : NULL;
}
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index aa16578..b62f236 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -262,11 +262,6 @@ static void nvmem_release(struct device *dev)
.name = "nvmem",
};

-static int of_nvmem_match(struct device *dev, void *nvmem_np)
-{
- return dev->of_node == nvmem_np;
-}
-
static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
{
struct device *d;
@@ -274,7 +269,7 @@ static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
if (!nvmem_np)
return NULL;

- d = bus_find_device(&nvmem_bus_type, NULL, nvmem_np, of_nvmem_match);
+ d = bus_find_device_by_fwnode(&nvmem_bus_type, NULL, &nvmem_np->fwnode);

if (!d)
return NULL;
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index e92391d..2906a6b 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -282,12 +282,6 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
}
EXPORT_SYMBOL(of_mdiobus_register);

-/* Helper function for of_phy_find_device */
-static int of_phy_match(struct device *dev, void *phy_np)
-{
- return dev->of_node == phy_np;
-}
-
/**
* of_phy_find_device - Give a PHY node, find the phy_device
* @phy_np: Pointer to the phy's device tree node
@@ -303,7 +297,7 @@ struct phy_device *of_phy_find_device(struct device_node *phy_np)
if (!phy_np)
return NULL;

- d = bus_find_device(&mdio_bus_type, NULL, phy_np, of_phy_match);
+ d = bus_find_device_by_fwnode(&mdio_bus_type, NULL, &phy_np->fwnode);
if (d) {
mdiodev = to_mdio_device(d);
if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 6c59673..36dd58e 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -37,11 +37,6 @@
{} /* Empty terminated list */
};

-static int of_dev_node_match(struct device *dev, void *data)
-{
- return dev->of_node == data;
-}
-
/**
* of_find_device_by_node - Find the platform_device associated with a node
* @np: Pointer to device tree node
@@ -55,7 +50,7 @@ struct platform_device *of_find_device_by_node(struct device_node *np)
{
struct device *dev;

- dev = bus_find_device(&platform_bus_type, NULL, np, of_dev_node_match);
+ dev = bus_find_device_by_fwnode(&platform_bus_type, NULL, &np->fwnode);
return dev ? to_platform_device(dev) : NULL;
}
EXPORT_SYMBOL(of_find_device_by_node);
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9da0bc5..97128a5 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3324,16 +3324,12 @@ int spi_write_then_read(struct spi_device *spi,
/*-------------------------------------------------------------------------*/

#if IS_ENABLED(CONFIG_OF_DYNAMIC)
-static int __spi_of_device_match(struct device *dev, void *data)
-{
- return dev->of_node == data;
-}
-
/* must call put_device() when done with returned spi_device device */
static struct spi_device *of_find_spi_device_by_node(struct device_node *node)
{
- struct device *dev = bus_find_device(&spi_bus_type, NULL, node,
- __spi_of_device_match);
+ struct device *dev = bus_find_device_by_fwnode(&spi_bus_type, NULL,
+ &node->fwnode);
+
return dev ? to_spi_device(dev) : NULL;
}

--
1.9.1


2018-10-09 10:21:32

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Driver core: add bus_find_device_by_fwnode

On Tue, Oct 9, 2018 at 12:18 PM Silesh C V <[email protected]> wrote:
>
> Some drivers need to find the device on a bus having a specific firmware
> node. Currently, such drivers have their own implementations to do this.
> Provide a helper similar to bus_find_device_by_name so that each driver
> does not have to reinvent this.
>
> Signed-off-by: Silesh C V <[email protected]>

Reviewed-by: Rafael J. Wysocki <[email protected]>

> ---
> Changes since v2:
> - make use of dev_fwnode in match_fwnode.
>
> drivers/base/bus.c | 20 ++++++++++++++++++++
> include/linux/device.h | 3 +++
> 2 files changed, 23 insertions(+)
>
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index 8bfd27e..a2f39db 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -17,6 +17,7 @@
> #include <linux/string.h>
> #include <linux/mutex.h>
> #include <linux/sysfs.h>
> +#include <linux/property.h>
> #include "base.h"
> #include "power/power.h"
>
> @@ -373,6 +374,25 @@ struct device *bus_find_device_by_name(struct bus_type *bus,
> }
> EXPORT_SYMBOL_GPL(bus_find_device_by_name);
>
> +static int match_fwnode(struct device *dev, void *fwnode)
> +{
> + return dev_fwnode(dev) == fwnode;
> +}
> +
> +/**
> + * bus_find_device_by_fwnode - device iterator for locating a particular device
> + * having a specific firmware node
> + * @bus: bus type
> + * @start: Device to begin with
> + * @fwnode: firmware node of the device to match
> + */
> +struct device *bus_find_device_by_fwnode(struct bus_type *bus, struct device *start,
> + struct fwnode_handle *fwnode)
> +{
> + return bus_find_device(bus, start, (void *)fwnode, match_fwnode);
> +}
> +EXPORT_SYMBOL_GPL(bus_find_device_by_fwnode);
> +
> /**
> * subsys_find_device_by_id - find a device with a specific enumeration number
> * @subsys: subsystem
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 8f88254..09384f6 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -171,6 +171,9 @@ struct device *bus_find_device(struct bus_type *bus, struct device *start,
> struct device *bus_find_device_by_name(struct bus_type *bus,
> struct device *start,
> const char *name);
> +struct device *bus_find_device_by_fwnode(struct bus_type *bus,
> + struct device *start,
> + struct fwnode_handle *fwnode);
> struct device *subsys_find_device_by_id(struct bus_type *bus, unsigned int id,
> struct device *hint);
> int bus_for_each_drv(struct bus_type *bus, struct device_driver *start,
> --
> 1.9.1
>

2018-10-09 11:04:15

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Driver core: add bus_find_device_by_fwnode

On Tue, Oct 09, 2018 at 03:47:24PM +0530, Silesh C V wrote:
> Some drivers need to find the device on a bus having a specific firmware
> node. Currently, such drivers have their own implementations to do this.
> Provide a helper similar to bus_find_device_by_name so that each driver
> does not have to reinvent this.
>
> Signed-off-by: Silesh C V <[email protected]>

Looks good in general, however:

We recently had this discussion in I2C world about using the parent if
the (logical) device has a NULL fw_node [1]. I don't know if the other
subsystems you modify use logical devices as well? If no, it seems we
need an additional check for the parent in the I2C core only. If yes,
this might be considered in your patchset?

Thanks,

Wolfram

[1] http://patchwork.ozlabs.org/patch/974584/


Attachments:
(No filename) (822.00 B)
signature.asc (849.00 B)
Download all attachments

2018-10-09 15:16:49

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Driver core: add bus_find_device_by_fwnode

On Tue, Oct 09, 2018 at 01:02:10PM +0200, Wolfram Sang wrote:

> We recently had this discussion in I2C world about using the parent if
> the (logical) device has a NULL fw_node [1]. I don't know if the other
> subsystems you modify use logical devices as well? If no, it seems we
> need an additional check for the parent in the I2C core only. If yes,
> this might be considered in your patchset?

SPI has a logical controller device as well.


Attachments:
(No filename) (453.00 B)
signature.asc (499.00 B)
Download all attachments

2018-10-09 17:29:24

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Driver core: add bus_find_device_by_fwnode

Hi Silesh,

On Tue, Oct 09, 2018 at 03:47:24PM +0530, Silesh C V wrote:
> Some drivers need to find the device on a bus having a specific firmware
> node. Currently, such drivers have their own implementations to do this.
> Provide a helper similar to bus_find_device_by_name so that each driver
> does not have to reinvent this.
>
> Signed-off-by: Silesh C V <[email protected]>
> ---
> Changes since v2:
> - make use of dev_fwnode in match_fwnode.
>
> drivers/base/bus.c | 20 ++++++++++++++++++++
> include/linux/device.h | 3 +++
> 2 files changed, 23 insertions(+)
>
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index 8bfd27e..a2f39db 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -17,6 +17,7 @@
> #include <linux/string.h>
> #include <linux/mutex.h>
> #include <linux/sysfs.h>
> +#include <linux/property.h>
> #include "base.h"
> #include "power/power.h"
>
> @@ -373,6 +374,25 @@ struct device *bus_find_device_by_name(struct bus_type *bus,
> }
> EXPORT_SYMBOL_GPL(bus_find_device_by_name);
>
> +static int match_fwnode(struct device *dev, void *fwnode)
> +{
> + return dev_fwnode(dev) == fwnode;
> +}
> +
> +/**
> + * bus_find_device_by_fwnode - device iterator for locating a particular device
> + * having a specific firmware node
> + * @bus: bus type
> + * @start: Device to begin with
> + * @fwnode: firmware node of the device to match
> + */
> +struct device *bus_find_device_by_fwnode(struct bus_type *bus, struct device *start,
> + struct fwnode_handle *fwnode)

I get the following when running checkpatch on your set:

mpoirier@xps15:~/work/linaro/coresight/kernel-maint$ ./scripts/checkpatch.pl
0001-Driver-core-add-bus_find_device_by_fwnode.patch
WARNING: line over 80 characters
#45: FILE: drivers/base/bus.c:389:
+struct device *bus_find_device_by_fwnode(struct bus_type *bus, struct device
*start,

total: 0 errors, 1 warnings, 41 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

0001-Driver-core-add-bus_find_device_by_fwnode.patch has style problems, please
review.

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

Regards,
Mathieu

> +{
> + return bus_find_device(bus, start, (void *)fwnode, match_fwnode);
> +}
> +EXPORT_SYMBOL_GPL(bus_find_device_by_fwnode);
> +
> /**
> * subsys_find_device_by_id - find a device with a specific enumeration number
> * @subsys: subsystem
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 8f88254..09384f6 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -171,6 +171,9 @@ struct device *bus_find_device(struct bus_type *bus, struct device *start,
> struct device *bus_find_device_by_name(struct bus_type *bus,
> struct device *start,
> const char *name);
> +struct device *bus_find_device_by_fwnode(struct bus_type *bus,
> + struct device *start,
> + struct fwnode_handle *fwnode);
> struct device *subsys_find_device_by_id(struct bus_type *bus, unsigned int id,
> struct device *hint);
> int bus_for_each_drv(struct bus_type *bus, struct device_driver *start,
> --
> 1.9.1
>

2018-10-09 17:39:54

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Driver core: add bus_find_device_by_fwnode

On Tue, Oct 9, 2018 at 7:27 PM Mathieu Poirier
<[email protected]> wrote:
>
> Hi Silesh,
>
> On Tue, Oct 09, 2018 at 03:47:24PM +0530, Silesh C V wrote:
> > Some drivers need to find the device on a bus having a specific firmware
> > node. Currently, such drivers have their own implementations to do this.
> > Provide a helper similar to bus_find_device_by_name so that each driver
> > does not have to reinvent this.
> >
> > Signed-off-by: Silesh C V <[email protected]>
> > ---
> > Changes since v2:
> > - make use of dev_fwnode in match_fwnode.
> >
> > drivers/base/bus.c | 20 ++++++++++++++++++++
> > include/linux/device.h | 3 +++
> > 2 files changed, 23 insertions(+)
> >
> > diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> > index 8bfd27e..a2f39db 100644
> > --- a/drivers/base/bus.c
> > +++ b/drivers/base/bus.c
> > @@ -17,6 +17,7 @@
> > #include <linux/string.h>
> > #include <linux/mutex.h>
> > #include <linux/sysfs.h>
> > +#include <linux/property.h>
> > #include "base.h"
> > #include "power/power.h"
> >
> > @@ -373,6 +374,25 @@ struct device *bus_find_device_by_name(struct bus_type *bus,
> > }
> > EXPORT_SYMBOL_GPL(bus_find_device_by_name);
> >
> > +static int match_fwnode(struct device *dev, void *fwnode)
> > +{
> > + return dev_fwnode(dev) == fwnode;
> > +}
> > +
> > +/**
> > + * bus_find_device_by_fwnode - device iterator for locating a particular device
> > + * having a specific firmware node
> > + * @bus: bus type
> > + * @start: Device to begin with
> > + * @fwnode: firmware node of the device to match
> > + */
> > +struct device *bus_find_device_by_fwnode(struct bus_type *bus, struct device *start,
> > + struct fwnode_handle *fwnode)
>
> I get the following when running checkpatch on your set:
>
> mpoirier@xps15:~/work/linaro/coresight/kernel-maint$ ./scripts/checkpatch.pl
> 0001-Driver-core-add-bus_find_device_by_fwnode.patch
> WARNING: line over 80 characters

Lines longer than 80 chars often are legitimate. No need to send
extra reports about those cases in general.

Thanks,
Rafael

2018-10-09 17:45:19

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] treewide: use bus_find_device_by_fwnode

On Tue, Oct 09, 2018 at 03:47:25PM +0530, Silesh C V wrote:
> Use bus_find_device_by_fwnode helper to find the device having a
> specific firmware node on a bus.
>
> Signed-off-by: Silesh C V <[email protected]>
> Reviewed-by: Rafael J. Wysocki <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> ---
> Changes since v2:
> - Add Reviewed-by tags
>
> drivers/hwtracing/coresight/of_coresight.c | 14 ++++----------
> drivers/i2c/i2c-core-of.c | 9 ++-------
> drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 8 +-------
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 8 +-------
> drivers/nvmem/core.c | 7 +------
> drivers/of/of_mdio.c | 8 +-------
> drivers/of/platform.c | 7 +------
> drivers/spi/spi.c | 10 +++-------
> 8 files changed, 14 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
> index 6880bee..8193b50 100644
> --- a/drivers/hwtracing/coresight/of_coresight.c
> +++ b/drivers/hwtracing/coresight/of_coresight.c
> @@ -17,12 +17,6 @@
> #include <linux/cpumask.h>
> #include <asm/smp_plat.h>
>
> -
> -static int of_dev_node_match(struct device *dev, void *data)
> -{
> - return dev->of_node == data;
> -}
> -
> static struct device *
> of_coresight_get_endpoint_device(struct device_node *endpoint)
> {
> @@ -32,8 +26,8 @@ static int of_dev_node_match(struct device *dev, void *data)
> * If we have a non-configurable replicator, it will be found on the
> * platform bus.
> */
> - dev = bus_find_device(&platform_bus_type, NULL,
> - endpoint, of_dev_node_match);
> + dev = bus_find_device_by_fwnode(&platform_bus_type, NULL,
> + &endpoint->fwnode);
> if (dev)
> return dev;
>
> @@ -41,8 +35,8 @@ static int of_dev_node_match(struct device *dev, void *data)
> * We have a configurable component - circle through the AMBA bus
> * looking for the device that matches the endpoint node.
> */
> - return bus_find_device(&amba_bustype, NULL,
> - endpoint, of_dev_node_match);
> + return bus_find_device_by_fwnode(&amba_bustype, NULL,
> + &endpoint->fwnode);

This doesn't apply on linux-next. This late in the cycle you may want to rebase
your work on that tree.

Otherwise and for the coresight part:

Tested-by: Mathieu Poirier <[email protected]>

Mathieu

> }
>
> static void of_coresight_get_ports(const struct device_node *node,
> diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
> index 6cb7ad6..2b8ef8d 100644
> --- a/drivers/i2c/i2c-core-of.c
> +++ b/drivers/i2c/i2c-core-of.c
> @@ -116,18 +116,13 @@ void of_i2c_register_devices(struct i2c_adapter *adap)
> of_node_put(bus);
> }
>
> -static int of_dev_node_match(struct device *dev, void *data)
> -{
> - return dev->of_node == data;
> -}
> -
> /* 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(&i2c_bus_type, NULL, node, of_dev_node_match);
> + dev = bus_find_device_by_fwnode(&i2c_bus_type, NULL, &node->fwnode);
> if (!dev)
> return NULL;
>
> @@ -145,7 +140,7 @@ 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_node_match);
> + dev = bus_find_device_by_fwnode(&i2c_bus_type, NULL, &node->fwnode);
> if (!dev)
> return NULL;
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 081aa91..b0d418e 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -4832,19 +4832,13 @@ static void hns_roce_v1_cleanup_eq_table(struct hns_roce_dev *hr_dev)
> };
> MODULE_DEVICE_TABLE(acpi, hns_roce_acpi_match);
>
> -static int hns_roce_node_match(struct device *dev, void *fwnode)
> -{
> - return dev->fwnode == fwnode;
> -}
> -
> static struct
> platform_device *hns_roce_find_pdev(struct fwnode_handle *fwnode)
> {
> struct device *dev;
>
> /* get the 'device' corresponding to the matching 'fwnode' */
> - dev = bus_find_device(&platform_bus_type, NULL,
> - fwnode, hns_roce_node_match);
> + dev = bus_find_device_by_fwnode(&platform_bus_type, NULL, fwnode);
> /* get the platform device */
> return dev ? to_platform_device(dev) : NULL;
> }
> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
> index 16294cd..d5d7c88 100644
> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
> @@ -758,17 +758,11 @@ struct dsaf_misc_op *hns_misc_op_get(struct dsaf_device *dsaf_dev)
> return (void *)misc_op;
> }
>
> -static int hns_dsaf_dev_match(struct device *dev, void *fwnode)
> -{
> - return dev->fwnode == fwnode;
> -}
> -
> struct
> platform_device *hns_dsaf_find_platform_device(struct fwnode_handle *fwnode)
> {
> struct device *dev;
>
> - dev = bus_find_device(&platform_bus_type, NULL,
> - fwnode, hns_dsaf_dev_match);
> + dev = bus_find_device_by_fwnode(&platform_bus_type, NULL, fwnode);
> return dev ? to_platform_device(dev) : NULL;
> }
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index aa16578..b62f236 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -262,11 +262,6 @@ static void nvmem_release(struct device *dev)
> .name = "nvmem",
> };
>
> -static int of_nvmem_match(struct device *dev, void *nvmem_np)
> -{
> - return dev->of_node == nvmem_np;
> -}
> -
> static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
> {
> struct device *d;
> @@ -274,7 +269,7 @@ static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
> if (!nvmem_np)
> return NULL;
>
> - d = bus_find_device(&nvmem_bus_type, NULL, nvmem_np, of_nvmem_match);
> + d = bus_find_device_by_fwnode(&nvmem_bus_type, NULL, &nvmem_np->fwnode);
>
> if (!d)
> return NULL;
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index e92391d..2906a6b 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -282,12 +282,6 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
> }
> EXPORT_SYMBOL(of_mdiobus_register);
>
> -/* Helper function for of_phy_find_device */
> -static int of_phy_match(struct device *dev, void *phy_np)
> -{
> - return dev->of_node == phy_np;
> -}
> -
> /**
> * of_phy_find_device - Give a PHY node, find the phy_device
> * @phy_np: Pointer to the phy's device tree node
> @@ -303,7 +297,7 @@ struct phy_device *of_phy_find_device(struct device_node *phy_np)
> if (!phy_np)
> return NULL;
>
> - d = bus_find_device(&mdio_bus_type, NULL, phy_np, of_phy_match);
> + d = bus_find_device_by_fwnode(&mdio_bus_type, NULL, &phy_np->fwnode);
> if (d) {
> mdiodev = to_mdio_device(d);
> if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY)
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 6c59673..36dd58e 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -37,11 +37,6 @@
> {} /* Empty terminated list */
> };
>
> -static int of_dev_node_match(struct device *dev, void *data)
> -{
> - return dev->of_node == data;
> -}
> -
> /**
> * of_find_device_by_node - Find the platform_device associated with a node
> * @np: Pointer to device tree node
> @@ -55,7 +50,7 @@ struct platform_device *of_find_device_by_node(struct device_node *np)
> {
> struct device *dev;
>
> - dev = bus_find_device(&platform_bus_type, NULL, np, of_dev_node_match);
> + dev = bus_find_device_by_fwnode(&platform_bus_type, NULL, &np->fwnode);
> return dev ? to_platform_device(dev) : NULL;
> }
> EXPORT_SYMBOL(of_find_device_by_node);
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 9da0bc5..97128a5 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -3324,16 +3324,12 @@ int spi_write_then_read(struct spi_device *spi,
> /*-------------------------------------------------------------------------*/
>
> #if IS_ENABLED(CONFIG_OF_DYNAMIC)
> -static int __spi_of_device_match(struct device *dev, void *data)
> -{
> - return dev->of_node == data;
> -}
> -
> /* must call put_device() when done with returned spi_device device */
> static struct spi_device *of_find_spi_device_by_node(struct device_node *node)
> {
> - struct device *dev = bus_find_device(&spi_bus_type, NULL, node,
> - __spi_of_device_match);
> + struct device *dev = bus_find_device_by_fwnode(&spi_bus_type, NULL,
> + &node->fwnode);
> +
> return dev ? to_spi_device(dev) : NULL;
> }
>
> --
> 1.9.1
>

2018-10-09 17:49:16

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Driver core: add bus_find_device_by_fwnode

On Tue, 9 Oct 2018 at 11:39, Rafael J. Wysocki <[email protected]> wrote:
>
> On Tue, Oct 9, 2018 at 7:27 PM Mathieu Poirier
> <[email protected]> wrote:
> >
> > Hi Silesh,
> >
> > On Tue, Oct 09, 2018 at 03:47:24PM +0530, Silesh C V wrote:
> > > Some drivers need to find the device on a bus having a specific firmware
> > > node. Currently, such drivers have their own implementations to do this.
> > > Provide a helper similar to bus_find_device_by_name so that each driver
> > > does not have to reinvent this.
> > >
> > > Signed-off-by: Silesh C V <[email protected]>
> > > ---
> > > Changes since v2:
> > > - make use of dev_fwnode in match_fwnode.
> > >
> > > drivers/base/bus.c | 20 ++++++++++++++++++++
> > > include/linux/device.h | 3 +++
> > > 2 files changed, 23 insertions(+)
> > >
> > > diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> > > index 8bfd27e..a2f39db 100644
> > > --- a/drivers/base/bus.c
> > > +++ b/drivers/base/bus.c
> > > @@ -17,6 +17,7 @@
> > > #include <linux/string.h>
> > > #include <linux/mutex.h>
> > > #include <linux/sysfs.h>
> > > +#include <linux/property.h>
> > > #include "base.h"
> > > #include "power/power.h"
> > >
> > > @@ -373,6 +374,25 @@ struct device *bus_find_device_by_name(struct bus_type *bus,
> > > }
> > > EXPORT_SYMBOL_GPL(bus_find_device_by_name);
> > >
> > > +static int match_fwnode(struct device *dev, void *fwnode)
> > > +{
> > > + return dev_fwnode(dev) == fwnode;
> > > +}
> > > +
> > > +/**
> > > + * bus_find_device_by_fwnode - device iterator for locating a particular device
> > > + * having a specific firmware node
> > > + * @bus: bus type
> > > + * @start: Device to begin with
> > > + * @fwnode: firmware node of the device to match
> > > + */
> > > +struct device *bus_find_device_by_fwnode(struct bus_type *bus, struct device *start,
> > > + struct fwnode_handle *fwnode)
> >
> > I get the following when running checkpatch on your set:
> >
> > mpoirier@xps15:~/work/linaro/coresight/kernel-maint$ ./scripts/checkpatch.pl
> > 0001-Driver-core-add-bus_find_device_by_fwnode.patch
> > WARNING: line over 80 characters
>
> Lines longer than 80 chars often are legitimate. No need to send
> extra reports about those cases in general.

In this case I don't see a reason not to abide to the guideline.
Wrapping the function declaration to 80 characters would be easy
without effecting code readability.

Mathieu

>
> Thanks,
> Rafael

2018-10-10 02:52:52

by Silesh C V

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Driver core: add bus_find_device_by_fwnode

Hello Wolfram,

On Tue, Oct 9, 2018 at 4:32 PM Wolfram Sang <[email protected]> wrote:
>
> On Tue, Oct 09, 2018 at 03:47:24PM +0530, Silesh C V wrote:
> > Some drivers need to find the device on a bus having a specific firmware
> > node. Currently, such drivers have their own implementations to do this.
> > Provide a helper similar to bus_find_device_by_name so that each driver
> > does not have to reinvent this.
> >
> > Signed-off-by: Silesh C V <[email protected]>
>
> Looks good in general, however:
>
> We recently had this discussion in I2C world about using the parent if
> the (logical) device has a NULL fw_node [1]. I don't know if the other
> subsystems you modify use logical devices as well? If no, it seems we
> need an additional check for the parent in the I2C core only. If yes,
> this might be considered in your patchset?

We can add an additional check for dev_fwnode(dev->parent) if match
for dev_fwnode(dev) fails in match_fwnode callback. Please correct me
if I am wrong. But then, we will be doing these two comparisons for
each device (until a match is found) on a bus for a bus_find_device
iteration(mostly unnecessarily because I could not find any "match
node" callbacks (for bus_find_device) currently doing this). So,
wouldn't it be better to add this check in exceptional cases (like in
the case of I2C) in the respective subsystems itself ?

Thanks,
Silesh

2018-10-10 02:56:33

by Silesh C V

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Driver core: add bus_find_device_by_fwnode

Hello Mathieu,

Thank you for the review.

On Tue, Oct 9, 2018 at 11:18 PM Mathieu Poirier
<[email protected]> wrote:
>
>
> In this case I don't see a reason not to abide to the guideline.
> Wrapping the function declaration to 80 characters would be easy
> without effecting code readability.
>

I had seen this warning but thought it looked better as is. But now,
as I have to resend the patches after rebasing on linux-next, I will
fix this too in v4.

Thanks,
Silesh

2018-10-10 03:16:32

by Silesh C V

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] treewide: use bus_find_device_by_fwnode

Hello Mathieu,

On Tue, Oct 9, 2018 at 11:14 PM Mathieu Poirier
<[email protected]> wrote:
>
>
> This doesn't apply on linux-next. This late in the cycle you may want to rebase
> your work on that tree.

OK. Will send v4 after rebasing onto linux-next.

>
> Otherwise and for the coresight part:
>
> Tested-by: Mathieu Poirier <[email protected]>
>

Thanks.

Regards,
Silesh.

2018-10-10 19:06:45

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] treewide: use bus_find_device_by_fwnode

On Tue, Oct 9, 2018 at 9:59 PM Silesh C V <[email protected]> wrote:
>
> Hello Mathieu,
>
> On Tue, Oct 9, 2018 at 11:14 PM Mathieu Poirier
> <[email protected]> wrote:
> >
> >
> > This doesn't apply on linux-next. This late in the cycle you may want to rebase
> > your work on that tree.
>
> OK. Will send v4 after rebasing onto linux-next.

That doesn't help anything unless you are targeting 4.21. If you are
it would be easiest to apply patch 1 in 4.20 and split this patch by
subsystem for 4.21. That avoids any cross subsystem dependencies.

Rob