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

2009-10-23 Thread Imberton Guilhem-gimb01 
 

 -Original Message-
 From: Nishanth Menon [mailto:n...@ti.com] 
 Sent: Friday, October 23, 2009 4:40 AM
 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..

Your right negative value wont be good
Let's use 10uSec (50 registers reading remains realy small amount of time)
And it will anyway give us a good idea of the time spent here

 -- 
 Regards,
 Nishanth Menon


Regards,

Guilhem 
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2009-10-22 Thread Nishanth Menon 

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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html