RE: [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hans,

Thanks for reviewing this.

>
>> +/* isif float type S8Q8/U8Q8 */
>> +struct isif_float_8 {
>> +	/* 8 bit integer part */
>> +	__u8 integer;
>> +	/* 8 bit decimal part */
>> +	__u8 decimal;
>
>Why __u8 instead of u8? This is a kernel-internal file, so it does not need
>to use the '__' variants.
>
This is a public header available for configuring the isif ip blocks such
as black clamp, defect correction, linearization etc. In future once
sub devices have its own node, application will direct the ioctls to
the sub device node. But as of now, it is going through video node.
I can think of splitting this into 2 files, isif.h that contains the
kernel part and isif-ioctl.h that will have structure and types for ioctls.
But for DM355 & DM644x ccdc, this is how it is done.

>If this needs to be publicly available, then it should be in include/linux.
Currently all of dm355 and dm644x header files for user space use are
under include/media/davinci. To me it makes sense since only davinci
specific applications will ever need to include them. I know you have
ivtv.h under linux/. So also several user space header files under
include/media and include/video. So not sure if there is a strict
rule that is followed. 

To a different note, Vaibhav had sent a patch for re-arranging the
driver files in a different folder to allow re-use of drivers
across DaVinci/OMAP devices. The proposal was to move all of TI
driver source files to

drivers/media/video/ti-media

and include files to
include/media/ti-media

What you think about this proposal? So at this point, shall we keep
it in it's current location?

>The comment before each shift define seems rather pointless.
>
>> +	/* Data shift applied before storing to SDRAM */
>> +	__u8 data_shift;
>> +	/* enable input test pattern generation */
>> +	__u8 test_pat_gen;
>> +};
>> +
>> +#ifdef __KERNEL__
>
>Hmm. Is this header supposed to be public or private to the kernel?
>

Actually it has both as is the case with dm355 & dm644x ccdc headers.
Do you want me to split the headers as suggested earlier?

Thanks

Murali
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux