diff mbox

[4/5] ARM: S3C6410: Support 800MHz operation in cpufreq

Message ID 1306921493-30911-4-git-send-email-broonie@opensource.wolfsonmicro.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mark Brown June 1, 2011, 9:44 a.m. UTC
At least some newer S3C6410 silicon supports operation up to 800MHz rather
than just 667MHz. Unfortunately I don't have access to any of documentation
of this other than some running systems, add a new cpufreq table entry for
this based on the behaviour of those systems.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/cpufreq/s3c64xx.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Comments

Sangbeom Kim June 1, 2011, 10:30 a.m. UTC | #1
Hi, Mark

800Mhz S3C6410 is only supplied by customer request.
800Mhz dvfs operation should be controlled carefully.
(Stable VDD_ARM should be supplied)
To optimize s3c6410 dvfs operation, not only ARMCLK
But also HCLK should do dvfs.
In other words, VDD_INT, as well as VDD_ARM should be controlled.

Thanks and regards,
SB Kim

On Wed, Jun 1, 2011 at 6:45 PM, Mark Brown
> At least some newer S3C6410 silicon supports operation up to 800MHz rather
> than just 667MHz. Unfortunately I don't have access to any of
> documentation
> of this other than some running systems, add a new cpufreq table entry for
> this based on the behaviour of those systems.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/cpufreq/s3c64xx.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/cpufreq/s3c64xx.c b/drivers/cpufreq/s3c64xx.c
> index fc3f180..fc69178 100644
> --- a/drivers/cpufreq/s3c64xx.c
> +++ b/drivers/cpufreq/s3c64xx.c
> @@ -31,6 +31,7 @@ static struct s3c64xx_dvfs s3c64xx_dvfs_table[] = {
>  	[1] = { 1050000, 1150000 },
>  	[2] = { 1100000, 1150000 },
>  	[3] = { 1200000, 1350000 },
> +	[4] = { 1300000, 1350000 },
>  };
> 
>  static struct cpufreq_frequency_table s3c64xx_freq_table[] = {
> @@ -43,6 +44,7 @@ static struct cpufreq_frequency_table
> s3c64xx_freq_table[] = {
>  	{ 2, 532000 },
>  	{ 2, 533000 },
>  	{ 3, 667000 },
> +	{ 4, 800000 },
>  	{ 0, CPUFREQ_TABLE_END },
>  };
>  #endif
> --
> 1.7.5.3
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Mark Brown June 1, 2011, 10:40 a.m. UTC | #2
On Wed, Jun 01, 2011 at 07:30:42PM +0900, Sangbeom Kim wrote:
> Hi, Mark

Don't top post.

> 800Mhz S3C6410 is only supplied by customer request.
> 800Mhz dvfs operation should be controlled carefully.
> (Stable VDD_ARM should be supplied)

The fact that the chip is only on limited distribution isn't really an
issue for supporting it in mainline - many CPUs currently supported in
Linux, including some Samsung ones, are only available to certain
customers.

Similarly, stable power supplies are a basic system design thing that
we shouldn't be worrying about here.

> To optimize s3c6410 dvfs operation, not only ARMCLK
> But also HCLK should do dvfs.
> In other words, VDD_INT, as well as VDD_ARM should be controlled.

I don't have any access to documentation on this part here and in any
case we've already got code in mainline which is doing DVFS on S3C6410
without varying VDD_INT so that's a bit of a moot point.  We need to add
the 800MHz support in order to have the existing code understand the
state the device boots up in on the system I'm currently working with,
never mind actually change anything at runtime.  

As things stand the system will boot and then immediately scale the
voltage down to 400MHz as it's the only supported rate for 800MHz (we're
only doing scaling by division of the ARM clock) so adding the operating
point for 800MHz at least means that the performance governor will do
the right thing.
Sangbeom Kim June 2, 2011, 12:11 a.m. UTC | #3
On Wed, Jun 1, 2011 at 7:41 PM, Mark Brown
> The fact that the chip is only on limited distribution isn't really an
> issue for supporting it in mainline - many CPUs currently supported in
> Linux, including some Samsung ones, are only available to certain
> customers.
As you said, limited distribution isn't issue,
Issue is that all s3c6410 can't support 800Mhz.
Even if some s3c6410 processor unable to support 800Mhz,
In some cases, It seems to work on 800Mhz.
But System suddenly can be dead after few days working.(system hang up)
So, If some developer want use 800Mhz, They should exactly know  that
their s3c6410 can support 800Mhz or not.
I just want to say this.
800Mhz s3c6410 can be distinguished by checking laser mark.

> Similarly, stable power supplies are a basic system design thing that
> we shouldn't be worrying about here.
Even if 6410 can support 800Mhz, If power is unstable, system can be dead.
It is not 6410 problem. It is board problem.
When system hang up happened, In the most cases, regulator give unstable
VDD_ARM
(Even if regulator register setting is right)
You can just say that I don't care about that.
But System hang up isn't simple problem.
I want developer to know that If They try to develop 800Mhz 6410,
They should consider many things like board design, 
selecting regulator, support 800Mhz etc.
So It is more desirable to add some comment for using 800Mhz
Mark Brown June 2, 2011, 8:28 a.m. UTC | #4
On Thu, Jun 02, 2011 at 09:11:39AM +0900, Sangbeom Kim wrote:

> I just want to say this.
> 800Mhz s3c6410 can be distinguished by checking laser mark.

It's really unfortunate that there's no software visible method I'm
aware of :/

> > Similarly, stable power supplies are a basic system design thing that
> > we shouldn't be worrying about here.

> Even if 6410 can support 800Mhz, If power is unstable, system can be dead.
> It is not 6410 problem. It is board problem.
> When system hang up happened, In the most cases, regulator give unstable
> VDD_ARM

Yes, this is very much the case with any board design - the regulators
need to be able to both sustain the full rate current draw and rapidly
respond to changes in load and voltage in order to make a viable system.
In the case of voltage changes we've got some support in the regulator
API for ramp delays which should allow us to cope with regulators that
are slow to implement voltage changes but for transient response there's
really nothing doing except for good hardware design.

I'm not sure it's a particular concern for the 800MHz part above others
simply because it's such a universal issue.
Sangbeom Kim June 2, 2011, 9:41 a.m. UTC | #5
On Wed, Jun 2, 2011 at 9:11 AM, Mark Brown
> 
> It's really unfortunate that there's no software visible method I'm
> aware of :/
Unfortunately, There is no method to detect 800Mhz s3c6410 by software.


> I'm not sure it's a particular concern for the 800MHz part above others
> simply because it's such a universal issue.
Because Voltage margin(VDD_ARM) of s3c6410 800Mhz is particularly weak.
Under 667Mhz, There is no issue for dvfs.
Kyungmin Park June 2, 2011, 9:44 a.m. UTC | #6
On Thu, Jun 2, 2011 at 6:41 PM, Sangbeom Kim <sbkim73@samsung.com> wrote:
> On Wed, Jun 2, 2011 at 9:11 AM, Mark Brown
>>
>> It's really unfortunate that there's no software visible method I'm
>> aware of :/
> Unfortunately, There is no method to detect 800Mhz s3c6410 by software.

Then how about to introduce the software flags. and set it at each
board. It can support the 800MHz.
Also this flags are handled at cpufreq drivers properly.

How do you think?

>
>
>> I'm not sure it's a particular concern for the 800MHz part above others
>> simply because it's such a universal issue.
> Because Voltage margin(VDD_ARM) of s3c6410 800Mhz is particularly weak.
> Under 667Mhz, There is no issue for dvfs.
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Jassi Brar June 2, 2011, 9:50 a.m. UTC | #7
On Thu, Jun 2, 2011 at 3:14 PM, Kyungmin Park <kmpark@infradead.org> wrote:
>>>
>>> It's really unfortunate that there's no software visible method I'm
>>> aware of :/
>> Unfortunately, There is no method to detect 800Mhz s3c6410 by software.
>
> Then how about to introduce the software flags. and set it at each
> board. It can support the 800MHz.
> Also this flags are handled at cpufreq drivers properly.
>
> How do you think?

Two SMDKs may have different versions of s3c6410.
Which file do we want to set the flags in ?
Vasily Khoruzhick June 2, 2011, 9:55 a.m. UTC | #8
On Thursday 02 June 2011 12:50:56 Jassi Brar wrote:
> On Thu, Jun 2, 2011 at 3:14 PM, Kyungmin Park <kmpark@infradead.org> wrote:
> >>> It's really unfortunate that there's no software visible method I'm
> >>> aware of :/
> >> 
> >> Unfortunately, There is no method to detect 800Mhz s3c6410 by software.
> > 
> > Then how about to introduce the software flags. and set it at each
> > board. It can support the 800MHz.
> > Also this flags are handled at cpufreq drivers properly.
> > 
> > How do you think?
> 
> Two SMDKs may have different versions of s3c6410.
> Which file do we want to set the flags in ?

What about adding cmdline arg like 's3c6410_maxfreq', and defaulting it to 667 
MHz?

Regards
Vasily
Mark Brown June 2, 2011, 9:56 a.m. UTC | #9
On Thu, Jun 02, 2011 at 03:20:56PM +0530, Jassi Brar wrote:
> On Thu, Jun 2, 2011 at 3:14 PM, Kyungmin Park <kmpark@infradead.org> wrote:

> > Then how about to introduce the software flags. and set it at each
> > board. It can support the 800MHz.
> > Also this flags are handled at cpufreq drivers properly.

> > How do you think?

> Two SMDKs may have different versions of s3c6410.
> Which file do we want to set the flags in ?

In this case the board can effectively eliminate 800MHz operation
through the regulation constraints as it requires 1.3V rather than 1.2V.

IIRC the device always comes up at full speed so the boot state also
constrains things with the current system - there's no code to reclock
the PLLs so if we boot up at 667MHz then 800MHz is inaccessible.
Kyungmin Park June 2, 2011, 9:57 a.m. UTC | #10
On Thu, Jun 2, 2011 at 6:50 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> On Thu, Jun 2, 2011 at 3:14 PM, Kyungmin Park <kmpark@infradead.org> wrote:
>>>>
>>>> It's really unfortunate that there's no software visible method I'm
>>>> aware of :/
>>> Unfortunately, There is no method to detect 800Mhz s3c6410 by software.
>>
>> Then how about to introduce the software flags. and set it at each
>> board. It can support the 800MHz.
>> Also this flags are handled at cpufreq drivers properly.
>>
>> How do you think?
>
> Two SMDKs may have different versions of s3c6410.
> Which file do we want to set the flags in ?

Does it general case in real? As SMDK is development board. If make a
real board, it usually uses the same revision chip.
>
diff mbox

Patch

diff --git a/drivers/cpufreq/s3c64xx.c b/drivers/cpufreq/s3c64xx.c
index fc3f180..fc69178 100644
--- a/drivers/cpufreq/s3c64xx.c
+++ b/drivers/cpufreq/s3c64xx.c
@@ -31,6 +31,7 @@  static struct s3c64xx_dvfs s3c64xx_dvfs_table[] = {
 	[1] = { 1050000, 1150000 },
 	[2] = { 1100000, 1150000 },
 	[3] = { 1200000, 1350000 },
+	[4] = { 1300000, 1350000 },
 };
 
 static struct cpufreq_frequency_table s3c64xx_freq_table[] = {
@@ -43,6 +44,7 @@  static struct cpufreq_frequency_table s3c64xx_freq_table[] = {
 	{ 2, 532000 },
 	{ 2, 533000 },
 	{ 3, 667000 },
+	{ 4, 800000 },
 	{ 0, CPUFREQ_TABLE_END },
 };
 #endif