[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20161115121032.GB7018@amd>
Date: Tue, 15 Nov 2016 13:10:32 +0100
From: Pavel Machek <pavel@....cz>
To: Ramiro Oliveira <Ramiro.Oliveira@...opsys.com>
Cc: mchehab@...nel.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, robh+dt@...nel.org,
devicetree@...r.kernel.org, davem@...emloft.net,
gregkh@...uxfoundation.org, geert+renesas@...der.be,
akpm@...ux-foundation.org, linux@...ck-us.net, hverkuil@...all.nl,
dheitmueller@...nellabs.com, slongerbeam@...il.com,
lars@...afoo.de, robert.jarzmik@...e.fr, pali.rohar@...il.com,
sakari.ailus@...ux.intel.com, mark.rutland@....com,
CARLOS.PALMINHA@...opsys.com
Subject: Re: [PATCH v4 2/2] Add support for OV5647 sensor
Hi!
> Add support for OV5647 sensor.
>
> +static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
> +{
> + int ret;
> + unsigned char data[3] = { reg >> 8, reg & 0xff, val};
> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> + ret = i2c_master_send(client, data, 3);
> + if (ret != 3) {
> + dev_dbg(&client->dev, "%s: i2c write error, reg: %x\n",
> + __func__, reg);
> + return ret < 0 ? ret : -EIO;
> + }
> + return 0;
> +}
Sorry, this is wrong. It should something <0 any time error is detected.
> +static int ov5647_write_array(struct v4l2_subdev *sd,
> + struct regval_list *regs, int array_size)
> +{
> + int i = 0;
> + int ret = 0;
> +
> + if (!regs)
> + return -EINVAL;
> +
> + while (i < array_size) {
> + ret = ov5647_write(sd, regs->addr, regs->data);
> + if (ret < 0)
> + return ret;
> + i++;
> + regs++;
> + }
> + return 0;
> +}
For example this expects <0 on error.
> +static int set_sw_standby(struct v4l2_subdev *sd, bool standby)
> +{
> + int ret;
> + unsigned char rdval;
> +
> + ret = ov5647_read(sd, 0x0100, &rdval);
> + if (ret != 0)
> + return ret;
> +
> + if (standby)
> + ret = ov5647_write(sd, 0x0100, rdval&0xfe);
> + else
> + ret = ov5647_write(sd, 0x0100, rdval|0x01);
> +
> + return ret;
if (standby)
rdval &= 0xfe;
else
rdval |= 0x01;
ret = ov5647_write(sd, 0x0100, rdval);
?
> +/**
> + * @short Store information about the video data format.
> + */
> +static struct sensor_format_struct {
> + __u8 *desc;
> + u32 mbus_code;
u8 is suitable here.
> + ov5647_read(sd, 0x0100, &resetval);
> + if (!resetval&0x01) {
add ()s here.
> +static int sensor_power(struct v4l2_subdev *sd, int on)
> +{
> + int ret;
> + struct ov5647 *ov5647 = to_state(sd);
> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> + ret = 0;
> + mutex_lock(&ov5647->lock);
> +
> + if (on) {
> + dev_dbg(&client->dev, "OV5647 power on!\n");
> +
> + ret = ov5647_write_array(sd, sensor_oe_enable_regs,
> + ARRAY_SIZE(sensor_oe_enable_regs));
> +
> + ret = __sensor_init(sd);
> +
> + if (ret < 0)
> + dev_err(&client->dev,
> + "Camera not available! Check Power!\n");
> + } else {
> + dev_dbg(&client->dev, "OV5647 power off!\n");
> +
> + dev_dbg(&client->dev, "disable oe\n");
> + ret = ov5647_write_array(sd, sensor_oe_disable_regs,
> + ARRAY_SIZE(sensor_oe_disable_regs));
> +
> + if (ret < 0)
> + dev_dbg(&client->dev, "disable oe failed!\n");
> +
> + ret = set_sw_standby(sd, true);
> +
> + if (ret < 0)
> + dev_dbg(&client->dev, "soft stby failed!\n");
dev_err for errors? Little less "!"s in the output?
> +static int sensor_get_register(struct v4l2_subdev *sd,
> + struct v4l2_dbg_register *reg)
> +{
> + unsigned char val = 0;
> + int ret;
> +
> + ret = ov5647_read(sd, reg->reg & 0xff, &val);
> + reg->val = val;
> + reg->size = 1;
> + return ret;
> +}
Filling reg->* when read failed is strange.
> +static int sensor_set_register(struct v4l2_subdev *sd,
> + const struct v4l2_dbg_register *reg)
> +{
> + ov5647_write(sd, reg->reg & 0xff, reg->val & 0xff);
> + return 0;
> +}
error handling?
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)
Powered by blists - more mailing lists