[RFC PATCH 0/5] USB: prepare for support port power off on non-ACPI device
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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