[v2,1/3] of: add common OF-based component functionality
diff mbox

Message ID E1b8pyR-0005OJ-V3@rmk-PC.armlinux.org.uk
State New
Headers show

Commit Message

Russell King June 3, 2016, 2:21 p.m. UTC
Add common OF-based component functionality for matching devices by
device node, and releasing the device node at the appropraite time.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/of/Makefile          |  2 +-
 drivers/of/of_component.c    | 41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/of_component.h | 14 ++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 drivers/of/of_component.c
 create mode 100644 include/linux/of_component.h

Comments

Rob Herring June 3, 2016, 3:29 p.m. UTC | #1
On Fri, Jun 3, 2016 at 9:21 AM, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> Add common OF-based component functionality for matching devices by
> device node, and releasing the device node at the appropraite time.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/of/Makefile          |  2 +-
>  drivers/of/of_component.c    | 41 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_component.h | 14 ++++++++++++++

I'd prefer this to go into drivers/base/component.c. That's the
general direction we've been moving.

I'd expect this would cause some build failures unless the cases you
converted all depend on CONFIG_OF.

Rob
Russell King - ARM Linux admin June 3, 2016, 3:36 p.m. UTC | #2
On Fri, Jun 03, 2016 at 10:29:40AM -0500, Rob Herring wrote:
> On Fri, Jun 3, 2016 at 9:21 AM, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > Add common OF-based component functionality for matching devices by
> > device node, and releasing the device node at the appropraite time.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/of/Makefile          |  2 +-
> >  drivers/of/of_component.c    | 41 +++++++++++++++++++++++++++++++++++++++++
> >  include/linux/of_component.h | 14 ++++++++++++++
> 
> I'd prefer this to go into drivers/base/component.c. That's the
> general direction we've been moving.

I'd prefer not to, I don't want to turn the component helpers into
something OF specific.  They aren't OF specific.

> I'd expect this would cause some build failures unless the cases you
> converted all depend on CONFIG_OF.

Okay, I'll stick with v1 then, and the duplication that v1 involves.
Thierry Reding June 3, 2016, 3:44 p.m. UTC | #3
On Fri, Jun 03, 2016 at 03:21:19PM +0100, Russell King wrote:
[...]
> diff --git a/drivers/of/of_component.c b/drivers/of/of_component.c
[...]
> +static void component_compare_of(struct device *dev, void *data)
> +{
> +	return dev->of_node == data;
> +}

The return statement here doesn't match the return value. Didn't GCC
complain about this?

Thierry
Russell King - ARM Linux admin June 3, 2016, 4:11 p.m. UTC | #4
On Fri, Jun 03, 2016 at 05:44:30PM +0200, Thierry Reding wrote:
> On Fri, Jun 03, 2016 at 03:21:19PM +0100, Russell King wrote:
> [...]
> > diff --git a/drivers/of/of_component.c b/drivers/of/of_component.c
> [...]
> > +static void component_compare_of(struct device *dev, void *data)
> > +{
> > +	return dev->of_node == data;
> > +}
> 
> The return statement here doesn't match the return value. Didn't GCC
> complain about this?

I didn't build-test it, because I wanted people's opinions on it first
(building means rebuilding my entire tree...)

Anyway, the patch series is dead because I'm not prepared to make the
changes which Rob mentioned, so we're back to v1 instead.
Rob Herring June 3, 2016, 7:52 p.m. UTC | #5
On Fri, Jun 3, 2016 at 10:36 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Fri, Jun 03, 2016 at 10:29:40AM -0500, Rob Herring wrote:
>> On Fri, Jun 3, 2016 at 9:21 AM, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>> > Add common OF-based component functionality for matching devices by
>> > device node, and releasing the device node at the appropraite time.
>> >
>> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> > ---
>> >  drivers/of/Makefile          |  2 +-
>> >  drivers/of/of_component.c    | 41 +++++++++++++++++++++++++++++++++++++++++
>> >  include/linux/of_component.h | 14 ++++++++++++++
>>
>> I'd prefer this to go into drivers/base/component.c. That's the
>> general direction we've been moving.
>
> I'd prefer not to, I don't want to turn the component helpers into
> something OF specific.  They aren't OF specific.

Fine, not enough code to argue about...

>> I'd expect this would cause some build failures unless the cases you
>> converted all depend on CONFIG_OF.
>
> Okay, I'll stick with v1 then, and the duplication that v1 involves.

Why? You don't want to add empty functions? Seems like good clean-up.

Rob

Patch
diff mbox

diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index d7efd9d458aa..6a4a5e2c0839 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -1,4 +1,4 @@ 
-obj-y = base.o device.o platform.o
+obj-y = base.o of_component.o device.o platform.o
 obj-$(CONFIG_OF_DYNAMIC) += dynamic.o
 obj-$(CONFIG_OF_FLATTREE) += fdt.o
 obj-$(CONFIG_OF_EARLY_FLATTREE) += fdt_address.o
diff --git a/drivers/of/of_component.c b/drivers/of/of_component.c
new file mode 100644
index 000000000000..41e6e817d264
--- /dev/null
+++ b/drivers/of/of_component.c
@@ -0,0 +1,41 @@ 
+/*
+ * Copyright (C) 2016 Russell King.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/component.h>
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_component.h>
+#include <linux/of_graph.h>
+
+static void component_release_of(struct device *dev, void *data)
+{
+	of_node_put(data);
+}
+
+void component_match_add_of_compare(struct device *master,
+	struct component_match **matchptr,
+	int (*compare)(struct device *, void *), struct device_node *node)
+{
+	of_node_get(node);
+	component_match_add_release(master, matchptr, component_release_of,
+				    compare, node);
+}
+EXPORT_SYMBOL_GPL(component_match_add_of_compare);
+
+static void component_compare_of(struct device *dev, void *data)
+{
+	return dev->of_node == data;
+}
+
+void component_match_add_of(struct device *master,
+	struct component_match **matchptr, struct device_node *node)
+{
+	of_node_get(node);
+	component_match_add_release(master, matchptr, component_release_of,
+				    component_compare_of, node);
+}
+EXPORT_SYMBOL_GPL(component_match_add_of);
diff --git a/include/linux/of_component.h b/include/linux/of_component.h
new file mode 100644
index 000000000000..a8170ba3b786
--- /dev/null
+++ b/include/linux/of_component.h
@@ -0,0 +1,14 @@ 
+#ifndef __LINUX_COMPONENT_OF_H
+#define __LINUX_COMPONENT_OF_H
+
+struct component_match;
+struct device;
+struct device_node;
+
+void component_match_add_of_compare(struct device *master,
+	struct component_match **matchptr,
+	int (*compare)(struct device *, void *), struct device_node *node);
+void component_match_add_of(struct device *master,
+	struct component_match **matchptr, struct device_node *node);
+
+#endif