| Message ID | 20190814202815.32491-5-jacopo@jmondi.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: v4l2-ctrls: Add camera 'location' support | expand |
Hi Jacopo, Thank you for the patch. On Wed, Aug 14, 2019 at 10:28:14PM +0200, Jacopo Mondi wrote: > The camera location is retrieved from the firmware interface parsing > the "location" device property and reported through the read-only > V4L2_CID_LOCATION control. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > drivers/media/i2c/ov5670.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c > index 041fcbb4eebd..1446aef1fc1e 100644 > --- a/drivers/media/i2c/ov5670.c > +++ b/drivers/media/i2c/ov5670.c > @@ -2059,10 +2059,12 @@ static const struct v4l2_ctrl_ops ov5670_ctrl_ops = { > /* Initialize control handlers */ > static int ov5670_init_controls(struct ov5670 *ov5670) > { > + struct i2c_client *client = v4l2_get_subdevdata(&ov5670->sd); > struct v4l2_ctrl_handler *ctrl_hdlr; > s64 vblank_max; > s64 vblank_def; > s64 vblank_min; > + u32 location; > s64 exposure_max; > int ret; > > @@ -2124,6 +2126,15 @@ static int ov5670_init_controls(struct ov5670 *ov5670) > ARRAY_SIZE(ov5670_test_pattern_menu) - 1, > 0, 0, ov5670_test_pattern_menu); > > + ret = device_property_read_u32(&client->dev, "location", &location); > + if (!ret) { > + v4l2_ctrl_new_std(ctrl_hdlr, &ov5670_ctrl_ops, > + V4L2_CID_LOCATION, V4L2_LOCATION_FRONT, > + V4L2_LOCATION_BACK, 1, > + location == V4L2_LOCATION_FRONT ? > + V4L2_LOCATION_FRONT : V4L2_LOCATION_BACK); > + } > + As most camera sensors should do this, I think it would make sense to create a helper function. We could add parsing of other standard sensor-related controls there (such as the link frequencies), and also create the pixel rate control. > if (ctrl_hdlr->error) { > ret = ctrl_hdlr->error; > goto error;
Hi Jacopo, On Wed, Aug 14, 2019 at 10:28:14PM +0200, Jacopo Mondi wrote: > The camera location is retrieved from the firmware interface parsing > the "location" device property and reported through the read-only > V4L2_CID_LOCATION control. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > drivers/media/i2c/ov5670.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c > index 041fcbb4eebd..1446aef1fc1e 100644 > --- a/drivers/media/i2c/ov5670.c > +++ b/drivers/media/i2c/ov5670.c > @@ -2059,10 +2059,12 @@ static const struct v4l2_ctrl_ops ov5670_ctrl_ops = { > /* Initialize control handlers */ > static int ov5670_init_controls(struct ov5670 *ov5670) > { > + struct i2c_client *client = v4l2_get_subdevdata(&ov5670->sd); > struct v4l2_ctrl_handler *ctrl_hdlr; > s64 vblank_max; > s64 vblank_def; > s64 vblank_min; > + u32 location; > s64 exposure_max; > int ret; > > @@ -2124,6 +2126,15 @@ static int ov5670_init_controls(struct ov5670 *ov5670) > ARRAY_SIZE(ov5670_test_pattern_menu) - 1, > 0, 0, ov5670_test_pattern_menu); > > + ret = device_property_read_u32(&client->dev, "location", &location); > + if (!ret) { > + v4l2_ctrl_new_std(ctrl_hdlr, &ov5670_ctrl_ops, > + V4L2_CID_LOCATION, V4L2_LOCATION_FRONT, > + V4L2_LOCATION_BACK, 1, > + location == V4L2_LOCATION_FRONT ? To do this, you'll need to document that the control values match DT binding values. I don't think that's there currently. A switch would be nicer, to check that the value that is not front is something else meaningful. Otherwise I wouldn't create the control at all. A helper function for drivers to use would be nice. > + V4L2_LOCATION_FRONT : V4L2_LOCATION_BACK); > + } > + > if (ctrl_hdlr->error) { > ret = ctrl_hdlr->error; > goto error;
diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c index 041fcbb4eebd..1446aef1fc1e 100644 --- a/drivers/media/i2c/ov5670.c +++ b/drivers/media/i2c/ov5670.c @@ -2059,10 +2059,12 @@ static const struct v4l2_ctrl_ops ov5670_ctrl_ops = { /* Initialize control handlers */ static int ov5670_init_controls(struct ov5670 *ov5670) { + struct i2c_client *client = v4l2_get_subdevdata(&ov5670->sd); struct v4l2_ctrl_handler *ctrl_hdlr; s64 vblank_max; s64 vblank_def; s64 vblank_min; + u32 location; s64 exposure_max; int ret; @@ -2124,6 +2126,15 @@ static int ov5670_init_controls(struct ov5670 *ov5670) ARRAY_SIZE(ov5670_test_pattern_menu) - 1, 0, 0, ov5670_test_pattern_menu); + ret = device_property_read_u32(&client->dev, "location", &location); + if (!ret) { + v4l2_ctrl_new_std(ctrl_hdlr, &ov5670_ctrl_ops, + V4L2_CID_LOCATION, V4L2_LOCATION_FRONT, + V4L2_LOCATION_BACK, 1, + location == V4L2_LOCATION_FRONT ? + V4L2_LOCATION_FRONT : V4L2_LOCATION_BACK); + } + if (ctrl_hdlr->error) { ret = ctrl_hdlr->error; goto error;
The camera location is retrieved from the firmware interface parsing the "location" device property and reported through the read-only V4L2_CID_LOCATION control. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- drivers/media/i2c/ov5670.c | 11 +++++++++++ 1 file changed, 11 insertions(+)