Hi, Guennadi Nice to hear you again after holidays. :) >-----Original Message----- >From: Guennadi Liakhovetski [mailto:g.liakhovetski@xxxxxx] >Sent: Tuesday, 27 November, 2012 18:16 >To: Albert Wang >Cc: corbet@xxxxxxx; linux-media@xxxxxxxxxxxxxxx; Libin Yang >Subject: Re: [PATCH 01/15] [media] marvell-ccic: use internal variable replace global frame >stats variable > >Hi Albert > >On Fri, 23 Nov 2012, Albert Wang wrote: > >> From: Libin Yang <lbyang@xxxxxxxxxxx> >> >> This patch replaces the global frame stats variables by using internal >> variables in mcam_camera structure. >> >> Signed-off-by: Albert Wang <twang13@xxxxxxxxxxx> >> Signed-off-by: Libin Yang <lbyang@xxxxxxxxxxx> > >Thanks for doing this work! Looks good just one remark below. > >> --- >> drivers/media/platform/marvell-ccic/mcam-core.c | 30 ++++++++++------------- >> drivers/media/platform/marvell-ccic/mcam-core.h | 9 +++++++ >> 2 files changed, 22 insertions(+), 17 deletions(-) > >[snip] > >> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h >> b/drivers/media/platform/marvell-ccic/mcam-core.h >> index bd6acba..e226de4 100755 >> --- a/drivers/media/platform/marvell-ccic/mcam-core.h >> +++ b/drivers/media/platform/marvell-ccic/mcam-core.h >> @@ -73,6 +73,14 @@ static inline int mcam_buffer_mode_supported(enum >mcam_buffer_mode mode) >> } >> } >> >> +/* >> + * Basic frame states >> + */ >> +struct mmp_frame_state { > >I think this should be "struct mcam_frame_state" - don't think we need to introduce a whole >new namespace in this header just because of this struct. > Yes, you are right. We should keep same namespace in this header. Maybe we did a typo. >> + unsigned int frames; >> + unsigned int singles; >> + unsigned int delivered; >> +}; >> >> /* >> * A description of one of our devices. >> @@ -108,6 +116,7 @@ struct mcam_camera { >> unsigned long flags; /* Buffer status, mainly (dev_lock) */ >> int users; /* How many open FDs */ >> >> + struct mmp_frame_state frame_state; /* Frame state counter */ >> /* >> * Subsystem structures. >> */ >> -- >> 1.7.9.5 > >Thanks >Guennadi >--- >Guennadi Liakhovetski, Ph.D. >Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
