[RFC PATCH 0/5] USB: prepare for support port power off on non-ACPI device

2012-12-02 Thread Ming Lei
Hi,

This patch set trys to prepare for support of usb port power
off mechanism on non-ACPI devices. The port power off mechasnism
has been discussed for some time[1][2], and support for ACPI
devices has been in shape:

- usb port device is introduced
- port connect type is introduced and set via ACPI
- usb root port power can be switched on/off via ACPI

The above has been merged to upstream, and Tianyu is pushing
patches[3] to enable auto port power feature.

So it is time to discuss how to support it for non-ACPI usb
devices, and there are many embedded system in which some of
usb devices are hardwired(often self-powered) into board, such as,
beagle board, pandaboard, Arndaleboard, etc. So powering on/off
these devices may involve platform dependent way, just like
ACPI power control is used in Tianyu's patch. 

This patch set introduces power controller to hide the power
switch implementation detail to usb subsytem, in theroy it can
be used to other devices and subsystem, and the power controller
instance is stored as device's resource. So the power controller
can be used to power on/off the power supply from the usb port,
and makes support of port power off possible on non-ACPI devices.

Associating power controller with one device(such as usb port)
which providing power logically is not an easy thing, because
platform dependent knowledge(port topology, hardware power domain
on the port, ...) has to be applied and the usb port device isn't
a platform device and it is created during hub probe(). So looks
there is no general approach to do it. This patchset introduces
one global device ADD/DEL notifier in driver core, so when a new
device is added, one match function in platform code is called to
check if the device can be associated with the power controller.
This patchset implements the match function on OMAP4 based Pandaboard,
and defines the power controller to control power for lan95xx hub
and ethernet device.

Another idea of associating power controller with port device
is by introducing usb port driver, and move all this port power
control stuff from platform code to port driver, which is just
what I think of and looks doable. I'd like to get some feedback
about which one is better, then I can do it in next cycle.

Also the two things on Pandaboard have been done at the same time:

- powering on the two devices can be delayed to the ehci-omap
root hub probing.

- the regulatores which are used for these two hardwired and
self-powered devices are moved out from ehci-omap driver

In fact, Andy Green has been working[4][5] on the above two things,
but which isn't the main purpose of these patches, and they might be
seen as byproduct of the patchset. Inevitably, there are some overlap
between Andy's work and this patchset, hope we can compare, discuss
and figure out the perfect solution.

 arch/arm/mach-omap2/board-omap4panda.c |   99 +++-
 drivers/base/core.c|   21 ++
 drivers/base/power/Makefile|1 +
 drivers/base/power/power_controller.c  |  110 
 drivers/usb/core/hub.c |   31 -
 drivers/usb/host/ehci-omap.c   |   36 ---
 include/linux/device.h |   13 
 include/linux/power_controller.h   |   48 ++
 include/linux/usb/port.h   |   16 +
 kernel/power/Kconfig   |6 ++
 10 files changed, 339 insertions(+), 42 deletions(-)

Thanks,
--
Ming Lei


[1], http://marc.info/?t=13481835493r=1w=2
[2], http://marc.info/?t=13430338453r=1w=2
[3], http://marc.info/?l=linux-usbm=135314427413307w=2
[4], http://marc.info/?t=13539339473r=1w=2
[5], http://www.spinics.net/lists/linux-omap/msg83191.html


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH 1/5] Device Power: introduce power controller

2012-12-02 Thread Ming Lei
Power controller is an abstract on simple power on/off switch.

One power controller can bind to more than one device, which
provides power logically, for example, we can think one usb port
in hub provides power to the usb device attached to the port, even
though the power is supplied actually by other ways, eg. the usb
hub is a self-power device. From hardware view, more than one
device can share one power domain, and power controller can power
on if one of these devices need to provide power, and power off if
all these devices don't need to provide power.

Cc: Rafael J. Wysocki r...@sisk.pl
Cc: Andy Green andy.gr...@linaro.org
Cc: Roger Quadros rog...@ti.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Ming Lei tom.leim...@gmail.com
---
 drivers/base/power/Makefile   |1 +
 drivers/base/power/power_controller.c |  110 +
 include/linux/power_controller.h  |   48 ++
 kernel/power/Kconfig  |6 ++
 4 files changed, 165 insertions(+)
 create mode 100644 drivers/base/power/power_controller.c
 create mode 100644 include/linux/power_controller.h

diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile
index 2e58ebb..c08bfc9 100644
--- a/drivers/base/power/Makefile
+++ b/drivers/base/power/Makefile
@@ -5,5 +5,6 @@ obj-$(CONFIG_PM_TRACE_RTC)  += trace.o
 obj-$(CONFIG_PM_OPP)   += opp.o
 obj-$(CONFIG_PM_GENERIC_DOMAINS)   +=  domain.o domain_governor.o
 obj-$(CONFIG_HAVE_CLK) += clock_ops.o
+obj-$(CONFIG_POWER_CONTROLLER) += power_controller.o
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
diff --git a/drivers/base/power/power_controller.c 
b/drivers/base/power/power_controller.c
new file mode 100644
index 000..d7671fb
--- /dev/null
+++ b/drivers/base/power/power_controller.c
@@ -0,0 +1,110 @@
+#include linux/init.h
+#include linux/kernel.h
+#include linux/power_controller.h
+#include linux/slab.h
+#include linux/err.h
+#include linux/export.h
+
+static DEFINE_MUTEX(pc_lock);
+
+static void pc_devm_release(struct device *dev, void *res)
+{
+}
+
+static int pc_devm_match(struct device *dev, void *res, void *match_data)
+{
+   struct pc_dev_data *data = res;
+
+   return (data-magic == (unsigned long)pc_devm_release);
+}
+
+static struct pc_dev_data *dev_pc_data(struct device *dev)
+{
+   struct pc_dev_data *data;
+
+   data = devres_find(dev, pc_devm_release, pc_devm_match, NULL);
+   return data;
+}
+
+static int pc_add_devm_data(struct device *dev, struct power_controller *pc,
+   void *dev_data, int dev_data_size)
+{
+   struct pc_dev_data *data;
+   int ret = 0;
+
+   mutex_lock(pc_lock);
+
+   /* each device should only have one power controller resource */
+   data = dev_pc_data(dev);
+   if (data) {
+   ret = 1;
+   goto exit;
+   }
+
+   data = devres_alloc(pc_devm_release, sizeof(struct pc_dev_data) +
+   dev_data_size, GFP_KERNEL);
+   if (!data) {
+   ret = -ENOMEM;
+   goto exit;
+   }
+
+   data-magic = (unsigned long)pc_devm_release;
+   data-pc = pc;
+   data-dev_data = data[1];
+   memcpy(data-dev_data, dev_data, dev_data_size);
+   devres_add(dev, data);
+
+exit:
+   mutex_unlock(pc_lock);
+   return ret;
+}
+
+static struct power_controller *dev_pc(struct device *dev)
+{
+   struct pc_dev_data *data = dev_pc_data(dev);
+
+   if (data)
+   return data-pc;
+   return NULL;
+}
+
+struct pc_dev_data *dev_pc_get_data(struct device *dev)
+{
+   struct pc_dev_data *data = dev_pc_data(dev);
+   return data;
+}
+EXPORT_SYMBOL(dev_pc_get_data);
+
+int dev_pc_bind(struct power_controller *pc, struct device *dev,
+   void *dev_data, int dev_data_size)
+{
+   return pc_add_devm_data(dev, pc, dev_data, dev_data_size);
+}
+EXPORT_SYMBOL(dev_pc_bind);
+
+void dev_pc_unbind(struct power_controller *pc, struct device *dev)
+{
+}
+EXPORT_SYMBOL(dev_pc_unbind);
+
+void dev_pc_power_on(struct device *dev)
+{
+   struct power_controller *pc = dev_pc(dev);
+
+   if (!pc)return;
+
+   if (atomic_inc_return(pc-count) == 1)
+   pc-power_on(pc, dev);
+}
+EXPORT_SYMBOL(dev_pc_power_on);
+
+void dev_pc_power_off(struct device *dev)
+{
+   struct power_controller *pc = dev_pc(dev);
+
+   if (!pc)return;
+
+   if (!atomic_dec_return(pc-count))
+   pc-power_off(pc, dev);
+}
+EXPORT_SYMBOL(dev_pc_power_off);
diff --git a/include/linux/power_controller.h b/include/linux/power_controller.h
new file mode 100644
index 000..772f6d7
--- /dev/null
+++ b/include/linux/power_controller.h
@@ -0,0 +1,48 @@
+#ifndef _LINUX_POWER_CONTROLLER_H
+#define _LINUX_POWER_CONTROLLER_H
+
+#include linux/device.h
+
+/*
+ * One power controller provides simple power on and power off.
+ *
+ * One power controller can 

[RFC PATCH 2/5] driver core: introduce global device ADD/DEL notifier

2012-12-02 Thread Ming Lei
The global device ADD/DEL notifier is introduced so that
some platform code can bind some device resource to the
device. When this platform code runs, there is no any bus
information about the device, so have to resort to the
global notifier.

Cc: Andy Green andy.gr...@linaro.org
Cc: Roger Quadros rog...@ti.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Ming Lei tom.leim...@gmail.com
---
 drivers/base/core.c|   21 +
 include/linux/device.h |   13 +
 2 files changed, 34 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index a235085..37f11ff 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -43,6 +43,9 @@ static __init int sysfs_deprecated_setup(char *arg)
 early_param(sysfs.deprecated, sysfs_deprecated_setup);
 #endif
 
+/* global device nofifier */
+struct blocking_notifier_head dev_notifier;
+
 int (*platform_notify)(struct device *dev) = NULL;
 int (*platform_notify_remove)(struct device *dev) = NULL;
 static struct kobject *dev_kobj;
@@ -1041,6 +1044,8 @@ int device_add(struct device *dev)
if (platform_notify)
platform_notify(dev);
 
+   blocking_notifier_call_chain(dev_notifier, DEV_NOTIFY_ADD_DEVICE, dev);
+
error = device_create_file(dev, uevent_attr);
if (error)
goto attrError;
@@ -1228,6 +1233,7 @@ void device_del(struct device *dev)
device_pm_remove(dev);
driver_deferred_probe_del(dev);
 
+   blocking_notifier_call_chain(dev_notifier, DEV_NOTIFY_DEL_DEVICE, dev);
/* Notify the platform of the removal, in case they
 * need to do anything...
 */
@@ -1376,6 +1382,8 @@ struct device *device_find_child(struct device *parent, 
void *data,
 
 int __init devices_init(void)
 {
+   BLOCKING_INIT_NOTIFIER_HEAD(dev_notifier);
+
devices_kset = kset_create_and_add(devices, device_uevent_ops, NULL);
if (!devices_kset)
return -ENOMEM;
@@ -1995,6 +2003,19 @@ int dev_printk(const char *level, const struct device 
*dev,
 }
 EXPORT_SYMBOL(dev_printk);
 
+/* The notifier should be avoided as far as possible */
+int dev_register_notifier(struct notifier_block *nb)
+{
+   return blocking_notifier_chain_register(dev_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(dev_register_notifier);
+
+int dev_unregister_notifier(struct notifier_block *nb)
+{
+   return blocking_notifier_chain_unregister(dev_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(dev_unregister_notifier);
+
 #define define_dev_printk_level(func, kern_level)  \
 int func(const struct device *dev, const char *fmt, ...)   \
 {  \
diff --git a/include/linux/device.h b/include/linux/device.h
index 43dcda9..aeb54f6 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -179,6 +179,19 @@ extern int bus_unregister_notifier(struct bus_type *bus,
 #define BUS_NOTIFY_UNBOUND_DRIVER  0x0006 /* driver is unbound
  from the device */
 
+/* All 2 notifers below get called with the target struct device *
+ * as an argument. Note that those functions are likely to be called
+ * with the device lock held in the core, so be careful.
+ */
+#define DEV_NOTIFY_ADD_DEVICE  0x0001 /* device added */
+#define DEV_NOTIFY_DEL_DEVICE  0x0002 /* device removed */
+extern int dev_register_notifier(struct notifier_block *nb);
+extern int dev_unregister_notifier(struct notifier_block *nb);
+
+extern struct kset *bus_get_kset(struct bus_type *bus);
+extern struct klist *bus_get_device_klist(struct bus_type *bus);
+
+
 extern struct kset *bus_get_kset(struct bus_type *bus);
 extern struct klist *bus_get_device_klist(struct bus_type *bus);
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH 3/5] USB: hub: apply power controller on usb port

2012-12-02 Thread Ming Lei
This patch applies the power controller on usb port, so that
hub driver can power on one port which isn't provided power
by bus.

Cc: Andy Green andy.gr...@linaro.org
Cc: Roger Quadros rog...@ti.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Ming Lei tom.leim...@gmail.com
---
 drivers/usb/core/hub.c   |   31 ---
 include/linux/usb/port.h |   16 
 2 files changed, 44 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/usb/port.h

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a815fd2..f8075d7 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -26,6 +26,8 @@
 #include linux/mutex.h
 #include linux/freezer.h
 #include linux/random.h
+#include linux/power_controller.h
+#include linux/usb/port.h
 
 #include asm/uaccess.h
 #include asm/byteorder.h
@@ -848,8 +850,15 @@ static unsigned hub_power_on(struct usb_hub *hub, bool 
do_delay)
else
dev_dbg(hub-intfdev, trying to enable port power on 
non-switchable hub\n);
-   for (port1 = 1; port1 = hub-descriptor-bNbrPorts; port1++)
+   for (port1 = 1; port1 = hub-descriptor-bNbrPorts; port1++) {
+   struct usb_port *port = hub-ports[port1 - 1];
+   struct pc_dev_data *pc_data = dev_pc_get_data(port-dev);
+
+   /* The power supply for this port isn't managed by bus only */
+   if (pc_data)
+   dev_pc_power_on(port-dev);
set_port_feature(hub-hdev, port1, USB_PORT_FEAT_POWER);
+   }
 
/* Wait at least 100 msec for power to become stable */
delay = max(pgood_delay, (unsigned) 100);
@@ -1541,10 +1550,20 @@ static int hub_configure(struct usb_hub *hub,
if (hub-has_indicators  blinkenlights)
hub-indicator [0] = INDICATOR_CYCLE;
 
-   for (i = 0; i  hdev-maxchild; i++)
+   for (i = 0; i  hdev-maxchild; i++) {
if (usb_hub_create_port_device(hub, i + 1)  0)
dev_err(hub-intfdev,
couldn't create port%d device.\n, i + 1);
+   else {
+   struct usb_port *port = hub-ports[i];
+   struct pc_dev_data *pc_data = 
dev_pc_get_data(port-dev);
+   if (pc_data  pc_data-dev_data) {
+   struct usb_port_power_switch_data *up =
+   pc_data-dev_data;
+   usb_set_hub_port_connect_type(hdev, i + 1, 
up-type);
+   }
+   }
+   }
 
hub_activate(hub, HUB_INIT);
return 0;
@@ -1587,8 +1606,14 @@ static void hub_disconnect(struct usb_interface *intf)
 
usb_set_intfdata (intf, NULL);
 
-   for (i = 0; i  hdev-maxchild; i++)
+   for (i = 0; i  hdev-maxchild; i++) {
+   struct usb_port *port = hub-ports[i];
+   struct pc_dev_data *pc_data = dev_pc_get_data(port-dev);
+   if (pc_data)
+   dev_pc_power_off(port-dev);
+
usb_hub_remove_port_device(hub, i + 1);
+   }
hub-hdev-maxchild = 0;
 
if (hub-hdev-speed == USB_SPEED_HIGH)
diff --git a/include/linux/usb/port.h b/include/linux/usb/port.h
new file mode 100644
index 000..a853d5e
--- /dev/null
+++ b/include/linux/usb/port.h
@@ -0,0 +1,16 @@
+#ifndef __USB_CORE_PORT_H
+#define __USB_CORE_PORT_H
+
+#include linux/usb.h
+
+/*
+ * Only used for describing power switch which provide power to
+ * hardwired self-powered device which attached to the port
+ */
+struct usb_port_power_switch_data {
+   int hub_tier;   /* root hub is zero, next tier is 1, ... */
+   int port_number;
+   enum usb_port_connect_type type;
+};
+
+#endif
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices

2012-12-02 Thread Ming Lei
This patch defines power controller for powering on/off LAN95xx
USB hub and USB ethernet devices, and implements one match function
to associate the power controller with related USB port device.
The big problem of this approach is that it depends on the global
device ADD/DEL notifier.

Another idea of associating power controller with port device
is by introducing usb port driver, and move all this port power
control stuff from platform code to the port driver, which is just
what I think of and looks doable. The problem of the idea is that
port driver is per board, so maybe cause lots of platform sort of
code to be put under drivers/usb/port/, but this approach can avoid
global device ADD/DEL notifier.

I'd like to get some feedback about which one is better or other choice,
then I may do it in next cycle.

Cc: Andy Green andy.gr...@linaro.org
Cc: Roger Quadros rog...@ti.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Ming Lei tom.leim...@gmail.com
---
 arch/arm/mach-omap2/board-omap4panda.c |   99 +++-
 1 file changed, 96 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap4panda.c 
b/arch/arm/mach-omap2/board-omap4panda.c
index 5c8e9ce..3183832 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -32,6 +32,8 @@
 #include linux/usb/musb.h
 #include linux/wl12xx.h
 #include linux/platform_data/omap-abe-twl6040.h
+#include linux/power_controller.h
+#include linux/usb/port.h
 
 #include asm/hardware/gic.h
 #include asm/mach-types.h
@@ -154,6 +156,99 @@ static struct gpio panda_ehci_gpios[] __initdata = {
{ GPIO_HUB_NRESET,  GPIOF_OUT_INIT_LOW,  hub_nreset },
 };
 
+static void ehci_hub_power_on(struct power_controller *pc, struct device *dev)
+{
+   gpio_set_value(GPIO_HUB_NRESET, 1);
+   gpio_set_value(GPIO_HUB_POWER, 1);
+}
+
+static void ehci_hub_power_off(struct power_controller *pc, struct device *dev)
+{
+   gpio_set_value(GPIO_HUB_NRESET, 0);
+   gpio_set_value(GPIO_HUB_POWER, 0);
+}
+
+static struct usb_port_power_switch_data root_hub_port_data = {
+   .hub_tier   = 0,
+   .port_number = 1,
+   .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
+};
+
+static struct usb_port_power_switch_data smsc_hub_port_data = {
+   .hub_tier   = 1,
+   .port_number = 1,
+   .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
+};
+
+static struct power_controller pc = {
+   .name = omap_hub_eth_pc,
+   .count = ATOMIC_INIT(0),
+   .power_on = ehci_hub_power_on,
+   .power_off = ehci_hub_power_off,
+};
+
+static inline int omap_ehci_hub_port(struct device *dev)
+{
+   /* we expect dev-parent points to ehcd controller */
+   if (dev-parent  !strcmp(dev_name(dev-parent), ehci-omap.0))
+   return 1;
+   return 0;
+}
+
+static inline int dev_pc_match(struct device *dev)
+{
+   struct device *anc;
+   int ret = 0;
+
+   if (likely(strcmp(dev_name(dev), port1)))
+   goto exit;
+
+   if (dev-parent  (anc = dev-parent-parent)) {
+   if (omap_ehci_hub_port(anc)) {
+   ret = 1;
+   goto exit;
+   }
+
+   /* is it port of lan95xx hub? */
+   if ((anc = anc-parent)  omap_ehci_hub_port(anc)) {
+   ret = 2;
+   goto exit;
+   }
+   }
+exit:
+   return ret;
+}
+
+/*
+ * Notifications of device registration
+ */
+static int device_notify(struct notifier_block *nb, unsigned long action, void 
*data)
+{
+   struct device *dev = data;
+   int ret;
+
+   switch (action) {
+   case DEV_NOTIFY_ADD_DEVICE:
+   ret = dev_pc_match(dev);
+   if (likely(!ret))
+   goto exit;
+   if (ret == 1)
+   dev_pc_bind(pc, dev, root_hub_port_data, 
sizeof(root_hub_port_data));
+   else
+   dev_pc_bind(pc, dev, smsc_hub_port_data, 
sizeof(smsc_hub_port_data));
+   break;
+
+   case DEV_NOTIFY_DEL_DEVICE:
+   break;
+   }
+exit:
+   return 0;
+}
+
+static struct notifier_block usb_port_nb = {
+   .notifier_call = device_notify,
+};
+
 static void __init omap4_ehci_init(void)
 {
int ret;
@@ -178,12 +273,10 @@ static void __init omap4_ehci_init(void)
 
gpio_export(GPIO_HUB_POWER, 0);
gpio_export(GPIO_HUB_NRESET, 0);
-   gpio_set_value(GPIO_HUB_NRESET, 1);
 
usbhs_init(usbhs_bdata);
 
-   /* enable power to hub */
-   gpio_set_value(GPIO_HUB_POWER, 1);
+   dev_register_notifier(usb_port_nb);
 }
 
 static struct omap_musb_board_data musb_board_data = {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH 5/5] usb: omap ehci: remove all regulator control from ehci omap

2012-12-02 Thread Ming Lei
From: Andy Green andy.gr...@linaro.org

This series migrates it to the hub driver as suggested by
Felipe Balbi.

Cc: Andy Green andy.gr...@linaro.org
Cc: Roger Quadros rog...@ti.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Andy Green andy.gr...@linaro.org
Signed-off-by: Ming Lei tom.leim...@gmail.com
---
 drivers/usb/host/ehci-omap.c |   36 
 1 file changed, 36 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index ac17a7c..d25e39e 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -39,7 +39,6 @@
 #include linux/platform_device.h
 #include linux/slab.h
 #include linux/usb/ulpi.h
-#include linux/regulator/consumer.h
 #include linux/pm_runtime.h
 #include linux/gpio.h
 #include linux/clk.h
@@ -150,19 +149,6 @@ static int omap_ehci_init(struct usb_hcd *hcd)
return rc;
 }
 
-static void disable_put_regulator(
-   struct ehci_hcd_omap_platform_data *pdata)
-{
-   int i;
-
-   for (i = 0 ; i  OMAP3_HS_USB_PORTS ; i++) {
-   if (pdata-regulator[i]) {
-   regulator_disable(pdata-regulator[i]);
-   regulator_put(pdata-regulator[i]);
-   }
-   }
-}
-
 /* configure so an HC device and id are always provided */
 /* always called with process context; sleeping is OK */
 
@@ -176,14 +162,11 @@ static void disable_put_regulator(
 static int ehci_hcd_omap_probe(struct platform_device *pdev)
 {
struct device   *dev = pdev-dev;
-   struct ehci_hcd_omap_platform_data  *pdata = dev-platform_data;
struct resource *res;
struct usb_hcd  *hcd;
void __iomem*regs;
int ret = -ENODEV;
int irq;
-   int i;
-   charsupply[7];
 
if (usb_disabled())
return -ENODEV;
@@ -224,23 +207,6 @@ static int ehci_hcd_omap_probe(struct platform_device 
*pdev)
hcd-rsrc_len = resource_size(res);
hcd-regs = regs;
 
-   /* get ehci regulator and enable */
-   for (i = 0 ; i  OMAP3_HS_USB_PORTS ; i++) {
-   if (pdata-port_mode[i] != OMAP_EHCI_PORT_MODE_PHY) {
-   pdata-regulator[i] = NULL;
-   continue;
-   }
-   snprintf(supply, sizeof(supply), hsusb%d, i);
-   pdata-regulator[i] = regulator_get(dev, supply);
-   if (IS_ERR(pdata-regulator[i])) {
-   pdata-regulator[i] = NULL;
-   dev_dbg(dev,
-   failed to get ehci port%d regulator\n, i);
-   } else {
-   regulator_enable(pdata-regulator[i]);
-   }
-   }
-
pm_runtime_enable(dev);
pm_runtime_get_sync(dev);
 
@@ -266,7 +232,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
return 0;
 
 err_pm_runtime:
-   disable_put_regulator(pdata);
pm_runtime_put_sync(dev);
usb_put_hcd(hcd);
 
@@ -291,7 +256,6 @@ static int ehci_hcd_omap_remove(struct platform_device 
*pdev)
struct ehci_hcd_omap_platform_data *pdata   = dev-platform_data;
 
usb_remove_hcd(hcd);
-   disable_put_regulator(dev-platform_data);
iounmap(hcd-regs);
usb_put_hcd(hcd);
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] spi: omap2-mcspi: Fix the redifine warning

2012-12-02 Thread Mark Brown
On Mon, Nov 19, 2012 at 12:12:29PM +0530, Shubhrajyoti D wrote:
 Fix the below warning
 drivers/spi/spi-omap2-mcspi.c:336:34: warning: symbol 'tx' shadows an earlier 
 one
 drivers/spi/spi-omap2-mcspi.c:327:12: originally declared here

Applied, thanks.


signature.asc
Description: Digital signature


Re: [RFC PATCH 1/5] Device Power: introduce power controller

2012-12-02 Thread Andy Green

On 02/12/12 23:01, the mail apparently from Ming Lei included:

Power controller is an abstract on simple power on/off switch.

One power controller can bind to more than one device, which
provides power logically, for example, we can think one usb port
in hub provides power to the usb device attached to the port, even
though the power is supplied actually by other ways, eg. the usb
hub is a self-power device. From hardware view, more than one
device can share one power domain, and power controller can power
on if one of these devices need to provide power, and power off if
all these devices don't need to provide power.


What stops us using struct regulator here?  If you have child regulators 
supplied by a parent supply, isn't that the right semantic already 
without introducing a whole new thing?  Apologies if I missed the point.


-Andy


Cc: Rafael J. Wysocki r...@sisk.pl
Cc: Andy Green andy.gr...@linaro.org
Cc: Roger Quadros rog...@ti.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Ming Lei tom.leim...@gmail.com
---
  drivers/base/power/Makefile   |1 +
  drivers/base/power/power_controller.c |  110 +
  include/linux/power_controller.h  |   48 ++
  kernel/power/Kconfig  |6 ++
  4 files changed, 165 insertions(+)
  create mode 100644 drivers/base/power/power_controller.c
  create mode 100644 include/linux/power_controller.h

diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile
index 2e58ebb..c08bfc9 100644
--- a/drivers/base/power/Makefile
+++ b/drivers/base/power/Makefile
@@ -5,5 +5,6 @@ obj-$(CONFIG_PM_TRACE_RTC)  += trace.o
  obj-$(CONFIG_PM_OPP)  += opp.o
  obj-$(CONFIG_PM_GENERIC_DOMAINS)  +=  domain.o domain_governor.o
  obj-$(CONFIG_HAVE_CLK)+= clock_ops.o
+obj-$(CONFIG_POWER_CONTROLLER) += power_controller.o

  ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
diff --git a/drivers/base/power/power_controller.c 
b/drivers/base/power/power_controller.c
new file mode 100644
index 000..d7671fb
--- /dev/null
+++ b/drivers/base/power/power_controller.c
@@ -0,0 +1,110 @@
+#include linux/init.h
+#include linux/kernel.h
+#include linux/power_controller.h
+#include linux/slab.h
+#include linux/err.h
+#include linux/export.h
+
+static DEFINE_MUTEX(pc_lock);
+
+static void pc_devm_release(struct device *dev, void *res)
+{
+}
+
+static int pc_devm_match(struct device *dev, void *res, void *match_data)
+{
+   struct pc_dev_data *data = res;
+
+   return (data-magic == (unsigned long)pc_devm_release);
+}
+
+static struct pc_dev_data *dev_pc_data(struct device *dev)
+{
+   struct pc_dev_data *data;
+
+   data = devres_find(dev, pc_devm_release, pc_devm_match, NULL);
+   return data;
+}
+
+static int pc_add_devm_data(struct device *dev, struct power_controller *pc,
+   void *dev_data, int dev_data_size)
+{
+   struct pc_dev_data *data;
+   int ret = 0;
+
+   mutex_lock(pc_lock);
+
+   /* each device should only have one power controller resource */
+   data = dev_pc_data(dev);
+   if (data) {
+   ret = 1;
+   goto exit;
+   }
+
+   data = devres_alloc(pc_devm_release, sizeof(struct pc_dev_data) +
+   dev_data_size, GFP_KERNEL);
+   if (!data) {
+   ret = -ENOMEM;
+   goto exit;
+   }
+
+   data-magic = (unsigned long)pc_devm_release;
+   data-pc = pc;
+   data-dev_data = data[1];
+   memcpy(data-dev_data, dev_data, dev_data_size);
+   devres_add(dev, data);
+
+exit:
+   mutex_unlock(pc_lock);
+   return ret;
+}
+
+static struct power_controller *dev_pc(struct device *dev)
+{
+   struct pc_dev_data *data = dev_pc_data(dev);
+
+   if (data)
+   return data-pc;
+   return NULL;
+}
+
+struct pc_dev_data *dev_pc_get_data(struct device *dev)
+{
+   struct pc_dev_data *data = dev_pc_data(dev);
+   return data;
+}
+EXPORT_SYMBOL(dev_pc_get_data);
+
+int dev_pc_bind(struct power_controller *pc, struct device *dev,
+   void *dev_data, int dev_data_size)
+{
+   return pc_add_devm_data(dev, pc, dev_data, dev_data_size);
+}
+EXPORT_SYMBOL(dev_pc_bind);
+
+void dev_pc_unbind(struct power_controller *pc, struct device *dev)
+{
+}
+EXPORT_SYMBOL(dev_pc_unbind);
+
+void dev_pc_power_on(struct device *dev)
+{
+   struct power_controller *pc = dev_pc(dev);
+
+   if (!pc)return;
+
+   if (atomic_inc_return(pc-count) == 1)
+   pc-power_on(pc, dev);
+}
+EXPORT_SYMBOL(dev_pc_power_on);
+
+void dev_pc_power_off(struct device *dev)
+{
+   struct power_controller *pc = dev_pc(dev);
+
+   if (!pc)return;
+
+   if (!atomic_dec_return(pc-count))
+   pc-power_off(pc, dev);
+}
+EXPORT_SYMBOL(dev_pc_power_off);
diff --git a/include/linux/power_controller.h b/include/linux/power_controller.h
new file 

Re: [RFC PATCH 2/5] driver core: introduce global device ADD/DEL notifier

2012-12-02 Thread Andy Green

On 02/12/12 23:01, the mail apparently from Ming Lei included:

The global device ADD/DEL notifier is introduced so that
some platform code can bind some device resource to the
device. When this platform code runs, there is no any bus
information about the device, so have to resort to the
global notifier.

Cc: Andy Green andy.gr...@linaro.org
Cc: Roger Quadros rog...@ti.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Ming Lei tom.leim...@gmail.com
---
  drivers/base/core.c|   21 +
  include/linux/device.h |   13 +
  2 files changed, 34 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index a235085..37f11ff 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -43,6 +43,9 @@ static __init int sysfs_deprecated_setup(char *arg)
  early_param(sysfs.deprecated, sysfs_deprecated_setup);
  #endif

+/* global device nofifier */
+struct blocking_notifier_head dev_notifier;
+
  int (*platform_notify)(struct device *dev) = NULL;
  int (*platform_notify_remove)(struct device *dev) = NULL;
  static struct kobject *dev_kobj;
@@ -1041,6 +1044,8 @@ int device_add(struct device *dev)
if (platform_notify)
platform_notify(dev);

+   blocking_notifier_call_chain(dev_notifier, DEV_NOTIFY_ADD_DEVICE, dev);
+
error = device_create_file(dev, uevent_attr);
if (error)
goto attrError;
@@ -1228,6 +1233,7 @@ void device_del(struct device *dev)
device_pm_remove(dev);
driver_deferred_probe_del(dev);

+   blocking_notifier_call_chain(dev_notifier, DEV_NOTIFY_DEL_DEVICE, dev);
/* Notify the platform of the removal, in case they
 * need to do anything...
 */
@@ -1376,6 +1382,8 @@ struct device *device_find_child(struct device *parent, 
void *data,

  int __init devices_init(void)
  {
+   BLOCKING_INIT_NOTIFIER_HEAD(dev_notifier);
+
devices_kset = kset_create_and_add(devices, device_uevent_ops, NULL);
if (!devices_kset)
return -ENOMEM;
@@ -1995,6 +2003,19 @@ int dev_printk(const char *level, const struct device 
*dev,
  }
  EXPORT_SYMBOL(dev_printk);

+/* The notifier should be avoided as far as possible */
+int dev_register_notifier(struct notifier_block *nb)
+{
+   return blocking_notifier_chain_register(dev_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(dev_register_notifier);
+
+int dev_unregister_notifier(struct notifier_block *nb)
+{
+   return blocking_notifier_chain_unregister(dev_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(dev_unregister_notifier);
+
  #define define_dev_printk_level(func, kern_level) \
  int func(const struct device *dev, const char *fmt, ...)  \
  { \
diff --git a/include/linux/device.h b/include/linux/device.h
index 43dcda9..aeb54f6 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -179,6 +179,19 @@ extern int bus_unregister_notifier(struct bus_type *bus,
  #define BUS_NOTIFY_UNBOUND_DRIVER 0x0006 /* driver is unbound
  from the device */

+/* All 2 notifers below get called with the target struct device *
+ * as an argument. Note that those functions are likely to be called
+ * with the device lock held in the core, so be careful.
+ */
+#define DEV_NOTIFY_ADD_DEVICE  0x0001 /* device added */
+#define DEV_NOTIFY_DEL_DEVICE  0x0002 /* device removed */
+extern int dev_register_notifier(struct notifier_block *nb);
+extern int dev_unregister_notifier(struct notifier_block *nb);
+
+extern struct kset *bus_get_kset(struct bus_type *bus);
+extern struct klist *bus_get_device_klist(struct bus_type *bus);
+
+
  extern struct kset *bus_get_kset(struct bus_type *bus);
  extern struct klist *bus_get_device_klist(struct bus_type *bus);


Device de/registraton time is not necessarily a good choice for the 
notification point.  At boot time, platform_devices may be being 
registered with no sign of driver and anything getting enabled in the 
notifier without further filtering (at each notifier...) will then 
always be enabled the whole session whether in use or not.


probe() and remove() are more interesting because at that point the 
driver is present and we're definitely instantiating the thing or 
finished with its instantiation, and it's valid for non-usb devices too.


In the one case you're servicing in the series, usb hub port, the device 
is very unusual in that it has no driver.  However if you're going to 
add a global notifier in generic device code, you should have it do the 
right thing for normal device case so other people can use it... how I 
solved this for hub port was to simply normalize hub port by introducing 
a stub driver for it, so you get a probe / remove like anything else.


-Andy

--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro

Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices

2012-12-02 Thread Andy Green

On 02/12/12 23:01, the mail apparently from Ming Lei included:

Hi -


This patch defines power controller for powering on/off LAN95xx
USB hub and USB ethernet devices, and implements one match function
to associate the power controller with related USB port device.
The big problem of this approach is that it depends on the global
device ADD/DEL notifier.

Another idea of associating power controller with port device
is by introducing usb port driver, and move all this port power
control stuff from platform code to the port driver, which is just
what I think of and looks doable. The problem of the idea is that
port driver is per board, so maybe cause lots of platform sort of
code to be put under drivers/usb/port/, but this approach can avoid
global device ADD/DEL notifier.

I'd like to get some feedback about which one is better or other choice,
then I may do it in next cycle.

Cc: Andy Green andy.gr...@linaro.org
Cc: Roger Quadros rog...@ti.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Ming Lei tom.leim...@gmail.com
---
  arch/arm/mach-omap2/board-omap4panda.c |   99 +++-
  1 file changed, 96 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap4panda.c 
b/arch/arm/mach-omap2/board-omap4panda.c
index 5c8e9ce..3183832 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -32,6 +32,8 @@
  #include linux/usb/musb.h
  #include linux/wl12xx.h
  #include linux/platform_data/omap-abe-twl6040.h
+#include linux/power_controller.h
+#include linux/usb/port.h

  #include asm/hardware/gic.h
  #include asm/mach-types.h
@@ -154,6 +156,99 @@ static struct gpio panda_ehci_gpios[] __initdata = {
{ GPIO_HUB_NRESET,  GPIOF_OUT_INIT_LOW,  hub_nreset },
  };

+static void ehci_hub_power_on(struct power_controller *pc, struct device *dev)
+{
+   gpio_set_value(GPIO_HUB_NRESET, 1);
+   gpio_set_value(GPIO_HUB_POWER, 1);
+}


You should wait a bit after applying power to the smsc chip before 
letting go of nReset too.  In the regulator-based implementation I sent 
it's handled by delays encoded in the regulator structs.



+static void ehci_hub_power_off(struct power_controller *pc, struct device *dev)
+{
+   gpio_set_value(GPIO_HUB_NRESET, 0);
+   gpio_set_value(GPIO_HUB_POWER, 0);
+}
+
+static struct usb_port_power_switch_data root_hub_port_data = {
+   .hub_tier   = 0,
+   .port_number = 1,
+   .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
+};
+
+static struct usb_port_power_switch_data smsc_hub_port_data = {
+   .hub_tier   = 1,
+   .port_number = 1,
+   .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
+};
+
+static struct power_controller pc = {
+   .name = omap_hub_eth_pc,
+   .count = ATOMIC_INIT(0),
+   .power_on = ehci_hub_power_on,
+   .power_off = ehci_hub_power_off,
+};
+
+static inline int omap_ehci_hub_port(struct device *dev)
+{
+   /* we expect dev-parent points to ehcd controller */
+   if (dev-parent  !strcmp(dev_name(dev-parent), ehci-omap.0))
+   return 1;
+   return 0;
+}
+
+static inline int dev_pc_match(struct device *dev)
+{
+   struct device *anc;
+   int ret = 0;
+
+   if (likely(strcmp(dev_name(dev), port1)))
+   goto exit;
+
+   if (dev-parent  (anc = dev-parent-parent)) {
+   if (omap_ehci_hub_port(anc)) {
+   ret = 1;
+   goto exit;
+   }
+
+   /* is it port of lan95xx hub? */
+   if ((anc = anc-parent)  omap_ehci_hub_port(anc)) {
+   ret = 2;
+   goto exit;
+   }
+   }
+exit:
+   return ret;
+}
+
+/*
+ * Notifications of device registration
+ */
+static int device_notify(struct notifier_block *nb, unsigned long action, void 
*data)
+{
+   struct device *dev = data;
+   int ret;
+
+   switch (action) {
+   case DEV_NOTIFY_ADD_DEVICE:
+   ret = dev_pc_match(dev);
+   if (likely(!ret))
+   goto exit;
+   if (ret == 1)
+   dev_pc_bind(pc, dev, root_hub_port_data, 
sizeof(root_hub_port_data));
+   else
+   dev_pc_bind(pc, dev, smsc_hub_port_data, 
sizeof(smsc_hub_port_data));
+   break;
+
+   case DEV_NOTIFY_DEL_DEVICE:
+   break;
+   }
+exit:
+   return 0;
+}
+
+static struct notifier_block usb_port_nb = {
+   .notifier_call = device_notify,
+};
+


Some thoughts on trying to make this functionality specific to power 
only and ehci hub port only:


 - Quite a few boards have smsc95xx... they're all going to carry these 
additions in the board file?  Surely you'll have to generalize the 
pieces that perform device_path business out of the omap4panda board 
file at least.  At that point the path matching code becomes generic 

Re: [PATCH v2 22/22] mfd: omap-usb-host: Don't spam console on clk_set_parent failure

2012-12-02 Thread Sergei Shtylyov

Hello.

On 28-11-2012 18:49, Roger Quadros wrote:


clk_set_parent is expected to fail on OMAP3 platforms. We don't
consider that as fatal so don't spam console.



Signed-off-by: Roger Quadros rog...@ti.com
---
  drivers/mfd/omap-usb-host.c |   10 +-
  1 files changed, 5 insertions(+), 5 deletions(-)



diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 6ede319..493e010 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -653,17 +653,17 @@ static int __devinit usbhs_omap_probe(struct 
platform_device *pdev)
}

if (is_ehci_phy_mode(pdata-port_mode[0])) {
-   /* for OMAP3 , the clk set paretn fails */
+   /* for OMAP3 , the clk set parent fails */


   Worth removing spave before comma too.


ret = clk_set_parent(omap-utmi_clk[0],
omap-xclk60mhsp1_ck);
if (ret != 0)
-   dev_err(dev, xclk60mhsp1_ck set parent
+   dev_dbg(dev, xclk60mhsp1_ck set parent


   Need space at the end of this substring, else you get parentfailed


failed error:%d\n, ret);
} else if (is_ehci_tll_mode(pdata-port_mode[0])) {
ret = clk_set_parent(omap-utmi_clk[0],
omap-init_60m_fclk);
if (ret != 0)
-   dev_err(dev, init_60m_fclk set parent
+   dev_dbg(dev, init_60m_fclk set parent


   Same here.


failed error:%d\n, ret);
}

@@ -671,13 +671,13 @@ static int __devinit usbhs_omap_probe(struct 
platform_device *pdev)
ret = clk_set_parent(omap-utmi_clk[1],
omap-xclk60mhsp2_ck);
if (ret != 0)
-   dev_err(dev, xclk60mhsp2_ck set parent
+   dev_dbg(dev, xclk60mhsp2_ck set parent


   Same here.


failed error:%d\n, ret);
} else if (is_ehci_tll_mode(pdata-port_mode[1])) {
ret = clk_set_parent(omap-utmi_clk[1],
omap-init_60m_fclk);
if (ret != 0)
-   dev_err(dev, init_60m_fclk set parent
+   dev_dbg(dev, init_60m_fclk set parent


   And here.


failed error:%d\n, ret);


WBR, Sergei


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 13/22] mfd: omap-usb-host: override number of ports from platform data

2012-12-02 Thread Sergei Shtylyov

Hello.

On 28-11-2012 18:49, Roger Quadros wrote:


Both OMAP4 and 5 exhibit the same revision ID in the REVISION register
but they have different number of ports i.e. 2 and 3 respectively.
So we can't rely on REVISION register for number of ports on OMAP5
and depend on platform data (or device tree) instead.



Signed-off-by: Roger Quadros rog...@ti.com

[...]


diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 87b574b..fda235a 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -495,19 +495,27 @@ static int __devinit usbhs_omap_probe(struct 
platform_device *pdev)
 */
pm_runtime_put_sync(dev);

-   switch (omap-usbhs_rev) {
-   case OMAP_USBHS_REV1:
-   omap-nports = 3;
-   break;
-   case OMAP_USBHS_REV2:
-   omap-nports = 2;
-   break;
-   default:
-   omap-nports = OMAP3_HS_USB_PORTS;
-   dev_dbg(dev,
- USB HOST Rev : 0x%d not recognized, assuming %d ports\n,
-  omap-usbhs_rev, omap-nports);
-   break;
+   /*
+* If platform data contains nports then use that
+* else make out number of ports from USBHS revision
+*/
+   if (pdata-nports) {
+   omap-nports = pdata-nports;


   Overindented line?


+   } else {
+   switch (omap-usbhs_rev) {
+   case OMAP_USBHS_REV1:
+   omap-nports = 3;
+   break;
+   case OMAP_USBHS_REV2:
+   omap-nports = 2;
+   break;
+   default:
+   omap-nports = OMAP3_HS_USB_PORTS;
+   dev_dbg(dev,
+   USB HOST Rev:0x%d not recognized, assuming %d ports\n,


   Indent this string a bit to the right please.

WBR, Sergei

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] Device Power: introduce power controller

2012-12-02 Thread Ming Lei
On Mon, Dec 3, 2012 at 12:02 AM, Andy Green andy.gr...@linaro.org wrote:
 On 02/12/12 23:01, the mail apparently from Ming Lei included:

 Power controller is an abstract on simple power on/off switch.

 One power controller can bind to more than one device, which
 provides power logically, for example, we can think one usb port
 in hub provides power to the usb device attached to the port, even
 though the power is supplied actually by other ways, eg. the usb
 hub is a self-power device. From hardware view, more than one
 device can share one power domain, and power controller can power
 on if one of these devices need to provide power, and power off if
 all these devices don't need to provide power.


 What stops us using struct regulator here?  If you have child regulators
 supplied by a parent supply, isn't that the right semantic already without
 introducing a whole new thing?  Apologies if I missed the point.

There are two purposes:

One is to hide the implementation details of the power controller because
the user doesn't care how it is implemented, maybe clock, regulator, gpio
and other platform dependent stuffs involved, so the patch simplify the usage
from the view of users.

Another is that several users may share one power controller, and the
introduced power controller can help users to use it.

Also the power controller is stored as device resource, not any new
stuff added into 'struct device', and all users of the power controller
needn't write code to operate device resource things too.

Thanks,
-- 
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 2/5] driver core: introduce global device ADD/DEL notifier

2012-12-02 Thread Ming Lei
On Mon, Dec 3, 2012 at 12:13 AM, Andy Green andy.gr...@linaro.org wrote:
 On 02/12/12 23:01, the mail apparently from Ming Lei included:

 The global device ADD/DEL notifier is introduced so that
 some platform code can bind some device resource to the
 device. When this platform code runs, there is no any bus
 information about the device, so have to resort to the
 global notifier.

 Cc: Andy Green andy.gr...@linaro.org
 Cc: Roger Quadros rog...@ti.com
 Cc: Alan Stern st...@rowland.harvard.edu
 Cc: Felipe Balbi ba...@ti.com
 Signed-off-by: Ming Lei tom.leim...@gmail.com
 ---
   drivers/base/core.c|   21 +
   include/linux/device.h |   13 +
   2 files changed, 34 insertions(+)

 diff --git a/drivers/base/core.c b/drivers/base/core.c
 index a235085..37f11ff 100644
 --- a/drivers/base/core.c
 +++ b/drivers/base/core.c
 @@ -43,6 +43,9 @@ static __init int sysfs_deprecated_setup(char *arg)
   early_param(sysfs.deprecated, sysfs_deprecated_setup);
   #endif

 +/* global device nofifier */
 +struct blocking_notifier_head dev_notifier;
 +
   int (*platform_notify)(struct device *dev) = NULL;
   int (*platform_notify_remove)(struct device *dev) = NULL;
   static struct kobject *dev_kobj;
 @@ -1041,6 +1044,8 @@ int device_add(struct device *dev)
 if (platform_notify)
 platform_notify(dev);

 +   blocking_notifier_call_chain(dev_notifier, DEV_NOTIFY_ADD_DEVICE,
 dev);
 +
 error = device_create_file(dev, uevent_attr);
 if (error)
 goto attrError;
 @@ -1228,6 +1233,7 @@ void device_del(struct device *dev)
 device_pm_remove(dev);
 driver_deferred_probe_del(dev);

 +   blocking_notifier_call_chain(dev_notifier, DEV_NOTIFY_DEL_DEVICE,
 dev);
 /* Notify the platform of the removal, in case they
  * need to do anything...
  */
 @@ -1376,6 +1382,8 @@ struct device *device_find_child(struct device
 *parent, void *data,

   int __init devices_init(void)
   {
 +   BLOCKING_INIT_NOTIFIER_HEAD(dev_notifier);
 +
 devices_kset = kset_create_and_add(devices, device_uevent_ops,
 NULL);
 if (!devices_kset)
 return -ENOMEM;
 @@ -1995,6 +2003,19 @@ int dev_printk(const char *level, const struct
 device *dev,
   }
   EXPORT_SYMBOL(dev_printk);

 +/* The notifier should be avoided as far as possible */
 +int dev_register_notifier(struct notifier_block *nb)
 +{
 +   return blocking_notifier_chain_register(dev_notifier, nb);
 +}
 +EXPORT_SYMBOL_GPL(dev_register_notifier);
 +
 +int dev_unregister_notifier(struct notifier_block *nb)
 +{
 +   return blocking_notifier_chain_unregister(dev_notifier, nb);
 +}
 +EXPORT_SYMBOL_GPL(dev_unregister_notifier);
 +
   #define define_dev_printk_level(func, kern_level) \
   int func(const struct device *dev, const char *fmt, ...)  \
   { \
 diff --git a/include/linux/device.h b/include/linux/device.h
 index 43dcda9..aeb54f6 100644
 --- a/include/linux/device.h
 +++ b/include/linux/device.h
 @@ -179,6 +179,19 @@ extern int bus_unregister_notifier(struct bus_type
 *bus,
   #define BUS_NOTIFY_UNBOUND_DRIVER 0x0006 /* driver is unbound
   from the device */

 +/* All 2 notifers below get called with the target struct device *
 + * as an argument. Note that those functions are likely to be called
 + * with the device lock held in the core, so be careful.
 + */
 +#define DEV_NOTIFY_ADD_DEVICE  0x0001 /* device added */
 +#define DEV_NOTIFY_DEL_DEVICE  0x0002 /* device removed */
 +extern int dev_register_notifier(struct notifier_block *nb);
 +extern int dev_unregister_notifier(struct notifier_block *nb);
 +
 +extern struct kset *bus_get_kset(struct bus_type *bus);
 +extern struct klist *bus_get_device_klist(struct bus_type *bus);
 +
 +
   extern struct kset *bus_get_kset(struct bus_type *bus);
   extern struct klist *bus_get_device_klist(struct bus_type *bus);


 Device de/registraton time is not necessarily a good choice for the
 notification point.  At boot time, platform_devices may be being registered
 with no sign of driver and anything getting enabled in the notifier without
 further filtering (at each notifier...) will then always be enabled the
 whole session whether in use or not.

 probe() and remove() are more interesting because at that point the driver
 is present and we're definitely instantiating the thing or finished with its
 instantiation, and it's valid for non-usb devices too.

 In the one case you're servicing in the series, usb hub port, the device is
 very unusual in that it has no driver.  However if you're going to add a
 global notifier in generic device code, you should have it do the right
 thing for normal device case so other people can use it... how I solved this
 for hub port was to simply normalize hub port by introducing a stub driver
 for 

Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices

2012-12-02 Thread Ming Lei
On Mon, Dec 3, 2012 at 12:37 AM, Andy Green andy.gr...@linaro.org wrote:
 On 02/12/12 23:01, the mail apparently from Ming Lei included:

 Hi -


 This patch defines power controller for powering on/off LAN95xx
 USB hub and USB ethernet devices, and implements one match function
 to associate the power controller with related USB port device.
 The big problem of this approach is that it depends on the global
 device ADD/DEL notifier.

 Another idea of associating power controller with port device
 is by introducing usb port driver, and move all this port power
 control stuff from platform code to the port driver, which is just
 what I think of and looks doable. The problem of the idea is that
 port driver is per board, so maybe cause lots of platform sort of
 code to be put under drivers/usb/port/, but this approach can avoid
 global device ADD/DEL notifier.

 I'd like to get some feedback about which one is better or other choice,
 then I may do it in next cycle.

 Cc: Andy Green andy.gr...@linaro.org
 Cc: Roger Quadros rog...@ti.com
 Cc: Alan Stern st...@rowland.harvard.edu
 Cc: Felipe Balbi ba...@ti.com
 Signed-off-by: Ming Lei tom.leim...@gmail.com
 ---
   arch/arm/mach-omap2/board-omap4panda.c |   99
 +++-
   1 file changed, 96 insertions(+), 3 deletions(-)

 diff --git a/arch/arm/mach-omap2/board-omap4panda.c
 b/arch/arm/mach-omap2/board-omap4panda.c
 index 5c8e9ce..3183832 100644
 --- a/arch/arm/mach-omap2/board-omap4panda.c
 +++ b/arch/arm/mach-omap2/board-omap4panda.c
 @@ -32,6 +32,8 @@
   #include linux/usb/musb.h
   #include linux/wl12xx.h
   #include linux/platform_data/omap-abe-twl6040.h
 +#include linux/power_controller.h
 +#include linux/usb/port.h

   #include asm/hardware/gic.h
   #include asm/mach-types.h
 @@ -154,6 +156,99 @@ static struct gpio panda_ehci_gpios[] __initdata = {
 { GPIO_HUB_NRESET,  GPIOF_OUT_INIT_LOW,  hub_nreset },
   };

 +static void ehci_hub_power_on(struct power_controller *pc, struct device
 *dev)
 +{
 +   gpio_set_value(GPIO_HUB_NRESET, 1);
 +   gpio_set_value(GPIO_HUB_POWER, 1);
 +}


 You should wait a bit after applying power to the smsc chip before letting
 go of nReset too.  In the regulator-based implementation I sent it's handled
 by delays encoded in the regulator structs.

It isn't a big thing about the discussion. If these code is only platform code,
we can use gpio or regulator or other thing.



 +static void ehci_hub_power_off(struct power_controller *pc, struct device
 *dev)
 +{
 +   gpio_set_value(GPIO_HUB_NRESET, 0);
 +   gpio_set_value(GPIO_HUB_POWER, 0);
 +}
 +
 +static struct usb_port_power_switch_data root_hub_port_data = {
 +   .hub_tier   = 0,
 +   .port_number = 1,
 +   .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
 +};
 +
 +static struct usb_port_power_switch_data smsc_hub_port_data = {
 +   .hub_tier   = 1,
 +   .port_number = 1,
 +   .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
 +};
 +
 +static struct power_controller pc = {
 +   .name = omap_hub_eth_pc,
 +   .count = ATOMIC_INIT(0),
 +   .power_on = ehci_hub_power_on,
 +   .power_off = ehci_hub_power_off,
 +};
 +
 +static inline int omap_ehci_hub_port(struct device *dev)
 +{
 +   /* we expect dev-parent points to ehcd controller */
 +   if (dev-parent  !strcmp(dev_name(dev-parent), ehci-omap.0))
 +   return 1;
 +   return 0;
 +}
 +
 +static inline int dev_pc_match(struct device *dev)
 +{
 +   struct device *anc;
 +   int ret = 0;
 +
 +   if (likely(strcmp(dev_name(dev), port1)))
 +   goto exit;
 +
 +   if (dev-parent  (anc = dev-parent-parent)) {
 +   if (omap_ehci_hub_port(anc)) {
 +   ret = 1;
 +   goto exit;
 +   }
 +
 +   /* is it port of lan95xx hub? */
 +   if ((anc = anc-parent)  omap_ehci_hub_port(anc)) {
 +   ret = 2;
 +   goto exit;
 +   }
 +   }
 +exit:
 +   return ret;
 +}
 +
 +/*
 + * Notifications of device registration
 + */
 +static int device_notify(struct notifier_block *nb, unsigned long action,
 void *data)
 +{
 +   struct device *dev = data;
 +   int ret;
 +
 +   switch (action) {
 +   case DEV_NOTIFY_ADD_DEVICE:
 +   ret = dev_pc_match(dev);
 +   if (likely(!ret))
 +   goto exit;
 +   if (ret == 1)
 +   dev_pc_bind(pc, dev, root_hub_port_data,
 sizeof(root_hub_port_data));
 +   else
 +   dev_pc_bind(pc, dev, smsc_hub_port_data,
 sizeof(smsc_hub_port_data));
 +   break;
 +
 +   case DEV_NOTIFY_DEL_DEVICE:
 +   break;
 +   }
 +exit:
 +   return 0;
 +}
 +
 +static struct notifier_block usb_port_nb = {
 +   .notifier_call = device_notify,
 +};
 +


 Some thoughts on trying to make this functionality specific to power 

Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices

2012-12-02 Thread Andy Green

On 03/12/12 12:11, the mail apparently from Ming Lei included:

On Mon, Dec 3, 2012 at 12:37 AM, Andy Green andy.gr...@linaro.org wrote:

On 02/12/12 23:01, the mail apparently from Ming Lei included:

Hi -



This patch defines power controller for powering on/off LAN95xx
USB hub and USB ethernet devices, and implements one match function
to associate the power controller with related USB port device.
The big problem of this approach is that it depends on the global
device ADD/DEL notifier.

Another idea of associating power controller with port device
is by introducing usb port driver, and move all this port power
control stuff from platform code to the port driver, which is just
what I think of and looks doable. The problem of the idea is that
port driver is per board, so maybe cause lots of platform sort of
code to be put under drivers/usb/port/, but this approach can avoid
global device ADD/DEL notifier.

I'd like to get some feedback about which one is better or other choice,
then I may do it in next cycle.

Cc: Andy Green andy.gr...@linaro.org
Cc: Roger Quadros rog...@ti.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Ming Lei tom.leim...@gmail.com
---
   arch/arm/mach-omap2/board-omap4panda.c |   99
+++-
   1 file changed, 96 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap4panda.c
b/arch/arm/mach-omap2/board-omap4panda.c
index 5c8e9ce..3183832 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -32,6 +32,8 @@
   #include linux/usb/musb.h
   #include linux/wl12xx.h
   #include linux/platform_data/omap-abe-twl6040.h
+#include linux/power_controller.h
+#include linux/usb/port.h

   #include asm/hardware/gic.h
   #include asm/mach-types.h
@@ -154,6 +156,99 @@ static struct gpio panda_ehci_gpios[] __initdata = {
 { GPIO_HUB_NRESET,  GPIOF_OUT_INIT_LOW,  hub_nreset },
   };

+static void ehci_hub_power_on(struct power_controller *pc, struct device
*dev)
+{
+   gpio_set_value(GPIO_HUB_NRESET, 1);
+   gpio_set_value(GPIO_HUB_POWER, 1);
+}



You should wait a bit after applying power to the smsc chip before letting
go of nReset too.  In the regulator-based implementation I sent it's handled
by delays encoded in the regulator structs.


It isn't a big thing about the discussion. If these code is only platform code,
we can use gpio or regulator or other thing.


Well, you need a delay there FYI.  It's just a nit.


+static void ehci_hub_power_off(struct power_controller *pc, struct device
*dev)
+{
+   gpio_set_value(GPIO_HUB_NRESET, 0);
+   gpio_set_value(GPIO_HUB_POWER, 0);
+}
+
+static struct usb_port_power_switch_data root_hub_port_data = {
+   .hub_tier   = 0,
+   .port_number = 1,
+   .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
+};
+
+static struct usb_port_power_switch_data smsc_hub_port_data = {
+   .hub_tier   = 1,
+   .port_number = 1,
+   .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
+};
+
+static struct power_controller pc = {
+   .name = omap_hub_eth_pc,
+   .count = ATOMIC_INIT(0),
+   .power_on = ehci_hub_power_on,
+   .power_off = ehci_hub_power_off,
+};
+
+static inline int omap_ehci_hub_port(struct device *dev)
+{
+   /* we expect dev-parent points to ehcd controller */
+   if (dev-parent  !strcmp(dev_name(dev-parent), ehci-omap.0))
+   return 1;
+   return 0;
+}
+
+static inline int dev_pc_match(struct device *dev)
+{
+   struct device *anc;
+   int ret = 0;
+
+   if (likely(strcmp(dev_name(dev), port1)))
+   goto exit;
+
+   if (dev-parent  (anc = dev-parent-parent)) {
+   if (omap_ehci_hub_port(anc)) {
+   ret = 1;
+   goto exit;
+   }
+
+   /* is it port of lan95xx hub? */
+   if ((anc = anc-parent)  omap_ehci_hub_port(anc)) {
+   ret = 2;
+   goto exit;
+   }
+   }
+exit:
+   return ret;
+}
+
+/*
+ * Notifications of device registration
+ */
+static int device_notify(struct notifier_block *nb, unsigned long action,
void *data)
+{
+   struct device *dev = data;
+   int ret;
+
+   switch (action) {
+   case DEV_NOTIFY_ADD_DEVICE:
+   ret = dev_pc_match(dev);
+   if (likely(!ret))
+   goto exit;
+   if (ret == 1)
+   dev_pc_bind(pc, dev, root_hub_port_data,
sizeof(root_hub_port_data));
+   else
+   dev_pc_bind(pc, dev, smsc_hub_port_data,
sizeof(smsc_hub_port_data));
+   break;
+
+   case DEV_NOTIFY_DEL_DEVICE:
+   break;
+   }
+exit:
+   return 0;
+}
+
+static struct notifier_block usb_port_nb = {
+   .notifier_call = device_notify,
+};
+



Some thoughts on trying to make this functionality specific to power only
and 

Re: [PATCH 2/7] OMAPDSS: DISPC: Move DISPC specific dss_reg_fields to dispc_features

2012-12-02 Thread Chandrabhanu Mahapatra
On Thursday 29 November 2012 05:48 PM, Tomi Valkeinen wrote:
 On 2012-11-28 12:41, Chandrabhanu Mahapatra wrote:
 The register fields in dss_reg_fields specific to DISPC are moved from struct
 omap_dss_features to corresponding dispc_reg_fields, initialized in struct
 dispc_features, thereby enabling local access.

 Signed-off-by: Chandrabhanu Mahapatra cmahapa...@ti.com
 ---
  drivers/video/omap2/dss/dispc.c|   87 
 
  drivers/video/omap2/dss/dss.h  |4 ++
  drivers/video/omap2/dss/dss_features.c |   28 --
  drivers/video/omap2/dss/dss_features.h |7 ---
  4 files changed, 80 insertions(+), 46 deletions(-)

 diff --git a/drivers/video/omap2/dss/dispc.c 
 b/drivers/video/omap2/dss/dispc.c
 index 9f259ba..21fc522 100644
 --- a/drivers/video/omap2/dss/dispc.c
 +++ b/drivers/video/omap2/dss/dispc.c
 @@ -80,6 +80,16 @@ struct dispc_irq_stats {
  unsigned irqs[32];
  };
  
 +enum dispc_feat_reg_field {
 +FEAT_REG_FIRHINC,
 +FEAT_REG_FIRVINC,
 +FEAT_REG_FIFOLOWTHRESHOLD,
 +FEAT_REG_FIFOHIGHTHRESHOLD,
 +FEAT_REG_FIFOSIZE,
 +FEAT_REG_HORIZONTALACCU,
 +FEAT_REG_VERTICALACCU,
 +};
 +
  struct dispc_features {
  u8 sw_start;
  u8 fp_start;
 @@ -107,6 +117,8 @@ struct dispc_features {
  
  u32 buffer_size_unit;
  u32 burst_size_unit;
 +
 +struct register_field *reg_fields;
  };
 
 Hmm, would it be simpler to have an explicit struct for the reg fields.
 I mean something like:
 
 struct dispc_reg_fields {
   struct register_field firhinc;
   struct register_field firvinc;
   struct register_field fifo_low_threshold;
   ...
 };
 
 Then accessing it would be
 
 dispc.feat-reg_fields.firhinc.start;
 
 instead of
 
 dispc.feat-reg_fields[FEAT_REG_FIFOSIZE].start;
 
 Not a big difference, but I don't see any benefit in having an array of
 reg fields here.
 
  Tomi
 
 

I was thinking to move reg_fields into the dispc_feats structure as

.burst_size_unit=   8,
.reg_fields =   {
.firhinc= { 12,  0 },
.firvinc= { 28, 16 },
.fifo_low_thresh= { 11,  0 },
.fifo_high_thresh   = { 27, 16 },
.fifosize   = { 10,  0 },
.hori_accu  = {  9,  0 },
.vert_accu  = { 25, 16 },
},

This would give us dispc.feat-reg_fields.firhinc.start;
but at the same time would create duplicate information for
omap34xx_rev1_0_dispc_feats and omap34xx_rev3_0_dispc_feats. However,
this duplication never occurs anywhere else in dss.c or dsi.c.

If we still go with the older approach of having dispc_reg_fields
outside dispc_feats the only way  it works is

.reg_fields =   omap2_dispc_reg_fields

which changes as dispc.feat-reg_fields-firhinc.start;

but avoids duplicate information. Both approaches seem good enough to me.


-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 22/22] mfd: omap-usb-host: Don't spam console on clk_set_parent failure

2012-12-02 Thread Bjørn Mork
Sergei Shtylyov sshtyl...@mvista.com writes:
 On 28-11-2012 18:49, Roger Quadros wrote:
  ret = clk_set_parent(omap-utmi_clk[0],
  omap-xclk60mhsp1_ck);
  if (ret != 0)
 -dev_err(dev, xclk60mhsp1_ck set parent
 +dev_dbg(dev, xclk60mhsp1_ck set parent

Need space at the end of this substring, else you get parentfailed

  failed error:%d\n, ret);


Wouldn't it be better to change all these to conform with
Documentation/CodingStyle when touching them anyway?  That would make
the missing space problem obvious as well as making the messages
greppable.

From Chapter 2: Breaking long lines and strings:

However, never break user-visible strings such as printk messages,
because that breaks the ability to grep for them.



Bjørn
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html