Re: [PATCHv3 02/16] drivers: thermal: introduce device tree parser

[Eduardo Valentin]

On 18-09-2013 15:59, Joe Perches wrote:
> On Wed, 2013-09-18 at 15:44 -0400, Eduardo Valentin wrote:
>> Hello Joe,
> 
> Hello Eduardo.
> 
>>  18-09-2013 15:11, Joe Perches wrote:
>>> On Wed, 2013-09-18 at 15:02 -0400, Eduardo Valentin wrote:
>>>> This patch introduces a device tree bindings for
>>>> describing the hardware thermal behavior and limits.
>>>> Also a parser to read and interpret the data and feed
>>>> it in the thermal framework is presented.
>>>
>>> trivial notes:
> []
>>> If readings are within some non zero noise level,
>>> perhaps stable should be returned.
>>
>> Yes, there should be some sort of threshold for temperature trend. But I
>> am not sure this is the right place to implement this. This type of
>> feature is in my TODO list, but I am planing to get it done within the
>> core code of the thermal framework.
> 
> No worries, one day...  Just wanted to bring it to attention.

Thanks, no worries, I should be working on it at some point in near
future. It does not hurt to add a TODO mark above this code though.

> 
>>>> +static struct __thermal_zone *
>>>> +thermal_of_build_thermal_zone(struct device_node *np)
>>>> +{
> []
>>>> +	tz = kzalloc(sizeof(*tz), GFP_KERNEL);
>>>> +	if (!tz) {
>>>> +		pr_err("not enough memory for thermal of zone\n");
>>>
>>> Unnecessary OOM message.
>>> All allocs without GFP_NOWARN get a dump_stack()
> 
> []
> 
>> Hmmm.. I am pretty sure you have a good point. But to me seams to be
>> still a common practice to have drivers outputing error messages when
>> allocation fails. A simple git grep -A 4 kzalloc for instance, shows
>> that there are still quite a considerable amount of occurrences of such
>> practice.
> 
> True though a considerable number have been removed.
> I think these should all be deleted eventually and
> there's no good reason to add more.


I see.  I will repost with your suggestion. Thanks.

> 
> 
> 
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin

Attachment: signature.asc
Description: OpenPGP digital signature