All, Please see my response inline.
> >>>> > >>>> [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 > >>>> > > >>>> > [MK] dm646x has vpif (video peripheral/port interface). So my > >>>> suggestion > >>>> > is to prefix the bridge driver file by the IP name. For example, > >>>> DM6467 > >>>> > has vpif IP for capture and display. So I would go for > vpif_capture.c > >>>> for > >>>> > bridge driver and vpif_display.c for display driver. However for > HW > >>>> > modules responsible for configuring video port parameters (vpif > for > >>>> > DM6467 and ccdc for DM355/6446), I would suggest adding dmxxx > prefix > >>>> > along with IP name since the hw may differ from one dmxxx family > to > >>>> > another (in terms of features). In future, if TI come up with > another > >>>> > DMxxx SoC similar to DM6467 that too uses vpif, then we don't > have to > >>>> > change the names again to re-use the bridge driver. So I propose > the > >>>> > following for DM6467 (I expect Chaithrika to comment on this). > >>>> > > >>>> > vpif_capture.[ch] -> DM6467 Capture bridge driver > >>>> > vpif_display.[ch] -> DM6467 Display bridge driver > >>>> > dm6467_vpif.[ch] -> DM6467 capture & display hw module > >>>> > > >>>> > For future DM6467 variants, we may able to use the same bridge > driver > >>>> as > >>>> > was done in DM355/DM6446. I am not sure if the DM6467 capture > driver > >>>> > requires change to re-use across another DM6467 variant. I will > let > >>>> > Chaithrika comment on this. > >>>> > > >>>> > >>>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? > >>>> > > >>>> > [MK] See my explanation above. Let us also hear from Chaithrika > on > >>>> this. > >>>> > >>>> OK. Using vpif_ and vpfe_ prefixes is OK by me if Chaithrika > agrees. As > >>>> long > >>>> as the naming is consistent for both dm355/dm6446 and dm646x. > >>> > >>>Currently the Video Port Interface driver is common to both display > and > >>>capture, therefore it is named as 'vpif.c'. I do not see a need to > create > >>>separate files for display and capture as mentioned above. > [MK] No. If you see my above suggestion, I was asking to re-name vpif.c > to > dm6467_vpif.[ch] (reproduced below). May be better dm646x_vpif.c > > >>>> > dm6467_vpif.[ch] -> DM6467 capture & display hw module > > I think the risk of going with this generic name is that if we got a > SoC in the future that has more hw registers than DM6467 ( Example CCDC > in DM355 vs that in DM6446) and if we want to develop a new hw module, > then we might end up with vpif.c for DM6467 and dm646<TBD>_vpif.c for > the future SoC. Ultimately the idea is to avoid confusion as pointed to > by Hans. So please take a decision accordingly. My preference for HW > module would be dm646x_vpif.c. > If a SoC in future uses similar VPIF, then the differences between the existing and the new VPIF can be handled in the existing driver. A version information can be included in the platform data to differentiate the VPIFs on different SoCs. Therefore it is not necessary to rename the file. > >>>the, > >>>VPIF bridge driver can retain the existing name. Also, TI is > planning to > >>>have VPIF in an upcoming SoC and this will be similar to the one on > >>>DM646x. > [MK] I think bridge driver can work across multiple SoCs as long as the > IP is similar (2 channels in your case). My suggestion was to name the > bridge driver for capture and display with a vpif prefix to indicate > they can work with any future SoC that has vpif. > > >>>> > vpif_capture.[ch] -> DM6467 Capture bridge driver > >>>> > vpif_display.[ch] -> DM6467 Display bridge driver > > If you name them with SoC prefix rather than vpif prefix you are making > an assumption that all future SoCs that uses vpif will be named in the > dm646x format. I think this bridge drivers can work across multiple > SoCs that has similar architecture, but variants of vpif and adding > dm646x prefix makes it vulnerable for a name change in future if this > needs to be re-used on a future SoC that doesn't follow DM646x format. > > >>>If there is anything specific to the hardware that can go into the > >>>dm646x_vpif.[ch]. > [MK] I think we shouldn't put the hardware specific changes in the > bridge driver, but must go into the hw module (in your case it should > be in vpif.c) > >>>At this point of time though, such a file may not be needed. > >>> > >>>The files that will be added by DM646x display driver will be > >>>1] drivers/media/video/davinci/dm646x_display.c - Is it better to > name > >>>this file > >>> 'dm646x_evm_display.c' since this is specific to the TI DM646x > EVM? > >>>2] drivers/media/video/davinci/vpif.c - no change > >>>3] include/media/davinci/dm646x_display.h - Should this be > >>>'dm646x_evm_display.h'? > >>>4] include/media/davinci/vpif.h - no change > >>> > [MK] Please re-look into the names based on my comments above. As long > as the names chosen doesn't results in confusion I am okay with the > names, but in it's present form, they can result in confusion in the > future. > Based on the discussions, the file names for DM646x display will be as follows 1] drivers/media/video/davinci/vpif_display.c - earlier 'dm646x_display.c' 2] drivers/media/video/davinci/vpif.c - no change 3] include/media/davinci/vpif_display.h - earlier 'dm646x_display.h' 4] include/media/davinci/vpif.h - no change Thanks, Chaithrika_______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
