LinuxLists.cc - [PATCH v1 0/10] Fixes and API additions for firmware nodes

[permalink] [raw]

Subject: [PATCH v1 01/10] of/platform: Set fwnode field for new devices

When allocating a new platform device, set the fwnode field in the
struct device to point to the device_node.

Signed-off-by: Tomeu Vizoso <[email protected]>
---

drivers/of/platform.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index ddf8e42..e880f55 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -139,6 +139,7 @@ struct platform_device *of_device_alloc(struct device_node *np,
}

dev->dev.of_node = of_node_get(np);
+ dev->dev.fwnode = &dev->dev.of_node->fwnode;
dev->dev.parent = parent ? : &platform_bus;

if (bus_id)
--
2.4.1

2015-06-30 14:57:04

[permalink] [raw]

Subject: [PATCH v1 02/10] i2c: core: Have clients point to their firmware nodes

This is needed by platform-independent code that needs to do something
with devices based on the data provided by the firmware.

Signed-off-by: Tomeu Vizoso <[email protected]>
---

drivers/i2c/i2c-core.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 069a41f..707850a 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1295,6 +1295,7 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
}

info.of_node = of_node_get(node);
+ info.fwnode = info.of_node ? &info.of_node->fwnode : NULL;
info.archdata = &dev_ad;

if (of_get_property(node, "wakeup-source", NULL))
--
2.4.1

2015-06-30 14:57:23

[permalink] [raw]

Subject: [PATCH v1 03/10] mfd: Have child devices point to their firmware nodes

This is needed by platform-independent code that needs to do something
with devices based on the data provided by the firmware.

Signed-off-by: Tomeu Vizoso <[email protected]>
---

drivers/mfd/mfd-core.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 14fd5cb..e2422ea 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -158,6 +158,7 @@ static int mfd_add_device(struct device *parent, int id,
for_each_child_of_node(parent->of_node, np) {
if (of_device_is_compatible(np, cell->of_compatible)) {
pdev->dev.of_node = np;
+ pdev->dev.fwnode = &pdev->dev.of_node->fwnode;
break;
}
}
--
2.4.1

2015-06-30 14:59:23

[permalink] [raw]

Subject: [PATCH v1 04/10] device property: add fwnode_get_parent()

So we can query the parent of a fwnode without having to resort to API
that is specific to a firmware data format.

Also adds a acpi_get_parent_dev() function to retrieve the parent
of an acpi_device. acpi_get_parent() already existed but it works with
acpi_handles.

The interface covers both ACPI and Device Trees.

Signed-off-by: Tomeu Vizoso <[email protected]>
---

drivers/base/property.c | 23 +++++++++++++++++++++++
include/acpi/acpi_bus.h | 5 +++++
include/linux/acpi.h | 5 +++++
include/linux/property.h | 2 ++
4 files changed, 35 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index f3f6d16..a7fb46b 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -533,3 +533,26 @@ bool device_dma_is_coherent(struct device *dev)
return coherent;
}
EXPORT_SYMBOL_GPL(device_dma_is_coherent);
+
+/**
+ * fwnode_get_parent - return the parent node of a device node
+ * @fwnode: Device node to find the parent node of
+ */
+struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode)
+{
+ if (is_of_node(fwnode)) {
+ struct device_node *node = to_of_node(fwnode);
+
+ if (node->parent)
+ return &node->parent->fwnode;
+ } else if (is_acpi_node(fwnode)) {
+ struct acpi_device *node;
+
+ node = acpi_get_parent_dev(to_acpi_node(fwnode));
+ if (node)
+ return acpi_fwnode_handle(node);
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_parent);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 83061ca..93c42a6 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -436,6 +436,11 @@ static inline void *acpi_driver_data(struct acpi_device *d)
return d->driver_data;
}

+static inline struct acpi_device *acpi_get_parent_dev(struct acpi_device *adev)
+{
+ return adev->parent;
+}
+
#define to_acpi_device(d) container_of(d, struct acpi_device, dev)
#define to_acpi_driver(d) container_of(d, struct acpi_driver, drv)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index ed79b06..fc84e42 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -804,6 +804,11 @@ static inline struct acpi_device *acpi_get_next_child(struct device *dev,
return NULL;
}

+static inline struct acpi_device *acpi_get_parent_dev(struct acpi_device *adev)
+{
+ return NULL;
+}
+
#endif

#endif /*_LINUX_ACPI_H*/
diff --git a/include/linux/property.h b/include/linux/property.h
index 76ebde9..f47092d 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -63,6 +63,8 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
struct fwnode_handle *device_get_next_child_node(struct device *dev,
struct fwnode_handle *child);

+struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);
+
#define device_for_each_child_node(dev, child) \
for (child = device_get_next_child_node(dev, NULL); child; \
child = device_get_next_child_node(dev, child))
--
2.4.1

2015-06-30 14:57:55

[permalink] [raw]

Subject: [PATCH v1 05/10] device property: add fwnode_get_name()

Getting a textual representation of a device node can be very useful for
debugging.

Signed-off-by: Tomeu Vizoso <[email protected]>
---

drivers/base/property.c | 15 +++++++++++++++
include/linux/property.h | 2 ++
2 files changed, 17 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index a7fb46b..3280b04 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -556,3 +556,18 @@ struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode)
return NULL;
}
EXPORT_SYMBOL_GPL(fwnode_get_parent);
+
+/**
+ * fwnode_get_name - return the name of a device node
+ * @fwnode: Device node to find the name of
+ */
+const char *fwnode_get_name(struct fwnode_handle *fwnode)
+{
+ if (is_of_node(fwnode))
+ return to_of_node(fwnode)->full_name;
+ else if (is_acpi_node(fwnode))
+ return acpi_dev_name(to_acpi_node(fwnode));
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_name);
diff --git a/include/linux/property.h b/include/linux/property.h
index f47092d..020a53c 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -65,6 +65,8 @@ struct fwnode_handle *device_get_next_child_node(struct device *dev,

struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);

+const char *fwnode_get_name(struct fwnode_handle *fwnode);
+
#define device_for_each_child_node(dev, child) \
for (child = device_get_next_child_node(dev, NULL); child; \
child = device_get_next_child_node(dev, child))
--
2.4.1

2015-06-30 14:57:32

[permalink] [raw]

Subject: [PATCH v1 06/10] ACPI / scan: Add acpi_dev_get_next_child()

So fwnode_get_next_child_node() can be implemented for ACPI firmware
nodes.

This re-implements acpi_get_next_child() in terms of
acpi_dev_get_next_child().

Signed-off-by: Tomeu Vizoso <[email protected]>
---

drivers/acpi/scan.c | 5 ++---
include/linux/acpi.h | 17 +++++++++++++++--
2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 2649a06..45cf1b7 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1482,10 +1482,9 @@ int acpi_device_add(struct acpi_device *device,
return result;
}

-struct acpi_device *acpi_get_next_child(struct device *dev,
- struct acpi_device *child)
+struct acpi_device *acpi_dev_get_next_child(struct acpi_device *adev,
+ struct acpi_device *child)
{
- struct acpi_device *adev = ACPI_COMPANION(dev);
struct list_head *head, *next;

if (!adev)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index fc84e42..2afcdb9 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -752,8 +752,14 @@ int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
enum dev_prop_type proptype, void *val, size_t nval);

-struct acpi_device *acpi_get_next_child(struct device *dev,
- struct acpi_device *child);
+struct acpi_device *acpi_dev_get_next_child(struct acpi_device *adev,
+ struct acpi_device *child);
+
+static inline struct acpi_device *acpi_get_next_child(struct device *dev,
+ struct acpi_device *child)
+{
+ return acpi_dev_get_next_child(ACPI_COMPANION(dev), child);
+}
#else
static inline int acpi_dev_get_property(struct acpi_device *adev,
const char *name, acpi_object_type type,
@@ -804,6 +810,13 @@ static inline struct acpi_device *acpi_get_next_child(struct device *dev,
return NULL;
}

+static inline struct acpi_device *acpi_dev_get_next_child(
+ struct acpi_device *adev,
+ struct acpi_device *child)
+{
+ return NULL;
+}
+
static inline struct acpi_device *acpi_get_parent_dev(struct acpi_device *adev)
{
return NULL;
--
2.4.1

2015-06-30 14:57:39

[permalink] [raw]

Subject: [PATCH v1 07/10] device property: Add fwnode_get_next_child_node()

So callers that have a handle to a firmware node but not any of the
devices related to it can iterate over their descendants.

Also adds fwnode_for_each_child_node().

Signed-off-by: Tomeu Vizoso <[email protected]>
---

drivers/base/property.c | 30 ++++++++++++++++++++++--------
include/linux/property.h | 7 +++++++
2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 3280b04..92cdbb3 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -466,28 +466,42 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
EXPORT_SYMBOL_GPL(fwnode_property_read_string);

/**
- * device_get_next_child_node - Return the next child node handle for a device
- * @dev: Device to find the next child node for.
- * @child: Handle to one of the device's child nodes or a null handle.
+ * fwnode_get_next_child_node - Return the next child node handle for a fwnode
+ * @fwnode: Firmware node to find the next child node for.
+ * @child: Handle to one of the firmware node's child nodes or a null handle.
*/
-struct fwnode_handle *device_get_next_child_node(struct device *dev,
+struct fwnode_handle *fwnode_get_next_child_node(struct fwnode_handle *fwnode,
struct fwnode_handle *child)
{
- if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+ if (is_of_node(fwnode)) {
struct device_node *node;

- node = of_get_next_available_child(dev->of_node, to_of_node(child));
+ node = of_get_next_available_child(to_of_node(fwnode),
+ to_of_node(child));
if (node)
return &node->fwnode;
- } else if (IS_ENABLED(CONFIG_ACPI)) {
+ } else if (is_acpi_node(fwnode)) {
struct acpi_device *node;

- node = acpi_get_next_child(dev, to_acpi_node(child));
+ node = acpi_dev_get_next_child(to_acpi_node(fwnode),
+ to_acpi_node(child));
if (node)
return acpi_fwnode_handle(node);
}
return NULL;
}
+EXPORT_SYMBOL_GPL(fwnode_get_next_child_node);
+
+/**
+ * device_get_next_child_node - Return the next child node handle for a device
+ * @dev: Device to find the next child node for.
+ * @child: Handle to one of the device's child nodes or a null handle.
+ */
+struct fwnode_handle *device_get_next_child_node(struct device *dev,
+ struct fwnode_handle *child)
+{
+ return fwnode_get_next_child_node(dev->fwnode, child);
+}
EXPORT_SYMBOL_GPL(device_get_next_child_node);

/**
diff --git a/include/linux/property.h b/include/linux/property.h
index 020a53c..cfd1eb2 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -63,6 +63,9 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
struct fwnode_handle *device_get_next_child_node(struct device *dev,
struct fwnode_handle *child);

+struct fwnode_handle *fwnode_get_next_child_node(struct fwnode_handle *fwnode,
+ struct fwnode_handle *child);
+
struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);

const char *fwnode_get_name(struct fwnode_handle *fwnode);
@@ -71,6 +74,10 @@ const char *fwnode_get_name(struct fwnode_handle *fwnode);
for (child = device_get_next_child_node(dev, NULL); child; \
child = device_get_next_child_node(dev, child))

+#define fwnode_for_each_child_node(fwnode, child) \
+ for (child = fwnode_get_next_child_node(fwnode, NULL); child; \
+ child = fwnode_get_next_child_node(fwnode, child))
+
void fwnode_handle_put(struct fwnode_handle *fwnode);

unsigned int device_get_child_node_count(struct device *dev);
--
2.4.1

2015-06-30 14:57:48

[permalink] [raw]

Subject: [PATCH v1 08/10] device property: add fwnode_is_compatible()

This is being added so code that is independent of the firmware being
used can match firmware nodes to devices.

This commit only implements it for OF nodes.

Signed-off-by: Tomeu Vizoso <[email protected]>
---

drivers/base/property.c | 9 +++++++++
include/linux/property.h | 2 ++
2 files changed, 11 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 92cdbb3..9c8be31 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -585,3 +585,12 @@ const char *fwnode_get_name(struct fwnode_handle *fwnode)
return NULL;
}
EXPORT_SYMBOL_GPL(fwnode_get_name);
+
+bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible)
+{
+ if (is_of_node(fwnode))
+ return of_device_is_compatible(to_of_node(fwnode), compatible);
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(fwnode_is_compatible);
diff --git a/include/linux/property.h b/include/linux/property.h
index cfd1eb2..bf10074 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -80,6 +80,8 @@ const char *fwnode_get_name(struct fwnode_handle *fwnode);

void fwnode_handle_put(struct fwnode_handle *fwnode);

+bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible);
+
unsigned int device_get_child_node_count(struct device *dev);

static inline bool device_property_read_bool(struct device *dev,
--
2.4.1

2015-06-30 14:58:02

[permalink] [raw]

Subject: [PATCH v1 09/10] device: property: add fwnode_driver_match_device()

So buses can match drivers to devices without having to care about what
firmware is used to describe the device.

The introduction of this function will also allow us to introduce
matching behavior common to the different type of firmwares.

Signed-off-by: Tomeu Vizoso <[email protected]>
---

drivers/base/property.c | 18 ++++++++++++++++++
include/linux/property.h | 4 ++++
2 files changed, 22 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 9c8be31..8528eb9 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -15,6 +15,7 @@
#include <linux/kernel.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/property.h>

/**
@@ -594,3 +595,20 @@ bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible)
return false;
}
EXPORT_SYMBOL_GPL(fwnode_is_compatible);
+
+/**
+ * fwnode_driver_match_device - Tell if a driver matches a device.
+ * @drv: the device_driver structure to test
+ * @dev: the device structure to match against
+ */
+bool fwnode_driver_match_device(struct device *dev,
+ const struct device_driver *drv)
+{
+ if (is_of_node(dev->fwnode))
+ return of_driver_match_device(dev, drv);
+ else if (is_acpi_node(dev->fwnode))
+ return acpi_driver_match_device(dev, drv);
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(fwnode_driver_match_device);
diff --git a/include/linux/property.h b/include/linux/property.h
index bf10074..4e453c4 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -17,6 +17,7 @@
#include <linux/types.h>

struct device;
+struct device_driver;

enum dev_prop_type {
DEV_PROP_U8,
@@ -82,6 +83,9 @@ void fwnode_handle_put(struct fwnode_handle *fwnode);

bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible);

+bool fwnode_driver_match_device(struct device *dev,
+ const struct device_driver *drv);
+
unsigned int device_get_child_node_count(struct device *dev);

static inline bool device_property_read_bool(struct device *dev,
--
2.4.1

2015-06-30 14:59:07

[permalink] [raw]

Subject: [PATCH v1 10/10] core: platform: use fwnode_driver_match_device()

Instead of calling both of_driver_match_device() and
acpi_driver_match_device(), call fwnode_driver_match_device() which
should be able to sort out what firmware describes the device in
question.

Signed-off-by: Tomeu Vizoso <[email protected]>
---

drivers/base/platform.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 063f0ab..a7e7757 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -843,12 +843,8 @@ static int platform_match(struct device *dev, struct device_driver *drv)
if (pdev->driver_override)
return !strcmp(pdev->driver_override, drv->name);

- /* Attempt an OF style match first */
- if (of_driver_match_device(dev, drv))
- return 1;
-
- /* Then try ACPI style match */
- if (acpi_driver_match_device(dev, drv))
+ /* Attempt a firmware match first */
+ if (fwnode_driver_match_device(dev, drv))
return 1;

/* Then try to match against the id table */
--
2.4.1

2015-06-30 15:06:53

[permalink] [raw]

Subject: Re: [PATCH v1 0/10] Fixes and API additions for firmware nodes

On Tuesday, June 30, 2015 04:54:58 PM Tomeu Vizoso wrote:
> Hi,
>
> this series makes it possible to extract device dependencies from firmware data
> with fwnode, thus not depending on the particular format of that data.
>
> The first patches are about making sure that we can reach the fwnode
> handle of the firmware data associated with a given device, and the rest
> add API that makes dependency extraction possible.

It would really help if you sent the whole series to linux-acpi. People on
that list would see the entire context then.

Thanks,
Rafael

2015-06-30 20:25:05

[permalink] [raw]

Subject: Re: [PATCH v1 01/10] of/platform: Set fwnode field for new devices

On Tue, 30 Jun 2015 16:54:59 +0200
, Tomeu Vizoso <[email protected]>
wrote:
> When allocating a new platform device, set the fwnode field in the
> struct device to point to the device_node.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>

Acked-by: Grant Likely <[email protected]>

g.

> ---
>
> drivers/of/platform.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index ddf8e42..e880f55 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -139,6 +139,7 @@ struct platform_device *of_device_alloc(struct device_node *np,
> }
>
> dev->dev.of_node = of_node_get(np);
> + dev->dev.fwnode = &dev->dev.of_node->fwnode;
> dev->dev.parent = parent ? : &platform_bus;
>
> if (bus_id)
> --
> 2.4.1
>

2015-06-30 20:23:00

[permalink] [raw]

Subject: Re: [PATCH v1 03/10] mfd: Have child devices point to their firmware nodes

On Tue, 30 Jun 2015 16:55:01 +0200
, Tomeu Vizoso <[email protected]>
wrote:
> This is needed by platform-independent code that needs to do something
> with devices based on the data provided by the firmware.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>

Acked-by: Grant Likely <[email protected]>
> ---
>
> drivers/mfd/mfd-core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> index 14fd5cb..e2422ea 100644
> --- a/drivers/mfd/mfd-core.c
> +++ b/drivers/mfd/mfd-core.c
> @@ -158,6 +158,7 @@ static int mfd_add_device(struct device *parent, int id,
> for_each_child_of_node(parent->of_node, np) {
> if (of_device_is_compatible(np, cell->of_compatible)) {
> pdev->dev.of_node = np;
> + pdev->dev.fwnode = &pdev->dev.of_node->fwnode;
> break;
> }
> }
> --
> 2.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-06-30 20:22:45

[permalink] [raw]

Subject: Re: [PATCH v1 05/10] device property: add fwnode_get_name()

On Tue, 30 Jun 2015 16:55:03 +0200
, Tomeu Vizoso <[email protected]>
wrote:
> Getting a textual representation of a device node can be very useful for
> debugging.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>
> ---
>
> drivers/base/property.c | 15 +++++++++++++++
> include/linux/property.h | 2 ++
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index a7fb46b..3280b04 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -556,3 +556,18 @@ struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode)
> return NULL;
> }
> EXPORT_SYMBOL_GPL(fwnode_get_parent);
> +
> +/**
> + * fwnode_get_name - return the name of a device node
> + * @fwnode: Device node to find the name of
> + */
> +const char *fwnode_get_name(struct fwnode_handle *fwnode)
> +{
> + if (is_of_node(fwnode))
> + return to_of_node(fwnode)->full_name;

of_node_full_name() please

> + else if (is_acpi_node(fwnode))
> + return acpi_dev_name(to_acpi_node(fwnode));
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_get_name);
> diff --git a/include/linux/property.h b/include/linux/property.h
> index f47092d..020a53c 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -65,6 +65,8 @@ struct fwnode_handle *device_get_next_child_node(struct device *dev,
>
> struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);
>
> +const char *fwnode_get_name(struct fwnode_handle *fwnode);
> +
> #define device_for_each_child_node(dev, child) \
> for (child = device_get_next_child_node(dev, NULL); child; \
> child = device_get_next_child_node(dev, child))
> --
> 2.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-06-30 20:22:55

[permalink] [raw]

Subject: Re: [PATCH v1 08/10] device property: add fwnode_is_compatible()

On Tue, 30 Jun 2015 16:55:06 +0200
, Tomeu Vizoso <[email protected]>
wrote:
> This is being added so code that is independent of the firmware being
> used can match firmware nodes to devices.
>
> This commit only implements it for OF nodes.

Hmmm, if it only works for OF nodes, then how is it firmware
independent? What is the use case?

>
> Signed-off-by: Tomeu Vizoso <[email protected]>
> ---
>
> drivers/base/property.c | 9 +++++++++
> include/linux/property.h | 2 ++
> 2 files changed, 11 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 92cdbb3..9c8be31 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -585,3 +585,12 @@ const char *fwnode_get_name(struct fwnode_handle *fwnode)
> return NULL;
> }
> EXPORT_SYMBOL_GPL(fwnode_get_name);
> +
> +bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible)
> +{
> + if (is_of_node(fwnode))
> + return of_device_is_compatible(to_of_node(fwnode), compatible);
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_is_compatible);
> diff --git a/include/linux/property.h b/include/linux/property.h
> index cfd1eb2..bf10074 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -80,6 +80,8 @@ const char *fwnode_get_name(struct fwnode_handle *fwnode);
>
> void fwnode_handle_put(struct fwnode_handle *fwnode);
>
> +bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible);
> +
> unsigned int device_get_child_node_count(struct device *dev);
>
> static inline bool device_property_read_bool(struct device *dev,
> --
> 2.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-06-30 20:23:52

[permalink] [raw]

Subject: Re: [PATCH v1 09/10] device: property: add fwnode_driver_match_device()

On Tue, 30 Jun 2015 16:55:07 +0200
, Tomeu Vizoso <[email protected]>
wrote:
> So buses can match drivers to devices without having to care about what
> firmware is used to describe the device.
>
> The introduction of this function will also allow us to introduce
> matching behavior common to the different type of firmwares.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>

The code looks fine... but the series only has one user! You should
update both SPI and I2C for the next version to show how this does
consolidate matching behaviour.

g.

> ---
>
> drivers/base/property.c | 18 ++++++++++++++++++
> include/linux/property.h | 4 ++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 9c8be31..8528eb9 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -15,6 +15,7 @@
> #include <linux/kernel.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/of_device.h>
> #include <linux/property.h>
>
> /**
> @@ -594,3 +595,20 @@ bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible)
> return false;
> }
> EXPORT_SYMBOL_GPL(fwnode_is_compatible);
> +
> +/**
> + * fwnode_driver_match_device - Tell if a driver matches a device.
> + * @drv: the device_driver structure to test
> + * @dev: the device structure to match against
> + */
> +bool fwnode_driver_match_device(struct device *dev,
> + const struct device_driver *drv)
> +{
> + if (is_of_node(dev->fwnode))
> + return of_driver_match_device(dev, drv);
> + else if (is_acpi_node(dev->fwnode))
> + return acpi_driver_match_device(dev, drv);
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_driver_match_device);
> diff --git a/include/linux/property.h b/include/linux/property.h
> index bf10074..4e453c4 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -17,6 +17,7 @@
> #include <linux/types.h>
>
> struct device;
> +struct device_driver;
>
> enum dev_prop_type {
> DEV_PROP_U8,
> @@ -82,6 +83,9 @@ void fwnode_handle_put(struct fwnode_handle *fwnode);
>
> bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible);
>
> +bool fwnode_driver_match_device(struct device *dev,
> + const struct device_driver *drv);
> +
> unsigned int device_get_child_node_count(struct device *dev);
>
> static inline bool device_property_read_bool(struct device *dev,
> --
> 2.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-07-01 07:36:54

by Lee Jones

[permalink] [raw]

Subject: Re: [PATCH v1 03/10] mfd: Have child devices point to their firmware nodes

On Tue, 30 Jun 2015, Tomeu Vizoso wrote:

> This is needed by platform-independent code that needs to do something
> with devices based on the data provided by the firmware.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>
> ---
>
> drivers/mfd/mfd-core.c | 1 +
> 1 file changed, 1 insertion(+)

Acked-by: Lee Jones <[email protected]>

> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> index 14fd5cb..e2422ea 100644
> --- a/drivers/mfd/mfd-core.c
> +++ b/drivers/mfd/mfd-core.c
> @@ -158,6 +158,7 @@ static int mfd_add_device(struct device *parent, int id,
> for_each_child_of_node(parent->of_node, np) {
> if (of_device_is_compatible(np, cell->of_compatible)) {
> pdev->dev.of_node = np;
> + pdev->dev.fwnode = &pdev->dev.of_node->fwnode;
> break;
> }
> }

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2015-07-01 23:06:03

[permalink] [raw]

Subject: Re: [PATCH v1 01/10] of/platform: Set fwnode field for new devices

On Tuesday, June 30, 2015 04:54:59 PM Tomeu Vizoso wrote:
> When allocating a new platform device, set the fwnode field in the
> struct device to point to the device_node.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>
> ---
>
> drivers/of/platform.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index ddf8e42..e880f55 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -139,6 +139,7 @@ struct platform_device *of_device_alloc(struct device_node *np,
> }
>
> dev->dev.of_node = of_node_get(np);
> + dev->dev.fwnode = &dev->dev.of_node->fwnode;

What about using set_primary_fwnode() instead of the direct assignment?

Or even having a macro analogous to ACPI_COMPANION_SET() for this purpose?

> dev->dev.parent = parent ? : &platform_bus;
>
> if (bus_id)
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2015-07-01 23:07:17

[permalink] [raw]

Subject: Re: [PATCH v1 03/10] mfd: Have child devices point to their firmware nodes

On Tuesday, June 30, 2015 04:55:01 PM Tomeu Vizoso wrote:
> This is needed by platform-independent code that needs to do something
> with devices based on the data provided by the firmware.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>
> ---
>
> drivers/mfd/mfd-core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> index 14fd5cb..e2422ea 100644
> --- a/drivers/mfd/mfd-core.c
> +++ b/drivers/mfd/mfd-core.c
> @@ -158,6 +158,7 @@ static int mfd_add_device(struct device *parent, int id,
> for_each_child_of_node(parent->of_node, np) {
> if (of_device_is_compatible(np, cell->of_compatible)) {
> pdev->dev.of_node = np;
> + pdev->dev.fwnode = &pdev->dev.of_node->fwnode;

Same comment as for the platform bus type.

> break;
> }
> }
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2015-07-01 23:11:46

[permalink] [raw]

Subject: Re: [PATCH v1 04/10] device property: add fwnode_get_parent()

On Tuesday, June 30, 2015 04:55:02 PM Tomeu Vizoso wrote:
> So we can query the parent of a fwnode without having to resort to API
> that is specific to a firmware data format.
>
> Also adds a acpi_get_parent_dev() function to retrieve the parent
> of an acpi_device. acpi_get_parent() already existed but it works with
> acpi_handles.
>
> The interface covers both ACPI and Device Trees.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>

This one looks good to me.

> ---
>
> drivers/base/property.c | 23 +++++++++++++++++++++++
> include/acpi/acpi_bus.h | 5 +++++
> include/linux/acpi.h | 5 +++++
> include/linux/property.h | 2 ++
> 4 files changed, 35 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index f3f6d16..a7fb46b 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -533,3 +533,26 @@ bool device_dma_is_coherent(struct device *dev)
> return coherent;
> }
> EXPORT_SYMBOL_GPL(device_dma_is_coherent);
> +
> +/**
> + * fwnode_get_parent - return the parent node of a device node
> + * @fwnode: Device node to find the parent node of
> + */
> +struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode)
> +{
> + if (is_of_node(fwnode)) {
> + struct device_node *node = to_of_node(fwnode);
> +
> + if (node->parent)
> + return &node->parent->fwnode;
> + } else if (is_acpi_node(fwnode)) {
> + struct acpi_device *node;
> +
> + node = acpi_get_parent_dev(to_acpi_node(fwnode));
> + if (node)
> + return acpi_fwnode_handle(node);
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_get_parent);
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 83061ca..93c42a6 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -436,6 +436,11 @@ static inline void *acpi_driver_data(struct acpi_device *d)
> return d->driver_data;
> }
>
> +static inline struct acpi_device *acpi_get_parent_dev(struct acpi_device *adev)
> +{
> + return adev->parent;
> +}
> +
> #define to_acpi_device(d) container_of(d, struct acpi_device, dev)
> #define to_acpi_driver(d) container_of(d, struct acpi_driver, drv)
>
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index ed79b06..fc84e42 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -804,6 +804,11 @@ static inline struct acpi_device *acpi_get_next_child(struct device *dev,
> return NULL;
> }
>
> +static inline struct acpi_device *acpi_get_parent_dev(struct acpi_device *adev)
> +{
> + return NULL;
> +}
> +
> #endif
>
> #endif /*_LINUX_ACPI_H*/
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 76ebde9..f47092d 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -63,6 +63,8 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
> struct fwnode_handle *device_get_next_child_node(struct device *dev,
> struct fwnode_handle *child);
>
> +struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);
> +
> #define device_for_each_child_node(dev, child) \
> for (child = device_get_next_child_node(dev, NULL); child; \
> child = device_get_next_child_node(dev, child))
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2015-07-01 23:12:24

[permalink] [raw]

Subject: Re: [PATCH v1 05/10] device property: add fwnode_get_name()

On Tuesday, June 30, 2015 04:55:03 PM Tomeu Vizoso wrote:
> Getting a textual representation of a device node can be very useful for
> debugging.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>

Fine by me.

> ---
>
> drivers/base/property.c | 15 +++++++++++++++
> include/linux/property.h | 2 ++
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index a7fb46b..3280b04 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -556,3 +556,18 @@ struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode)
> return NULL;
> }
> EXPORT_SYMBOL_GPL(fwnode_get_parent);
> +
> +/**
> + * fwnode_get_name - return the name of a device node
> + * @fwnode: Device node to find the name of
> + */
> +const char *fwnode_get_name(struct fwnode_handle *fwnode)
> +{
> + if (is_of_node(fwnode))
> + return to_of_node(fwnode)->full_name;
> + else if (is_acpi_node(fwnode))
> + return acpi_dev_name(to_acpi_node(fwnode));
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_get_name);
> diff --git a/include/linux/property.h b/include/linux/property.h
> index f47092d..020a53c 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -65,6 +65,8 @@ struct fwnode_handle *device_get_next_child_node(struct device *dev,
>
> struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);
>
> +const char *fwnode_get_name(struct fwnode_handle *fwnode);
> +
> #define device_for_each_child_node(dev, child) \
> for (child = device_get_next_child_node(dev, NULL); child; \
> child = device_get_next_child_node(dev, child))
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2015-07-01 23:13:39

[permalink] [raw]

Subject: Re: [PATCH v1 06/10] ACPI / scan: Add acpi_dev_get_next_child()

On Tuesday, June 30, 2015 04:55:04 PM Tomeu Vizoso wrote:
> So fwnode_get_next_child_node() can be implemented for ACPI firmware
> nodes.
>
> This re-implements acpi_get_next_child() in terms of
> acpi_dev_get_next_child().
>
> Signed-off-by: Tomeu Vizoso <[email protected]>

Looks good to me.

> ---
>
> drivers/acpi/scan.c | 5 ++---
> include/linux/acpi.h | 17 +++++++++++++++--
> 2 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 2649a06..45cf1b7 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1482,10 +1482,9 @@ int acpi_device_add(struct acpi_device *device,
> return result;
> }
>
> -struct acpi_device *acpi_get_next_child(struct device *dev,
> - struct acpi_device *child)
> +struct acpi_device *acpi_dev_get_next_child(struct acpi_device *adev,
> + struct acpi_device *child)
> {
> - struct acpi_device *adev = ACPI_COMPANION(dev);
> struct list_head *head, *next;
>
> if (!adev)
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index fc84e42..2afcdb9 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -752,8 +752,14 @@ int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
> int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
> enum dev_prop_type proptype, void *val, size_t nval);
>
> -struct acpi_device *acpi_get_next_child(struct device *dev,
> - struct acpi_device *child);
> +struct acpi_device *acpi_dev_get_next_child(struct acpi_device *adev,
> + struct acpi_device *child);
> +
> +static inline struct acpi_device *acpi_get_next_child(struct device *dev,
> + struct acpi_device *child)
> +{
> + return acpi_dev_get_next_child(ACPI_COMPANION(dev), child);
> +}
> #else
> static inline int acpi_dev_get_property(struct acpi_device *adev,
> const char *name, acpi_object_type type,
> @@ -804,6 +810,13 @@ static inline struct acpi_device *acpi_get_next_child(struct device *dev,
> return NULL;
> }
>
> +static inline struct acpi_device *acpi_dev_get_next_child(
> + struct acpi_device *adev,
> + struct acpi_device *child)
> +{
> + return NULL;
> +}
> +
> static inline struct acpi_device *acpi_get_parent_dev(struct acpi_device *adev)
> {
> return NULL;
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2015-07-01 23:14:14

[permalink] [raw]

Subject: Re: [PATCH v1 07/10] device property: Add fwnode_get_next_child_node()

On Tuesday, June 30, 2015 04:55:05 PM Tomeu Vizoso wrote:
> So callers that have a handle to a firmware node but not any of the
> devices related to it can iterate over their descendants.
>
> Also adds fwnode_for_each_child_node().
>
> Signed-off-by: Tomeu Vizoso <[email protected]>

Looks OK to me.

> ---
>
> drivers/base/property.c | 30 ++++++++++++++++++++++--------
> include/linux/property.h | 7 +++++++
> 2 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 3280b04..92cdbb3 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -466,28 +466,42 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
> EXPORT_SYMBOL_GPL(fwnode_property_read_string);
>
> /**
> - * device_get_next_child_node - Return the next child node handle for a device
> - * @dev: Device to find the next child node for.
> - * @child: Handle to one of the device's child nodes or a null handle.
> + * fwnode_get_next_child_node - Return the next child node handle for a fwnode
> + * @fwnode: Firmware node to find the next child node for.
> + * @child: Handle to one of the firmware node's child nodes or a null handle.
> */
> -struct fwnode_handle *device_get_next_child_node(struct device *dev,
> +struct fwnode_handle *fwnode_get_next_child_node(struct fwnode_handle *fwnode,
> struct fwnode_handle *child)
> {
> - if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
> + if (is_of_node(fwnode)) {
> struct device_node *node;
>
> - node = of_get_next_available_child(dev->of_node, to_of_node(child));
> + node = of_get_next_available_child(to_of_node(fwnode),
> + to_of_node(child));
> if (node)
> return &node->fwnode;
> - } else if (IS_ENABLED(CONFIG_ACPI)) {
> + } else if (is_acpi_node(fwnode)) {
> struct acpi_device *node;
>
> - node = acpi_get_next_child(dev, to_acpi_node(child));
> + node = acpi_dev_get_next_child(to_acpi_node(fwnode),
> + to_acpi_node(child));
> if (node)
> return acpi_fwnode_handle(node);
> }
> return NULL;
> }
> +EXPORT_SYMBOL_GPL(fwnode_get_next_child_node);
> +
> +/**
> + * device_get_next_child_node - Return the next child node handle for a device
> + * @dev: Device to find the next child node for.
> + * @child: Handle to one of the device's child nodes or a null handle.
> + */
> +struct fwnode_handle *device_get_next_child_node(struct device *dev,
> + struct fwnode_handle *child)
> +{
> + return fwnode_get_next_child_node(dev->fwnode, child);
> +}
> EXPORT_SYMBOL_GPL(device_get_next_child_node);
>
> /**
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 020a53c..cfd1eb2 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -63,6 +63,9 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
> struct fwnode_handle *device_get_next_child_node(struct device *dev,
> struct fwnode_handle *child);
>
> +struct fwnode_handle *fwnode_get_next_child_node(struct fwnode_handle *fwnode,
> + struct fwnode_handle *child);
> +
> struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);
>
> const char *fwnode_get_name(struct fwnode_handle *fwnode);
> @@ -71,6 +74,10 @@ const char *fwnode_get_name(struct fwnode_handle *fwnode);
> for (child = device_get_next_child_node(dev, NULL); child; \
> child = device_get_next_child_node(dev, child))
>
> +#define fwnode_for_each_child_node(fwnode, child) \
> + for (child = fwnode_get_next_child_node(fwnode, NULL); child; \
> + child = fwnode_get_next_child_node(fwnode, child))
> +
> void fwnode_handle_put(struct fwnode_handle *fwnode);
>
> unsigned int device_get_child_node_count(struct device *dev);
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2015-07-01 23:18:33

[permalink] [raw]

Subject: Re: [PATCH v1 08/10] device property: add fwnode_is_compatible()

On Tuesday, June 30, 2015 04:55:06 PM Tomeu Vizoso wrote:
> This is being added so code that is independent of the firmware being
> used can match firmware nodes to devices.
>
> This commit only implements it for OF nodes.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>

I don't really like this one, as it is hiding OF-specific stuff under
a seemingly generic API.

The "compatible" property also makes sense for certain ACPI device objects
nowadays, so at least this should take that case into accout.

What do you need it for, specifically?

> ---
>
> drivers/base/property.c | 9 +++++++++
> include/linux/property.h | 2 ++
> 2 files changed, 11 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 92cdbb3..9c8be31 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -585,3 +585,12 @@ const char *fwnode_get_name(struct fwnode_handle *fwnode)
> return NULL;
> }
> EXPORT_SYMBOL_GPL(fwnode_get_name);
> +
> +bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible)
> +{
> + if (is_of_node(fwnode))
> + return of_device_is_compatible(to_of_node(fwnode), compatible);
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_is_compatible);
> diff --git a/include/linux/property.h b/include/linux/property.h
> index cfd1eb2..bf10074 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -80,6 +80,8 @@ const char *fwnode_get_name(struct fwnode_handle *fwnode);
>
> void fwnode_handle_put(struct fwnode_handle *fwnode);
>
> +bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible);
> +
> unsigned int device_get_child_node_count(struct device *dev);
>
> static inline bool device_property_read_bool(struct device *dev,
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2015-07-01 23:19:00

[permalink] [raw]

Subject: Re: [PATCH v1 09/10] device: property: add fwnode_driver_match_device()

On Tuesday, June 30, 2015 04:55:07 PM Tomeu Vizoso wrote:
> So buses can match drivers to devices without having to care about what
> firmware is used to describe the device.
>
> The introduction of this function will also allow us to introduce
> matching behavior common to the different type of firmwares.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>

Looks OK to me.

> ---
>
> drivers/base/property.c | 18 ++++++++++++++++++
> include/linux/property.h | 4 ++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 9c8be31..8528eb9 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -15,6 +15,7 @@
> #include <linux/kernel.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/of_device.h>
> #include <linux/property.h>
>
> /**
> @@ -594,3 +595,20 @@ bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible)
> return false;
> }
> EXPORT_SYMBOL_GPL(fwnode_is_compatible);
> +
> +/**
> + * fwnode_driver_match_device - Tell if a driver matches a device.
> + * @drv: the device_driver structure to test
> + * @dev: the device structure to match against
> + */
> +bool fwnode_driver_match_device(struct device *dev,
> + const struct device_driver *drv)
> +{
> + if (is_of_node(dev->fwnode))
> + return of_driver_match_device(dev, drv);
> + else if (is_acpi_node(dev->fwnode))
> + return acpi_driver_match_device(dev, drv);
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_driver_match_device);
> diff --git a/include/linux/property.h b/include/linux/property.h
> index bf10074..4e453c4 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -17,6 +17,7 @@
> #include <linux/types.h>
>
> struct device;
> +struct device_driver;
>
> enum dev_prop_type {
> DEV_PROP_U8,
> @@ -82,6 +83,9 @@ void fwnode_handle_put(struct fwnode_handle *fwnode);
>
> bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible);
>
> +bool fwnode_driver_match_device(struct device *dev,
> + const struct device_driver *drv);
> +
> unsigned int device_get_child_node_count(struct device *dev);
>
> static inline bool device_property_read_bool(struct device *dev,
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2015-07-01 23:20:04

[permalink] [raw]

Subject: Re: [PATCH v1 10/10] core: platform: use fwnode_driver_match_device()

On Tuesday, June 30, 2015 04:55:08 PM Tomeu Vizoso wrote:
> Instead of calling both of_driver_match_device() and
> acpi_driver_match_device(), call fwnode_driver_match_device() which
> should be able to sort out what firmware describes the device in
> question.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>

This one should be part of the other set IMO.

Or please just combine the two sets so it is more clear what the new helpers
are intended for.

> ---
>
> drivers/base/platform.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 063f0ab..a7e7757 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -843,12 +843,8 @@ static int platform_match(struct device *dev, struct device_driver *drv)
> if (pdev->driver_override)
> return !strcmp(pdev->driver_override, drv->name);
>
> - /* Attempt an OF style match first */
> - if (of_driver_match_device(dev, drv))
> - return 1;
> -
> - /* Then try ACPI style match */
> - if (acpi_driver_match_device(dev, drv))
> + /* Attempt a firmware match first */
> + if (fwnode_driver_match_device(dev, drv))
> return 1;
>
> /* Then try to match against the id table */
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2015-07-10 12:54:22

[permalink] [raw]

Subject: Re: [PATCH v1 10/10] core: platform: use fwnode_driver_match_device()

On 2 July 2015 at 01:46, Rafael J. Wysocki <[email protected]> wrote:
> On Tuesday, June 30, 2015 04:55:08 PM Tomeu Vizoso wrote:
>> Instead of calling both of_driver_match_device() and
>> acpi_driver_match_device(), call fwnode_driver_match_device() which
>> should be able to sort out what firmware describes the device in
>> question.
>>
>> Signed-off-by: Tomeu Vizoso <[email protected]>
>
> This one should be part of the other set IMO.

Yeah, I wasn't sure about it myself, but in any case the other series
won't depend on this patch any more because I have moved the match
delay into platform.c. I plan though to resend this with Rob's
comments addressed in the next revision, so they can be taken if they
are seen worthy in themselves.

> Or please just combine the two sets so it is more clear what the new helpers
> are intended for.

Sorry about that, I did the split because Mark suggested so in the
previous review round. I understand that depending on each person's
workflow it may be more convenient in one way or the other, so I'm not
sure I can find a way that fits everybody's preferences.

Thanks,

Tomeu

>> ---
>>
>> drivers/base/platform.c | 8 ++------
>> 1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
>> index 063f0ab..a7e7757 100644
>> --- a/drivers/base/platform.c
>> +++ b/drivers/base/platform.c
>> @@ -843,12 +843,8 @@ static int platform_match(struct device *dev, struct device_driver *drv)
>> if (pdev->driver_override)
>> return !strcmp(pdev->driver_override, drv->name);
>>
>> - /* Attempt an OF style match first */
>> - if (of_driver_match_device(dev, drv))
>> - return 1;
>> -
>> - /* Then try ACPI style match */
>> - if (acpi_driver_match_device(dev, drv))
>> + /* Attempt a firmware match first */
>> + if (fwnode_driver_match_device(dev, drv))
>> return 1;
>>
>> /* Then try to match against the id table */
>>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-07-11 06:21:09

[permalink] [raw]

Subject: Re: [PATCH v1 10/10] core: platform: use fwnode_driver_match_device()

On Friday, July 10, 2015 02:53:53 PM Tomeu Vizoso wrote:
> On 2 July 2015 at 01:46, Rafael J. Wysocki <[email protected]> wrote:
> > On Tuesday, June 30, 2015 04:55:08 PM Tomeu Vizoso wrote:
> >> Instead of calling both of_driver_match_device() and
> >> acpi_driver_match_device(), call fwnode_driver_match_device() which
> >> should be able to sort out what firmware describes the device in
> >> question.
> >>
> >> Signed-off-by: Tomeu Vizoso <[email protected]>
> >
> > This one should be part of the other set IMO.
>
> Yeah, I wasn't sure about it myself, but in any case the other series
> won't depend on this patch any more because I have moved the match
> delay into platform.c. I plan though to resend this with Rob's
> comments addressed in the next revision, so they can be taken if they
> are seen worthy in themselves.
>
> > Or please just combine the two sets so it is more clear what the new helpers
> > are intended for.
>
> Sorry about that, I did the split because Mark suggested so in the
> previous review round. I understand that depending on each person's
> workflow it may be more convenient in one way or the other, so I'm not
> sure I can find a way that fits everybody's preferences.

The ordering/form in which it is merged need not reflect the ordering/form
in which it is reviewed and it is much easier to follow changes having full
context available.

So you can split it later if need be, but for now let's keep them all together
for the completeness of the context.

Thanks,
Rafael

2015-07-13 06:56:01

[permalink] [raw]

Subject: Re: [PATCH v1 10/10] core: platform: use fwnode_driver_match_device()

On 11 July 2015 at 04:47, Rafael J. Wysocki <[email protected]> wrote:
> On Friday, July 10, 2015 02:53:53 PM Tomeu Vizoso wrote:
>> On 2 July 2015 at 01:46, Rafael J. Wysocki <[email protected]> wrote:
>> > On Tuesday, June 30, 2015 04:55:08 PM Tomeu Vizoso wrote:
>> >> Instead of calling both of_driver_match_device() and
>> >> acpi_driver_match_device(), call fwnode_driver_match_device() which
>> >> should be able to sort out what firmware describes the device in
>> >> question.
>> >>
>> >> Signed-off-by: Tomeu Vizoso <[email protected]>
>> >
>> > This one should be part of the other set IMO.
>>
>> Yeah, I wasn't sure about it myself, but in any case the other series
>> won't depend on this patch any more because I have moved the match
>> delay into platform.c. I plan though to resend this with Rob's
>> comments addressed in the next revision, so they can be taken if they
>> are seen worthy in themselves.
>>
>> > Or please just combine the two sets so it is more clear what the new helpers
>> > are intended for.
>>
>> Sorry about that, I did the split because Mark suggested so in the
>> previous review round. I understand that depending on each person's
>> workflow it may be more convenient in one way or the other, so I'm not
>> sure I can find a way that fits everybody's preferences.
>
> The ordering/form in which it is merged need not reflect the ordering/form
> in which it is reviewed and it is much easier to follow changes having full
> context available.

Sure, nobody was talking yet about how this could be merged.

> So you can split it later if need be, but for now let's keep them all together
> for the completeness of the context.

That's fine with me, really. I've been trying to find a way to send
such big series that will work for most people, but that's probably
the hardest part of it all.

Regards,

Tomeu

> Thanks,
> Rafael
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-07-22 11:54:07

[permalink] [raw]

Subject: Re: [PATCH v1 05/10] device property: add fwnode_get_name()

On 2 July 2015 at 01:38, Rafael J. Wysocki <[email protected]> wrote:
> On Tuesday, June 30, 2015 04:55:03 PM Tomeu Vizoso wrote:
>> Getting a textual representation of a device node can be very useful for
>> debugging.
>>
>> Signed-off-by: Tomeu Vizoso <[email protected]>
>
> Fine by me.

Thanks, Rafael. Is this a Reviewed-by?

Tomeu

>
>> ---
>>
>> drivers/base/property.c | 15 +++++++++++++++
>> include/linux/property.h | 2 ++
>> 2 files changed, 17 insertions(+)
>>
>> diff --git a/drivers/base/property.c b/drivers/base/property.c
>> index a7fb46b..3280b04 100644
>> --- a/drivers/base/property.c
>> +++ b/drivers/base/property.c
>> @@ -556,3 +556,18 @@ struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode)
>> return NULL;
>> }
>> EXPORT_SYMBOL_GPL(fwnode_get_parent);
>> +
>> +/**
>> + * fwnode_get_name - return the name of a device node
>> + * @fwnode: Device node to find the name of
>> + */
>> +const char *fwnode_get_name(struct fwnode_handle *fwnode)
>> +{
>> + if (is_of_node(fwnode))
>> + return to_of_node(fwnode)->full_name;
>> + else if (is_acpi_node(fwnode))
>> + return acpi_dev_name(to_acpi_node(fwnode));
>> +
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(fwnode_get_name);
>> diff --git a/include/linux/property.h b/include/linux/property.h
>> index f47092d..020a53c 100644
>> --- a/include/linux/property.h
>> +++ b/include/linux/property.h
>> @@ -65,6 +65,8 @@ struct fwnode_handle *device_get_next_child_node(struct device *dev,
>>
>> struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);
>>
>> +const char *fwnode_get_name(struct fwnode_handle *fwnode);
>> +
>> #define device_for_each_child_node(dev, child) \
>> for (child = device_get_next_child_node(dev, NULL); child; \
>> child = device_get_next_child_node(dev, child))
>>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-07-31 10:36:30

by Wolfram Sang

[permalink] [raw]

Subject: Re: [PATCH v1 02/10] i2c: core: Have clients point to their firmware nodes

On Tue, Jun 30, 2015 at 04:55:00PM +0200, Tomeu Vizoso wrote:
> This is needed by platform-independent code that needs to do something
> with devices based on the data provided by the firmware.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>

Dunno the status of this series, but the i2c change is trivial enough.
So in case, the rest is accepted:

Acked-by: Wolfram Sang <[email protected]>

Attachments:

(No filename) (407.00 B)
signature.asc (819.00 B)
Digital signature Download all attachments

2015-08-06 14:17:51