Re: Query: EHCI clock management approach

2009-10-08 Thread Kevin Hilman
Gadiyar, Anand gadi...@ti.com writes:

 I'd like to post a patch in a couple of days to refactor the EHCI
 clock management code. This would be useful to do aggressive clock
 management in the idle path.

 A current implementation I have is to simply factor out the
 clock_enable/disable calls out. Does it make sense to move this out
 to mach-omap2/* and provide the function pointer through platform
 data? Does the driver need to be aware of the clocks that it needs,
 or is it sufficient for it to just call a platform-specific function
 that turns on/off all the clocks needed by the driver?

 The reason I ask is, in a future OMAP, we may need to enable a
 different set of clocks, but we should be able to re-use the rest of
 the code directly.

I am so glad you asked...

The short answer is: use omap_device.

Now that omap_hwmod + omap_device are in mainline (thanks to Paul
Walmsley), all new drivers need to use this.

Once an omap_hwmod and omap_device for EHCI are implemented, the
driver calls would be abstracted just like you are looking for.
Below[3], I extracted the comments dirctly from
arch/arm/plat-omap/omap_device.c which describe how the drivers would
use these.  You can also look at an example of a converted MMC
driver[1] done by Paul.

Note that the abstraction via pdata to omap_device will be going away
by the time we reach 2.6.33 (hopefully.)  By then the runtime PM
framework[2] will be available for OMAP and, drivers will use runtime
PM.  The runtime PM core for OMAP will then call the omap_device layer
directly.

Kevin

[1] http://marc.info/?l=linux-omapm=124419789124570w=2

[2] http://elinux.org/OMAP_Power_Management#future_directions

[3] This is extracted directly from the header of
arch/arm/plat-omap/omap_device.c


Guidelines for usage by driver authors:

1. These functions are intended to be used by device drivers via
function pointers in struct platform_data.  As an example,
omap_device_enable() should be passed to the driver as

struct foo_driver_platform_data {
...
 int (*device_enable)(struct platform_device *pdev);
...
}

Note that the generic device_enable name is used, rather than
omap_device_enable.  This is so other architectures can pass in their
own enable/disable functions here.

This should be populated during device setup:

...
pdata-device_enable = omap_device_enable;
...

2. Drivers should first check to ensure the function pointer is not null
before calling it, as in:

if (pdata-device_enable)
pdata-device_enable(pdev);

This allows other architectures that don't use similar device_enable()/
device_shutdown() functions to execute normally.

...

Suggested usage by device drivers:

During device initialization:
device_enable()

During device idle:
(save remaining device context if necessary)
device_idle();

During device resume:
device_enable();
(restore context if necessary)

During device shutdown:
device_shutdown()
(device must be reinitialized at this point to use it again)

--
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


Query: EHCI clock management approach

2009-10-07 Thread Gadiyar, Anand
Hi all,

I'd like to post a patch in a couple of days to refactor the EHCI clock
management code. This would be useful to do aggressive clock
management in the idle path.

A current implementation I have is to simply factor out the clock_enable/disable
calls out. Does it make sense to move this out to mach-omap2/* and provide
the function pointer through platform data? Does the driver need to be aware
of the clocks that it needs, or is it sufficient for it to just call a 
platform-specific
function that turns on/off all the clocks needed by the driver?

The reason I ask is, in a future OMAP, we may need to enable a different set of
clocks, but we should be able to re-use the rest of the code directly.


- Anand
--
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