From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Hilman Subject: Re: [PATCH V2] OMAP3: PM: Set/reset T2 bit for Smartreflex on TWL. Date: Thu, 03 Feb 2011 10:05:40 -0800 Message-ID: <87mxmd2du3.fsf@ti.com> References: <1295847446-20667-1-git-send-email-shweta.gulati@ti.com> <87wrljnzri.fsf@ti.com> <4D48C1A4.6040600@ti.com> <87y65y9kw4.fsf@ti.com> <4D4A023D.8030908@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from na3sys009aog116.obsmtp.com ([74.125.149.240]:41780 "EHLO na3sys009aog116.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756418Ab1BCSFp (ORCPT ); Thu, 3 Feb 2011 13:05:45 -0500 Received: by pwj6 with SMTP id 6so374781pwj.18 for ; Thu, 03 Feb 2011 10:05:44 -0800 (PST) In-Reply-To: <4D4A023D.8030908@ti.com> (Nishanth Menon's message of "Thu, 03 Feb 2011 06:47:49 +0530") Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: Nishanth Menon Cc: Shweta Gulati , linux-omap@vger.kernel.org, Thara Gopinath Nishanth Menon writes: > Kevin Hilman wrote, on 02/03/2011 03:09 AM: >> Nishanth Menon writes: >> >>> Kevin Hilman wrote, on 02/02/2011 04:11 AM: >>>> Shweta Gulati writes: >>>> >>>>> From: Thara Gopinath >>>>> >>>>> The smartreflex bit on twl4030 needs to be enabled by default irrespective >>>>> of whether smartreflex module is enabled on the OMAP side or not. >>>>> This is because without this bit enabled the voltage scaling through >>>>> vp forceupdate does not function properly on OMAP3.API added >>>>> 'omap3_twl_set_sr_bit' with parameter to set/clear SR bit. It is cleared >>>>> for platforms where voltage is not scaled using vpforceupdate >>>>> or vc_bypass Method. In those cases 'omap3_twl_set_sr_bit' is called >>>>> from board file, to make sure this bit is not overwritten in >>>>> 'omap3_twl_init', a flag 'twl_sr_enable' >>>>> is added. >>>> >>>> As Sanjeev pointed out, the use of 'irrespective' above is confusing, in >>>> fact the whole changelog is kind of confusing. >>>> >>>> The changelog states that it has to always be enabled, but then goes on >>>> to describe the situation(s) where it would be disabled. >>>> >>>> Here's my rephrasing of how I understand the above changelog >>>> >>>> - enable: *always* be enabled >>>> - enable: needed for VP force update >>>> - disable: platforms using VP forced update or VP bypass >>>> >>>> -ECONFUSED >>>> >>>> Kevin >>> >>> How about this as the commit log? >>> >>> The smartreflex bit on twl4030 specifies if the setting of voltage >>> is done over the I2C_SR path. Given that there are platforms that >>> do not use I2C_SR path for voltage scaling, a new function >>> 'omap3_twl_set_sr_bit' with parameter to set/clear SR bit has been >>> provided for flexibility. >> >> So far so good. >> >>> It is called with appropriate param >>> for platforms where voltage is not scaled using I2C_SR path >>> from board file, to make sure this bit is not overwritten in >>> 'omap3_twl_init'. >> >> -ENOPARSE > k, How about this: > Voltage control on TWL can be done using VMODE/I2C1/I2C_SR. Since > almost all platforms use I2C_SR on omap3, omap3_twl_init by default > defaults expects that OMAP's I2C_SR is plugged in to TWL's I2C and > calls omap3_twl_set_sr_bit. On platforms where I2C_SR is not > connected, the board files are expected to call > omap3_twl_set_sr_bit(false) to ensure that I2C_SR path is not set for > voltage control and prevent the default behavior of omap3_twl_init. Nice, thanks. >> >>> >>> >>>> >>>>> This patch is based on LO PM Branch and Smartreflex has been >>>>> tested on OMAP3430 SDP, OMAP3630 SDP and boot tested on >>>>> OMAP2430 SDP. >>> >>> this belongs into the diffstat. >>> >>> Attached is a modified version of this patch - i'vent tested it >>> though.. but basically improves the logic a little: >>> >>> *) made the comments more generic to ensure that this is more of >>> I2C_SR path as far as TWL is concerned(yes, from OMAP perspective it >>> is vp forceupdate/bypass), but it is more of an OMAP problem than >>> omap_twl.c problem. >>> *) modified the function call sequences to prevent rentry even if >>> board file calls with various other params >>> *) shifted to using bool >>> *) use init and initdata to free up the space once we are done with >>> init sequence >> >> All good changes, but I don't think they're incorporated in V3. > > could you be more clear inline on v3? Will look closer at v4. Kevin