Hi, 

>A few minor comments based on a quick look.
>
>In terms of the OMAP3 code, is that for retention-idle only, 
>or does it work with off-idle also?  

That patch should basically work in off-mode also, however it requires
correct settings in the padconfig registers which are currently done in
the boot code. Also, currently I have not had any chance to actually
test this in off-mode, I am still trying to get the off-mode work on my
HW.

>
>On Tue, 10 Jun 2008, Tero Kristo wrote:
>
>> +const u32 omap2_uart_wk_bit[OMAP_MAX_NR_PORTS] = {
>> +       OMAP24XX_ST_UART1, OMAP24XX_ST_UART2, OMAP24XX_ST_UART3 };
>
>Looks like that can be static.  

True.

>
>> +/* UART padconfig registers, these may differ if 
>non-default padconfig
>> +   is used */
>> +#define CONTROL_PADCONF_UART1_RX 0x48002182 #define 
>> +CONTROL_PADCONF_UART2_RX 0x4800217A #define 
>CONTROL_PADCONF_UART3_RX 
>> +0x4800219E #define PADCONF_WAKEUP_ST 0x8000
>
>Please use omap_ctrl_read{b,w,l}() rather than 
>omap_read{b,w,l}() for SCM addresses.

Hmm yea, could do.

>Also, the above are register addresses + 2.  Is it possible to 
>use the actual register addresses (ideally they should be 
>defined in include/asm-arm/arch-omap/control.h), do long 
>reads, and simply shift the register contents for the serial 
>ports that need it?

These registers are currently not defined anywhere and I did not feel
like wanting to put all the padconfig register definitions somewhere
(all 200+ of them).... I don't think it is currently very clear how we
want to handle these registers in general. Also, this fix is more or
less temporary anyway while we are waiting for a real driver that
handles UART clocks properly (the final solution will most likely use
some of the elements of this though.)

Also, the spec says that these registers can be accessed in either 8, 16
or 32 bit modes so why make it unnecessarily complicated and potentially
buggy with shifts (race conditions)?

-Tero
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to