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.
--
Regards,
Sakari Ailus
