Sent from Anson's iPhone
> 在 2013年12月17日，21:00，"Mark Brown" <broo...@kernel.org> 写道： > > On Tue, Dec 17, 2013 at 12:38:33PM +0000, anson.hu...@freescale.com wrote: > >>> better to add the error checking there wouldn't it? > >> Okay, than what about other functions? there is such condition check >> there is other functions too, that is why I add it here. if you think >> it is no necessary, I will remove this check in my patch? > > It shouldn't be in the other functions either. got it, I will remove it later. > >>> This sounds like you need to have some higher level synchronisation >>> between whatever is managing this supply and your cpufreq driver - if >>> you rely on reading back the current status from the hardware there will >>> always be races between reading the state and the other thing doing the >>> enable or disable. > >> yes, you are right. but I think we have handled that, all the >> operations of this LDO will via regulator interface, and regulator >> framework already has mutex lock. so there should be no such race in >> kernel as long as we all use regulator interface to access anatop LDO. > > No, that's not going to work. Consider this sequence: > > cpufreq other driver > disable() > is_enabled() > enable() > > The locking the regulator core does won't help you here, nothing stops > the state of the regulator changing after it's read. Your cpufreq > driver should just change the voltage and not worry if the regulator is > enabled, the voltage change won't have any effect while the regulator is > off and presumably if it does get enabled then it needs to be enabled at > whatever voltage cpufreq set anyway. > > In any case I'd be much happier with this patch if it implemented the > enable and disable operations as well. understand now. then maybe I should remove the PU check in cpufreq, although setting PU LDO if it is off before would bring unnessary power leak. I will add all these changes together with the dynamic PU LDO management feature, thanks for your time!
