Hi,

On Mon, Nov 26, 2012 at 05:14:45PM +0200, Roger Quadros wrote:
> Felipe,
> 
> On 11/21/2012 03:39 PM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Nov 15, 2012 at 04:34:04PM +0200, Roger Quadros wrote:
> >> All ports have similarly named port clocks so we can
> >> bunch them into a port data structure and use for loop
> >> to enable/disable the clocks.
> >>
> >> Signed-off-by: Roger Quadros <rog...@ti.com>
> >> ---
> >>  drivers/mfd/omap-usb-host.c |  208 
> >> +++++++++++++++++++++----------------------
> >>  1 files changed, 101 insertions(+), 107 deletions(-)
> >>
> >> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> >> index 23cec57..7303c41 100644
> >> --- a/drivers/mfd/omap-usb-host.c
> >> +++ b/drivers/mfd/omap-usb-host.c
> >> @@ -76,6 +76,8 @@
> >>  
> >>  #define   OMAP_UHH_DEBUG_CSR                              (0x44)
> >>  
> >> +#define MAX_HS_USB_PORTS  3       /* Increase this if any chip has more */
> >> +
> >>  /* Values of UHH_REVISION - Note: these are not given in the TRM */
> >>  #define OMAP_USBHS_REV1           0x00000010      /* OMAP3 */
> >>  #define OMAP_USBHS_REV2           0x50700100      /* OMAP4 */
> >> @@ -87,14 +89,15 @@
> >>  #define is_ehci_tll_mode(x)       (x == OMAP_EHCI_PORT_MODE_TLL)
> >>  #define is_ehci_hsic_mode(x)      (x == OMAP_EHCI_PORT_MODE_HSIC)
> >>  
> >> +struct usbhs_port {
> >> +  struct clk      *utmi_clk;
> >> +};
> > 
> > I rather not since this will make it a lot more difficult to use
> > pm_clk_add() :-s Also, this sort of thing should be dynamically
> > allocated anyway ;-)
> > 
> 
> Why do you say so? The whole point of this patch is to group similarly
> named clocks so that we can use a for loop and set number of ports (or
> clocks) dynamically. I suppose it would be just a matter of replacing
> clk_enable/disable() with pm_clk_add() later, right?
> 
> If you see patch 11, we are adding 2 HSIC related clocks to this
> structure. This means 9 clocks (i.e. 3 clocks for 3 ports) can be
> managed using a simple for loop instead of coding each clock name by hand.

that's usually not what you want, actually. You want clock management to
be explicit so you can have micro-power optimizations. I fear that the
time omap-usb-host.c gets *truly* stabilized and we move to more
aggressive PM, we will undo these changes just to have a more granular
control of the clocks, at which point your patch would be rendered
useless.

-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to