Subject: Re: [PATCH 1/6] kvmppc: read device tree hypervisor node infrastructure

From: Christian Ehrhardt <[email-masked]>
Date: Thu, 24 Jul 2008 09:44:19 +0200

Tony Breeds wrote:
> On Wed, Jul 23, 2008 at 10:36:42AM +0200, [email-masked] wrote:
>
> Hi Christian,
> 	A few comments inlined ...
>
>   
[...]
>> +
>>  static inline int kvm_para_available(void)
>>  {
>> -	return 0;
>> +	struct device_node *dn;
>> +
>> +	dn = of_find_node_by_path("/hypervisor");
>>     
>
> You need an of_node_put(dn);
>
>   
I just looked at the linux/of.h and did not see that I have to free it 
again.
Thanks for the hint, I inserted both calls.
>> +
>> +	return !!dn;
>>  }
>>  
>>  static inline unsigned int kvm_arch_para_features(void)
>>  {
>> -	return 0;
>> +	struct device_node *dn;
>> +	const int *dtval;
>> +	unsigned int features = 0;
>> +	int i;
>> +
>> +	dn = of_find_node_by_path("/hypervisor");
>> +	if (!dn)
>> +		return 0;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(para_features)-1; i++) {
>>     
>
> Why -1?  Isn't ARRAY_SIZE(para_features) adequate?
>   

yeah I already had this, bit the change was folded into the wrong patch, 
fixed now

[...]
> Yours Tony
>
>   linux.conf.au    http://www.marchsouth.org/
>   Jan 19 - 24 2009 The Australian Linux Technical Conference!
>
>   

-- 

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization