Paul
>Sent: Friday, January 25, 2008 1:47 AM
>
>Hello Vikram,
>
>a few comments:
>
>On Thu, 24 Jan 2008, Pandita, Vikram wrote:
>
>> + omap_writel((12 << CM_CLKSEL4_PLL_PERIPH2_DPLL_DIV_SHIFT)|
>> + (120 << CM_CLKSEL4_PLL_PERIPH2_DPLL_MULT_SHIFT),
>> + CM_CLKSEL4_PLL);
>
>Please change reads and writes to CM registers to use
>cm_{read,write}_{mod,}_reg(). These should use the defines in cm.h and
>cm_regbits_34xx.h and prcm_common.h. One advantage of doing this are that
>CM register debugging can be enabled to track all CM register writes.
>Plus it will automatically compute the CM module offset, so you won't need
>to create your own register defines, like CM_CLKSEL4_PLL.
>
>Regarding the register bit shift defines, perhaps you can add
>OMAP3430ES2_PERIPH2_DPLL_MULT_SHIFT and OMAP3430ES2_PERIPH2_DPLL_DIV_SHIFT
>to cm_regbits_34xx.h, next to the mask definitions for the same bits, and
>use those. This keeps all of these definitions in the same place.
>
>So the above could be written as:
>
>cm_write_mod_reg((12 << OMAP3430ES2_PERIPH2_DPLL_DIV_SHIFT) |
> (120 << OMAP3430ES2_PERIPH2_DPLL_MULT_SHIFT),
> PLL_MOD, OMAP3430ES2_CM_CLKSEL4);
DPLL5 enabling should be made part of CM/PRM framework.
It should be enabled upfront or tied to 120m clk and put in autoidle mode and in that sense USB should not care about DPLL5.
Since the framework does not have it now, I have put it in USB code.
>
>
>> Index: omap-submit/drivers/usb/host/ehci-omap.h
>> ===================================================================
>>
>> +/* PRCM: CLOCK_Control_CM */
>> +#define PM_PREPWSTST_USBHOST\
>> + (OMAP2_PRM_BASE + OMAP3430ES2_USBHOST_MOD + OMAP3430_PM_PREPWSTST)
>
>Please use what's defined in the CM and PRM include files here. Defining
>new macros there if needed.
Are there any exported header files for this? I think all files are present in arch/arm/mach-omap2/ and how will any driver access the .h?
>
>Regards,
>
>- Paul
-
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
