Re: [PATCH RFC v2 00/35] Second preview of imx-drm cleanup series
On Mon, 10 Feb 2014 15:18:21 +
Russell King - ARM Linux wrote:
> Now, mind explaining what "v4l2 style device tree bindings" means? I've
> no idea since I'm relatively new to DT.
Documentation/devicetree/bindings/media/video-interfaces.txt
For the Cubox, I have:
tda998x: hdmi-encoder {
compatible = "nxp,tda998x";
...
port {
tda998x_0: endpoint@0 {
remote-endpoint = <&lcd0_0>;
};
};
};
&lcd0 {
status = "okay";
...
port {
lcd0_0: endpoint@0 {
remote-endpoint = <&tda998x_0>;
};
};
};
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/2] drivers/base: permit base components to omit the bind/unbind ops
On Mon, 10 Feb 2014 13:12:33 + Russell King - ARM Linux wrote: > I've NAK'd these patches already - I believe they're based on a > mis-understanding of how this should be used. I believe Jean-Francois > has only looked at the core, rather than looking at the imx-drm example > it was posted with in an attempt to understand it. > > Omitting the component bind operations is absurd because it makes the > component code completely pointless, since there is then no way to > control the sequencing of driver initialisation - something which is > one of the primary reasons for this code existing in the first place. I perfectly looked at your example and I use it now in my system. You did not see what could be done with your component code. For example, since november, I have not yet the clock probe_defer in the mainline (http://www.spinics.net/lists/arm-kernel/msg306072.html), so, there are 3 solutions: - hope the patch will be some day in the mainline and, today, reboot when the system does not start correctly, - insert a delay in the tda998x and kirkwood probe sequences (delay long enough to be sure the si5351 is started, or loop), - use your component work. In the last case, it is easy: - the si5351 driver calls component_add (with empty ops: it has no interest in the bind/unbind functions) when it is fully started (i.e. registered - that was the subject of my patch), - in the DRM driver, look for the si5351 as a clock in the DT (drm -> encoder -> clock), and add it to the awaited components (CRTCs, encoders..), - in the audio subsystem, look for the si5351 as an external clock in the DT (simple-card -> CPU DAI -> clock) and add it to the awaited components (CPU and CODEC DAIs - yes, the S/PDIF CODEC should also be a component with no bin/unbind ops). Then, when the si5351 is registered, both master components video and audio can safely run. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 0/2] drivers/base: simplify simple DT-based components
On Fri, 7 Feb 2014 20:23:51 + Russell King - ARM Linux wrote: > Here's my changes to the TDA998x driver to add support for the component > helper. The TDA998x driver retains support for the old way so that > drivers can be transitioned. For any one DRM "card" the transition to I rewrote the tda998x as a simple encoder+connector (i.e. not a slave_encoder) with your component helper, and the code is much clearer and simpler: the DRM driver has nothing to do except to know that the tda998x is a component and to set the possible_crtcs. AFAIK, only the tilcdc drm driver is using the tda998x as a slave_encoder. It does a (encoder+connector) conversion to (slave_encoder). Then, in your changes in the TDA998x, you do a (slave_encoder) translation to (encoder+connector). This seems rather complicated! I think it would be easier to use your component helper and rewrite (remove?) tilcdc_slave.c. > And yes, I'm thinking that maybe moving compare_of() into the component > support so that drivers can share this generic function may be a good > idea. This function exists already in drivers/of/platform.c as of_dev_node_match(). It just needs to be exported. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 0/2] drivers/base: simplify simple DT-based components
On Fri, 7 Feb 2014 17:33:26 + Russell King - ARM Linux wrote: > On Fri, Feb 07, 2014 at 06:11:08PM +0100, Jean-Francois Moine wrote: > > This patch series tries to simplify the code of simple devices in case > > they are part of componentised subsystems, are declared in a DT, and > > are not using the component bin/unbind functions. > > I wonder - I said earlier today that this works absolutely fine without > modification with DT, so why are you messing about with it adding DT > support? > > This is totally the wrong approach. The idea is that this deals with > /devices/ and /devices/ only. It groups up /devices/. > > It's up to the add_component callback to the master device to decide > how to deal with that. > > > Jean-Francois Moine (2): > > drivers/base: permit base components to omit the bind/unbind ops > > And this patch has me wondering if you even understand how to use > this... The master bind/unbind callbacks are the ones which establish > the "card" based context with the subsystem. > > Please, before buggering up this nicely designed implementation, please > /first/ look at the imx-drm rework which was posted back in early January > which illustrates how this is used in a DT context - which is something > I've already pointed you at once today already. As I told in a previous mail, your code works fine in my DT-based Cubox. I am rewriting the TDA988x as a normal encoder/connector, and, yes, the bind/unbind functions are useful in this case. But you opened a door. In a DT context, you know that the probe_defer mechanism does not work correctly. Your work permits to solve delicate cases: your component_add tells exactly when a device is available, and the master bind callback is the green signal for the device waiting for its resources. Indeed, your system was not created for such a usage, but it works as it is (anyway, the component bind/unbind functions may be empty...). -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/2] *** SUBJECT HERE ***
*** BLURB HERE *** Jean-Francois Moine (2): drivers/base: permit base components to omit the bind/unbind ops drivers/base: declare phandle DT nodes as components drivers/base/component.c | 21 +++-- drivers/base/core.c | 18 ++ include/linux/of.h | 2 ++ 3 files changed, 39 insertions(+), 2 deletions(-) -- 1.9.rc1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/2] drivers/base: declare phandle DT nodes as components
At system startup time, some devices depends on the availability of
some other devices before starting. The infrastructure for componentised
subsystems permits to handle this dependence, each driver defining
its own role.
This patch does an automatic creation of the lowest components in
case of DT. This permits simple devices to be part of complex
componentised subsystems without any specific code.
When created from DT, the device dependence is generally indicated by
phandle: a device which is pointed to by a phandle must be started
before the pointing device(s).
So, at device register time, the devices which are pointed to by a
phandle are declared as components, except when they declared
themselves as such in their probe function.
Signed-off-by: Jean-Francois Moine
---
drivers/base/component.c | 12
drivers/base/core.c | 18 ++
include/linux/of.h | 2 ++
3 files changed, 32 insertions(+)
diff --git a/drivers/base/component.c b/drivers/base/component.c
index 0a39d7a..3cab26b 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
struct master {
struct list_head node;
@@ -336,6 +337,7 @@ EXPORT_SYMBOL_GPL(component_bind_all);
int component_add(struct device *dev, const struct component_ops *ops)
{
struct component *component;
+ struct device_node *np;
int ret;
component = kzalloc(sizeof(*component), GFP_KERNEL);
@@ -356,6 +358,11 @@ int component_add(struct device *dev, const struct
component_ops *ops)
kfree(component);
}
+
+ np = dev->of_node;
+ if (np)
+ np->_flags |= OF_DEV_COMPONENT;
+
mutex_unlock(&component_mutex);
return ret < 0 ? ret : 0;
@@ -365,6 +372,7 @@ EXPORT_SYMBOL_GPL(component_add);
void component_del(struct device *dev, const struct component_ops *ops)
{
struct component *c, *component = NULL;
+ struct device_node *np;
mutex_lock(&component_mutex);
list_for_each_entry(c, &component_list, node)
@@ -377,6 +385,10 @@ void component_del(struct device *dev, const struct
component_ops *ops)
if (component && component->master)
take_down_master(component->master);
+ np = dev->of_node;
+ if (np)
+ np->_flags &= ~OF_DEV_COMPONENT;
+
mutex_unlock(&component_mutex);
WARN_ON(!component);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 2b56717..0517b91 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "base.h"
#include "power/power.h"
@@ -980,6 +981,7 @@ int device_add(struct device *dev)
struct device *parent = NULL;
struct kobject *kobj;
struct class_interface *class_intf;
+ struct device_node *np;
int error = -EINVAL;
dev = get_device(dev);
@@ -1088,6 +1090,15 @@ int device_add(struct device *dev)
class_intf->add_dev(dev, class_intf);
mutex_unlock(&dev->class->p->mutex);
}
+
+ /* if DT-created device referenced by phandle, create a component */
+ np = dev->of_node;
+ if (np && np->phandle &&
+ !(np->_flags & OF_DEV_COMPONENT)) {
+ component_add(dev, NULL);
+ np->_flags |= OF_DEV_IMPLICIT_COMPONENT;
+ }
+
done:
put_device(dev);
return error;
@@ -1189,10 +1200,17 @@ void device_del(struct device *dev)
{
struct device *parent = dev->parent;
struct class_interface *class_intf;
+ struct device_node *np;
/* Notify clients of device removal. This call must come
* before dpm_sysfs_remove().
*/
+ np = dev->of_node;
+ if (np && (np->_flags & OF_DEV_COMPONENT)) {
+ component_del(dev, NULL);
+ np->_flags &= ~OF_DEV_IMPLICIT_COMPONENT;
+ }
+
if (dev->bus)
blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
BUS_NOTIFY_DEL_DEVICE, dev);
diff --git a/include/linux/of.h b/include/linux/of.h
index 70c64ba..40f1c34 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -59,6 +59,8 @@ struct device_node {
struct proc_dir_entry *pde;/* this node's proc directory */
struct kref kref;
unsigned long _flags;
+#define OF_DEV_COMPONENT 1
+#define OF_DEV_IMPLICIT_COMPONENT 2
void*data;
#if defined(CONFIG_SPARC)
const char *path_component_name;
--
1.9.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 1/2] drivers/base: permit base components to omit the bind/unbind ops
Some simple components don't need to do any specific action on
bind to / unbind from a master component.
This patch permits such components to omit the bind/unbind
operations.
Signed-off-by: Jean-Francois Moine
---
drivers/base/component.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/base/component.c b/drivers/base/component.c
index c53efe6..0a39d7a 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -225,7 +225,8 @@ static void component_unbind(struct component *component,
{
WARN_ON(!component->bound);
- component->ops->unbind(component->dev, master->dev, data);
+ if (component->ops)
+ component->ops->unbind(component->dev, master->dev, data);
component->bound = false;
/* Release all resources claimed in the binding of this component */
@@ -274,7 +275,11 @@ static int component_bind(struct component *component,
struct master *master,
dev_dbg(master->dev, "binding %s (ops %ps)\n",
dev_name(component->dev), component->ops);
- ret = component->ops->bind(component->dev, master->dev, data);
+ if (component->ops)
+ ret = component->ops->bind(component->dev, master->dev, data);
+ else
+ ret = 0;
+
if (!ret) {
component->bound = true;
--
1.9.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 0/2] drivers/base: simplify simple DT-based components
This patch series tries to simplify the code of simple devices in case they are part of componentised subsystems, are declared in a DT, and are not using the component bin/unbind functions. Jean-Francois Moine (2): drivers/base: permit base components to omit the bind/unbind ops drivers/base: declare phandle DT nodes as components drivers/base/component.c | 21 +++-- drivers/base/core.c | 18 ++ include/linux/of.h | 2 ++ 3 files changed, 39 insertions(+), 2 deletions(-) -- 1.9.rc1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH RFC 2/2] drivers/base: declare phandle DT nodes as components
At system startup time, some devices depends on the availability of
some other devices before starting. The infrastructure for componentised
subsystems permits to handle this dependence, each driver defining
its own role.
This patch does an automatic creation of the lowest components in
case of DT. This permits simple devices to be part of complex
componentised subsystems without any specific code.
When created from DT, the device dependence is generally indicated by
phandle: a device which is pointed to by a phandle must be started
before the pointing device(s).
So, at device register time, the devices which are pointed to by a
phandle are declared as components, except when they declared
themselves as such in their probe function.
Signed-off-by: Jean-Francois Moine
---
drivers/base/component.c | 12
drivers/base/core.c | 18 ++
include/linux/of.h | 2 ++
3 files changed, 32 insertions(+)
diff --git a/drivers/base/component.c b/drivers/base/component.c
index 0a39d7a..3cab26b 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
struct master {
struct list_head node;
@@ -336,6 +337,7 @@ EXPORT_SYMBOL_GPL(component_bind_all);
int component_add(struct device *dev, const struct component_ops *ops)
{
struct component *component;
+ struct device_node *np;
int ret;
component = kzalloc(sizeof(*component), GFP_KERNEL);
@@ -356,6 +358,11 @@ int component_add(struct device *dev, const struct
component_ops *ops)
kfree(component);
}
+
+ np = dev->of_node;
+ if (np)
+ np->_flags |= OF_DEV_COMPONENT;
+
mutex_unlock(&component_mutex);
return ret < 0 ? ret : 0;
@@ -365,6 +372,7 @@ EXPORT_SYMBOL_GPL(component_add);
void component_del(struct device *dev, const struct component_ops *ops)
{
struct component *c, *component = NULL;
+ struct device_node *np;
mutex_lock(&component_mutex);
list_for_each_entry(c, &component_list, node)
@@ -377,6 +385,10 @@ void component_del(struct device *dev, const struct
component_ops *ops)
if (component && component->master)
take_down_master(component->master);
+ np = dev->of_node;
+ if (np)
+ np->_flags &= ~OF_DEV_COMPONENT;
+
mutex_unlock(&component_mutex);
WARN_ON(!component);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 2b56717..0517b91 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "base.h"
#include "power/power.h"
@@ -980,6 +981,7 @@ int device_add(struct device *dev)
struct device *parent = NULL;
struct kobject *kobj;
struct class_interface *class_intf;
+ struct device_node *np;
int error = -EINVAL;
dev = get_device(dev);
@@ -1088,6 +1090,15 @@ int device_add(struct device *dev)
class_intf->add_dev(dev, class_intf);
mutex_unlock(&dev->class->p->mutex);
}
+
+ /* if DT-created device referenced by phandle, create a component */
+ np = dev->of_node;
+ if (np && np->phandle &&
+ !(np->_flags & OF_DEV_COMPONENT)) {
+ component_add(dev, NULL);
+ np->_flags |= OF_DEV_IMPLICIT_COMPONENT;
+ }
+
done:
put_device(dev);
return error;
@@ -1189,10 +1200,17 @@ void device_del(struct device *dev)
{
struct device *parent = dev->parent;
struct class_interface *class_intf;
+ struct device_node *np;
/* Notify clients of device removal. This call must come
* before dpm_sysfs_remove().
*/
+ np = dev->of_node;
+ if (np && (np->_flags & OF_DEV_COMPONENT)) {
+ component_del(dev, NULL);
+ np->_flags &= ~OF_DEV_IMPLICIT_COMPONENT;
+ }
+
if (dev->bus)
blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
BUS_NOTIFY_DEL_DEVICE, dev);
diff --git a/include/linux/of.h b/include/linux/of.h
index 70c64ba..40f1c34 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -59,6 +59,8 @@ struct device_node {
struct proc_dir_entry *pde;/* this node's proc directory */
struct kref kref;
unsigned long _flags;
+#define OF_DEV_COMPONENT 1
+#define OF_DEV_IMPLICIT_COMPONENT 2
void*data;
#if defined(CONFIG_SPARC)
const char *path_component_name;
--
1.9.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/2] drivers/base: permit base components to omit the bind/unbind ops
Some simple components don't need to do any specific action on
bind to / unbind from a master component.
This patch permits such components to omit the bind/unbind
operations.
Signed-off-by: Jean-Francois Moine
---
drivers/base/component.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/base/component.c b/drivers/base/component.c
index c53efe6..0a39d7a 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -225,7 +225,8 @@ static void component_unbind(struct component *component,
{
WARN_ON(!component->bound);
- component->ops->unbind(component->dev, master->dev, data);
+ if (component->ops)
+ component->ops->unbind(component->dev, master->dev, data);
component->bound = false;
/* Release all resources claimed in the binding of this component */
@@ -274,7 +275,11 @@ static int component_bind(struct component *component,
struct master *master,
dev_dbg(master->dev, "binding %s (ops %ps)\n",
dev_name(component->dev), component->ops);
- ret = component->ops->bind(component->dev, master->dev, data);
+ if (component->ops)
+ ret = component->ops->bind(component->dev, master->dev, data);
+ else
+ ret = 0;
+
if (!ret) {
component->bound = true;
--
1.9.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems
On Fri, 7 Feb 2014 09:46:56 + Russell King - ARM Linux wrote: > On Fri, Feb 07, 2014 at 10:04:30AM +0100, Daniel Vetter wrote: > > I've chatted a bit with Hans Verkuil about this topic at fosdem and > > apparently both v4l and alsa have something like this already in their > > helper libraries. Adding more people as fyi in case they want to > > switch to the new driver core stuff from Russell. > > It's not ALSA, but ASoC which has this. Mark is already aware of this > and will be looking at it from an ASoC perspective. Russell, I started to use your code (which works fine, thanks), and it avoids a lot of problems, especially, about probe_defer in a DT context. I was wondering if your componentised mechanism could be extended to the devices defined by DT. In the DT, when a device_node is a phandle, this means it is referenced by some other device(s), and these device(s) will not start until the phandle device is registered. Then, the idea is to do a component_add() for such phandle devices in device_add() (device_register). Pratically, - the component_add() call in device_register would not include any bind/unbind callback function, so, this should be tested in component_bind/unbind(), - component_add would not be called if the device being added already called component_add in its probe function. A simple flag in the struct device_node should solve this problem. What do you think about this? -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
