RE: [PATCHv3 18/22] OMAP3: PM: Optional reset of voltage during Smartreflex disable.
-Original Message- From: Kevin Hilman [mailto:khil...@deeprootsystems.com] Sent: Wednesday, April 28, 2010 12:45 AM To: Gopinath, Thara Cc: linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit; Sripathy, Vishwanath; Sawant, Anand Subject: Re: [PATCHv3 18/22] OMAP3: PM: Optional reset of voltage during Smartreflex disable. Thara Gopinath th...@ti.com writes: Currently whenever smartreflex is disabled the voltage for the particular VDD is reset to the non-smartreflex compensated level. This step is unnecessary during dvfs because anyways in the next couple of steps before re-enabling smartreflex , the voltage level is changed. This patch adds the flexibility in the smartreflex framework for the user to specify whether or not a voltage reset is required after disabling of smartrefelx. The smartreflex driver just passes on this info to the smartreflex class driver, which ultimately takes the decision to reset the voltage or not. Signed-off-by: Thara Gopinath th...@ti.com I don't think this option should be a decision made for each call to omap_smartreflex_[en|dis]able(). Rather it should be an init time option. Hello Kevin, Why do you say this? Anytime we do a disable of smartreflex auto compensation from user space we need a reset of the voltage is required. During dvfs during smartreflex disable a reset of the voltage is not required. And in both these scenarios it is the same class API that gets called. So the only way for the API to know whether to reset the voltage or not is through this parameter. Also IMHO keeping it parameter based allows more flexibility in the framework for voltage reset. Regards Thara Kevin -- 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: [PATCHv3 18/22] OMAP3: PM: Optional reset of voltage during Smartreflex disable.
Gopinath, Thara th...@ti.com writes: -Original Message- From: Kevin Hilman [mailto:khil...@deeprootsystems.com] Sent: Wednesday, April 28, 2010 12:45 AM To: Gopinath, Thara Cc: linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit; Sripathy, Vishwanath; Sawant, Anand Subject: Re: [PATCHv3 18/22] OMAP3: PM: Optional reset of voltage during Smartreflex disable. Thara Gopinath th...@ti.com writes: Currently whenever smartreflex is disabled the voltage for the particular VDD is reset to the non-smartreflex compensated level. This step is unnecessary during dvfs because anyways in the next couple of steps before re-enabling smartreflex , the voltage level is changed. This patch adds the flexibility in the smartreflex framework for the user to specify whether or not a voltage reset is required after disabling of smartrefelx. The smartreflex driver just passes on this info to the smartreflex class driver, which ultimately takes the decision to reset the voltage or not. Signed-off-by: Thara Gopinath th...@ti.com I don't think this option should be a decision made for each call to omap_smartreflex_[en|dis]able(). Rather it should be an init time option. Why do you say this? Anytime we do a disable of smartreflex auto compensation from user space we need a reset of the voltage is required. During dvfs during smartreflex disable a reset of the voltage is not required. And in both these scenarios it is the same class API that gets called. So the only way for the API to know whether to reset the voltage or not is through this parameter. Also IMHO keeping it parameter based allows more flexibility in the framework for voltage reset. OK, I see now. I was looking at this patch in isolation and didn't notice the usage of it in the next DVFS patch where the -disable is done without reset. Also, since this new flag is a bool, change it from int to bool. Or better yet add a new function. One thing I'm not a fan of for readability sake is the use of bool flags. It makes me have to go look at the function to see what the flag is used for. I'd rather just see a new function added. Something like omap_smartreflex_disable_noreset() would make the code a bit more readable. Thanks, Kevin -- 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: [PATCHv3 18/22] OMAP3: PM: Optional reset of voltage during Smartreflex disable.
Thara Gopinath th...@ti.com writes: Currently whenever smartreflex is disabled the voltage for the particular VDD is reset to the non-smartreflex compensated level. This step is unnecessary during dvfs because anyways in the next couple of steps before re-enabling smartreflex , the voltage level is changed. This patch adds the flexibility in the smartreflex framework for the user to specify whether or not a voltage reset is required after disabling of smartrefelx. The smartreflex driver just passes on this info to the smartreflex class driver, which ultimately takes the decision to reset the voltage or not. Signed-off-by: Thara Gopinath th...@ti.com I don't think this option should be a decision made for each call to omap_smartreflex_[en|dis]able(). Rather it should be an init time option. Kevin -- 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
