All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: Re: [PATCH] PM / devfreq: Use freq_table for available_frequencies
@ 2014-04-15  1:36 ` 함명주
  0 siblings, 0 replies; 8+ messages in thread
From: 함명주 @ 2014-04-15  1:36 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: 박경민,
	linux-pm, linux-kernel, linux-arm-msm, linux-arm-kernel

> MyungJoo/Kyungmin,
> 
> Bump. Can we accept this patch please?
> 
> -Saravana

Nack.

Please note that freq_table is also an optional value, which may
be null.

Besides, please be aware that your code is under rcu_read_lock().


Cheers,
MyungJoo.

ps. I'll send a related patch (avoid accessing null but not-an-error
pointer at other sysfs nodes). Thank you for letting me catch such bugs anyway.

> 
> 
> On 04/10/2014 07:54 PM, Saravana Kannan wrote:
> > Some devices use freq_table instead of OPP. For those devices, the
> > available_frequencies file shows up empty. Fix that by using freq_table to
> > generate the available_frequencies data when OPP is not present.
> >
> > Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
> > ---
> >   drivers/devfreq/devfreq.c | 15 +++++++++++----
> >   1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > index 2042ec3..a715d15 100644
> > --- a/drivers/devfreq/devfreq.c
> > +++ b/drivers/devfreq/devfreq.c
> > @@ -912,19 +912,26 @@ static ssize_t available_frequencies_show(struct device *d,
> >   	struct devfreq *df = to_devfreq(d);
> >   	struct device *dev = df->dev.parent;
> >   	struct dev_pm_opp *opp;
> > +	unsigned int i = 0, max_state = df->profile->max_state;
> > +	bool use_opp;
> >   	ssize_t count = 0;
> >   	unsigned long freq = 0;
> >
> >   	rcu_read_lock();
> > +	use_opp = dev_pm_opp_get_opp_count(dev) > 0;
> >   	do {
> > -		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> > -		if (IS_ERR(opp))
> > -			break;
> > +		if (use_opp) {
> > +			opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> > +			if (IS_ERR(opp))
> > +				break;
> > +		} else {
> > +			freq = df->profile->freq_table[i++];
> > +		}
> >
> >   		count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
> >   				   "%lu ", freq);
> >   		freq++;
> > -	} while (1);
> > +	} while (use_opp || (!use_opp && i < max_state));
> >   	rcu_read_unlock();
> >
> >   	/* Truncate the trailing space */
> >
> 
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 
> 
> 
>        
>   
>          
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: Re: [PATCH] PM / devfreq: Use freq_table for available_frequencies
@ 2014-04-15  6:30 ` 함명주
  0 siblings, 0 replies; 8+ messages in thread
From: 함명주 @ 2014-04-15  6:30 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: 박경민,
	linux-pm, linux-kernel, linux-arm-msm, linux-arm-kernel

> On 04/14/2014 06:36 PM, 함명주 wrote:
> >> MyungJoo/Kyungmin,
> >>
> >> Bump. Can we accept this patch please?
> >>
> >> -Saravana
> > 
> > Nack.
> > 
> > Please note that freq_table is also an optional value, which may
> > be null.
> 
> Ah, I saw that the max_freq would be zero if freq_table was NULL and I
> assumed that it can't be NULL. But I see that the max_freq limit is not
> applied if it's zero. Thanks for catching it.
> 
> > Besides, please be aware that your code is under rcu_read_lock().
> 
> Valid point. I was just trying to keep the diff simple. No one's really
> going to be catting this file often when performance matters.
> 
> > 
> > 
> > Cheers,
> > MyungJoo.
> > 
> > ps. I'll send a related patch (avoid accessing null but not-an-error
> > pointer at other sysfs nodes). Thank you for letting me catch such bugs anyway.
> 
> I can go ahead and do this myself if you don't mind.

No, we don't need it. It was a false alarm.
Reading again, I've found that we've already made other sysfs nodes
check if either freq_table is null or its size is 0.

So, we only need to look at this available_frequencies node now.

I'll add some notes on the ABI doc for available_frequencies soon.

Cheers,
MyungJoo.

> 
> -Saravana
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 
> 
> 
>        
>   
>          
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: Re: [PATCH] PM / devfreq: Use freq_table for available_frequencies
@ 2014-04-18 10:47 ` MyungJoo Ham
  0 siblings, 0 replies; 8+ messages in thread
From: MyungJoo Ham @ 2014-04-18 10:47 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: 박경민,
	linux-pm, linux-kernel, linux-arm-msm, linux-arm-kernel

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

> On 04/15/2014 11:41 AM, Saravana Kannan wrote:
> > Ah, I misunderstood your previous email. I thought you Nack-ed my patch
> > and decided to send your own patch to replace mine. Ok, I'll fix up mine
> > and send it out.
> 
> MyungJoo/Kyungmin,
> 
> I sent out an updated patch. Can you please take a look?
> 
> Thanks,
> Saravana

I'll be away from most of network system during the weekend.
And I'll be only accessing mobile phones (no laptops or desktops) until next Wednesday.

Please let me look at this a while later. (I'll have other pending tons of messages as well at that time)

Sorry for the extended delay.

Cheers,
MyungJoo.

> 
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 
> 
ÿôèº{.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¥

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-04-18 10:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-15  1:36 Re: [PATCH] PM / devfreq: Use freq_table for available_frequencies 함명주
2014-04-15  1:36 ` 함명주
2014-04-15  5:16 ` Saravana Kannan
2014-04-15  5:16   ` Saravana Kannan
2014-04-15  6:30 함명주
2014-04-15  6:30 ` 함명주
2014-04-18 10:47 MyungJoo Ham
2014-04-18 10:47 ` MyungJoo Ham

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.