Re: [PATCH 1/5 v13] arm: omap: usb: ehci and ohci hwmod structuresfor omap4

[Felipe Balbi]

On Mon, Oct 10, 2011 at 12:26:15PM +0300, Felipe Balbi wrote:
> On Mon, Oct 10, 2011 at 02:47:42PM +0530, Munegowda, Keshava wrote:
> > On Mon, Oct 10, 2011 at 2:33 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> > > Hi,
> > >
> > > On Mon, Oct 10, 2011 at 02:22:23PM +0530, Munegowda, Keshava wrote:
> > >> Hi paul and Felipe
> > >>
> > >> Here is the highlights of the change in the design of  USB Host which
> > >> we can do after kernel 3.2 release;
> > >>
> > >> 1. separate the TLL changes  from UHH
> > >> 2. The TLL is be a new platform driver in ./drivers/mfd
> > >> 3. the TLL platform driver will export apis  for enable/disable clocks
> > >> and settings.
> > >
> > > TLL should control its clocks through pm_runtime APIs like anything
> > > else. If you really must export APIs to be used by UHH, you need to have
> > > an API so that you can claim/release a TLL channel and get/put for
> > > increasing/decreasing PM counters.
> > >
> > > I still think though, you should try to avoid exporting OMAP-specific
> > > APIs all over the place. Ideally, we would be able to have some way of
> > > saying that UHH and TLL are closely related... something like having the
> > > ability to say e.g. two devices are sibblings of each other, so that we
> > > could ask for a sibbling to wakeup/sleep depending if we need it or not.
> > 
> > do we have sibling structures today? I dont think so.
> 
> no we don't.

Ok, here's a first shot at it:

From 600ae62f4b4a832d90a83d43227deb6f84b9def1 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <balbi@xxxxxx>
Date: Mon, 10 Oct 2011 12:56:34 +0300
Subject: [RFC/NOT-FOR-MERGING/PATCH] base: add the idea of sibling devices
Organization: Texas Instruments\n

It's possible that some devices, which share a
common parent, need to talk to each due to a
very close relationship between them.

Generally, one device will provide some sort of
resources to the other (e.g. clocks) while still
both sharing another common parent.

In such cases, it seems necessary to define those
two devices as siblings, rather than a virtual
parent relationship, and have means for one device
to ask the sibling device to e.g. turn on its clocks.

Signed-off-by: Felipe Balbi <balbi@xxxxxx>
---
 drivers/base/core.c    |   41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/device.h |    7 +++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index bc8729d..3b7f2e5 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -589,6 +589,7 @@ void device_initialize(struct device *dev)
 	dev->kobj.kset = devices_kset;
 	kobject_init(&dev->kobj, &device_ktype);
 	INIT_LIST_HEAD(&dev->dma_pools);
+	INIT_LIST_HEAD(&dev->siblings);
 	mutex_init(&dev->mutex);
 	lockdep_set_novalidate_class(&dev->mutex);
 	spin_lock_init(&dev->devres_lock);
@@ -889,6 +890,7 @@ int device_private_init(struct device *dev)
  */
 int device_add(struct device *dev)
 {
+	struct device *sibling, *n;
 	struct device *parent = NULL;
 	struct class_interface *class_intf;
 	int error = -EINVAL;
@@ -923,6 +925,10 @@ int device_add(struct device *dev)
 	parent = get_device(dev->parent);
 	setup_parent(dev, parent);
 
+	/* setup siblings */
+	list_for_each_entry_safe(sibling, n, &dev->siblings, sibling_node)
+		get_device(sibling);
+
 	/* use parent numa_node */
 	if (parent)
 		set_dev_node(dev, dev_to_node(parent));
@@ -1071,6 +1077,31 @@ void put_device(struct device *dev)
 }
 
 /**
+ * get_sibling_device_byname - finds a sibling device by its name
+ * @dev: device.
+ * @name: name for the sibling to find.
+ *
+ * This is here to help drivers which need to ask their siblings
+ * for something in particular (like ask sibling to turn clocks on)
+ * achieve that by first finding the correct device pointer for
+ * that sibling.
+ */
+struct device *get_sibling_device_byname(struct device *dev, const char *name)
+{
+	struct device	*sibling, *n;
+	struct device	*found = NULL;
+
+	list_for_each_entry_safe(sibling, n, &dev->siblings, sibling_node) {
+		if (!strcmp(dev_name(sibling), name))
+			found = sibling;
+			goto found;
+	}
+
+found:
+	return found;
+}
+
+/**
  * device_del - delete device from system.
  * @dev: device.
  *
@@ -1085,6 +1116,7 @@ void put_device(struct device *dev)
  */
 void device_del(struct device *dev)
 {
+	struct device *sibling, *n;
 	struct device *parent = dev->parent;
 	struct class_interface *class_intf;
 
@@ -1120,6 +1152,15 @@ void device_del(struct device *dev)
 	device_remove_attrs(dev);
 	bus_remove_device(dev);
 
+	/* teardown siblings */
+	list_for_each_entry_safe(sibling, n, &dev->siblings, sibling_node) {
+		/* siblings must have  the same parent */
+		WARN(sibling->parent != parent,
+				"siblings must have a common parent\n");
+
+		get_device(sibling);
+	}
+
 	/*
 	 * Some platform devices are driven without driver attached
 	 * and managed resources may have been acquired.  Make sure
diff --git a/include/linux/device.h b/include/linux/device.h
index c20dfbf..ae9cec9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -551,6 +551,9 @@ struct device_dma_parameters {
 struct device {
 	struct device		*parent;
 
+	struct list_head	sibling_node;
+	struct list_head	siblings;
+
 	struct device_private	*p;
 
 	struct kobject kobj;
@@ -764,6 +767,10 @@ extern int (*platform_notify_remove)(struct device *dev);
 extern struct device *get_device(struct device *dev);
 extern void put_device(struct device *dev);
 
+/* finds a sibling struct device pointer */
+extern struct device *get_sibling_device_byname(struct device *dev,
+		const char *name);
+
 extern void wait_for_device_probe(void);
 
 #ifdef CONFIG_DEVTMPFS
-- 
1.7.6.396.ge0613

one way to use this would be to mark both hwmods has having the same
parent and pointing to each other as siblings. Then, from UHH you could:

if (port->mode == TLL) {
	tll = get_sibling_device_by_name(dev, "usbtll");
	if (!tll)
		error();

	pm_runtime_get_sync(tll);
}

or something similar. As of now, I'm not so sure this is a very good
idea, but decided to gather some comments anyway.

Does anybody see any change of this getting used in more cases ? Greg,
I'm adding you to this thread, if you have any comments to this, I'd
like to hear them.

-- 
balbi

Attachment: signature.asc
Description: Digital signature