Re: [PATCH] PM / devfreq: Fix incorrect type issue.

Re: [PATCH] PM / devfreq: Fix incorrect type issue.

[MyungJoo Ham]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 1361 bytes --]

  
> time_in_state in struct devfreq is defined as unsigned long, so
> devm_kzalloc should use sizeof(unsigned long) as argument instead
> of sizeof(unsigned int), otherwise it will cause unexpected result
> in 64bit system.
> 
> Signed-off-by: Xiaolong Ye <yexl@marvell.com>
> Signed-off-by: Kevin Liu <kliu5@marvell.com>

Thanks!

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>


Which SoC are you working with?
Are you going to upstream your 64bit devfreq driver soon?


Cheers,
MyungJoo

> ---
>  drivers/devfreq/devfreq.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index ca1b362..ac9845a 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -482,7 +482,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  						devfreq->profile->max_state *
>  						devfreq->profile->max_state,
>  						GFP_KERNEL);
> -	devfreq->time_in_state = devm_kzalloc(dev, sizeof(unsigned int) *
> +	devfreq->time_in_state = devm_kzalloc(dev, sizeof(unsigned long) *
>  						devfreq->profile->max_state,
>  						GFP_KERNEL);
>  	devfreq->last_stat_updated = jiffies;
> -- 
> 1.7.9.5
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH] PM / devfreq: Fix incorrect type issue.

[Xiaolong Ye]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1727 bytes --]

> 
> 
> > time_in_state in struct devfreq is defined as unsigned long, so
> > devm_kzalloc should use sizeof(unsigned long) as argument instead of
> > sizeof(unsigned int), otherwise it will cause unexpected result in
> > 64bit system.
> >
> > Signed-off-by: Xiaolong Ye <yexl@marvell.com>
> > Signed-off-by: Kevin Liu <kliu5@marvell.com>
> 
> Thanks!
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> 
> 
> Which SoC are you working with?

I am working on MARVELL PXA1928 SoC platform(with 4 ARM CA53 cores), and we are using devfreq framework to implement our 
ddr frequency change design, I found this issue when adapting our driver to 64bit system.

> Are you going to upstream your 64bit devfreq driver soon?

Currently, we don’t have a plan to upstream our ddr devfreq driver.

> 
> 
> Cheers,
> MyungJoo
> 
> > ---
> >  drivers/devfreq/devfreq.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > index ca1b362..ac9845a 100644
> > --- a/drivers/devfreq/devfreq.c
> > +++ b/drivers/devfreq/devfreq.c
> > @@ -482,7 +482,7 @@ struct devfreq *devfreq_add_device(struct device
> *dev,
> >  						devfreq->profile->max_state *
> >  						devfreq->profile->max_state,
> >  						GFP_KERNEL);
> > -	devfreq->time_in_state = devm_kzalloc(dev, sizeof(unsigned int) *
> > +	devfreq->time_in_state = devm_kzalloc(dev, sizeof(unsigned long) *
> >  						devfreq->profile->max_state,
> >  						GFP_KERNEL);
> >  	devfreq->last_stat_updated = jiffies;
> > --
> > 1.7.9.5
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH] fix incorrect initialization code for trans_stat of devfreq

[Xiaolong Ye]

Hi,

I met trans_stat abnormal issue while debugging devfreq feature for ddr in arm64 system, cat trans_stat 
would get extremely large time value which is unreasonable, I checked the code and found it was caused by
incorrect initialization code, time_in_state is initialized by devm_kzalloc and wrongly use sizeof(unsigned int)
as argument, but time_in_state which is defined in struct devfreq actually is defined as unsigned long, so
it is ok for 32 bit system, but will cause unexpected result in 64 bit system.

Xiaolong Ye (1):
  PM / devfreq: Fix incorrect type issue.

 drivers/devfreq/devfreq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
1.7.9.5

[PATCH] PM / devfreq: Fix incorrect type issue.

[Xiaolong Ye]

time_in_state in struct devfreq is defined as unsigned long, so
devm_kzalloc should use sizeof(unsigned long) as argument instead
of sizeof(unsigned int), otherwise it will cause unexpected result
in 64bit system.

Signed-off-by: Xiaolong Ye <yexl@marvell.com>
Signed-off-by: Kevin Liu <kliu5@marvell.com>
---
 drivers/devfreq/devfreq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index ca1b362..ac9845a 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -482,7 +482,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 						devfreq->profile->max_state *
 						devfreq->profile->max_state,
 						GFP_KERNEL);
-	devfreq->time_in_state = devm_kzalloc(dev, sizeof(unsigned int) *
+	devfreq->time_in_state = devm_kzalloc(dev, sizeof(unsigned long) *
 						devfreq->profile->max_state,
 						GFP_KERNEL);
 	devfreq->last_stat_updated = jiffies;
-- 
1.7.9.5