| Message ID | 20240416193319.778192-43-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Generic line based metadata support, internal pads | expand |
Hi Sakari, Thank you for the patch. On Tue, Apr 16, 2024 at 10:33:15PM +0300, Sakari Ailus wrote: > Add generic sensor property information as controĺs by using > v4l2_fwnode_device_parse() and v4l2_ctrl_new_fwnode_properties(). > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > drivers/media/i2c/ov2740.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c > index dc0931308053..e37d824291fe 100644 > --- a/drivers/media/i2c/ov2740.c > +++ b/drivers/media/i2c/ov2740.c > @@ -779,6 +779,8 @@ static const struct v4l2_ctrl_ops ov2740_ctrl_ops = { > > static int ov2740_init_controls(struct ov2740 *ov2740) > { > + struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd); > + struct v4l2_fwnode_device_properties props; > struct v4l2_ctrl_handler *ctrl_hdlr; > s64 exposure_max, h_blank, pixel_rate; > u32 vblank_min, vblank_max, vblank_default; > @@ -789,6 +791,10 @@ static int ov2740_init_controls(struct ov2740 *ov2740) > if (ret) > return ret; > > + if (!v4l2_fwnode_device_parse(&client->dev, &props)) If you moved the parsing earlier, you could set the right number of controls when initializing the handler. This being said, maybe we should instead try to get rid of the controls count hint to the handler initialization function. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ov2740_ctrl_ops, > + &props); > + > ov2740->link_freq = > v4l2_ctrl_new_int_menu(ctrl_hdlr, &ov2740_ctrl_ops, > V4L2_CID_LINK_FREQ,
Hi Laurent, On Sat, Apr 20, 2024 at 12:40:16PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Tue, Apr 16, 2024 at 10:33:15PM +0300, Sakari Ailus wrote: > > Add generic sensor property information as controĺs by using > > v4l2_fwnode_device_parse() and v4l2_ctrl_new_fwnode_properties(). > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > drivers/media/i2c/ov2740.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c > > index dc0931308053..e37d824291fe 100644 > > --- a/drivers/media/i2c/ov2740.c > > +++ b/drivers/media/i2c/ov2740.c > > @@ -779,6 +779,8 @@ static const struct v4l2_ctrl_ops ov2740_ctrl_ops = { > > > > static int ov2740_init_controls(struct ov2740 *ov2740) > > { > > + struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd); > > + struct v4l2_fwnode_device_properties props; > > struct v4l2_ctrl_handler *ctrl_hdlr; > > s64 exposure_max, h_blank, pixel_rate; > > u32 vblank_min, vblank_max, vblank_default; > > @@ -789,6 +791,10 @@ static int ov2740_init_controls(struct ov2740 *ov2740) > > if (ret) > > return ret; > > > > + if (!v4l2_fwnode_device_parse(&client->dev, &props)) > > If you moved the parsing earlier, you could set the right number of > controls when initializing the handler. This being said, maybe we should > instead try to get rid of the controls count hint to the handler > initialization function. I'm not quite sure how that's related. But I'll update the number. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks! > > > + v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ov2740_ctrl_ops, > > + &props); > > + > > ov2740->link_freq = > > v4l2_ctrl_new_int_menu(ctrl_hdlr, &ov2740_ctrl_ops, > > V4L2_CID_LINK_FREQ, >
On Tue, Apr 23, 2024 at 04:17:23PM +0000, Sakari Ailus wrote: > On Sat, Apr 20, 2024 at 12:40:16PM +0300, Laurent Pinchart wrote: > > On Tue, Apr 16, 2024 at 10:33:15PM +0300, Sakari Ailus wrote: > > > Add generic sensor property information as controĺs by using > > > v4l2_fwnode_device_parse() and v4l2_ctrl_new_fwnode_properties(). > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > --- > > > drivers/media/i2c/ov2740.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c > > > index dc0931308053..e37d824291fe 100644 > > > --- a/drivers/media/i2c/ov2740.c > > > +++ b/drivers/media/i2c/ov2740.c > > > @@ -779,6 +779,8 @@ static const struct v4l2_ctrl_ops ov2740_ctrl_ops = { > > > > > > static int ov2740_init_controls(struct ov2740 *ov2740) > > > { > > > + struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd); > > > + struct v4l2_fwnode_device_properties props; > > > struct v4l2_ctrl_handler *ctrl_hdlr; > > > s64 exposure_max, h_blank, pixel_rate; > > > u32 vblank_min, vblank_max, vblank_default; > > > @@ -789,6 +791,10 @@ static int ov2740_init_controls(struct ov2740 *ov2740) > > > if (ret) > > > return ret; > > > > > > + if (!v4l2_fwnode_device_parse(&client->dev, &props)) > > > > If you moved the parsing earlier, you could set the right number of > > controls when initializing the handler. This being said, maybe we should > > instead try to get rid of the controls count hint to the handler > > initialization function. > > I'm not quite sure how that's related. The move is related because you need to know if v4l2_fwnode_device_parse() succeeded to know how many controls v4l2_ctrl_new_fwnode_properties() will add. > But I'll update the number. > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Thanks! > > > > + v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ov2740_ctrl_ops, > > > + &props); > > > + > > > ov2740->link_freq = > > > v4l2_ctrl_new_int_menu(ctrl_hdlr, &ov2740_ctrl_ops, > > > V4L2_CID_LINK_FREQ,
diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c index dc0931308053..e37d824291fe 100644 --- a/drivers/media/i2c/ov2740.c +++ b/drivers/media/i2c/ov2740.c @@ -779,6 +779,8 @@ static const struct v4l2_ctrl_ops ov2740_ctrl_ops = { static int ov2740_init_controls(struct ov2740 *ov2740) { + struct i2c_client *client = v4l2_get_subdevdata(&ov2740->sd); + struct v4l2_fwnode_device_properties props; struct v4l2_ctrl_handler *ctrl_hdlr; s64 exposure_max, h_blank, pixel_rate; u32 vblank_min, vblank_max, vblank_default; @@ -789,6 +791,10 @@ static int ov2740_init_controls(struct ov2740 *ov2740) if (ret) return ret; + if (!v4l2_fwnode_device_parse(&client->dev, &props)) + v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ov2740_ctrl_ops, + &props); + ov2740->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr, &ov2740_ctrl_ops, V4L2_CID_LINK_FREQ,
Add generic sensor property information as controĺs by using v4l2_fwnode_device_parse() and v4l2_ctrl_new_fwnode_properties(). Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/media/i2c/ov2740.c | 6 ++++++ 1 file changed, 6 insertions(+)