Re: [PATCH v2 4/8] davinci: vpfe: add support for CCDC hardware for dm365

From: Sylwester Nawrocki <snjw23@gmail.com>
To: "Hadli, Manjunath" <manjunath.hadli@ti.com>
Cc: LMML <linux-media@vger.kernel.org>,
	dlos <davinci-linux-open-source@linux.davincidsp.com>,
	"Netagunte, Nagabhushana" <nagabhushana.netagunte@ti.com>
Subject: Re: [PATCH v2 4/8] davinci: vpfe: add support for CCDC hardware for dm365
Date: Fri, 09 Sep 2011 18:39:38 +0200	[thread overview]
Message-ID: <4E6A414A.8070207@gmail.com> (raw)
In-Reply-To: <B85A65D85D7EB246BE421B3FB0FBB593025743F3C9@dbde02.ent.ti.com>

Hi Manju,

On 09/09/2011 03:30 PM, Hadli, Manjunath wrote:
> Thank you for these comments too.
> 
> My responses inlined.
> -Manju

Thanks for addressing my comments.

> 
> On Fri, Sep 02, 2011 at 02:53:08, Sylwester Nawrocki wrote:
...
>>> +/**
>>> + * ccdc float type S8Q8/U8Q8
>>> + */
>>> +struct ccdc_float_8 {
>>> +	/* 8 bit integer part */
>>> +	unsigned char integer;
>>> +	/* 8 bit decimal part */
>>> +	unsigned char decimal;
>>> +};
>>
>> Isn't it better to use explicit width type, like u8, u16, etc. ?
>> Then we could just have:
>>
>> +struct ccdc_float_8 {
>> +	u8 integer;
>> +	u8 decimal;
>> +};
>>
> This is an interface header which is also used by apps. So we have
> kept it as unsigned char. Any suggestions on that?

OK, sorry, I haven't noticed that. Anyway, I think you could use the
double underscore prefixed types (__[u,s][8,32,16]), from linux/types.h.
They are widely used in in linux/videodev2.h for instance.

--
Regards,
Sylwester