lkml.org 
[lkml]   [2018]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v3 2/2] [media] imx214: Add imx214 camera sensor driver
HI Sakari

Thanks a lot for your review!

On Tue, Oct 2, 2018 at 6:27 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
> > +static int imx214_s_power(struct v4l2_subdev *sd, int on)
> > +{
> > + struct imx214 *imx214 = to_imx214(sd);
> > + int ret = 0;
> > +
> > + on = !!on;
> > +
> > + if (imx214->power_on == on)
> > + return 0;
> > +
> > + if (on)
> > + ret = imx214_set_power_on(imx214);
> > + else
> > + imx214_set_power_off(imx214);
> > +
> > + imx214->power_on = on;
> > +
> > + return 0;
>
> Using runtime PM would relieve you of this function.

Tried using runtime PM, but did not manage to get it working with the
qcom i2c driver. Will try again when this is merged on a separated
patch.
>
> > +static int imx214_find_mode(u32 height)
> > +{
> > + int i;
>
> Unsigned int.
>
> > +
> > + for (i = 0; (i < ARRAY_SIZE(imx214_modes) - 1) ; i++)
>
> Extra parentheses.
>
> How about using v4l2_find_nearest_size()?
>
> > +static int imx214_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct imx214 *imx214;
> > + int ret;
> > + static const s64 link_freq[] = {
> > + IMX214_DEFAULT_LINK_FREQ,
>
> This should come from firmware.

Thanks for clarifying this on the IRC. The link frequencies should
come from the firmware, but since we only support one and is probed it
is fine to get a static list.

> Kind regards,
>
> Sakari Ailus
> sakari.ailus@linux.intel.com

Thanks again

--
Ricardo Ribalda

\
 
 \ /
  Last update: 2018-10-03 09:18    [W:0.057 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site