Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver

Sakari Ailus
2020-03-10T10:05:39Z

Hi Dongchun,

On Tue, Mar 10, 2020 at 06:00:19PM +0800, Dongchun Zhu wrote:
> Hi Sakari, Rob, Andy, Tomasz,
>
> On Fri, 2020-02-28 at 23:59 +0800, Dongchun Zhu wrote:
...
> > +static int dw9768_init(struct dw9768 *dw9768)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > + int ret, val;
> > +
> > + /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
> > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > + DW9768_PD_MODE_OFF);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /*
> > + * DW9769 requires waiting delay time of t_OPR
> > + * after PD reset takes place.
> > + */
> > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > +
> > + ret = dw9768_write_array(dw9768, dw9768_init_regs,
> > + ARRAY_SIZE(dw9768_init_regs));
> > + if (ret)
> > + return ret;
> > +
> > + for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
> > + val <= dw9768->focus->val;
> > + val += DW9768_MOVE_STEPS) {
> > + ret = dw9768_set_dac(dw9768, val);
> > + if (ret) {
> > + dev_err(&client->dev, "%s I2C failure: %d",
> > + __func__, ret);
> > + return ret;
> > + }
> > + usleep_range(DW9768_MOVE_DELAY_US,
> > + DW9768_MOVE_DELAY_US + 1000);
> > + }
> > +
>
> What do you think about the approach taken by this patch?
> From the view of VCM hardware, the collision sound of lens should only
> happen when moving position back to zero.
> When opening camera, one should be able to move lens to the position
> directly.
> I tried to replace this code to a single dw9768_set_dac(dw9768,
> dw9768->focus->val),
> there is no collision sound when open camera and it could reduce several
> hundred ms when open lens driver fd.
> Are we okay with this?

I think so. Usually on VCMs with ringing compensation the only problematic
case is when the power is cut. :-)

--
Sakari Ailus