From: Daniel Scally <djrscally@gmail.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: linux-media@vger.kernel.org, laurent.pinchart@ideasonboard.com,
yong.zhi@intel.com, bingbu.cao@intel.com, tian.shu.qiu@intel.com,
kieran.bingham@ideasonboard.com,
jeanmichel.hautbois@ideasonboard.com
Subject: Re: [PATCH v3 2/2] media: i2c: Add support for ov5693 sensor
Date: Wed, 22 Sep 2021 13:26:06 +0100 [thread overview]
Message-ID: <f19d6824-07e7-1647-db81-5c98d25308d1@gmail.com> (raw)
In-Reply-To: <YUnUtdXpYdoIyb4s@paasikivi.fi.intel.com>
Hi Sakari
On 21/09/2021 13:48, Sakari Ailus wrote:
> Hi Daniel,
>
> On Tue, Sep 21, 2021 at 12:47:56PM +0100, Daniel Scally wrote:
>>> +
>>> +static int ov5693_get_exposure(struct ov5693_device *ov5693, s32 *value)
>>> +{
>>> + u8 exposure_hh = 0, exposure_h = 0, exposure_l = 0;
>>> + int ret;
>>> +
>>> + ret = ov5693_read_reg(ov5693, OV5693_EXPOSURE_L_CTRL_HH_REG, &exposure_hh);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = ov5693_read_reg(ov5693, OV5693_EXPOSURE_L_CTRL_H_REG, &exposure_h);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = ov5693_read_reg(ov5693, OV5693_EXPOSURE_L_CTRL_L_REG, &exposure_l);
>>> + if (ret)
>>> + return ret;
>>> Does the sensor not allow reading this register as a single operation?
>>>
>>> Just a question. Some sensors from the vendor do not. Same for the writes.
>>
>> It does; if I'm honest I just preferred the individual read/writes. I
>> find it's easier to see exactly what's going on. Happy to change it if
>> you prefer though - it's less important now that the work is mostly done.
> It's certainly not wrong to do that but it takes a longer time. So you're
> much, much more likely to miss the frame you intended the settings to take
> effect. Also note the device could have a specific order in which to write
> them for the update to be atomic. Missing this could cause wildly
> misexposed frames. I don't know if this one does.
>
The datasheet doesn't mention a specific ordering, but I take your point
about the timings. I'll look at switching it to a single write operation.
prev parent reply other threads:[~2021-09-22 12:26 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-20 22:54 [PATCH v3 0/2] Add support for OV5693 Sensor Daniel Scally
2021-09-20 22:54 ` [PATCH v3 1/2] media: ipu3-cio2: Toggle sensor streaming in pm runtime ops Daniel Scally
2021-09-20 22:54 ` [PATCH v3 2/2] media: i2c: Add support for ov5693 sensor Daniel Scally
2021-09-21 9:06 ` Daniel Scally
2021-09-21 9:11 ` Sakari Ailus
2021-09-21 11:47 ` Daniel Scally
2021-09-21 12:48 ` Sakari Ailus
2021-09-22 12:26 ` Daniel Scally [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f19d6824-07e7-1647-db81-5c98d25308d1@gmail.com \
--to=djrscally@gmail.com \
--cc=bingbu.cao@intel.com \
--cc=jeanmichel.hautbois@ideasonboard.com \
--cc=kieran.bingham@ideasonboard.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=tian.shu.qiu@intel.com \
--cc=yong.zhi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.