Re: [PATCH V2] OMAP3: PM: Set/reset T2 bit for Smartreflex on TWL.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kevin Hilman wrote, on 02/03/2011 03:09 AM:
Nishanth Menon<nm@xxxxxx>  writes:

Kevin Hilman wrote, on 02/02/2011 04:11 AM:
Shweta Gulati<shweta.gulati@xxxxxx>   writes:

From: Thara Gopinath<thara@xxxxxx>

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.





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?


--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux