RE: [PATCH 5/5] [CPUFREQ] EXYNOS4210: Add support ASV feature

[Kukjin Kim <kgene.kim@samsung.com>]

MyungJoo Ham wrote:
> 
> Hello,
> 
Hi,

I got the comments from Jaecheol Lee but his mail client has some problem so
I'm sending instead.

> On Wed, Nov 2, 2011 at 9:43 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> []
> > +static void __init set_volt_table(void)
> > +{
> > +       unsigned int tmp, i, asv_group = 0;
> > +
> > +       tmp = __raw_readl(S5P_INFORM2);
> 
> As I've mentioned in the ASV patch thread, do we really need to use an
> INFORM register simply to save the id of supported voltage ranges?
> 
> Why aren't we using an extern variable here? For example, "extern int
> asv_group_id;" and define it at "asv.h" or somewhere else.
> 
> At reboot, we are going to init ASV driver and will get the ASV value
> again; thus, we don't need to use such a preserving register anyway.
> At suspend/resume, the value in RAM does not disappear and the IPL
> does not care this value; thus, it is meaningless to use INFORM2 for
> this value.
> 
ASV feature had been implemented in bootloader hence inform register was
used to return the result value to cpufreq driver. If there is no problem to
use the inform register why don't you keep this manner.

(snip)

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.