Hi, Guennadi

Nice to hear you again after holidays. :)

>-----Original Message-----
>From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de]
>Sent: Tuesday, 27 November, 2012 18:16
>To: Albert Wang
>Cc: cor...@lwn.net; linux-media@vger.kernel.org; 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 <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.
>
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to