On Tue, Jul 24, 2012 at 12:47 PM, Manjunathappa, Prakash
[off-list ref] wrote:

Hi Sriram,

On Tue, Jul 24, 2012 at 10:11:55, Madhvapathi Sriram wrote:

quoted

On Tue, Jul 24, 2012 at 9:39 AM, Manjunathappa, Prakash
[off-list ref] wrote:

[...]

quoted

quoted

 /* Disable the Raster Engine of the LCD Controller */
-static inline void lcd_disable_raster(void)
+static inline void lcd_disable_raster(bool wait_for_frame_done)
 {
        u32 reg;
+       u32 loop_cnt = 0;
+       u32 stat;
+       u32 i = 0;
+
+       /* 50 milli seconds should be sufficient for a frame to complete
*/
+       if (wait_for_frame_done)
+               loop_cnt = 50;

        reg = lcdc_read(LCD_RASTER_CTRL_REG);
        if (reg & LCD_RASTER_ENABLE)
                lcdc_write(reg & ~LCD_RASTER_ENABLE, LCD_RASTER_CTRL_REG);
+
+       /* Wait for the current frame to complete */
+       do {
+               if (lcd_revision = LCD_VERSION_1)
+                       stat = lcdc_read(LCD_STAT_REG);
+               else
+                       stat = lcdc_read(LCD_RAW_STAT_REG);
+               mdelay(1);
+       } while (!(stat & LCD_FRAME_DONE) && (i++ < loop_cnt));

Are you sure it should be do while? You call it from interrupt handler
too (sync lost handling). In that case mdelay() from interrupt
context..not a good idea I feel.

Ok. I will change it as below:

while (1) {
               if (lcd_revision = LCD_VERSION_1)
                       stat = lcdc_read(LCD_STAT_REG);
               else
                       stat = lcdc_read(LCD_RAW_STAT_REG);
               if((stat & LCD_FRAME_DONE) || (i++ > loop_cnt))
                    break;
               mdelay(1);
}

Hmm.. while(1) could be avoided though. Club all frame-done-wait
related at one place. I leave it open for your choice.

Some where in the beginning of fxn....
u32 stat_reg = LCD_STAT_REG;

Avoids conditions in loop....
if (lcd_revision = LCD_VERSION_2)
       stat_reg = LCD_RAW_STAT_REG;

if (wait_for_frame_done) {
    u32 loop_count = 50;

    while (!(lcdc_read(stat_reg) & LCD_FRAME_DONE)) {
           /* Handle timeout */
           if(unlikely(0 = --loop_count)) {
                pr_err("LCD Controller timed out\n");
                break;
           }
           mdelay(1);
    }
}

quoted

quoted

+
+       if (lcd_revision = LCD_VERSION_1)
+               lcdc_write(stat, LCD_STAT_REG);
+       else
+               lcdc_write(stat, LCD_MASKED_STAT_REG);
+
+       if ((loop_cnt != 0) && (i >= loop_cnt)) {
+               pr_err("LCD Controller timed out\n");
+               return;

return may not be necessary?

Agree, I will remove it.

Thanks,
Prakash

quoted

quoted

+       }
 }

[...]