Hi Albert

On Fri, 23 Nov 2012, Albert Wang wrote:

> From: Libin Yang <lby...@marvell.com>
> 
> This patch replaces the global frame stats variables by using
> internal variables in mcam_camera structure.
> 
> Signed-off-by: Albert Wang <twan...@marvell.com>
> Signed-off-by: Libin Yang <lby...@marvell.com>

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.

> +     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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to