Re: [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params
Em Mon, 13 Aug 2012 19:43:29 +0530 Manjunath Hadli manjunath.ha...@ti.com 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. Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params
Hi Dror, On Thu, Sep 27, 2012 at 1:50 AM, Mauro Carvalho Chehab mche...@infradead.org wrote: Em Mon, 13 Aug 2012 19:43:29 +0530 Manjunath Hadli manjunath.ha...@ti.com 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 majord...@vger.kernel.org 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params
Hi Dror, Thanks for the patch. Mauro, I'll queue this patch for v3.7 through my tree. 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 Dror Cohen (1): fixing function name start to vpif_config_params drivers/media/video/davinci/vpif.c |6 +++--- drivers/media/video/davinci/vpif_capture.c |2 +- drivers/media/video/davinci/vpif_capture.h |2 +- drivers/media/video/davinci/vpif_display.c |2 +- drivers/media/video/davinci/vpif_display.h |2 +- 5 files changed, 7 insertions(+), 7 deletions(-) Acked-by: Manjunath Hadli manjunath.ha...@ti.com Thx, --Manju -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params
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 Dror Cohen (1): fixing function name start to vpif_config_params drivers/media/video/davinci/vpif.c |6 +++--- drivers/media/video/davinci/vpif_capture.c |2 +- drivers/media/video/davinci/vpif_capture.h |2 +- drivers/media/video/davinci/vpif_display.c |2 +- drivers/media/video/davinci/vpif_display.h |2 +- 5 files changed, 7 insertions(+), 7 deletions(-) -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html