* [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params @ 2012-08-09 6:33 Dror Cohen 2012-08-09 6:33 ` [PATCH 1/1 " Dror Cohen 2012-08-13 14:13 ` [PATCH 0/1 " Manjunath Hadli 0 siblings, 2 replies; 5+ messages in thread From: Dror Cohen @ 2012-08-09 6:33 UTC (permalink / raw) To: linux-media; +Cc: mchehab, nsekhar, davinci-linux-open-source, Dror Cohen 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1 v2] media/video: vpif: fixing function name start to vpif_config_params 2012-08-09 6:33 [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params Dror Cohen @ 2012-08-09 6:33 ` Dror Cohen 2012-08-13 14:13 ` [PATCH 0/1 " Manjunath Hadli 1 sibling, 0 replies; 5+ messages in thread From: Dror Cohen @ 2012-08-09 6:33 UTC (permalink / raw) To: linux-media; +Cc: mchehab, nsekhar, davinci-linux-open-source, Dror Cohen diff --git a/drivers/media/video/davinci/vpif.c b/drivers/media/video/davinci/vpif.c index af96802..04dd8fa 100644 --- a/drivers/media/video/davinci/vpif.c +++ b/drivers/media/video/davinci/vpif.c @@ -301,12 +301,12 @@ static void vpif_set_mode_info(const struct vpif_channel_config_params *config, regw(value, vpifregs[channel_id].v_cfg); } -/* config_vpif_params +/* vpif_config_params * Function to set the parameters of a channel * Mainly modifies the channel ciontrol register * It sets frame format, yc mux mode */ -static void config_vpif_params(struct vpif_params *vpifparams, +static void vpif_config_params(struct vpif_params *vpifparams, u8 channel_id, u8 found) { const struct vpif_channel_config_params *config = &vpifparams->std_info; @@ -374,7 +374,7 @@ int vpif_set_video_params(struct vpif_params *vpifparams, u8 channel_id) found = 2; } - config_vpif_params(vpifparams, channel_id, found); + vpif_config_params(vpifparams, channel_id, found); regw(0x80, VPIF_REQ_SIZE); regw(0x01, VPIF_EMULATION_CTRL); diff --git a/drivers/media/video/davinci/vpif_capture.c b/drivers/media/video/davinci/vpif_capture.c index 9604695..59104e6 100644 --- a/drivers/media/video/davinci/vpif_capture.c +++ b/drivers/media/video/davinci/vpif_capture.c @@ -67,7 +67,7 @@ MODULE_PARM_DESC(ch3_numbuffers, "Channel1 buffer count (default:3)"); MODULE_PARM_DESC(ch2_bufsize, "Channel0 buffer size (default:1920 x 1080 x 2)"); MODULE_PARM_DESC(ch3_bufsize, "Channel1 buffer size (default:720 x 576 x 2)"); -static struct vpif_config_params config_params = { +static struct config_vpif_params config_params = { .min_numbuffers = 3, .numbuffers[0] = 3, .numbuffers[1] = 3, diff --git a/drivers/media/video/davinci/vpif_capture.h b/drivers/media/video/davinci/vpif_capture.h index a693d4e..8863de1 100644 --- a/drivers/media/video/davinci/vpif_capture.h +++ b/drivers/media/video/davinci/vpif_capture.h @@ -144,7 +144,7 @@ struct vpif_device { struct v4l2_subdev **sd; }; -struct vpif_config_params { +struct config_vpif_params { u8 min_numbuffers; u8 numbuffers[VPIF_CAPTURE_NUM_CHANNELS]; s8 device_type; diff --git a/drivers/media/video/davinci/vpif_display.c b/drivers/media/video/davinci/vpif_display.c index e6488ee..652440d 100644 --- a/drivers/media/video/davinci/vpif_display.c +++ b/drivers/media/video/davinci/vpif_display.c @@ -70,7 +70,7 @@ MODULE_PARM_DESC(ch3_numbuffers, "Channel3 buffer count (default:3)"); MODULE_PARM_DESC(ch2_bufsize, "Channel2 buffer size (default:1920 x 1080 x 2)"); MODULE_PARM_DESC(ch3_bufsize, "Channel3 buffer size (default:720 x 576 x 2)"); -static struct vpif_config_params config_params = { +static struct config_vpif_params config_params = { .min_numbuffers = 3, .numbuffers[0] = 3, .numbuffers[1] = 3, diff --git a/drivers/media/video/davinci/vpif_display.h b/drivers/media/video/davinci/vpif_display.h index 56879d1..3e14807 100644 --- a/drivers/media/video/davinci/vpif_display.h +++ b/drivers/media/video/davinci/vpif_display.h @@ -154,7 +154,7 @@ struct vpif_device { }; -struct vpif_config_params { +struct config_vpif_params { u32 min_bufsize[VPIF_DISPLAY_NUM_CHANNELS]; u32 channel_bufsize[VPIF_DISPLAY_NUM_CHANNELS]; u8 numbuffers[VPIF_DISPLAY_NUM_CHANNELS]; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params 2012-08-09 6:33 [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params Dror Cohen 2012-08-09 6:33 ` [PATCH 1/1 " Dror Cohen @ 2012-08-13 14:13 ` Manjunath Hadli 2012-09-26 20:20 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 5+ messages in thread From: Manjunath Hadli @ 2012-08-13 14:13 UTC (permalink / raw) To: Dror Cohen, mchehab Cc: linux-media, davinci-linux-open-source, Manjunath Hadli 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.hadli@ti.com> Thx, --Manju ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params 2012-08-13 14:13 ` [PATCH 0/1 " Manjunath Hadli @ 2012-09-26 20:20 ` Mauro Carvalho Chehab 2012-09-27 5:16 ` Prabhakar Lad 0 siblings, 1 reply; 5+ messages in thread From: Mauro Carvalho Chehab @ 2012-09-26 20:20 UTC (permalink / raw) To: Manjunath Hadli; +Cc: Dror Cohen, linux-media, davinci-linux-open-source Em Mon, 13 Aug 2012 19:43:29 +0530 Manjunath Hadli <manjunath.hadli@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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params 2012-09-26 20:20 ` Mauro Carvalho Chehab @ 2012-09-27 5:16 ` Prabhakar Lad 0 siblings, 0 replies; 5+ messages in thread From: Prabhakar Lad @ 2012-09-27 5:16 UTC (permalink / raw) To: Dror Cohen Cc: Manjunath Hadli, linux-media, davinci-linux-open-source, Mauro Carvalho Chehab, Prabhakar Lad Hi Dror, On Thu, Sep 27, 2012 at 1:50 AM, Mauro Carvalho Chehab <mchehab@infradead.org> wrote: > Em Mon, 13 Aug 2012 19:43:29 +0530 > Manjunath Hadli <manjunath.hadli@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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-09-27 5:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-09 6:33 [PATCH 0/1 v2] media/video: vpif: fixing function name start to vpif_config_params Dror Cohen 2012-08-09 6:33 ` [PATCH 1/1 " Dror Cohen 2012-08-13 14:13 ` [PATCH 0/1 " Manjunath Hadli 2012-09-26 20:20 ` Mauro Carvalho Chehab 2012-09-27 5:16 ` Prabhakar Lad
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).