[PATCH v2 0/6] ARM: S5PV210: Add support S5PV210/S5PC110 CPUFREQ

[MyungJoo Ham]

On Tue, Oct 12, 2010 at 9:15 AM, Kukjin Kim <kgene.kim at samsung.com> wrote:
> MyungJoo Ham wrote:
>>
>> On Mon, Oct 11, 2010 at 5:04 PM, Sangbeom Kim <sbkim73 at samsung.com>
>> wrote:
>> > On Monday, Oct 11, 2010 9:12 AM, MyungJoo Ham
>> <myungjoo.ham at samsung.com> wrote:
>> >>
>> >> Hello,
>> >>
>> >> On Fri, Oct 8, 2010 at 6:52 PM, Kukjin Kim <kgene.kim at samsung.com>
> wrote:
>> >> > Sangbeom Kim wrote:
>> >> >>
>> >> >> Hi all,
>> >> >> I know that there is another S5PV210 CPUFREQ patch from Mr. Ham.
>> >> >> But, That patchset seems to have unclear part.
>> >> >>
>> >> > http://lists.infradead.org/pipermail/linux-arm-kernel/2010-
>> >> August/023626.htm
>> >> >> l
>> >> >> The part of 'CONFIG_PM' is unclear and still have PM support part.
>> >> >> So, I think that this patch is more suitable for S5PV210/S5PC110
>> >> CPUFREQ
>> >> > in
>> >> >> the mainline.
>> >> >> because this patch implemented basic pm functionality and already
>> >> tested
>> >> > on
>> >> >> smdkv210 board.
>> >> >>
>> >> > Hi,
>> >> >
>> >> > So...I'd like to merge this for this merge window.
>> >> >
>> >> > Mr. Ham, how do you think?
>> >> > If any problems or opinions, please let me know.
>> >>
>> >>
>> >> You can go ahead as long as this patch include features that the
>> >> another addresses.
>> >>
>> >>
>> >> Anyway, the part that is under "CONFIG_PM" is intended to block
>> >> frequency changing while PM is doing something. For example, when the
>> >> system enters a sleep, if CPUFREQ somehow enters "100MHz/0.95V" just
>> >> before the sleep and wakes up, the voltage may stay at 0.95V (PMIC is
>> >> not turned off during sleep) while the frequency is reset to 800MHz
>> >> (by the bootloader), which requires, let's say, 1.1V. Such
>> >> inconsistency may lead to system hang and we have observed such cases
>> >> a few times. I intended to refuse any CPUFREQ requests at the
>> >> /mach-s5pv210/cpufreq.c when PM seems to start.
>> >>
>> >
>> > We knew already that issue,
>> > So we have a plan to implement with other solution.
>> > (using regulator suspend feature)
>> >
>> >> It may seem that such feature is not needed if the voltage is not
>> >> optimized, however, CPUFREQ is useless if the voltage stays at 1.2V
>> >> and if the voltage changes properly according to the frequency, such
>> >> feature is required anyway.
>> >
>> > Currently, there is no max8698 regulator driver on mainline.
>> > After submission that driver, we can add voltage scaling feature.
>> >
>> > Both issues have dependency on regulator driver.
>>
>> Nope. it does not need to depend on any regulator drivers. We can
>> simply stop changing frequency (or hold at a specific frequency) after
>> suspend-to-mem is called. (probably at ".begin" stage). This depends
>> on PM, but does NOT depend on REGULATOR.
>>
>> The approach in the removed patch was to set a flag at the .begin PM
>> ops and deflag it at .end PM ops and let target function of CPUFREQ to
>> read the flag to decide whether to accept the frequency change request
>> and to set at the default frequency (i.e., 800MHz) when .begin ops is
>> called and restore the frequency when .end ops is called.
>>
>
> Hi all,
>
> Both looks reasonable to me, in my opinion, need to decide one thing.
>
> As I said, I'd like to apply basic functionality of CPUFREQ, Jaecheol Lee's
> patch in this merge window.
> It would be better if could keep going update its other functionality next
> time.
>
> Thanks.

Yes, we can focus on the basic functionality for now and add
additional features later.

However, if we implement "stop changing voltage in suspend ops of
regulator", there is a risk of "changing frequency in suspend routine"
while "voltage is not changing" depending on when it stops changing
the voltage. Note that "suspend" function of regulator driver would be
too soon because cpufreq may keep running at that time. Because the
duration between cpu enters wfi and running suspend ops of regulator
driver is very short, the system may seem reliable, but there still
exists a risk (like one fail every 10,000 sleeps?) Besides, utilizing
regulator_suspend_prepare() would be not a good solution either:
running regulator_suspend_prepare() before device suspend loop makes
devices fail (regulators turned off too soon) and running it after the
loop would make cpufreq fail.


Cheers!

- MyungJoo
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim at samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858