RE: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-25 Thread Mani, Rajmohan
> > > + if (ret) > > > + dev_err(dev, "%s I2C failure: %d", __func__, > > > + ret); > > > > I think we should just return an error code here and fail the suspend. > > The result from an error here is that the user would hear an audible click. > I don't think it'

RE: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-25 Thread Mani, Rajmohan
Hi Sakari, Sylwester, > > > > You're right, sorry. I'd expect such things to be better covered in > > the API documentation. Probably pm_runtime_put_noidle() is a better > > Well, the documentation tells what the function does. It'd be good if it > pointed > that the usage count needs to be dec

RE: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-25 Thread Mani, Rajmohan
Hi Sakari, > -Original Message- > From: Sakari Ailus [mailto:sakari.ai...@iki.fi] > Sent: Thursday, May 11, 2017 12:41 AM > To: Mani, Rajmohan > Cc: linux-media@vger.kernel.org; mche...@kernel.org; hverk...@xs4all.nl; > tf...@chromium.org > Subject: Re: [PATCH v4] d

RE: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-25 Thread Mani, Rajmohan
> Cc: linux-media@vger.kernel.org; mche...@kernel.org; Hans Verkuil > ; Sakari Ailus > Subject: Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM > > Hi Raj, > > Thanks for re-spin. Still a bit more comments inline. (I missed few more > before, > sorry.) > > O

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-25 Thread Tomasz Figa
Hi Raj, On Thu, May 11, 2017 at 3:30 PM, Tomasz Figa wrote: > Hi Raj, > > Thanks for re-spin. Still a bit more comments inline. (I missed few > more before, sorry.) > > On Thu, May 11, 2017 at 1:00 PM, Rajmohan Mani > wrote: >> DW9714 is a 10 bit DAC, designed for linear >> control of voice coi

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-12 Thread Sakari Ailus
Hi Sylwester, On Fri, May 12, 2017 at 10:57:39AM +0200, Sylwester Nawrocki wrote: > On 05/11/2017 04:59 PM, Sakari Ailus wrote: > >>On 05/11/2017 08:30 AM, Tomasz Figa wrote: > [...] > >>>rval = pm_runtime_get_sync(dev); > >>>if (rval < 0) { > >>>pm_runtime_put(dev); > >>>r

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-12 Thread Sylwester Nawrocki
On 05/11/2017 04:59 PM, Sakari Ailus wrote: On 05/11/2017 08:30 AM, Tomasz Figa wrote: [...] rval = pm_runtime_get_sync(dev); if (rval < 0) { pm_runtime_put(dev); return rval; } Aren't we supposed to call pm_runtime_put() only when corresponding pm_runtime_get() suc

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-11 Thread Sakari Ailus
On Thu, May 11, 2017 at 04:39:41PM +0200, Sylwester Nawrocki wrote: > Hi, > > On 05/11/2017 08:30 AM, Tomasz Figa wrote: > >> +static int dw9714_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) > >> +{ > >> + struct dw9714_device *dw9714_dev = container_of(sd, > >> +

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-11 Thread Sylwester Nawrocki
Hi, On 05/11/2017 08:30 AM, Tomasz Figa wrote: >> +static int dw9714_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) >> +{ >> + struct dw9714_device *dw9714_dev = container_of(sd, >> + struct dw9714_device, >> +

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-11 Thread Sakari Ailus
Hi Tomasz, On Thu, May 11, 2017 at 04:37:08PM +0800, Tomasz Figa wrote: > >> >> > +/* > >> >> > + * This function sets the vcm position to the value set by the user > >> >> > + * through v4l2_ctrl_ops s_ctrl handler > >> >> > + * The lens position is gradually moved in units of > >> >> > DW9714_C

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-11 Thread Tomasz Figa
On Thu, May 11, 2017 at 4:24 PM, Sakari Ailus wrote: > Hi Tomasz, > > On Thu, May 11, 2017 at 04:02:35PM +0800, Tomasz Figa wrote: >> Hi Sakari, >> >> On Thu, May 11, 2017 at 3:55 PM, Sakari Ailus wrote: >> > Hi Tomasz, >> > >> > On Thu, May 11, 2017 at 02:30:31PM +0800, Tomasz Figa wrote: >> > .

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-11 Thread Sakari Ailus
Hi Tomasz, On Thu, May 11, 2017 at 04:02:35PM +0800, Tomasz Figa wrote: > Hi Sakari, > > On Thu, May 11, 2017 at 3:55 PM, Sakari Ailus wrote: > > Hi Tomasz, > > > > On Thu, May 11, 2017 at 02:30:31PM +0800, Tomasz Figa wrote: > > ... > >> > + > >> > +/* > >> > + * This function sets the vcm posi

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-11 Thread Tomasz Figa
Hi Sakari, On Thu, May 11, 2017 at 3:55 PM, Sakari Ailus wrote: > Hi Tomasz, > > On Thu, May 11, 2017 at 02:30:31PM +0800, Tomasz Figa wrote: > ... >> > + >> > +/* >> > + * This function sets the vcm position, so it consumes least current >> > + * The lens position is gradually moved in units of

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-11 Thread Sakari Ailus
Hi Tomasz, On Thu, May 11, 2017 at 02:30:31PM +0800, Tomasz Figa wrote: ... > > + > > +/* > > + * This function sets the vcm position, so it consumes least current > > + * The lens position is gradually moved in units of DW9714_CTRL_STEPS, > > + * to make the movements smoothly. > > + */ > > +stat

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-11 Thread Sakari Ailus
Hi Rajmohan, A few minor comments below. On Wed, May 10, 2017 at 10:00:20PM -0700, Rajmohan Mani wrote: > DW9714 is a 10 bit DAC, designed for linear > control of voice coil motor. > > This driver creates a V4L2 subdevice and > provides control to set the desired focus. > > Signed-off-by: Rajmo

Re: [PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-10 Thread Tomasz Figa
Hi Raj, Thanks for re-spin. Still a bit more comments inline. (I missed few more before, sorry.) On Thu, May 11, 2017 at 1:00 PM, Rajmohan Mani wrote: > DW9714 is a 10 bit DAC, designed for linear > control of voice coil motor. [snip] > +static int dw9714_i2c_write(struct i2c_client *client, u16

[PATCH v4] dw9714: Initial driver for dw9714 VCM

2017-05-10 Thread Rajmohan Mani
DW9714 is a 10 bit DAC, designed for linear control of voice coil motor. This driver creates a V4L2 subdevice and provides control to set the desired focus. Signed-off-by: Rajmohan Mani --- Changes in v4: - Addressed review comments from Tomasz Changes in v3: - Addressed most of