Re: [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params

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

 



Hi Dror,

On Thu, Sep 27, 2012 at 1:50 AM, Mauro Carvalho Chehab
<mchehab@xxxxxxxxxxxxx> wrote:
> Em Mon, 13 Aug 2012 19:43:29 +0530
> Manjunath Hadli <manjunath.hadli@xxxxxx> escreveu:
>
>> Hi Dror,
>>
>> Thanks for the patch.
>>
>> Mauro,
>>
>> I'll queue this patch for v3.7 through my tree.
>
> Sure.
>
>>
>> On Thursday 09 August 2012 12:03 PM, Dror Cohen wrote:
>> > This patch address the issue that a function named config_vpif_params should
>> > be vpif_config_params. However this name is shared with two structures defined
>> > already. So I changed the structures to config_vpif_params (origin was
>> > vpif_config_params)
>> >
>> > v2 changes: softer wording in description and the structs are now
>> > defined without _t
>
> Hmm... I didn't understand what you're wanting with this change. Before this patch,
> there are:
>
> v4l@pedra ~/v4l/patchwork $ git grep config_vpif_params
> drivers/media/platform/davinci/vpif.c:/* config_vpif_params
> drivers/media/platform/davinci/vpif.c:static void config_vpif_params(struct vpif_params *vpifparams,
> drivers/media/platform/davinci/vpif.c:    config_vpif_params(vpifparams, channel_id, found);
> v4l@pedra ~/v4l/patchwork $ git grep vpif_config_params
> drivers/media/platform/davinci/vpif_capture.c:static struct vpif_config_params config_params = {
> drivers/media/platform/davinci/vpif_capture.h:struct vpif_config_params {
> drivers/media/platform/davinci/vpif_display.c:static struct vpif_config_params config_params = {
> drivers/media/platform/davinci/vpif_display.h:struct vpif_config_params {
>
> After that, there are:
>
> v4l@pedra ~/v4l/patchwork $ git grep vpif_config_params
> drivers/media/platform/davinci/vpif.c:/* vpif_config_params
> drivers/media/platform/davinci/vpif.c:static void vpif_config_params(struct vpif_params *vpifparams,
> drivers/media/platform/davinci/vpif.c:    vpif_config_params(vpifparams, channel_id, found);
> v4l@pedra ~/v4l/patchwork $ git grep config_vpif_params
> drivers/media/platform/davinci/vpif_capture.c:static struct config_vpif_params config_params = {
> drivers/media/platform/davinci/vpif_capture.h:struct config_vpif_params {
> drivers/media/platform/davinci/vpif_display.c:static struct config_vpif_params config_params = {
> drivers/media/platform/davinci/vpif_display.h:struct config_vpif_params {
>
> So, I can't really see any improvement on avoiding duplicate names.
>
> IMHO, the better would be to name those functions as:
>
> vpif:           vpif_config_params (or, even better, vpif_core_config_params)
> vpif_capture:   vpif_capture_config_params
> vpif_display:   vpif_display_config_params
>
> This way, duplication will be avoided and will avoid the confusing inversion between
> vpif and config.
>
I agree with Mauro here, Can you do the above changes and post
a v2.( Rebase it on
http://git.linuxtv.org/media_tree.git/shortlog/refs/heads/staging/for_v3.7
branch)

Regards,
--Prabhakar Lad

> Regards,
> Mauro
> --
> 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
--
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