RE: [PATCH 14/15] [media] marvell-ccic: use unsigned int type replace int type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Guennadi


>-----Original Message-----
>From: Guennadi Liakhovetski [mailto:g.liakhovetski@xxxxxx]
>Sent: Wednesday, 28 November, 2012 00:02
>To: Albert Wang
>Cc: corbet@xxxxxxx; linux-media@xxxxxxxxxxxxxxx; Libin Yang
>Subject: Re: [PATCH 14/15] [media] marvell-ccic: use unsigned int type replace int type
>
>On Fri, 23 Nov 2012, Albert Wang wrote:
>
>> This patch use unsigned int type replace int type in marvell-ccic.
>>
>> These variables: frame number, buf number, irq... should be unsigned.
>
>Several issues to be considered here:
>
>* most these variables will never take values > INT_MAX, so, this isn't
>  very important
>* sometimes it is convenient to use a variable, that normally should only
>  take positive values, to also check for negative values. These variables
>  should be signed.
>* compile-time compatibility: if variables are used as arguments of
>  functions or are compared or assigned to other variables, their types
>  should be compatible.
>* my "old" cross-compiler was hiding many such problems, I'm sure at
>  marvell you use new enough compilers to warn you about any such
>  issues:-)
>
>So, mainly, just make sure you get no compiler warnings, otherwise it's not very
>important, IMHO.
>

Yes, usually you are right.
For frame number and buf number, the one of prime reasons is it is prepared for the next patch:
add 3 frame buffer support, we must use unsigned type in some operations. :)

For irq, it's for compiler warning. :)


>Thanks
>Guennadi
>


Thanks
Albert Wang
86-21-61092656
>>
>> Signed-off-by: Albert Wang <twang13@xxxxxxxxxxx>
>> ---
>>  .../media/platform/marvell-ccic/mcam-core-soc.h    |    2 +-
>>  .../platform/marvell-ccic/mcam-core-standard.h     |   10 ++++-----
>>  drivers/media/platform/marvell-ccic/mcam-core.c    |   22 ++++++++++----------
>>  drivers/media/platform/marvell-ccic/mcam-core.h    |    2 +-
>>  drivers/media/platform/marvell-ccic/mmp-driver.c   |    2 +-
>>  5 files changed, 19 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/media/platform/marvell-ccic/mcam-core-soc.h
>> b/drivers/media/platform/marvell-ccic/mcam-core-soc.h
>> index a5b5fa6..bff8b2a 100644
>> --- a/drivers/media/platform/marvell-ccic/mcam-core-soc.h
>> +++ b/drivers/media/platform/marvell-ccic/mcam-core-soc.h
>> @@ -11,7 +11,7 @@ extern const struct vb2_ops mcam_soc_vb2_ops;
>>
>>  extern void mcam_ctlr_power_up(struct mcam_camera *cam);  extern void
>> mcam_ctlr_power_down(struct mcam_camera *cam); -extern void
>> mcam_dma_contig_done(struct mcam_camera *cam, int frame);
>> +extern void mcam_dma_contig_done(struct mcam_camera *cam, unsigned
>> +int frame);
>>  extern void mcam_ctlr_stop(struct mcam_camera *cam);  extern int
>> mcam_config_mipi(struct mcam_camera *mcam, int enable);  extern void
>> mcam_ctlr_image(struct mcam_camera *cam); diff --git
>> a/drivers/media/platform/marvell-ccic/mcam-core-standard.h
>> b/drivers/media/platform/marvell-ccic/mcam-core-standard.h
>> index 148a1a1..090c1a2 100644
>> --- a/drivers/media/platform/marvell-ccic/mcam-core-standard.h
>> +++ b/drivers/media/platform/marvell-ccic/mcam-core-standard.h
>> @@ -4,8 +4,8 @@
>>   * Copyright 2011 Jonathan Corbet corbet@xxxxxxx
>>   */
>>  extern bool alloc_bufs_at_read;
>> -extern int n_dma_bufs;
>> -extern int buffer_mode;
>> +extern unsigned int n_dma_bufs;
>> +extern unsigned int buffer_mode;
>>  extern const struct vb2_ops mcam_vb2_sg_ops;  extern const struct
>> vb2_ops mcam_vb2_ops;
>>
>> @@ -17,12 +17,12 @@ extern void mcam_ctlr_init(struct mcam_camera
>> *cam);  extern int mcam_cam_init(struct mcam_camera *cam);  extern
>> void mcam_free_dma_bufs(struct mcam_camera *cam);  extern void
>> mcam_ctlr_dma_sg(struct mcam_camera *cam); -extern void
>> mcam_dma_sg_done(struct mcam_camera *cam, int frame);
>> +extern void mcam_dma_sg_done(struct mcam_camera *cam, unsigned int
>> +frame);
>>  extern int mcam_check_dma_buffers(struct mcam_camera *cam);  extern
>> void mcam_set_config_needed(struct mcam_camera *cam, int needed);
>> extern int __mcam_cam_reset(struct mcam_camera *cam);  extern int
>> mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime);  extern
>> void mcam_ctlr_dma_contig(struct mcam_camera *cam); -extern void
>> mcam_dma_contig_done(struct mcam_camera *cam, int frame);
>> +extern void mcam_dma_contig_done(struct mcam_camera *cam, unsigned
>> +int frame);
>>  extern void mcam_ctlr_dma_vmalloc(struct mcam_camera *cam); -extern
>> void mcam_vmalloc_done(struct mcam_camera *cam, int frame);
>> +extern void mcam_vmalloc_done(struct mcam_camera *cam, unsigned int
>> +frame);
>> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
>> b/drivers/media/platform/marvell-ccic/mcam-core.c
>> index 3b05d8c..2d200d6 100755
>> --- a/drivers/media/platform/marvell-ccic/mcam-core.c
>> +++ b/drivers/media/platform/marvell-ccic/mcam-core.c
>> @@ -111,7 +111,7 @@ static inline struct mcam_vb_buffer
>> *vb_to_mvb(struct vb2_buffer *vb)
>>  /*
>>   * Hand a completed buffer back to user space.
>>   */
>> -static void mcam_buffer_done(struct mcam_camera *cam, int frame,
>> +static void mcam_buffer_done(struct mcam_camera *cam, unsigned int
>> +frame,
>>  		struct vb2_buffer *vbuf)
>>  {
>>  	vbuf->v4l2_buf.bytesused = cam->pix_format.sizeimage; @@ -125,7
>> +125,7 @@ static void mcam_buffer_done(struct mcam_camera *cam, int frame,
>>   */
>>  static void mcam_reset_buffers(struct mcam_camera *cam)  {
>> -	int i;
>> +	unsigned int i;
>>
>>  	cam->next_buf = -1;
>>  	for (i = 0; i < cam->nbufs; i++) {
>> @@ -216,7 +216,7 @@ int mcam_config_mipi(struct mcam_camera *mcam, int enable)
>>   */
>>  int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime)  {
>> -	int i;
>> +	unsigned int i;
>>
>>  	mcam_set_config_needed(cam, 1);
>>  	if (loadtime)
>> @@ -257,7 +257,7 @@ int mcam_alloc_dma_bufs(struct mcam_camera *cam,
>> int loadtime)
>>
>>  void mcam_free_dma_bufs(struct mcam_camera *cam)  {
>> -	int i;
>> +	unsigned int i;
>>
>>  	for (i = 0; i < cam->nbufs; i++) {
>>  		dma_free_coherent(cam->dev, cam->dma_buf_size, @@ -296,7 +296,7
>@@
>> void mcam_ctlr_dma_vmalloc(struct mcam_camera *cam)  static void
>> mcam_frame_tasklet(unsigned long data)  {
>>  	struct mcam_camera *cam = (struct mcam_camera *) data;
>> -	int i;
>> +	unsigned int i;
>>  	unsigned long flags;
>>  	struct mcam_vb_buffer *buf;
>>
>> @@ -344,7 +344,7 @@ int mcam_check_dma_buffers(struct mcam_camera *cam)
>>  	return 0;
>>  }
>>
>> -void mcam_vmalloc_done(struct mcam_camera *cam, int frame)
>> +void mcam_vmalloc_done(struct mcam_camera *cam, unsigned int frame)
>>  {
>>  	tasklet_schedule(&cam->s_tasklet);
>>  }
>> @@ -396,7 +396,7 @@ static bool mcam_fmt_is_planar(__u32 pfmt)
>>   * space.  In this way, we always have a buffer to DMA to and don't
>>   * have to try to play games stopping and restarting the controller.
>>   */
>> -static void mcam_set_contig_buffer(struct mcam_camera *cam, int
>> frame)
>> +static void mcam_set_contig_buffer(struct mcam_camera *cam, unsigned
>> +int frame)
>>  {
>>  	struct mcam_vb_buffer *buf;
>>  	struct v4l2_pix_format *fmt = &cam->pix_format; @@ -442,7 +442,7 @@
>> void mcam_ctlr_dma_contig(struct mcam_camera *cam)
>>  /*
>>   * Frame completion handling.
>>   */
>> -void mcam_dma_contig_done(struct mcam_camera *cam, int frame)
>> +void mcam_dma_contig_done(struct mcam_camera *cam, unsigned int
>> +frame)
>>  {
>>  	struct mcam_vb_buffer *buf = cam->vb_bufs[frame];
>>
>> @@ -518,7 +518,7 @@ void mcam_ctlr_dma_sg(struct mcam_camera *cam)
>>   * safely change the DMA descriptor array here and restart things
>>   * (assuming there's another buffer waiting to go).
>>   */
>> -void mcam_dma_sg_done(struct mcam_camera *cam, int frame)
>> +void mcam_dma_sg_done(struct mcam_camera *cam, unsigned int frame)
>>  {
>>  	struct mcam_vb_buffer *buf = cam->vb_bufs[0];
>>
>> @@ -935,7 +935,7 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq,
>>  		void *alloc_ctxs[])
>>  {
>>  	struct mcam_camera *cam = get_mcam(vq);
>> -	int minbufs = (cam->buffer_mode == B_DMA_contig) ? 3 : 2;
>> +	unsigned int minbufs = (cam->buffer_mode == B_DMA_contig) ? 3 : 2;
>>
>>  	sizes[0] = cam->pix_format.sizeimage;
>>  	*num_planes = 1; /* Someday we have to support planar formats... */
>> @@ -1207,7 +1207,7 @@ const struct vb2_ops mcam_soc_vb2_ops = {
>>  /*
>>   * Interrupt handler stuff
>>   */
>> -static void mcam_frame_complete(struct mcam_camera *cam, int frame)
>> +static void mcam_frame_complete(struct mcam_camera *cam, unsigned int
>> +frame)
>>  {
>>  	/*
>>  	 * Basic frame housekeeping.
>> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h
>> b/drivers/media/platform/marvell-ccic/mcam-core.h
>> index 999b581..5b2cf6e 100755
>> --- a/drivers/media/platform/marvell-ccic/mcam-core.h
>> +++ b/drivers/media/platform/marvell-ccic/mcam-core.h
>> @@ -197,7 +197,7 @@ struct mcam_camera {
>>
>>  	/* Mode-specific ops, set at open time */
>>  	void (*dma_setup)(struct mcam_camera *cam);
>> -	void (*frame_complete)(struct mcam_camera *cam, int frame);
>> +	void (*frame_complete)(struct mcam_camera *cam, unsigned int frame);
>>
>>  	/* Current operating parameters */
>>  	u32 sensor_type;		/* Currently ov7670 only */
>> diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c
>> b/drivers/media/platform/marvell-ccic/mmp-driver.c
>> index e840941..9b631b7 100755
>> --- a/drivers/media/platform/marvell-ccic/mmp-driver.c
>> +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
>> @@ -52,7 +52,7 @@ struct mmp_camera {
>>  	struct list_head devlist;
>>  	/* will change here */
>>  	struct clk *clk[3];	/* CCIC_GATE, CCIC_RST, CCIC_DBG clocks */
>> -	int irq;
>> +	unsigned int irq;
>>  };
>>
>>  static inline struct mmp_camera *mcam_to_cam(struct mcam_camera
>> *mcam)
>> --
>> 1.7.9.5
>>
>
>---
>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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux