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

2012-12-06 Thread Alan Stern
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

2012-12-05 Thread Alan Stern
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

2012-12-05 Thread Tony Lindgren
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

2012-12-05 Thread Andy Green

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

2012-12-05 Thread Rafael J. Wysocki
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

2012-12-04 Thread Alan Stern
[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

2012-12-04 Thread Sarah Sharp
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

2012-12-04 Thread Andy Green

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

2012-12-04 Thread Andy Green

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

2012-12-03 Thread Alan Stern
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

2012-12-03 Thread Ming Lei
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

2012-12-03 Thread Ming Lei
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

2012-12-03 Thread Andy Green

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

2012-12-03 Thread Andy Green

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

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

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

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

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

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

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


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

2012-12-02 Thread Andy Green

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

Hi -


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

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

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

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

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

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

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


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



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


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


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

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

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

 Hi -


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

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

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

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

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

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

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


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

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



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


 Some thoughts on trying to make this functionality specific to power 

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

2012-12-02 Thread Andy Green

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

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

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

Hi -



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

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

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

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

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

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

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



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


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


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


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



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