diff mbox

[v9,4/6] ARM: Exynos: switch to using generic cpufreq driver for Exynos4210/5250/5420

Message ID CADWw7OgW+o3tB=ptnsy-PsmXmNv8oXzt_WfK6NswQiEWWwAzNA@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chander Kashyap Aug. 26, 2014, 5:25 a.m. UTC
Hi Kevin,

On Mon, Aug 25, 2014 at 9:02 PM, Kevin Hilman <khilman@linaro.org> wrote:
> Hi Chander,
>
> Chander Kashyap <k.chander@samsung.com> writes:
>
> [...]
>
>>>> I'm trying it on the 5800/Chromebook2 and it's not terribly stable.  I'm
>>>> testing along with CPUidle, so there may be some untested interactions
>>>> there as it seems a bit more stable without CPUidle enabled.
>>>>
>>>> I'd love to hear from anyone else that's testing CPUidle and CPUfreq
>>>> together big.LITTLE 5420/5800, with or without the switcher.
>>
>> I have tested this patch series on SMDK5420 with cpuidle (with and
>> without b.L switcher enabled).
>>
>> As of now voltage scaling support is not there in generic big-little
>> cpufreq driver (arm_big_little.c).
>> Hence need to tie arm and kfc voltages to highest level for testing.
>
>> Without this change stability issues are there, but with this change
>> everything is stable.
>
> Can you clarify how you're setting the voltages to ensure stability?

below is the diff :  wip/exynos/integ


>
> Tomasz, I didn't mean to suggest this isn't ready for mainline.  For the
> 5420/5800 it seems cpufreq support is a new feature, so this isn't a
> regression against previous (mainline) behavior.  Maybe the big.LITTLE
> cpufreq support should've been separated out from the cleanup since it's
> more of a new feature, but that's up to you.
>
> Kevin
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

regards,
Chander

Comments

Kevin Hilman Aug. 26, 2014, 3:15 p.m. UTC | #1
On Mon, Aug 25, 2014 at 10:25 PM, Chander Kashyap <k.chander@samsung.com> wrote:
> Hi Kevin,
>
> On Mon, Aug 25, 2014 at 9:02 PM, Kevin Hilman <khilman@linaro.org> wrote:
>> Hi Chander,
>>
>> Chander Kashyap <k.chander@samsung.com> writes:
>>
>> [...]
>>
>>>>> I'm trying it on the 5800/Chromebook2 and it's not terribly stable.  I'm
>>>>> testing along with CPUidle, so there may be some untested interactions
>>>>> there as it seems a bit more stable without CPUidle enabled.
>>>>>
>>>>> I'd love to hear from anyone else that's testing CPUidle and CPUfreq
>>>>> together big.LITTLE 5420/5800, with or without the switcher.
>>>
>>> I have tested this patch series on SMDK5420 with cpuidle (with and
>>> without b.L switcher enabled).
>>>
>>> As of now voltage scaling support is not there in generic big-little
>>> cpufreq driver (arm_big_little.c).
>>> Hence need to tie arm and kfc voltages to highest level for testing.
>>
>>> Without this change stability issues are there, but with this change
>>> everything is stable.
>>
>> Can you clarify how you're setting the voltages to ensure stability?
>
> below is the diff :  wip/exynos/integ

Thanks.

I've applied your patch, and bootup shows vdd_arm and vdd_kfc at
1500mV, but still when booting with cpuidle enabled (bL switcher
disabled), I'm seeing lockups with no kernel output.  With CPUidle
disabled, things are pretty stable.

What tree are you using to test this out on 5420?  I'm using mainline
v3.17-rc1 + DT patch for CPUidle and this cpufreq series.  See my
wip/exynos/integ branch at
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux.git.

Are there other out of tree dependencies that I'm missing?  Is the
max77802 regulator support that's in mainline sufficient?  or am I
missing some stuff there?

Kevin
Kevin Hilman Aug. 26, 2014, 10:25 p.m. UTC | #2
On Tue, Aug 26, 2014 at 8:15 AM, Kevin Hilman <khilman@linaro.org> wrote:
> On Mon, Aug 25, 2014 at 10:25 PM, Chander Kashyap <k.chander@samsung.com> wrote:

[...]

>>>
>>> Can you clarify how you're setting the voltages to ensure stability?
>>
>> below is the diff :  wip/exynos/integ
>
> Thanks.
>
> I've applied your patch, and bootup shows vdd_arm and vdd_kfc at
> 1500mV, but still when booting with cpuidle enabled (bL switcher
> disabled), I'm seeing lockups with no kernel output.  With CPUidle
> disabled, things are pretty stable.
>
> What tree are you using to test this out on 5420?  I'm using mainline
> v3.17-rc1 + DT patch for CPUidle and this cpufreq series.  See my
> wip/exynos/integ branch at
> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux.git.

I mis-stated this.  Actually my tree is based on the v3.17-rc1 branch
of the exynos-reference tree[1] + the above mentioned patches for
cpuidle and cpufreq.

Also, I've narrowed down the instability a bit, and it's not related
to CPUidle.  I can now trigger a boot hang even without CPUidle
enabled.  Here's a quick way to cause a boot lockup. With the switcher
disabled, I enable CPUfreq and set the default governor to
performance.  As soon as cpufreq driver loads, it tries to use the top
frequences for both clusters, and it hangs.

Selectively disabling frequencies, I narrowed it down to the 1.3GHz
and 1.2GHz frequencies of the little cluster.  With these commented
out in the DT, it will fully boot with the performance governor
enabled.

So that leads to the question.  Are all of the operating points in
exynos5420.dtsi valid for exynos5800, and have they been validated?

Kevin

[1]https://github.com/exynos-reference/kernel.git
Thomas Abraham Aug. 29, 2014, 12:52 p.m. UTC | #3
Hi Kevin,

On Wed, Aug 27, 2014 at 3:55 AM, Kevin Hilman <khilman@linaro.org> wrote:
> On Tue, Aug 26, 2014 at 8:15 AM, Kevin Hilman <khilman@linaro.org> wrote:
>> On Mon, Aug 25, 2014 at 10:25 PM, Chander Kashyap <k.chander@samsung.com> wrote:
>
> [...]
>
>>>>
>>>> Can you clarify how you're setting the voltages to ensure stability?
>>>
>>> below is the diff :  wip/exynos/integ
>>
>> Thanks.
>>
>> I've applied your patch, and bootup shows vdd_arm and vdd_kfc at
>> 1500mV, but still when booting with cpuidle enabled (bL switcher
>> disabled), I'm seeing lockups with no kernel output.  With CPUidle
>> disabled, things are pretty stable.
>>
>> What tree are you using to test this out on 5420?  I'm using mainline
>> v3.17-rc1 + DT patch for CPUidle and this cpufreq series.  See my
>> wip/exynos/integ branch at
>> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux.git.
>
> I mis-stated this.  Actually my tree is based on the v3.17-rc1 branch
> of the exynos-reference tree[1] + the above mentioned patches for
> cpuidle and cpufreq.
>
> Also, I've narrowed down the instability a bit, and it's not related
> to CPUidle.  I can now trigger a boot hang even without CPUidle
> enabled.  Here's a quick way to cause a boot lockup. With the switcher
> disabled, I enable CPUfreq and set the default governor to
> performance.  As soon as cpufreq driver loads, it tries to use the top
> frequences for both clusters, and it hangs.
>
> Selectively disabling frequencies, I narrowed it down to the 1.3GHz
> and 1.2GHz frequencies of the little cluster.  With these commented
> out in the DT, it will fully boot with the performance governor
> enabled.
>
> So that leads to the question.  Are all of the operating points in
> exynos5420.dtsi valid for exynos5800, and have they been validated?

I tried to recreate the boot lockup issue using the same steps you
listed above for the Exynos5800 peach-pi platform (Chromebook2), but I
do not see any issues. I can see both clusters with max clock speed
after boot (1.8GHz and 1.3GHz).

I am using v3.17-rc2 + CPUFreq Patches + max77802 regulator support
patches for Chromebook2 + temp hack to set A15 voltage to 1.35V and A7
voltage to 1.3V.

Sorry for the delaying in following up.

Thanks,
Thomas.

>
> Kevin
>
> [1]https://github.com/exynos-reference/kernel.git
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman Aug. 29, 2014, 3:03 p.m. UTC | #4
Hi Thomas,

On Fri, Aug 29, 2014 at 5:52 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
> Hi Kevin,
>
> On Wed, Aug 27, 2014 at 3:55 AM, Kevin Hilman <khilman@linaro.org> wrote:
>> On Tue, Aug 26, 2014 at 8:15 AM, Kevin Hilman <khilman@linaro.org> wrote:
>>> On Mon, Aug 25, 2014 at 10:25 PM, Chander Kashyap <k.chander@samsung.com> wrote:
>>
>> [...]
>>
>>>>>
>>>>> Can you clarify how you're setting the voltages to ensure stability?
>>>>
>>>> below is the diff :  wip/exynos/integ
>>>
>>> Thanks.
>>>
>>> I've applied your patch, and bootup shows vdd_arm and vdd_kfc at
>>> 1500mV, but still when booting with cpuidle enabled (bL switcher
>>> disabled), I'm seeing lockups with no kernel output.  With CPUidle
>>> disabled, things are pretty stable.
>>>
>>> What tree are you using to test this out on 5420?  I'm using mainline
>>> v3.17-rc1 + DT patch for CPUidle and this cpufreq series.  See my
>>> wip/exynos/integ branch at
>>> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux.git.
>>
>> I mis-stated this.  Actually my tree is based on the v3.17-rc1 branch
>> of the exynos-reference tree[1] + the above mentioned patches for
>> cpuidle and cpufreq.
>>
>> Also, I've narrowed down the instability a bit, and it's not related
>> to CPUidle.  I can now trigger a boot hang even without CPUidle
>> enabled.  Here's a quick way to cause a boot lockup. With the switcher
>> disabled, I enable CPUfreq and set the default governor to
>> performance.  As soon as cpufreq driver loads, it tries to use the top
>> frequences for both clusters, and it hangs.
>>
>> Selectively disabling frequencies, I narrowed it down to the 1.3GHz
>> and 1.2GHz frequencies of the little cluster.  With these commented
>> out in the DT, it will fully boot with the performance governor
>> enabled.
>>
>> So that leads to the question.  Are all of the operating points in
>> exynos5420.dtsi valid for exynos5800, and have they been validated?
>
> I tried to recreate the boot lockup issue using the same steps you
> listed above for the Exynos5800 peach-pi platform (Chromebook2), but I
> do not see any issues. I can see both clusters with max clock speed
> after boot (1.8GHz and 1.3GHz).
>
> I am using v3.17-rc2 + CPUFreq Patches + max77802 regulator support
> patches for Chromebook2 + temp hack to set A15 voltage to 1.35V and A7
> voltage to 1.3V.

Can you share your branch and temp hack(s) as well as your defconfig?

I'm using the v3.17-rc1 branch from the exynos tree (which includes
the max77802 series) but also has a bunch of other stuff which may be
causing the issue.

It would be good if I can reproduce your exact tree/branch and see if
I still have the same problem.

Thanks for looking into this,

Kevin
Thomas Abraham Sept. 1, 2014, 8:47 a.m. UTC | #5
Hi Kevin,


On Fri, Aug 29, 2014 at 8:33 PM, Kevin Hilman <khilman@kernel.org> wrote:
> Hi Thomas,
>
> On Fri, Aug 29, 2014 at 5:52 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
>> Hi Kevin,
>>
>> On Wed, Aug 27, 2014 at 3:55 AM, Kevin Hilman <khilman@linaro.org> wrote:
>>> On Tue, Aug 26, 2014 at 8:15 AM, Kevin Hilman <khilman@linaro.org> wrote:
>>>> On Mon, Aug 25, 2014 at 10:25 PM, Chander Kashyap <k.chander@samsung.com> wrote:
>>>
>>> [...]
>>>
>>>>>>
>>>>>> Can you clarify how you're setting the voltages to ensure stability?
>>>>>
>>>>> below is the diff :  wip/exynos/integ
>>>>
>>>> Thanks.
>>>>
>>>> I've applied your patch, and bootup shows vdd_arm and vdd_kfc at
>>>> 1500mV, but still when booting with cpuidle enabled (bL switcher
>>>> disabled), I'm seeing lockups with no kernel output.  With CPUidle
>>>> disabled, things are pretty stable.
>>>>
>>>> What tree are you using to test this out on 5420?  I'm using mainline
>>>> v3.17-rc1 + DT patch for CPUidle and this cpufreq series.  See my
>>>> wip/exynos/integ branch at
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux.git.
>>>
>>> I mis-stated this.  Actually my tree is based on the v3.17-rc1 branch
>>> of the exynos-reference tree[1] + the above mentioned patches for
>>> cpuidle and cpufreq.
>>>
>>> Also, I've narrowed down the instability a bit, and it's not related
>>> to CPUidle.  I can now trigger a boot hang even without CPUidle
>>> enabled.  Here's a quick way to cause a boot lockup. With the switcher
>>> disabled, I enable CPUfreq and set the default governor to
>>> performance.  As soon as cpufreq driver loads, it tries to use the top
>>> frequences for both clusters, and it hangs.
>>>
>>> Selectively disabling frequencies, I narrowed it down to the 1.3GHz
>>> and 1.2GHz frequencies of the little cluster.  With these commented
>>> out in the DT, it will fully boot with the performance governor
>>> enabled.
>>>
>>> So that leads to the question.  Are all of the operating points in
>>> exynos5420.dtsi valid for exynos5800, and have they been validated?
>>
>> I tried to recreate the boot lockup issue using the same steps you
>> listed above for the Exynos5800 peach-pi platform (Chromebook2), but I
>> do not see any issues. I can see both clusters with max clock speed
>> after boot (1.8GHz and 1.3GHz).
>>
>> I am using v3.17-rc2 + CPUFreq Patches + max77802 regulator support
>> patches for Chromebook2 + temp hack to set A15 voltage to 1.35V and A7
>> voltage to 1.3V.
>
> Can you share your branch and temp hack(s) as well as your defconfig?
>
> I'm using the v3.17-rc1 branch from the exynos tree (which includes
> the max77802 series) but also has a bunch of other stuff which may be
> causing the issue.
>
> It would be good if I can reproduce your exact tree/branch and see if
> I still have the same problem.

The branch with the patches that have been used to test cpufreq on
Exynos5800 is available at

https://github.com/exynos-reference/kernel/tree/exynos5-v3.17-rc3-temp-cpufreq

Please let me know if this works or if there are any issues.

Thanks,
Thomas.

>
> Thanks for looking into this,
>
> Kevin
Kevin Hilman Sept. 2, 2014, 7:32 p.m. UTC | #6
HI Thomas,

Thomas Abraham <ta.omasab@gmail.com> writes:

> On Fri, Aug 29, 2014 at 8:33 PM, Kevin Hilman <khilman@kernel.org> wrote:
>> Hi Thomas,
>>
>> On Fri, Aug 29, 2014 at 5:52 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
>>> Hi Kevin,
>>>
>>> On Wed, Aug 27, 2014 at 3:55 AM, Kevin Hilman <khilman@linaro.org> wrote:
>>>> On Tue, Aug 26, 2014 at 8:15 AM, Kevin Hilman <khilman@linaro.org> wrote:
>>>>> On Mon, Aug 25, 2014 at 10:25 PM, Chander Kashyap <k.chander@samsung.com> wrote:
>>>>
>>>> [...]
>>>>
>>>>>>>
>>>>>>> Can you clarify how you're setting the voltages to ensure stability?
>>>>>>
>>>>>> below is the diff :  wip/exynos/integ
>>>>>
>>>>> Thanks.
>>>>>
>>>>> I've applied your patch, and bootup shows vdd_arm and vdd_kfc at
>>>>> 1500mV, but still when booting with cpuidle enabled (bL switcher
>>>>> disabled), I'm seeing lockups with no kernel output.  With CPUidle
>>>>> disabled, things are pretty stable.
>>>>>
>>>>> What tree are you using to test this out on 5420?  I'm using mainline
>>>>> v3.17-rc1 + DT patch for CPUidle and this cpufreq series.  See my
>>>>> wip/exynos/integ branch at
>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux.git.
>>>>
>>>> I mis-stated this.  Actually my tree is based on the v3.17-rc1 branch
>>>> of the exynos-reference tree[1] + the above mentioned patches for
>>>> cpuidle and cpufreq.
>>>>
>>>> Also, I've narrowed down the instability a bit, and it's not related
>>>> to CPUidle.  I can now trigger a boot hang even without CPUidle
>>>> enabled.  Here's a quick way to cause a boot lockup. With the switcher
>>>> disabled, I enable CPUfreq and set the default governor to
>>>> performance.  As soon as cpufreq driver loads, it tries to use the top
>>>> frequences for both clusters, and it hangs.
>>>>
>>>> Selectively disabling frequencies, I narrowed it down to the 1.3GHz
>>>> and 1.2GHz frequencies of the little cluster.  With these commented
>>>> out in the DT, it will fully boot with the performance governor
>>>> enabled.
>>>>
>>>> So that leads to the question.  Are all of the operating points in
>>>> exynos5420.dtsi valid for exynos5800, and have they been validated?
>>>
>>> I tried to recreate the boot lockup issue using the same steps you
>>> listed above for the Exynos5800 peach-pi platform (Chromebook2), but I
>>> do not see any issues. I can see both clusters with max clock speed
>>> after boot (1.8GHz and 1.3GHz).
>>>
>>> I am using v3.17-rc2 + CPUFreq Patches + max77802 regulator support
>>> patches for Chromebook2 + temp hack to set A15 voltage to 1.35V and A7
>>> voltage to 1.3V.
>>
>> Can you share your branch and temp hack(s) as well as your defconfig?
>>
>> I'm using the v3.17-rc1 branch from the exynos tree (which includes
>> the max77802 series) but also has a bunch of other stuff which may be
>> causing the issue.
>>
>> It would be good if I can reproduce your exact tree/branch and see if
>> I still have the same problem.
>
> The branch with the patches that have been used to test cpufreq on
> Exynos5800 is available at
>
> https://github.com/exynos-reference/kernel/tree/exynos5-v3.17-rc3-temp-cpufreq
>
> Please let me know if this works or if there are any issues.

Yes, your branch works fine, but it's because of the last (unposted)
patch on your branch[1]: 
ARM: dts: remove all supplies sourced from tps65090 PMIC

That patch had not been posted, so I hadn't seen it before, but based on
the changelog, it's pretty clear you had the same problems that I had
without it, so I'm not sure why it wasn't mentioned earlier in this
thread.

I also noticed that the "force vdd_arm and vdd_kfc to max voltage" patch
is not actually using the max voltage, which appears to be 1.5V from the
DT, but actually using 1.35 V, however the changelog has no explanation
for this.

One other thing, your temp-cpufreq branch has conflicts with max77802
stuff in the v3.17-rc1 branch of the exynos-reference tree (which I'm
using for CPUidle dependencies, on the PMU series IIRC.)

Are there any plans to update the main referece branch and include
cpufreq?

Kevin

[1] https://github.com/exynos-reference/kernel/commit/f08be7e4296a3452ee5d1aae31e3de5bbff2cf1a
Thomas Abraham Sept. 3, 2014, 4:26 a.m. UTC | #7
Hi Kevin,

On Wed, Sep 3, 2014 at 1:02 AM, Kevin Hilman <khilman@kernel.org> wrote:
> HI Thomas,
>
> Thomas Abraham <ta.omasab@gmail.com> writes:
>
>> On Fri, Aug 29, 2014 at 8:33 PM, Kevin Hilman <khilman@kernel.org> wrote:
>>> Hi Thomas,
>>>
>>> On Fri, Aug 29, 2014 at 5:52 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
>>>> Hi Kevin,
>>>>
>>>> On Wed, Aug 27, 2014 at 3:55 AM, Kevin Hilman <khilman@linaro.org> wrote:
>>>>> On Tue, Aug 26, 2014 at 8:15 AM, Kevin Hilman <khilman@linaro.org> wrote:
>>>>>> On Mon, Aug 25, 2014 at 10:25 PM, Chander Kashyap <k.chander@samsung.com> wrote:
>>>>>
>>>>> [...]
>>>>>
>>>>>>>>
>>>>>>>> Can you clarify how you're setting the voltages to ensure stability?
>>>>>>>
>>>>>>> below is the diff :  wip/exynos/integ
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>> I've applied your patch, and bootup shows vdd_arm and vdd_kfc at
>>>>>> 1500mV, but still when booting with cpuidle enabled (bL switcher
>>>>>> disabled), I'm seeing lockups with no kernel output.  With CPUidle
>>>>>> disabled, things are pretty stable.
>>>>>>
>>>>>> What tree are you using to test this out on 5420?  I'm using mainline
>>>>>> v3.17-rc1 + DT patch for CPUidle and this cpufreq series.  See my
>>>>>> wip/exynos/integ branch at
>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux.git.
>>>>>
>>>>> I mis-stated this.  Actually my tree is based on the v3.17-rc1 branch
>>>>> of the exynos-reference tree[1] + the above mentioned patches for
>>>>> cpuidle and cpufreq.
>>>>>
>>>>> Also, I've narrowed down the instability a bit, and it's not related
>>>>> to CPUidle.  I can now trigger a boot hang even without CPUidle
>>>>> enabled.  Here's a quick way to cause a boot lockup. With the switcher
>>>>> disabled, I enable CPUfreq and set the default governor to
>>>>> performance.  As soon as cpufreq driver loads, it tries to use the top
>>>>> frequences for both clusters, and it hangs.
>>>>>
>>>>> Selectively disabling frequencies, I narrowed it down to the 1.3GHz
>>>>> and 1.2GHz frequencies of the little cluster.  With these commented
>>>>> out in the DT, it will fully boot with the performance governor
>>>>> enabled.
>>>>>
>>>>> So that leads to the question.  Are all of the operating points in
>>>>> exynos5420.dtsi valid for exynos5800, and have they been validated?
>>>>
>>>> I tried to recreate the boot lockup issue using the same steps you
>>>> listed above for the Exynos5800 peach-pi platform (Chromebook2), but I
>>>> do not see any issues. I can see both clusters with max clock speed
>>>> after boot (1.8GHz and 1.3GHz).
>>>>
>>>> I am using v3.17-rc2 + CPUFreq Patches + max77802 regulator support
>>>> patches for Chromebook2 + temp hack to set A15 voltage to 1.35V and A7
>>>> voltage to 1.3V.
>>>
>>> Can you share your branch and temp hack(s) as well as your defconfig?
>>>
>>> I'm using the v3.17-rc1 branch from the exynos tree (which includes
>>> the max77802 series) but also has a bunch of other stuff which may be
>>> causing the issue.
>>>
>>> It would be good if I can reproduce your exact tree/branch and see if
>>> I still have the same problem.
>>
>> The branch with the patches that have been used to test cpufreq on
>> Exynos5800 is available at
>>
>> https://github.com/exynos-reference/kernel/tree/exynos5-v3.17-rc3-temp-cpufreq
>>
>> Please let me know if this works or if there are any issues.
>
> Yes, your branch works fine, but it's because of the last (unposted)
> patch on your branch[1]:
> ARM: dts: remove all supplies sourced from tps65090 PMIC

I must have explicitly stated that I am using local changes to get
vdd_arm and vdd_kfc to required levels. I apologize for that. These
are local temporary changes which I did not want to post. I am working
on adding voltage scaling support in arm bL cpufreq driver with which
these local hacks would not be necessary.

>
> That patch had not been posted, so I hadn't seen it before, but based on
> the changelog, it's pretty clear you had the same problems that I had
> without it, so I'm not sure why it wasn't mentioned earlier in this
> thread.

At the time of posting, this patch series was only tested on
Exynos5420 based smdk5420 board. There was no regulator support for
peach-pit and peach-pi at that time and so I had not tested this
series on Exynos5800 Chromebook2. But the code was written to be fully
compatible for Exynos5800 as well. It was when you reported a problem
with Exynos5800 that I tested this series on Exynos5800 with the
regulator patches from Javier.

>
> I also noticed that the "force vdd_arm and vdd_kfc to max voltage" patch
> is not actually using the max voltage, which appears to be 1.5V from the
> DT, but actually using 1.35 V, however the changelog has no explanation
> for this.

This also is a temporary patch and by "max voltage" I actually meant
max voltage required to operate the cpus and not the max voltage that
the buck can supply.

>
> One other thing, your temp-cpufreq branch has conflicts with max77802
> stuff in the v3.17-rc1 branch of the exynos-reference tree (which I'm
> using for CPUidle dependencies, on the PMU series IIRC.)

I haven't checked but probably there is an older version of Javier's
regulator patches in the v3.17-rc1 branch.

>
> Are there any plans to update the main referece branch and include
> cpufreq?

Yes, a new branch with all the latest patches (cpufreq + regulator +
temp fixes) will be created. I will let you when that is ready.

Thanks,
Thomas.

>
> Kevin
>
> [1] https://github.com/exynos-reference/kernel/commit/f08be7e4296a3452ee5d1aae31e3de5bbff2cf1a
Thomas Abraham Sept. 3, 2014, 1:18 p.m. UTC | #8
On Wed, Sep 3, 2014 at 9:56 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
> Hi Kevin,
>
> On Wed, Sep 3, 2014 at 1:02 AM, Kevin Hilman <khilman@kernel.org> wrote:
>> HI Thomas,
>>
>> Thomas Abraham <ta.omasab@gmail.com> writes:
>>
>>> On Fri, Aug 29, 2014 at 8:33 PM, Kevin Hilman <khilman@kernel.org> wrote:
>>>> Hi Thomas,
>>>>
>>>> On Fri, Aug 29, 2014 at 5:52 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
>>>>> Hi Kevin,
>>>>>
>>>>> On Wed, Aug 27, 2014 at 3:55 AM, Kevin Hilman <khilman@linaro.org> wrote:
>>>>>> On Tue, Aug 26, 2014 at 8:15 AM, Kevin Hilman <khilman@linaro.org> wrote:
>>>>>>> On Mon, Aug 25, 2014 at 10:25 PM, Chander Kashyap <k.chander@samsung.com> wrote:
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>>>>
>>>>>>>>> Can you clarify how you're setting the voltages to ensure stability?
>>>>>>>>
>>>>>>>> below is the diff :  wip/exynos/integ
>>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>> I've applied your patch, and bootup shows vdd_arm and vdd_kfc at
>>>>>>> 1500mV, but still when booting with cpuidle enabled (bL switcher
>>>>>>> disabled), I'm seeing lockups with no kernel output.  With CPUidle
>>>>>>> disabled, things are pretty stable.
>>>>>>>
>>>>>>> What tree are you using to test this out on 5420?  I'm using mainline
>>>>>>> v3.17-rc1 + DT patch for CPUidle and this cpufreq series.  See my
>>>>>>> wip/exynos/integ branch at
>>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux.git.
>>>>>>
>>>>>> I mis-stated this.  Actually my tree is based on the v3.17-rc1 branch
>>>>>> of the exynos-reference tree[1] + the above mentioned patches for
>>>>>> cpuidle and cpufreq.
>>>>>>
>>>>>> Also, I've narrowed down the instability a bit, and it's not related
>>>>>> to CPUidle.  I can now trigger a boot hang even without CPUidle
>>>>>> enabled.  Here's a quick way to cause a boot lockup. With the switcher
>>>>>> disabled, I enable CPUfreq and set the default governor to
>>>>>> performance.  As soon as cpufreq driver loads, it tries to use the top
>>>>>> frequences for both clusters, and it hangs.
>>>>>>
>>>>>> Selectively disabling frequencies, I narrowed it down to the 1.3GHz
>>>>>> and 1.2GHz frequencies of the little cluster.  With these commented
>>>>>> out in the DT, it will fully boot with the performance governor
>>>>>> enabled.
>>>>>>
>>>>>> So that leads to the question.  Are all of the operating points in
>>>>>> exynos5420.dtsi valid for exynos5800, and have they been validated?
>>>>>
>>>>> I tried to recreate the boot lockup issue using the same steps you
>>>>> listed above for the Exynos5800 peach-pi platform (Chromebook2), but I
>>>>> do not see any issues. I can see both clusters with max clock speed
>>>>> after boot (1.8GHz and 1.3GHz).
>>>>>
>>>>> I am using v3.17-rc2 + CPUFreq Patches + max77802 regulator support
>>>>> patches for Chromebook2 + temp hack to set A15 voltage to 1.35V and A7
>>>>> voltage to 1.3V.
>>>>
>>>> Can you share your branch and temp hack(s) as well as your defconfig?
>>>>
>>>> I'm using the v3.17-rc1 branch from the exynos tree (which includes
>>>> the max77802 series) but also has a bunch of other stuff which may be
>>>> causing the issue.
>>>>
>>>> It would be good if I can reproduce your exact tree/branch and see if
>>>> I still have the same problem.
>>>
>>> The branch with the patches that have been used to test cpufreq on
>>> Exynos5800 is available at
>>>
>>> https://github.com/exynos-reference/kernel/tree/exynos5-v3.17-rc3-temp-cpufreq
>>>
>>> Please let me know if this works or if there are any issues.
>>
>> Yes, your branch works fine, but it's because of the last (unposted)
>> patch on your branch[1]:
>> ARM: dts: remove all supplies sourced from tps65090 PMIC
>
> I must have explicitly stated that I am using local changes to get
> vdd_arm and vdd_kfc to required levels. I apologize for that. These
> are local temporary changes which I did not want to post. I am working
> on adding voltage scaling support in arm bL cpufreq driver with which
> these local hacks would not be necessary.
>
>>
>> That patch had not been posted, so I hadn't seen it before, but based on
>> the changelog, it's pretty clear you had the same problems that I had
>> without it, so I'm not sure why it wasn't mentioned earlier in this
>> thread.
>
> At the time of posting, this patch series was only tested on
> Exynos5420 based smdk5420 board. There was no regulator support for
> peach-pit and peach-pi at that time and so I had not tested this
> series on Exynos5800 Chromebook2. But the code was written to be fully
> compatible for Exynos5800 as well. It was when you reported a problem
> with Exynos5800 that I tested this series on Exynos5800 with the
> regulator patches from Javier.
>
>>
>> I also noticed that the "force vdd_arm and vdd_kfc to max voltage" patch
>> is not actually using the max voltage, which appears to be 1.5V from the
>> DT, but actually using 1.35 V, however the changelog has no explanation
>> for this.
>
> This also is a temporary patch and by "max voltage" I actually meant
> max voltage required to operate the cpus and not the max voltage that
> the buck can supply.
>
>>
>> One other thing, your temp-cpufreq branch has conflicts with max77802
>> stuff in the v3.17-rc1 branch of the exynos-reference tree (which I'm
>> using for CPUidle dependencies, on the PMU series IIRC.)
>
> I haven't checked but probably there is an older version of Javier's
> regulator patches in the v3.17-rc1 branch.
>
>>
>> Are there any plans to update the main referece branch and include
>> cpufreq?
>
> Yes, a new branch with all the latest patches (cpufreq + regulator +
> temp fixes) will be created. I will let you when that is ready.

Hi Kevin,

A new branch [1] has been created using commits from exynos5-v3.17-rc1
branch + cpufreq + regulator + temp fixes. I have tested this branch
on Exynos5800 Chromebook2 and cpufreq works fine with both ondemand
and performance governors. Please let me know if there are any issues
with this new branch. It is based on v3.17-rc3.

[1] https://github.com/exynos-reference/kernel/tree/exynos5-v3.17-rc3

Thanks,
Thomas.

>
> Thanks,
> Thomas.
>
>>
>> Kevin
>>
>> [1] https://github.com/exynos-reference/kernel/commit/f08be7e4296a3452ee5d1aae31e3de5bbff2cf1a
Kevin Hilman Sept. 3, 2014, 11:15 p.m. UTC | #9
Hi Thomas,

Thomas Abraham <ta.omasab@gmail.com> writes:

[...]

> A new branch [1] has been created using commits from exynos5-v3.17-rc1
> branch + cpufreq + regulator + temp fixes. I have tested this branch
> on Exynos5800 Chromebook2 and cpufreq works fine with both ondemand
> and performance governors. Please let me know if there are any issues
> with this new branch. It is based on v3.17-rc3.

Excellent!  Thank you.

The only thing missing now is the CPUidle support for 5800, and all
that's needed for that is the compatible string patch[1] which Daniel
has queued up.

With that patch, display + CPUidle + CPUfreq are working pretty well on
my exynos5800/chromebook2 with the big.LITTLE switcher disabled.  If I
turn on the switcher, it boots OK, but as soon as I try to run powertop
(upstream v2.6.1) it gets stuck.  Have you tried this branch with the
switcher enabled?

Thanks again for your work on this, we're getting close!  

Kevin

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279028.html
Thomas Abraham Sept. 4, 2014, 10:22 a.m. UTC | #10
On Thu, Sep 4, 2014 at 4:45 AM, Kevin Hilman <khilman@kernel.org> wrote:
> Hi Thomas,
>
> Thomas Abraham <ta.omasab@gmail.com> writes:
>
> [...]
>
>> A new branch [1] has been created using commits from exynos5-v3.17-rc1
>> branch + cpufreq + regulator + temp fixes. I have tested this branch
>> on Exynos5800 Chromebook2 and cpufreq works fine with both ondemand
>> and performance governors. Please let me know if there are any issues
>> with this new branch. It is based on v3.17-rc3.
>
> Excellent!  Thank you.
>
> The only thing missing now is the CPUidle support for 5800, and all
> that's needed for that is the compatible string patch[1] which Daniel
> has queued up.
>
> With that patch, display + CPUidle + CPUfreq are working pretty well on
> my exynos5800/chromebook2 with the big.LITTLE switcher disabled.  If I
> turn on the switcher, it boots OK, but as soon as I try to run powertop
> (upstream v2.6.1) it gets stuck.  Have you tried this branch with the
> switcher enabled?

Yes, I have tested switcher + cpufreq + cpuidle with this branch and
there are no issues found. I haven't tested with powertop yet. I will
try and do that and let you know the result.

You mentioned that when you run powertop, it gets stuck. When that
happens, is there any log on the console or does system just turn
unresponsive?

Thanks,
Thomas.

>
> Thanks again for your work on this, we're getting close!
>
> Kevin
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279028.html
>
Kevin Hilman Sept. 4, 2014, 1:30 p.m. UTC | #11
Thomas Abraham <ta.omasab@gmail.com> writes:

> On Thu, Sep 4, 2014 at 4:45 AM, Kevin Hilman <khilman@kernel.org> wrote:
>> Hi Thomas,
>>
>> Thomas Abraham <ta.omasab@gmail.com> writes:
>>
>> [...]
>>
>>> A new branch [1] has been created using commits from exynos5-v3.17-rc1
>>> branch + cpufreq + regulator + temp fixes. I have tested this branch
>>> on Exynos5800 Chromebook2 and cpufreq works fine with both ondemand
>>> and performance governors. Please let me know if there are any issues
>>> with this new branch. It is based on v3.17-rc3.
>>
>> Excellent!  Thank you.
>>
>> The only thing missing now is the CPUidle support for 5800, and all
>> that's needed for that is the compatible string patch[1] which Daniel
>> has queued up.
>>
>> With that patch, display + CPUidle + CPUfreq are working pretty well on
>> my exynos5800/chromebook2 with the big.LITTLE switcher disabled.  If I
>> turn on the switcher, it boots OK, but as soon as I try to run powertop
>> (upstream v2.6.1) it gets stuck.  Have you tried this branch with the
>> switcher enabled?
>
> Yes, I have tested switcher + cpufreq + cpuidle with this branch and
> there are no issues found. I haven't tested with powertop yet. I will
> try and do that and let you know the result.
>
> You mentioned that when you run powertop, it gets stuck. When that
> happens, is there any log on the console or does system just turn
> unresponsive?

The console is not responsive, but kernel seems busy because I see
periodic timeout messages from the samsung clock driver.

Note that I see these messages when things are functioning normally
also.

Kevin


[1]
[  337.832031] wait_until_divider_stable: timeout in divider stablization
[  337.847024] wait_until_divider_stable: timeout in divider stablization
[  337.862024] wait_until_divider_stable: timeout in divider stablization
[  337.957028] wait_until_divider_stable: timeout in divider stablization
[  337.972024] wait_until_divider_stable: timeout in divider stablization
[  337.987024] wait_until_divider_stable: timeout in divider stablization
[  340.082029] wait_until_divider_stable: timeout in divider stablization
[  340.097024] wait_until_divider_stable: timeout in divider stablization
[  340.112024] wait_until_divider_stable: timeout in divider stablization
[  346.242030] wait_until_divider_stable: timeout in divider stablization
[  346.257024] wait_until_divider_stable: timeout in divider stablization
[  346.272024] wait_until_divider_stable: timeout in divider stablization
[  348.322029] wait_until_divider_stable: timeout in divider stablization
[  348.337025] wait_until_divider_stable: timeout in divider stablization
[  348.352024] wait_until_divider_stable: timeout in divider stablization
diff mbox

Patch

--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -225,7 +225,7 @@ 

                        buck2_reg: BUCK2 {
                                regulator-name = "vdd_arm";
-                               regulator-min-microvolt = <800000>;
+                               regulator-min-microvolt = <1500000>;
                                regulator-max-microvolt = <1500000>;
                                regulator-always-on;
                                regulator-boot-on;
@@ -260,7 +260,7 @@ 

                        buck6_reg: BUCK6 {
                                regulator-name = "vdd_kfc";
-                               regulator-min-microvolt = <800000>;
+                               regulator-min-microvolt = <1500000>;
                                regulator-max-microvolt = <1500000>;
                                regulator-always-on;
                                regulator-boot-on;