Re: [PATCH 1/4] soc-camera: tw9910: Add revision control

Guennadi Liakhovetski <[email protected]>
Newsgroups gmane.comp.video.video4linux
Message-ID <[email protected]>
On Mon, 2 Nov 2009, Kuninori Morimoto wrote:

> 
> Signed-off-by: Kuninori Morimoto <[email protected]>
> ---
>  drivers/media/video/tw9910.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/tw9910.c b/drivers/media/video/tw9910.c
> index 09ea042..a0b5bbe 100644
> --- a/drivers/media/video/tw9910.c
> +++ b/drivers/media/video/tw9910.c
> @@ -225,6 +225,7 @@ struct tw9910_priv {
>  	struct v4l2_subdev                subdev;
>  	struct tw9910_video_info       *info;
>  	const struct tw9910_scale_ctrl *scale;
> +	int rev;

Not critical, but I think we can afford a complete "revision" here.

>  };
>  
>  /*
> @@ -570,8 +571,11 @@ static int tw9910_enum_input(struct soc_camera_device *icd,
>  static int tw9910_g_chip_ident(struct v4l2_subdev *sd,
>  			       struct v4l2_dbg_chip_ident *id)
>  {
> +	struct i2c_client *client = sd->priv;
> +	struct tw9910_priv *priv = to_tw9910(client);
> +
>  	id->ident = V4L2_IDENT_TW9910;
> -	id->revision = 0;
> +	id->revision = priv->rev;
>  
>  	return 0;
>  }
> @@ -886,6 +890,8 @@ static int tw9910_video_probe(struct soc_camera_device *icd,
>  		return -ENODEV;
>  	}
>  
> +	priv->rev = GET_ReV(val);
> +

If you assign this directly after calling i2c_smbus_read_byte_data() you 
would save yourself a couple of "&" operations behind the GET_ReV() macro 
(btw, why such strange capitalisation? why not just GET_REV()?). Don't 
worry about the error case, priv will get freed then.

>  	dev_info(&client->dev,
>  		 "tw9910 Product ID %0x:%0x\n", GET_ID(val), GET_ReV(val));

you would also replace it here.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.