lkml.org 
[lkml]   [2010]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] power: introduce library for device-specific OPPs
Date
"Menon, Nishanth" <nm@ti.com> writes:

>> -----Original Message-----
>> From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
>> Sent: Thursday, October 07, 2010 4:55 PM
>
>
>>
>> Hi,
>>
>> On Wednesday, October 06, 2010, Nishanth Menon wrote:
>> > SoCs have a standard set of tuples consisting of frequency and
>> > voltage pairs that the device will support per voltage domain. These
>> > are called Operating Performance Points or OPPs. The actual
>> > definitions of OPP varies over silicon versions. For a specific domain,
>> > we can have a set of {frequency, voltage} pairs. As the kernel boots
>> > and more information is available, a default set of these are activated
>> > based on the precise nature of device. Further on operation, based on
>> > conditions prevailing in the system (such as temperature), some OPP
>> > availability may be temporarily controlled by the SoC frameworks.
>> >
>> > To implement an OPP, some sort of power management support is necessary
>> > hence this library depends on CONFIG_PM.
>>
>> The patch generally looks good to me, I only have a couple of cosmetic
> Thanks for the great reviews.. It did bump up the resultant patch.
>
>> remarks
>> (below).
>>
>> ...
>> > +static int opp_set_availability(struct device *dev, unsigned long freq,
>> > + bool availability_req)
>> > +{
>> > + struct device_opp *tmp_dev_opp, *dev_opp = NULL;
>> > + struct opp *new_opp, *tmp_opp, *opp = ERR_PTR(-ENODEV);
>> > + int r = 0;
>> > +
>> > + /* keep the node allocated */
>> > + new_opp = kmalloc(sizeof(struct opp), GFP_KERNEL);
>> > + if (!new_opp) {
>> > + pr_warning("Unable to allocate opp\n");
>>
>> Please add an identification string to the messages, something like
>> "OPP: Unable to allocat object\n" (and similarly in the other messages).
>> That would help to find the source of a message in case there's any
>> problem.
>
> pr_fmt has been reformatted for this. The actual message which will appear
> is as follows:
> opp_set_availability: Unable to allocate opp
>
> is'nt that good enough considering that all functions are opp_ prefixed?
> I can modify pr_fmt to add "OPP:" but I kinda think it is redundant. But I
> have no strong opinions on that and look forward to your recommendations.

Even more informative would be to use dev_warn() and include the func.
That way we'll even know which device has the problem.

Kevin


\
 
 \ /
  Last update: 2010-10-08 00:15    [W:0.052 / U:0.480 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site