"Nori, Sekhar" <[email protected]> writes:
> On Sat, Sep 05, 2009 at 01:16:43, Santiago Nunez-Corrales wrote:
>> Sekhar,
>>
>>
>> Thanks for your review. Comments inlined.
>>
>> Regards,
>>
>> Nori, Sekhar wrote:
>> > On Fri, Sep 04, 2009 at 04:22:03, [email protected] wrote:
>> >
>> >> From: Santiago Nunez-Corrales <[email protected]>
>> >>
>> >> This patch provides support for TVP7002 in architecture definitions
>> >> within DM365. Moved tvp7002 platform data here and cleaned up code.
>
> [...]
>
>> >> diff --git a/arch/arm/mach-davinci/board-dm365-evm.c
>> >> b/arch/arm/mach-davinci/board-dm365-evm.c
>
> [...]
>
>> >>
>> >>
>> >> static inline int have_imager(void)
>> >> @@ -53,14 +55,19 @@ static inline int have_imager(void)
>> >>
>> >> static inline int have_tvp7002(void)
>> >> {
>> >> - /* REVISIT when it's supported, trigger via Kconfig */
>> >> +#ifdef CONFIG_VIDEO_TVP7002
>> >> + return 1;
>> >> +#else
>> >> return 0;
>> >> +#endif
>> >> }
>> >>
>> >
>> > May be this can simply be:
>> >
>> > #ifdef CONFIG_VIDEO_TVP7002
>> > #define HAS_TVP7002 1
>> > #else
>> > #define HAS_TVP7002 0
>> > #endif
>> >
>> > However, you don't seem to use this in your
>> > patch set anyway.
>> >
>> >
>> [SN] It is used in this same file. The reason for coding this is to
>> follow the standard in the implementation.
>
> If using a function is must, then the implementation
> can be:
>
> #ifdef CONFIG_VIDEO_TVP7002
> static inline int have_tvp7002(void)
> {
> return 1;
> }
> #else
> static inline int have_tvp7002(void)
> {
> return 0;
> }
> #endif
>
Not sure that's any better.
What's the point of having a function that essentially returns the
value of a Kconfig varible?
What's being done is basically this:
static inline int have_tvp7002(void)
{
return CONFIG_VIDEO_TVP7002;
}
and I don't see the need for that.
Kevin
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
