Re: [PATCH 2/2 v2] OMAP3:PM:SR: SmartReflex Refactor Rev3.0

[Nishanth Menon <nm@ti.com>]

Imberton Guilhem-gimb01 had written, on 10/22/2009 04:04 PM, the following:
>  I have one small comment on the actual implementation
>> +     do {
>> +             value = prm_read_mod_reg(OMAP3430_GR_MOD,
>> +                                      OMAP3_PRM_VC_BYPASS_VAL_OFFSET)
>> +                 & OMAP3430_VALID;
>> +             /* should i wait? */
>> +             if (value && (count % 50))
>> +                     udelay(10);
>> +             count--;
>> +             *timeout_us -= 5;
>> +     } while (value && count);
> 
> Why are you removing 5us every read ?
Thanks for catching this. Yep, I have got my counters mixed up :(.. will 
add it as part of v2 for tomorrow evening with other potential comments 
if any.

> If 50 iteration is 1us and we delay every loop by 10us we should remove 11us
> 
> Shouldn't it be:
> 
> +       do {
> +               value = prm_read_mod_reg(OMAP3430_GR_MOD,
> +                                        OMAP3_PRM_VC_BYPASS_VAL_OFFSET)
> +                   & OMAP3430_VALID;
> +               /* should i wait? */
> +               if (value && (count % 50)) {
> +                       udelay(10);
> +                     *timeout_us -= 11;
> +           }
> +               count--;
> +       } while (value && count);
> 
here is a problem with the above logic:
a) I dont know for sure if 50 iterations == 1 uSec.
b) count = timeout*5
   ==> if function is called with timeout = 10 uSec, count = 50, and 
udelay(10) will be called once. now *timeout -=11 will make it a 
negative value, which is not helpeful.

so, the decrement has to be by 10 OR i can pad timeout_us .. not exactly 
useful either..
-- 
Regards,
Nishanth Menon