On Monday 16 March 2009 17:55:29 Karicheri, Muralidharan wrote:
> Hans,
>
> Thanks for taking some time to preview the patches and giving valuable
> comments.
>
> I would like to know if you agree with the following :-
>
> tvp5146 is used on DM6446, DM355 and DM365 platforms and our drivers
> needs to work with the driver tvp514x available in open source. However
> tvp514x driver is currently using v4l2-int-device frame work. So I have
> decided to use this frame work for the initial version of the driver
> until the tvp514x driver migrates to sub device framework. Migration of
> our driver to sub device frame work has to happen in sync with the
> migration plan for tvp514x. Please confirm if you agree with this
> approach.
Yes, that's OK. What timeframe for the subdev conversion did you have in
mind? Personally I would like to see v4l2-int-device removed for kernel
2.6.31.
> Other responses are inline.
>
> >>>I have one comment that also refers to the new DM646x driver that was
> >>>send to
> >>>the list for review, which is why I CC-ed this to Chaithrika as well:
> >>>
> >>>The DM335/DM6446 adds the following files:
> >>>
> >>> create mode 100644 drivers/media/video/davinci/ccdc_davinci.c
> >>> create mode 100644 drivers/media/video/davinci/ccdc_davinci.h
> >>> create mode 100644 drivers/media/video/davinci/vpfe_capture.c
> >>> create mode 100644 drivers/media/video/davinci/ccdc_dm355.c
> >>> create mode 100644 drivers/media/video/davinci/ccdc_dm355.h
> >>> create mode 100644 include/media/davinci/vpfe_capture.h
> >>> create mode 100644 include/media/davinci/vpfe_types.h
> >>> create mode 100644 include/media/davinci/ccdc_common.h
> >>> create mode 100644 include/media/davinci/ccdc_hw_device.h
>
> [MK] VPFE capture is a bridge driver that is used on DM355, DM6446 and
> DM365. Since all these DM SoCs have common VPFE (Video Processing front
> end) that is used for capturing video, I have named the capture driver as
> vpfe_capture.c. vpfe_types.h has common vpfe types used across all DMxxx
> family.
But the dm646x also has a vpfe, but that's different than these, right? So
that would have headers like dm646x_capture.h etc.
Given this I am inclined to rather go for dmxxx_foo.c/h. Not ideal, but
that's because you messed up your numbering scheme :-) :-)
What do you think?
> I agree with you on renaming following files as suggested. Also
> I will add a header description for the terms used (example ccdc, vpfe
> etc.)
>
> ccdc_davinci.c ->dm6446_ccdc.c
> ccdc_davinci.h ->dm6446_ccdc.h + dm6446_ccdc_regs.h
> vpfe_capture.c -> no change as explained above
> ccdc_dm355.c -> dm355_ccdc.c
> ccdc_dm355.h -> dm355_ccdc.h + dm355_ccdc_regs.h
> vpfe_capture.h -> no change
> vpfe_types.h -> no change
> ccdc_common.h -> ccdc_types.h (all common types used across DMxxx CCDC)
> ccdc_hw_device.h -> header file for hw APIs to configure the CCDC. Since
> same bridge driver is used across multiple DMxxx all functions used for
> configuring the CCDC are abstracted into these header file and
> implemented by the respective ccdc_dmxxx.c.
Shouldn't it be dm3x5? Or am I now too pedantic... Anyway, this is already a
lot better.
> >>>Unless you think it is really important I am not going to review these
> >>>drivers. I suggest that you watch my review of the dm646x and fix any
> >>>issues that will also apply to this driver before it comes up for
> >>> review on the linux-media list.
>
> [MK] As I have mentioned, my driver is using a different framework
> (v4l2-int-device) than DM646x and I will try to address any comments that
> are applicable in that context. I will leave it to you if you want review
> the driver now or later when I submit the same to v4l2 mailing list.
While I had some comments regarding the v4l2_device/v4l2_subdev framework,
most comments were about other things that might well apply to this driver
as well.
I do not have the time to look at this driver as well, so I'll postpone my
review until you submit it to the linux-media list.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
