Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
On Thu, 6 Dec 2012, Andy Green wrote: Right. The question is should it be done in this somewhat roundabout way (you've got two separate assets for setting up this one thing), or should the USB port code be responsible for explicitly checking if there are any applicable assets when the port is created? The advantange of the second approach is that it doesn't involve checking all the ancestors every time a new device is added (and it involves only one asset). The disadvantage is that it works only for USB ports. It's done in two steps to strongly filter candidate child devices against being children of a known platform device. If you go around that, you will be back to full device path matching with wildcards at the USB child to determine if he is the one. So that's a feature not an issue I think. I don't follow. Suppose an asset is attached to ehci-omap.0 with its string set to -0:1.0/port1 and the other members pointing to the regulator, clock and so on. And suppose the USB port code, when creating a new port, checks for a name match against all the assets attached to its bus's host controller device structure. That should do exactly what you want while using only one asset. I can see doing it generically or not is equally a political issue as a technical one, but I think if we bother to add this kind of support we should prefer to do it generally. It's going to be about the same amount of code. Not quite. If you do it generally then you have to insert hooks at all the places where a subsystem _might_ need it. If you do it specifically then no hooks are needed at all -- just a match call at the right place in the subsystem that needs it. As Tony Lindgren said, even board-omap4panda.c itself is deprecated, to project platform info into USB stack you either have to add DT code into usb stack then to go find things directly, or provide a generic methodology like assets which have the dt bindings done outside of any subsystem and apply their operations outside the subsystem too. Assuming DT can be extended to support assets, adding one asset will be simpler than adding two. To answer the question, we need to know how other subsystems might want to use the asset-matching approach. My guess is that only a small number of device types would care about assets (in the same way that assets matter to USB ports but not to other USB device types). And it might not be necessary to check against every ancestor; we might know beforehand where to check (just as the USB port would know to look for an asset attached to the host controller device). Yes I think it boils down to buses in general can benefit from this. They're the thing that dynamically - later - create child devices you might need to target with what was ultimately platform information. On Panda the other bus thing that can benefit is the WLAN module on SDIO. In fact that's a very illuminating case for your question above. Faced with exactly the same problem, that they needed to project platform info on to SDIO-probed device instance to tell it what clock rate it had been given, they invented an api which you can see today in board-omap4panda.c and other boards there, wl12xx_set_platform_data(). This is from board-4430sdp: static struct wl12xx_platform_data omap4_sdp4430_wlan_data __initdata = { .board_ref_clock = WL12XX_REFCLOCK_26, .board_tcxo_clock = WL12XX_TCXOCLOCK_26, }; ... ret = wl12xx_set_platform_data(omap4_sdp4430_wlan_data); You can find the other end of it here in drivers/net/wireless/ti/wlcore/wl12xx_platform_data.c static struct wl12xx_platform_data *platform_data; int __init wl12xx_set_platform_data(const struct wl12xx_platform_data *data) { if (platform_data) return -EBUSY; if (!data) return -EINVAL; platform_data = kmemdup(data, sizeof(*data), GFP_KERNEL); if (!platform_data) return -ENOMEM; return 0; } when the driver for the device instance wants to get its platform data it reads the static pointer via another api. Obviously if you want two modules on your platform that's not so good. Also it isn't type-safe, although that's probably not a big concern. I doubt that's the only SDIO device that wants to know platform info. So I think by providing a generic way we help other buses and devices. I agree, assets look like a good way to improve this. In fact, to some extent assets appear to be a generalization or extension of platform data. Alan Stern -- 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 4/5] arm: omap2: support port power on lan95xx devices
On Wed, 5 Dec 2012, Andy Green wrote: The details of this aren't clear yet. For instance, should the device core try to match the port with the asset info, or should this be done by the USB code when the port is created? Currently what I have (this is before changing it to pm domain, but this should be unchanged) lets the asset define a callback op which will receive notifications for all added child devices that have the device the asset is bound to as an ancestor. So you would bind an asset to the host controller device... static struct device_asset assets_ehci_omap0[] = { { .name = -0:1.0/port1, .asset = assets_ehci_omap0_smsc_port, .ops = device_descendant_attach_default_asset_ops, }, { } }; ...with this descendant filter callback pointing to a generic end of the device path matcher. when device_descendant_attach_default_asset_ops() sees the child that was foretold has appeared (and it only hears about children of ehci-omap.0 in this case), it binds the assets pointed to by .asset to the new child before its probe. assets_ehci_omap0_smsc_port is an array of the actual regulator and clock that need switching by the child. So the effect is to magic the right assets to the child device just before it gets added (and probe called etc). This is working well and the matcher helper is small and simple. Right. The question is should it be done in this somewhat roundabout way (you've got two separate assets for setting up this one thing), or should the USB port code be responsible for explicitly checking if there are any applicable assets when the port is created? The advantange of the second approach is that it doesn't involve checking all the ancestors every time a new device is added (and it involves only one asset). The disadvantage is that it works only for USB ports. To answer the question, we need to know how other subsystems might want to use the asset-matching approach. My guess is that only a small number of device types would care about assets (in the same way that assets matter to USB ports but not to other USB device types). And it might not be necessary to check against every ancestor; we might know beforehand where to check (just as the USB port would know to look for an asset attached to the host controller device). Alan Stern -- 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 4/5] arm: omap2: support port power on lan95xx devices
Hi, * Ming Lei tom.leim...@gmail.com [121202 07:05]: --- a/arch/arm/mach-omap2/board-omap4panda.c +++ b/arch/arm/mach-omap2/board-omap4panda.c ... + +static struct notifier_block usb_port_nb = { + .notifier_call = device_notify, +}; + We'll be flipping omap4 over to be device tree only soon. So let's not make the conversion more complex by adding more platform code. This means that for am33xx, omap4, and omap5 this code can be device tree only code. Regards, Tony -- 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 4/5] arm: omap2: support port power on lan95xx devices
On 06/12/12 00:42, the mail apparently from Alan Stern included: On Wed, 5 Dec 2012, Andy Green wrote: The details of this aren't clear yet. For instance, should the device core try to match the port with the asset info, or should this be done by the USB code when the port is created? Currently what I have (this is before changing it to pm domain, but this should be unchanged) lets the asset define a callback op which will receive notifications for all added child devices that have the device the asset is bound to as an ancestor. So you would bind an asset to the host controller device... static struct device_asset assets_ehci_omap0[] = { { .name = -0:1.0/port1, .asset = assets_ehci_omap0_smsc_port, .ops = device_descendant_attach_default_asset_ops, }, { } }; ...with this descendant filter callback pointing to a generic end of the device path matcher. when device_descendant_attach_default_asset_ops() sees the child that was foretold has appeared (and it only hears about children of ehci-omap.0 in this case), it binds the assets pointed to by .asset to the new child before its probe. assets_ehci_omap0_smsc_port is an array of the actual regulator and clock that need switching by the child. So the effect is to magic the right assets to the child device just before it gets added (and probe called etc). This is working well and the matcher helper is small and simple. Right. The question is should it be done in this somewhat roundabout way (you've got two separate assets for setting up this one thing), or should the USB port code be responsible for explicitly checking if there are any applicable assets when the port is created? The advantange of the second approach is that it doesn't involve checking all the ancestors every time a new device is added (and it involves only one asset). The disadvantage is that it works only for USB ports. It's done in two steps to strongly filter candidate child devices against being children of a known platform device. If you go around that, you will be back to full device path matching with wildcards at the USB child to determine if he is the one. So that's a feature not an issue I think. I can see doing it generically or not is equally a political issue as a technical one, but I think if we bother to add this kind of support we should prefer to do it generally. It's going to be about the same amount of code. As Tony Lindgren said, even board-omap4panda.c itself is deprecated, to project platform info into USB stack you either have to add DT code into usb stack then to go find things directly, or provide a generic methodology like assets which have the dt bindings done outside of any subsystem and apply their operations outside the subsystem too. To answer the question, we need to know how other subsystems might want to use the asset-matching approach. My guess is that only a small number of device types would care about assets (in the same way that assets matter to USB ports but not to other USB device types). And it might not be necessary to check against every ancestor; we might know beforehand where to check (just as the USB port would know to look for an asset attached to the host controller device). Yes I think it boils down to buses in general can benefit from this. They're the thing that dynamically - later - create child devices you might need to target with what was ultimately platform information. On Panda the other bus thing that can benefit is the WLAN module on SDIO. In fact that's a very illuminating case for your question above. Faced with exactly the same problem, that they needed to project platform info on to SDIO-probed device instance to tell it what clock rate it had been given, they invented an api which you can see today in board-omap4panda.c and other boards there, wl12xx_set_platform_data(). This is from board-4430sdp: static struct wl12xx_platform_data omap4_sdp4430_wlan_data __initdata = { .board_ref_clock = WL12XX_REFCLOCK_26, .board_tcxo_clock = WL12XX_TCXOCLOCK_26, }; ... ret = wl12xx_set_platform_data(omap4_sdp4430_wlan_data); You can find the other end of it here in drivers/net/wireless/ti/wlcore/wl12xx_platform_data.c static struct wl12xx_platform_data *platform_data; int __init wl12xx_set_platform_data(const struct wl12xx_platform_data *data) { if (platform_data) return -EBUSY; if (!data) return -EINVAL; platform_data = kmemdup(data, sizeof(*data), GFP_KERNEL); if (!platform_data) return -ENOMEM; return 0; } when the driver for the device instance wants to get its platform data it reads the static pointer via another api. Obviously if you want two modules on your platform that's not so good. I doubt that's the only SDIO device that wants to know platform info. So I think by
Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
Hi, Well, I'm not any less busy than yesterday, as it turns out, but I'm expecting that to continue tomorrow, so I may as well have a look at it now. :-) On Tuesday, December 04, 2012 12:10:32 PM Alan Stern wrote: [CC: list trimmed; the people who were on it should be subscribed to at least one of the lists anyway.] [...] Not only regulators involved, clock or other things might be involved too. Also the same power domain might be shared with more than one port, so it is better to introduce power domain to the problem. Looks generic_pm_domain is overkill, so I introduced power controller which only focuses on power on/off and being shared by multiple devices. Even though it is overkill, it may be better than introducing a new abstraction. After all, this is exactly the sort of thing that PM domains were originally created to handle. Rafael, do you have any advice on this? The generic_pm_domain structure is fairly complicated, there's a lot of code in drivers/base/power/domain.c (it's the biggest source file in its directory), and I'm not aware of any documentation. Yeah, documentation is missing, which obviously is my fault. That code is designed to cover the case in which multiple devices share a power switch that can be used to remove power from all of them at once (eg. through a register write). It also assumes that there will be a stop operation, such as disable clock, allowing each device in the domain to be put into a shallow low-power state (individually) without the necessity to save its state. Device states only have to be saved when the power switch is about to be used, which generally happens when they all have been stopped (their runtime PM status is RPM_SUSPENDED). The stop operation may be defined in a different way for each device in the domain (actually, that applies to the save state, restore state, and start - opposite to stop - operations too) and PM QoS latency constraints are used to decide if and when to turn the whole domain off. Moreover, it supports hierarchies of power domains that may be pretty much arbitarily complicated. A big part of this code is for the handling of system suspend/resume in such a way that it is consistent with runtime PM. For USB ports I'd recommend to use something simpler. :-) Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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 4/5] arm: omap2: support port power on lan95xx devices
[CC: list trimmed; the people who were on it should be subscribed to at least one of the lists anyway.] On Tue, 4 Dec 2012, Andy Green wrote: I think associating ULPI PHY reset and SMSC power and reset with the hub port power state is good. Then, you could have the driver in a device with multiple onboard USB devices, and individually control whether they're eating power or not. In the asset case, you'd associate mux assets with ehci-omap.0. Yesterday I studied the hub port code and have a couple of patches, one normalizes the hub port device to have a stub driver. The other then puts hub port power state signalling into runtime_pm handlers in the hub port device. Until now, actually there's no code in hub.c to switch off a port. In fact that's not quite true. You simply weren't aware of the new code; you can find a series of patches starting here: http://marc.info/?l=linux-usbm=135314427413307w=2 The parts of interest to us begin in patch 7/10. Assuming that's not insane, what should the user interface to disable a port power look like, something in sysfs? Until now it doesn't seem to exist. It will be implemented through PM QOS. (On the other hand, since the LAN95xx is the only thing connected to the root hub, it could be powered off and on by unbinding the ehci-omap.0 device from its driver and rebinding it.) We shouldn't get to tied up with Panda case, this will be there for all cases like PCs etc. It should work well if there are multiple ports with onboard assets. Okay, I'm fine with tying this to the port. 2. If we do choose the port, do we want to identify it by matching against a device name string or by matching a sequence of port numbers (in this case, a length-1 sequence)? The port numbers are fixed by the board design, whereas the device name strings might get changed in the future. On the other hand, the port numbers apply only to USB whereas device names can be used by any subsystem. USB device names contain the port information. The matching scheme I have currently just uses the right-hand side of the path information and nothing that is not defined by the USB subsystem. It uses a platform_device ancestor to restrict matches to descendants of the right host controller. So unlike try#1 the names are as stable as the subsystem code alone, however stable that is, it's not exposed to changes from anywhere else. As you mention it's then workable on any dynamically probed bus. 3. Should the matching mechanism go into the device core or into the USB port code? (This is related to the previous question.) Currently I am experimenting with having the asset pointer in struct device, but migrating the events into runtime_resume and runtime_suspend. If it works out that has advantages that assets follow not just the logical device existence but the pm state of the device closely. It also allows leveraging assets directly to the hub port runtime_pm state, so they follow enable state of the port without any additional code. If we use a PM domain then there won't be any need to hook the runtime PM events. The domain will automatically be notified about power changes. 4. Should this be implemented simply as a regulator (or a pair of regulators)? Or should it be generalized to some sort of PM domain thing? The generic_pm_domain structure defined in include/linux/pm_domain.h seems like overkill, but maybe it's the most appropriate thing to use. They should be regulators for that I think. But it's only part the problem since clocks and mux are also going to be commonly associated with device power state, and indeed are in Panda case. I realize restricting the scope is desirable to get something done, but I'm not sure supporting regulators only is enough of the job. Then why not use a PM domain? It will allow us to do whatever we want in the callbacks. On Tue, 4 Dec 2012, Ming Lei wrote: Alos, the same ehci-omap driver and same LAN95xx chip is used in beagle board and panda board with different power control approach, does port driver can distinguish these two cases? Port device is a general device(not platform device), how does port driver get platform/board dependent info? This is the part that Andy has been working on. The board-dependent info will be registered by the board file, and it will take effect either when the port is registered or when it is bound to a driver. The details of this aren't clear yet. For instance, should the device core try to match the port with the asset info, or should this be done by the USB code when the port is created? Not only regulators involved, clock or other things might be involved too. Also the same power domain might be shared with more than one port, so it is better to introduce power domain to the problem. Looks
Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
On Tue, Dec 04, 2012 at 11:40:05AM +0800, Andy Green wrote: On 04/12/12 01:09, the mail apparently from Alan Stern included: On Mon, 3 Dec 2012, Andy Green wrote: Unless someone NAKs it for sure already (if you're already sure you're going to, please do so to avoid wasting time), I'll issue a try#2 of my code later which demonstrates what I mean. At least I guess it's useful for comparative purposes. Before you go writing a whole lot more code, we should discuss the basics a bit more clearly. There are several unsettled issues here: 1. Should the LAN95xx stuff be associated with the ehci-omap.0's driver or with the hub port? The port would be more flexible, offering the ability to turn the power off and on while the system is running without affecting anything else. But the port code is currently in flux, which could cause this new addition to be delayed. I think associating ULPI PHY reset and SMSC power and reset with the hub port power state is good. Then, you could have the driver in a device with multiple onboard USB devices, and individually control whether they're eating power or not. In the asset case, you'd associate mux assets with ehci-omap.0. Yesterday I studied the hub port code and have a couple of patches, one normalizes the hub port device to have a stub driver. The other then puts hub port power state signalling into runtime_pm handlers in the hub port device. Until now, actually there's no code in hub.c to switch off a port. Did you take a look at the most recent patches from Tianyu to add support to power off a port if a device is suspended? Start of the series: http://marc.info/?l=linux-usbm=135314427413307w=2 Patch that adds power off on device suspend: http://marc.info/?l=linux-usbm=135314431913321w=2 Tianyu also added some code to the xHCI host controller driver to call into the ACPI methods to power off a port when the USB hub driver clears the port power feature. Sarah Sharp -- 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 4/5] arm: omap2: support port power on lan95xx devices
On 05/12/12 01:10, the mail apparently from Alan Stern included: [CC: list trimmed; the people who were on it should be subscribed to at least one of the lists anyway.] On Tue, 4 Dec 2012, Andy Green wrote: I think associating ULPI PHY reset and SMSC power and reset with the hub port power state is good. Then, you could have the driver in a device with multiple onboard USB devices, and individually control whether they're eating power or not. In the asset case, you'd associate mux assets with ehci-omap.0. Yesterday I studied the hub port code and have a couple of patches, one normalizes the hub port device to have a stub driver. The other then puts hub port power state signalling into runtime_pm handlers in the hub port device. Until now, actually there's no code in hub.c to switch off a port. In fact that's not quite true. You simply weren't aware of the new code; you can find a series of patches starting here: http://marc.info/?l=linux-usbm=135314427413307w=2 The parts of interest to us begin in patch 7/10. Yes I have been looking in usb-next. Assuming that's not insane, what should the user interface to disable a port power look like, something in sysfs? Until now it doesn't seem to exist. It will be implemented through PM QOS. OK. I saw [PATCH 09/10] usb: expose usb port's pm qos flags to user space now. (On the other hand, since the LAN95xx is the only thing connected to the root hub, it could be powered off and on by unbinding the ehci-omap.0 device from its driver and rebinding it.) We shouldn't get to tied up with Panda case, this will be there for all cases like PCs etc. It should work well if there are multiple ports with onboard assets. Okay, I'm fine with tying this to the port. OK. 2. If we do choose the port, do we want to identify it by matching against a device name string or by matching a sequence of port numbers (in this case, a length-1 sequence)? The port numbers are fixed by the board design, whereas the device name strings might get changed in the future. On the other hand, the port numbers apply only to USB whereas device names can be used by any subsystem. USB device names contain the port information. The matching scheme I have currently just uses the right-hand side of the path information and nothing that is not defined by the USB subsystem. It uses a platform_device ancestor to restrict matches to descendants of the right host controller. So unlike try#1 the names are as stable as the subsystem code alone, however stable that is, it's not exposed to changes from anywhere else. As you mention it's then workable on any dynamically probed bus. 3. Should the matching mechanism go into the device core or into the USB port code? (This is related to the previous question.) Currently I am experimenting with having the asset pointer in struct device, but migrating the events into runtime_resume and runtime_suspend. If it works out that has advantages that assets follow not just the logical device existence but the pm state of the device closely. It also allows leveraging assets directly to the hub port runtime_pm state, so they follow enable state of the port without any additional code. If we use a PM domain then there won't be any need to hook the runtime PM events. The domain will automatically be notified about power changes. OK. 4. Should this be implemented simply as a regulator (or a pair of regulators)? Or should it be generalized to some sort of PM domain thing? The generic_pm_domain structure defined in include/linux/pm_domain.h seems like overkill, but maybe it's the most appropriate thing to use. They should be regulators for that I think. But it's only part the problem since clocks and mux are also going to be commonly associated with device power state, and indeed are in Panda case. I realize restricting the scope is desirable to get something done, but I'm not sure supporting regulators only is enough of the job. Then why not use a PM domain? It will allow us to do whatever we want in the callbacks. I see, I never met them before now is the reason ^^. You're right it's already in struct device too, and it's much more plumbed into the future apis than what I have been doing. I'll study how to change what I have to fit this and do so. On Tue, 4 Dec 2012, Ming Lei wrote: Alos, the same ehci-omap driver and same LAN95xx chip is used in beagle board and panda board with different power control approach, does port driver can distinguish these two cases? Port device is a general device(not platform device), how does port driver get platform/board dependent info? This is the part that Andy has been working on. The board-dependent info will be registered by the board file, and it will take effect either when the port is registered or when it is bound
Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
On 05/12/12 02:14, the mail apparently from Sarah Sharp included: On Tue, Dec 04, 2012 at 11:40:05AM +0800, Andy Green wrote: On 04/12/12 01:09, the mail apparently from Alan Stern included: On Mon, 3 Dec 2012, Andy Green wrote: Unless someone NAKs it for sure already (if you're already sure you're going to, please do so to avoid wasting time), I'll issue a try#2 of my code later which demonstrates what I mean. At least I guess it's useful for comparative purposes. Before you go writing a whole lot more code, we should discuss the basics a bit more clearly. There are several unsettled issues here: 1. Should the LAN95xx stuff be associated with the ehci-omap.0's driver or with the hub port? The port would be more flexible, offering the ability to turn the power off and on while the system is running without affecting anything else. But the port code is currently in flux, which could cause this new addition to be delayed. I think associating ULPI PHY reset and SMSC power and reset with the hub port power state is good. Then, you could have the driver in a device with multiple onboard USB devices, and individually control whether they're eating power or not. In the asset case, you'd associate mux assets with ehci-omap.0. Yesterday I studied the hub port code and have a couple of patches, one normalizes the hub port device to have a stub driver. The other then puts hub port power state signalling into runtime_pm handlers in the hub port device. Until now, actually there's no code in hub.c to switch off a port. Did you take a look at the most recent patches from Tianyu to add support to power off a port if a device is suspended? Start of the series: http://marc.info/?l=linux-usbm=135314427413307w=2 Patch that adds power off on device suspend: http://marc.info/?l=linux-usbm=135314431913321w=2 Tianyu also added some code to the xHCI host controller driver to call into the ACPI methods to power off a port when the USB hub driver clears the port power feature. No I didn't know about it, I will study these along with pm_domain stuff thanks. -Andy -- Andy Green | TI Landing Team Leader Linaro.org │ Open source software for ARM SoCs | Follow Linaro http://facebook.com/pages/Linaro/155974581091106 - http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog -- 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 4/5] arm: omap2: support port power on lan95xx devices
On Mon, 3 Dec 2012, Andy Green wrote: Unless someone NAKs it for sure already (if you're already sure you're going to, please do so to avoid wasting time), I'll issue a try#2 of my code later which demonstrates what I mean. At least I guess it's useful for comparative purposes. Before you go writing a whole lot more code, we should discuss the basics a bit more clearly. There are several unsettled issues here: 1. Should the LAN95xx stuff be associated with the ehci-omap.0's driver or with the hub port? The port would be more flexible, offering the ability to turn the power off and on while the system is running without affecting anything else. But the port code is currently in flux, which could cause this new addition to be delayed. (On the other hand, since the LAN95xx is the only thing connected to the root hub, it could be powered off and on by unbinding the ehci-omap.0 device from its driver and rebinding it.) 2. If we do choose the port, do we want to identify it by matching against a device name string or by matching a sequence of port numbers (in this case, a length-1 sequence)? The port numbers are fixed by the board design, whereas the device name strings might get changed in the future. On the other hand, the port numbers apply only to USB whereas device names can be used by any subsystem. 3. Should the matching mechanism go into the device core or into the USB port code? (This is related to the previous question.) 4. Should this be implemented simply as a regulator (or a pair of regulators)? Or should it be generalized to some sort of PM domain thing? The generic_pm_domain structure defined in include/linux/pm_domain.h seems like overkill, but maybe it's the most appropriate thing to use. Until we decide on the answers to these questions, there's no point writing detailed patches. Alan Stern -- 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 4/5] arm: omap2: support port power on lan95xx devices
On Mon, Dec 3, 2012 at 12:52 PM, Andy Green andy.gr...@linaro.org wrote: +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 All things are board dependent because we are discussing peripheral device(not builtin SoC devices), so it is proper to put it in the board file. If some boards want to share it, we can put it in a single .c file and let board file include it. Where would the .c file go... SMSC is not platform, or even arch specific chip (eg, iMX or MIPS or even x86 boards can have it), so not arch/arm/mach- or arch/arm/plat-xxx or arch/arm. I guess it would go in drivers/usb or drivers/net. How does drivers/usb or drivers/net know the SMSC is used on beagle or panda? Different power control approach is used in the two boards even same SMSC chip is shipped in the two boards. Push in ARM-Land is towards single kernels which support many platforms, a good case in point is omap2plus_defconfg which wants to allow to support all OMAP2/3/4/5 platforms in one kernel. If you include that C file over and over in board files, it's not very nice for that. They anyway want to eliminate board files for the single kernel binary reason, and just have one load of config come in by dtb. I only mean it is reasonable to put the power control code into board file because each board may take different power control approach even same SMSC chip is used. I understand DT only describes the difference of the board via device node, and I am not sure if the current DT is enough to convert the board file into data as far as the problem is concerned. So it guides you towards having static helper code once in drivers/ for the scenarios you support... if that's where you end up smsc is less good a target for a helper than to have helpers for classes of object like regulator and clk, that you can combine and reuse on all sorts of target devices, which is device_asset approach. It also guides you to having the special platform sauce be something that can go into the dtb: per-board code can't. That's why device_asset stuff only places asset structs in the board file and is removing code from there. that perform device_path business out of the omap4panda board file at least. At that point the path matching code becomes generic
Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
On Tue, Dec 4, 2012 at 1:09 AM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 3 Dec 2012, Andy Green wrote: Unless someone NAKs it for sure already (if you're already sure you're going to, please do so to avoid wasting time), I'll issue a try#2 of my code later which demonstrates what I mean. At least I guess it's useful for comparative purposes. Before you go writing a whole lot more code, we should discuss the basics a bit more clearly. There are several unsettled issues here: 1. Should the LAN95xx stuff be associated with the ehci-omap.0's driver or with the hub port? The port would be more flexible, IMO, it shouldn't be associated with ehci-omap controller driver because the LAN95xx is a external USB device and should be nothing to do with ehci, but it is reasonable to be associated with the hub port because it takes a out of band power control method. offering the ability to turn the power off and on while the system is running without affecting anything else. But the port code is currently in flux, which could cause this new addition to be delayed. (On the other hand, since the LAN95xx is the only thing connected to the root hub, it could be powered off and on by unbinding the ehci-omap.0 device from its driver and rebinding it.) 2. If we do choose the port, do we want to identify it by matching against a device name string or by matching a sequence of port numbers (in this case, a length-1 sequence)? The port numbers are fixed by the board design, whereas the device name strings might get changed in the future. On the other hand, the port numbers apply only to USB whereas device names can be used by any subsystem. Alos, the same ehci-omap driver and same LAN95xx chip is used in beagle board and panda board with different power control approach, does port driver can distinguish these two cases? Port device is a general device(not platform device), how does port driver get platform/board dependent info? 3. Should the matching mechanism go into the device core or into the USB port code? (This is related to the previous question.) 4. Should this be implemented simply as a regulator (or a pair of regulators)? Or should it be generalized to some sort of PM domain thing? The generic_pm_domain structure defined in include/linux/pm_domain.h seems like overkill, but maybe it's the most appropriate thing to use. Not only regulators involved, clock or other things might be involved too. Also the same power domain might be shared with more than one port, so it is better to introduce power domain to the problem. Looks generic_pm_domain is overkill, so I introduced power controller which only focuses on power on/off and being shared by multiple devices. 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 4/5] arm: omap2: support port power on lan95xx devices
On 04/12/12 01:09, the mail apparently from Alan Stern included: On Mon, 3 Dec 2012, Andy Green wrote: Unless someone NAKs it for sure already (if you're already sure you're going to, please do so to avoid wasting time), I'll issue a try#2 of my code later which demonstrates what I mean. At least I guess it's useful for comparative purposes. Before you go writing a whole lot more code, we should discuss the basics a bit more clearly. There are several unsettled issues here: 1. Should the LAN95xx stuff be associated with the ehci-omap.0's driver or with the hub port? The port would be more flexible, offering the ability to turn the power off and on while the system is running without affecting anything else. But the port code is currently in flux, which could cause this new addition to be delayed. I think associating ULPI PHY reset and SMSC power and reset with the hub port power state is good. Then, you could have the driver in a device with multiple onboard USB devices, and individually control whether they're eating power or not. In the asset case, you'd associate mux assets with ehci-omap.0. Yesterday I studied the hub port code and have a couple of patches, one normalizes the hub port device to have a stub driver. The other then puts hub port power state signalling into runtime_pm handlers in the hub port device. Until now, actually there's no code in hub.c to switch off a port. Assuming that's not insane, what should the user interface to disable a port power look like, something in sysfs? Until now it doesn't seem to exist. (On the other hand, since the LAN95xx is the only thing connected to the root hub, it could be powered off and on by unbinding the ehci-omap.0 device from its driver and rebinding it.) We shouldn't get to tied up with Panda case, this will be there for all cases like PCs etc. It should work well if there are multiple ports with onboard assets. 2. If we do choose the port, do we want to identify it by matching against a device name string or by matching a sequence of port numbers (in this case, a length-1 sequence)? The port numbers are fixed by the board design, whereas the device name strings might get changed in the future. On the other hand, the port numbers apply only to USB whereas device names can be used by any subsystem. USB device names contain the port information. The matching scheme I have currently just uses the right-hand side of the path information and nothing that is not defined by the USB subsystem. It uses a platform_device ancestor to restrict matches to descendants of the right host controller. So unlike try#1 the names are as stable as the subsystem code alone, however stable that is, it's not exposed to changes from anywhere else. As you mention it's then workable on any dynamically probed bus. 3. Should the matching mechanism go into the device core or into the USB port code? (This is related to the previous question.) Currently I am experimenting with having the asset pointer in struct device, but migrating the events into runtime_resume and runtime_suspend. If it works out that has advantages that assets follow not just the logical device existence but the pm state of the device closely. It also allows leveraging assets directly to the hub port runtime_pm state, so they follow enable state of the port without any additional code. 4. Should this be implemented simply as a regulator (or a pair of regulators)? Or should it be generalized to some sort of PM domain thing? The generic_pm_domain structure defined in include/linux/pm_domain.h seems like overkill, but maybe it's the most appropriate thing to use. They should be regulators for that I think. But it's only part the problem since clocks and mux are also going to be commonly associated with device power state, and indeed are in Panda case. I realize restricting the scope is desirable to get something done, but I'm not sure supporting regulators only is enough of the job. Until we decide on the answers to these questions, there's no point writing detailed patches. I agree, however I can't get insight into and confidence about what to do without studying and meddling with the code. Some throwaway code is probably a reasonable price. -Andy -- Andy Green | TI Landing Team Leader Linaro.org │ Open source software for ARM SoCs | Follow Linaro http://facebook.com/pages/Linaro/155974581091106 - http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog -- 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 4/5] arm: omap2: support port power on lan95xx devices
On 04/12/12 10:39, the mail apparently from Ming Lei included: On Mon, Dec 3, 2012 at 12:52 PM, Andy Green andy.gr...@linaro.org wrote: +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 All things are board dependent because we are discussing peripheral device(not builtin SoC devices), so it is proper to put it in the board file. If some boards want to share it, we can put it in a single .c file and let board file include it. Where would the .c file go... SMSC is not platform, or even arch specific chip (eg, iMX or MIPS or even x86 boards can have it), so not arch/arm/mach- or arch/arm/plat-xxx or arch/arm. I guess it would go in drivers/usb or drivers/net. How does drivers/usb or drivers/net know the SMSC is used on beagle or panda? Different power control approach is used in the two boards even same SMSC chip is shipped in the two boards. You mention you're going to put it in a single .c file and let the board file include it, I am just wondering where that .c file will live. It seems it would have to live down drivers/ somewhere so MIPS, x86 etc that might have an SMSC chip onboard can also use it if they want. Push in ARM-Land is towards single kernels which support many platforms, a good case in point is omap2plus_defconfg which wants to allow to support all OMAP2/3/4/5 platforms in one kernel. If you include that C file over and over in board files, it's not very nice for that. They anyway want to eliminate board files for the single kernel binary reason, and just have one load of config come in by dtb. I only mean it is reasonable to put the power control code into board file because each board may take different power control approach even same SMSC chip is used. I understand DT only describes the difference of the board via device node, and I am not sure if the current DT is enough to convert the board file into data as far as the problem is concerned. No the approach with DT is to provide a dummy SoC board file with all data provided by dtb. This is already established, see ./arch/arm/mach-omap2/board-generic.c for example. You can see there's nothing in it other than minimum dt match business. People with new boards are getting pointed at that and told to sort it out in dtb and disallowed from creating new board files. So whatever support or helper scheme you're adding needs a story about how dtb can
[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
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: [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