| Message ID | 20220103162414.27723-3-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | b4bdc64613f1f29c955cbba2f0e298ca76f65f5f |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: Drop .set_mbus_config(), improve .get_mbus_config() | expand |
Hi Laurent, Thanks for the set. On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote: > The subdev .set_mbus_config() operation is deprecated. No code in the > kernel calls it, so drop its implementation from the ov6650 driver. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > drivers/media/i2c/ov6650.c | 37 ------------------------------------- > 1 file changed, 37 deletions(-) > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > index f67412150b16..455a627e35a0 100644 > --- a/drivers/media/i2c/ov6650.c > +++ b/drivers/media/i2c/ov6650.c > @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd, > return 0; > } > > -/* Alter bus settings on camera side */ > -static int ov6650_set_mbus_config(struct v4l2_subdev *sd, > - unsigned int pad, > - struct v4l2_mbus_config *cfg) > -{ > - struct i2c_client *client = v4l2_get_subdevdata(sd); > - int ret = 0; > - > - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0); > - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING); I think this configuration should come from the endpoint which the driver currently does not parse. In fact, there are no even DT bindings for the device. I wonder what kind of environment it is used in --- assuming it works somewhere. > - if (ret) > - return ret; > - > - if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW) > - ret = ov6650_reg_rmw(client, REG_COMF, COMF_HREF_LOW, 0); > - else if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) > - ret = ov6650_reg_rmw(client, REG_COMF, 0, COMF_HREF_LOW); > - if (ret) > - return ret; > - > - if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_VSYNC_HIGH, 0); > - else if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW) > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_VSYNC_HIGH); > - if (ret) > - return ret; > - > - /* > - * Update the configuration to report what is actually applied to > - * the hardware. > - */ > - return ov6650_get_mbus_config(sd, pad, cfg); > -} > - > static const struct v4l2_subdev_video_ops ov6650_video_ops = { > .s_stream = ov6650_s_stream, > .g_frame_interval = ov6650_g_frame_interval, > @@ -993,7 +957,6 @@ static const struct v4l2_subdev_pad_ops ov6650_pad_ops = { > .get_fmt = ov6650_get_fmt, > .set_fmt = ov6650_set_fmt, > .get_mbus_config = ov6650_get_mbus_config, > - .set_mbus_config = ov6650_set_mbus_config, > }; > > static const struct v4l2_subdev_ops ov6650_subdev_ops = {
Hi Sakari, On Wed, Jan 05, 2022 at 08:04:24PM +0200, Sakari Ailus wrote: > On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote: > > The subdev .set_mbus_config() operation is deprecated. No code in the > > kernel calls it, so drop its implementation from the ov6650 driver. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > --- > > drivers/media/i2c/ov6650.c | 37 ------------------------------------- > > 1 file changed, 37 deletions(-) > > > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > > index f67412150b16..455a627e35a0 100644 > > --- a/drivers/media/i2c/ov6650.c > > +++ b/drivers/media/i2c/ov6650.c > > @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd, > > return 0; > > } > > > > -/* Alter bus settings on camera side */ > > -static int ov6650_set_mbus_config(struct v4l2_subdev *sd, > > - unsigned int pad, > > - struct v4l2_mbus_config *cfg) > > -{ > > - struct i2c_client *client = v4l2_get_subdevdata(sd); > > - int ret = 0; > > - > > - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0); > > - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING); > > I think this configuration should come from the endpoint which the driver > currently does not parse. In fact, there are no even DT bindings for the > device. There's also no OF match table. While this isn't strictly required, it may indicate that the sensor hasn't been tested much on DT-based systems. I agree that the configuration should come from the device tree, but I can't test that, so I'm tempted to let someone else implement it if the driver is actually still in use (I can also write a patch if someone can test it). > I wonder what kind of environment it is used in --- assuming it works > somewhere. > > > - if (ret) > > - return ret; > > - > > - if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW) > > - ret = ov6650_reg_rmw(client, REG_COMF, COMF_HREF_LOW, 0); > > - else if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) > > - ret = ov6650_reg_rmw(client, REG_COMF, 0, COMF_HREF_LOW); > > - if (ret) > > - return ret; > > - > > - if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_VSYNC_HIGH, 0); > > - else if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW) > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_VSYNC_HIGH); > > - if (ret) > > - return ret; > > - > > - /* > > - * Update the configuration to report what is actually applied to > > - * the hardware. > > - */ > > - return ov6650_get_mbus_config(sd, pad, cfg); > > -} > > - > > static const struct v4l2_subdev_video_ops ov6650_video_ops = { > > .s_stream = ov6650_s_stream, > > .g_frame_interval = ov6650_g_frame_interval, > > @@ -993,7 +957,6 @@ static const struct v4l2_subdev_pad_ops ov6650_pad_ops = { > > .get_fmt = ov6650_get_fmt, > > .set_fmt = ov6650_set_fmt, > > .get_mbus_config = ov6650_get_mbus_config, > > - .set_mbus_config = ov6650_set_mbus_config, > > }; > > > > static const struct v4l2_subdev_ops ov6650_subdev_ops = {
Hi Laurent, On Wednesday, 5 January 2022 21:19:49 CET Laurent Pinchart wrote: > Hi Sakari, > > On Wed, Jan 05, 2022 at 08:04:24PM +0200, Sakari Ailus wrote: > > On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote: > > > The subdev .set_mbus_config() operation is deprecated. No code in the > > > kernel calls it, so drop its implementation from the ov6650 driver. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > --- > > > drivers/media/i2c/ov6650.c | 37 ------------------------------------- > > > 1 file changed, 37 deletions(-) > > > > > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > > > index f67412150b16..455a627e35a0 100644 > > > --- a/drivers/media/i2c/ov6650.c > > > +++ b/drivers/media/i2c/ov6650.c > > > @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd, > > > return 0; > > > } > > > > > > -/* Alter bus settings on camera side */ > > > -static int ov6650_set_mbus_config(struct v4l2_subdev *sd, > > > - unsigned int pad, > > > - struct v4l2_mbus_config *cfg) > > > -{ > > > - struct i2c_client *client = v4l2_get_subdevdata(sd); > > > - int ret = 0; > > > - > > > - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0); > > > - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING); > > > > I think this configuration should come from the endpoint which the driver > > currently does not parse. In fact, there are no even DT bindings for the > > device. > > There's also no OF match table. While this isn't strictly required, it > may indicate that the sensor hasn't been tested much on DT-based > systems. > > I agree that the configuration should come from the device tree, but I > can't test that, so I'm tempted to let someone else implement it if the > driver is actually still in use (I can also write a patch if someone can > test it). This driver was used with omap1_camera, removed from the tree a few years ago by Hans, despite my attempts to refresh it. I tried to keep ov6650 updated but I gave up due to lack of response to my submissions. That also blocked my attempts to rework and reintroduce omap1_camera. I think I'm still able to update my local (v4l2, non-mc) version of omap1_camera to the extent required to test any changes to ov6650. However, the OMAP1 platform does not support DT, and will probably never do. Then, I think that it makes sense to spend my time on that only if you (media maintainers) are not going to depreciate non-DT support any soon. Are you? Thanks, Janusz > > > I wonder what kind of environment it is used in --- assuming it works > > somewhere. > > > > > - if (ret) > > > - return ret; > > > - > > > - if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW) > > > - ret = ov6650_reg_rmw(client, REG_COMF, COMF_HREF_LOW, 0); > > > - else if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) > > > - ret = ov6650_reg_rmw(client, REG_COMF, 0, COMF_HREF_LOW); > > > - if (ret) > > > - return ret; > > > - > > > - if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_VSYNC_HIGH, 0); > > > - else if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW) > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_VSYNC_HIGH); > > > - if (ret) > > > - return ret; > > > - > > > - /* > > > - * Update the configuration to report what is actually applied to > > > - * the hardware. > > > - */ > > > - return ov6650_get_mbus_config(sd, pad, cfg); > > > -} > > > - > > > static const struct v4l2_subdev_video_ops ov6650_video_ops = { > > > .s_stream = ov6650_s_stream, > > > .g_frame_interval = ov6650_g_frame_interval, > > > @@ -993,7 +957,6 @@ static const struct v4l2_subdev_pad_ops ov6650_pad_ops = { > > > .get_fmt = ov6650_get_fmt, > > > .set_fmt = ov6650_set_fmt, > > > .get_mbus_config = ov6650_get_mbus_config, > > > - .set_mbus_config = ov6650_set_mbus_config, > > > }; > > > > > > static const struct v4l2_subdev_ops ov6650_subdev_ops = { > >
Hi Janusz, On Wed, Jan 05, 2022 at 10:31:41PM +0100, Janusz Krzysztofik wrote: > On Wednesday, 5 January 2022 21:19:49 CET Laurent Pinchart wrote: > > On Wed, Jan 05, 2022 at 08:04:24PM +0200, Sakari Ailus wrote: > > > On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote: > > > > The subdev .set_mbus_config() operation is deprecated. No code in the > > > > kernel calls it, so drop its implementation from the ov6650 driver. > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > --- > > > > drivers/media/i2c/ov6650.c | 37 ------------------------------------- > > > > 1 file changed, 37 deletions(-) > > > > > > > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > > > > index f67412150b16..455a627e35a0 100644 > > > > --- a/drivers/media/i2c/ov6650.c > > > > +++ b/drivers/media/i2c/ov6650.c > > > > @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd, > > > > return 0; > > > > } > > > > > > > > -/* Alter bus settings on camera side */ > > > > -static int ov6650_set_mbus_config(struct v4l2_subdev *sd, > > > > - unsigned int pad, > > > > - struct v4l2_mbus_config *cfg) > > > > -{ > > > > - struct i2c_client *client = v4l2_get_subdevdata(sd); > > > > - int ret = 0; > > > > - > > > > - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0); > > > > - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING); > > > > > > I think this configuration should come from the endpoint which the driver > > > currently does not parse. In fact, there are no even DT bindings for the > > > device. > > > > There's also no OF match table. While this isn't strictly required, it > > may indicate that the sensor hasn't been tested much on DT-based > > systems. > > > > I agree that the configuration should come from the device tree, but I > > can't test that, so I'm tempted to let someone else implement it if the > > driver is actually still in use (I can also write a patch if someone can > > test it). > > This driver was used with omap1_camera, removed from the tree a few years > ago by Hans, despite my attempts to refresh it. I tried to keep ov6650 > updated but I gave up due to lack of response to my submissions. That also > blocked my attempts to rework and reintroduce omap1_camera. > > I think I'm still able to update my local (v4l2, non-mc) version of > omap1_camera to the extent required to test any changes to ov6650. > However, the OMAP1 platform does not support DT, and will probably never > do. Then, I think that it makes sense to spend my time on that only if > you (media maintainers) are not going to depreciate non-DT support any > soon. Are you? Thank you for the quick reply, and for the offer to test this. I understand your frustration, and the impossibility (with reasonable effort) to move OMAP1 to DT. This means that we would need to add platform data support to the ov6650 driver, and specify the platform data in the corresponding board file. On the ov6650 driver side I have no issue with that, and while platform data is deprecated for new platforms, it can be kept around for older ones as long as needed. I however don't know if changes to board files in arch/arm/mach-omap1 would be accepted. I also don't see any mention of ov6650 there, leading me to believe nobody can use this driver with the mainline kernel without resurecting the omap1_camera driver. I'm thus wondering if this would be a good use of your time, or if we should just merge this patch as-is. > > > I wonder what kind of environment it is used in --- assuming it works > > > somewhere. > > > > > > > - if (ret) > > > > - return ret; > > > > - > > > > - if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW) > > > > - ret = ov6650_reg_rmw(client, REG_COMF, COMF_HREF_LOW, 0); > > > > - else if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) > > > > - ret = ov6650_reg_rmw(client, REG_COMF, 0, COMF_HREF_LOW); > > > > - if (ret) > > > > - return ret; > > > > - > > > > - if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_VSYNC_HIGH, 0); > > > > - else if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW) > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_VSYNC_HIGH); > > > > - if (ret) > > > > - return ret; > > > > - > > > > - /* > > > > - * Update the configuration to report what is actually applied to > > > > - * the hardware. > > > > - */ > > > > - return ov6650_get_mbus_config(sd, pad, cfg); > > > > -} > > > > - > > > > static const struct v4l2_subdev_video_ops ov6650_video_ops = { > > > > .s_stream = ov6650_s_stream, > > > > .g_frame_interval = ov6650_g_frame_interval, > > > > @@ -993,7 +957,6 @@ static const struct v4l2_subdev_pad_ops ov6650_pad_ops = { > > > > .get_fmt = ov6650_get_fmt, > > > > .set_fmt = ov6650_set_fmt, > > > > .get_mbus_config = ov6650_get_mbus_config, > > > > - .set_mbus_config = ov6650_set_mbus_config, > > > > }; > > > > > > > > static const struct v4l2_subdev_ops ov6650_subdev_ops = {
On Wednesday, 5 January 2022 22:42:38 CET Laurent Pinchart wrote: > Hi Janusz, > > On Wed, Jan 05, 2022 at 10:31:41PM +0100, Janusz Krzysztofik wrote: > > On Wednesday, 5 January 2022 21:19:49 CET Laurent Pinchart wrote: > > > On Wed, Jan 05, 2022 at 08:04:24PM +0200, Sakari Ailus wrote: > > > > On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote: > > > > > The subdev .set_mbus_config() operation is deprecated. No code in the > > > > > kernel calls it, so drop its implementation from the ov6650 driver. > > > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > > --- > > > > > drivers/media/i2c/ov6650.c | 37 ------------------------------------- > > > > > 1 file changed, 37 deletions(-) > > > > > > > > > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ ov6650.c > > > > > index f67412150b16..455a627e35a0 100644 > > > > > --- a/drivers/media/i2c/ov6650.c > > > > > +++ b/drivers/media/i2c/ov6650.c > > > > > @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd, > > > > > return 0; > > > > > } > > > > > > > > > > -/* Alter bus settings on camera side */ > > > > > -static int ov6650_set_mbus_config(struct v4l2_subdev *sd, > > > > > - unsigned int pad, > > > > > - struct v4l2_mbus_config *cfg) > > > > > -{ > > > > > - struct i2c_client *client = v4l2_get_subdevdata(sd); > > > > > - int ret = 0; > > > > > - > > > > > - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0); > > > > > - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING); > > > > > > > > I think this configuration should come from the endpoint which the driver > > > > currently does not parse. In fact, there are no even DT bindings for the > > > > device. > > > > > > There's also no OF match table. While this isn't strictly required, it > > > may indicate that the sensor hasn't been tested much on DT-based > > > systems. > > > > > > I agree that the configuration should come from the device tree, but I > > > can't test that, so I'm tempted to let someone else implement it if the > > > driver is actually still in use (I can also write a patch if someone can > > > test it). > > > > This driver was used with omap1_camera, removed from the tree a few years > > ago by Hans, despite my attempts to refresh it. I tried to keep ov6650 > > updated but I gave up due to lack of response to my submissions. That also > > blocked my attempts to rework and reintroduce omap1_camera. > > > > I think I'm still able to update my local (v4l2, non-mc) version of > > omap1_camera to the extent required to test any changes to ov6650. > > However, the OMAP1 platform does not support DT, and will probably never > > do. Then, I think that it makes sense to spend my time on that only if > > you (media maintainers) are not going to depreciate non-DT support any > > soon. Are you? > > Thank you for the quick reply, and for the offer to test this. I > understand your frustration, and the impossibility (with reasonable > effort) to move OMAP1 to DT. I forgot to mention one more limitation of OMAP1 platform: it does not support CCF. With removal of v4l2-clk support from ov6650 a year ago, the driver is probably no longer usable with OMAP1 boards. > This means that we would need to add > platform data support to the ov6650 driver, and specify the platform > data in the corresponding board file. On the ov6650 driver side I have > no issue with that, and while platform data is deprecated for new > platforms, it can be kept around for older ones as long as needed. I > however don't know if changes to board files in arch/arm/mach-omap1 > would be accepted. I think that shouldn't be a problem, I could take care. > I also don't see any mention of ov6650 there, Respective i2c_board_info was removed from arch/arm/mach-omap1/board-ams- delta.c together with removal of soc_camera support. I had a patch in my queue that was registering the sensor info with I2C sybsystem at boot time for v4l2_async use but that alone wouldn't help much. > leading > me to believe nobody can use this driver with the mainline kernel > without resurecting the omap1_camera driver. I'm thus wondering if this > would be a good use of your time, or if we should just merge this patch > as-is. Yes, please feel free to merge it. Acked-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> Thanks, Janusz > > > > > I wonder what kind of environment it is used in --- assuming it works > > > > somewhere. > > > > > > > > > - if (ret) > > > > > - return ret; > > > > > - > > > > > - if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW) > > > > > - ret = ov6650_reg_rmw(client, REG_COMF, COMF_HREF_LOW, 0); > > > > > - else if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) > > > > > - ret = ov6650_reg_rmw(client, REG_COMF, 0, COMF_HREF_LOW); > > > > > - if (ret) > > > > > - return ret; > > > > > - > > > > > - if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_VSYNC_HIGH, 0); > > > > > - else if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW) > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_VSYNC_HIGH); > > > > > - if (ret) > > > > > - return ret; > > > > > - > > > > > - /* > > > > > - * Update the configuration to report what is actually applied to > > > > > - * the hardware. > > > > > - */ > > > > > - return ov6650_get_mbus_config(sd, pad, cfg); > > > > > -} > > > > > - > > > > > static const struct v4l2_subdev_video_ops ov6650_video_ops = { > > > > > .s_stream = ov6650_s_stream, > > > > > .g_frame_interval = ov6650_g_frame_interval, > > > > > @@ -993,7 +957,6 @@ static const struct v4l2_subdev_pad_ops ov6650_pad_ops = { > > > > > .get_fmt = ov6650_get_fmt, > > > > > .set_fmt = ov6650_set_fmt, > > > > > .get_mbus_config = ov6650_get_mbus_config, > > > > > - .set_mbus_config = ov6650_set_mbus_config, > > > > > }; > > > > > > > > > > static const struct v4l2_subdev_ops ov6650_subdev_ops = { > >
Hi Janusz, On Wed, Jan 05, 2022 at 10:31:41PM +0100, Janusz Krzysztofik wrote: > Hi Laurent, > > On Wednesday, 5 January 2022 21:19:49 CET Laurent Pinchart wrote: > > Hi Sakari, > > > > On Wed, Jan 05, 2022 at 08:04:24PM +0200, Sakari Ailus wrote: > > > On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote: > > > > The subdev .set_mbus_config() operation is deprecated. No code in the > > > > kernel calls it, so drop its implementation from the ov6650 driver. > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > --- > > > > drivers/media/i2c/ov6650.c | 37 ------------------------------------- > > > > 1 file changed, 37 deletions(-) > > > > > > > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > > > > index f67412150b16..455a627e35a0 100644 > > > > --- a/drivers/media/i2c/ov6650.c > > > > +++ b/drivers/media/i2c/ov6650.c > > > > @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd, > > > > return 0; > > > > } > > > > > > > > -/* Alter bus settings on camera side */ > > > > -static int ov6650_set_mbus_config(struct v4l2_subdev *sd, > > > > - unsigned int pad, > > > > - struct v4l2_mbus_config *cfg) > > > > -{ > > > > - struct i2c_client *client = v4l2_get_subdevdata(sd); > > > > - int ret = 0; > > > > - > > > > - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0); > > > > - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING); > > > > > > I think this configuration should come from the endpoint which the driver > > > currently does not parse. In fact, there are no even DT bindings for the > > > device. > > > > There's also no OF match table. While this isn't strictly required, it > > may indicate that the sensor hasn't been tested much on DT-based > > systems. > > > > I agree that the configuration should come from the device tree, but I > > can't test that, so I'm tempted to let someone else implement it if the > > driver is actually still in use (I can also write a patch if someone can > > test it). > > This driver was used with omap1_camera, removed from the tree a few years > ago by Hans, despite my attempts to refresh it. I tried to keep ov6650 > updated but I gave up due to lack of response to my submissions. That also > blocked my attempts to rework and reintroduce omap1_camera. My apologies for this --- I indeed to see a set of unreviewed ov6650 patches from you. Please do ping me if you expect an answer but do not get one. > > I think I'm still able to update my local (v4l2, non-mc) version of > omap1_camera to the extent required to test any changes to ov6650. > However, the OMAP1 platform does not support DT, and will probably never > do. Then, I think that it makes sense to spend my time on that only if > you (media maintainers) are not going to depreciate non-DT support any > soon. Are you? Commenting just this and not the discussion later in this thread --- it is possible to support such sensor drivers without DT or ACPI nowadays, through software nodes. See e.g. drivers/media/pci/intel/ipu3/cio2-bridge.c .
Hi Janusz, On Thu, Jan 06, 2022 at 01:13:56AM +0100, Janusz Krzysztofik wrote: > On Wednesday, 5 January 2022 22:42:38 CET Laurent Pinchart wrote: > > On Wed, Jan 05, 2022 at 10:31:41PM +0100, Janusz Krzysztofik wrote: > > > On Wednesday, 5 January 2022 21:19:49 CET Laurent Pinchart wrote: > > > > On Wed, Jan 05, 2022 at 08:04:24PM +0200, Sakari Ailus wrote: > > > > > On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote: > > > > > > The subdev .set_mbus_config() operation is deprecated. No code in the > > > > > > kernel calls it, so drop its implementation from the ov6650 driver. > > > > > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > > > --- > > > > > > drivers/media/i2c/ov6650.c | 37 ------------------------------------- > > > > > > 1 file changed, 37 deletions(-) > > > > > > > > > > > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ ov6650.c > > > > > > index f67412150b16..455a627e35a0 100644 > > > > > > --- a/drivers/media/i2c/ov6650.c > > > > > > +++ b/drivers/media/i2c/ov6650.c > > > > > > @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd, > > > > > > return 0; > > > > > > } > > > > > > > > > > > > -/* Alter bus settings on camera side */ > > > > > > -static int ov6650_set_mbus_config(struct v4l2_subdev *sd, > > > > > > - unsigned int pad, > > > > > > - struct v4l2_mbus_config *cfg) > > > > > > -{ > > > > > > - struct i2c_client *client = v4l2_get_subdevdata(sd); > > > > > > - int ret = 0; > > > > > > - > > > > > > - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0); > > > > > > - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) > > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING); > > > > > > > > > > I think this configuration should come from the endpoint which the driver > > > > > currently does not parse. In fact, there are no even DT bindings for the > > > > > device. > > > > > > > > There's also no OF match table. While this isn't strictly required, it > > > > may indicate that the sensor hasn't been tested much on DT-based > > > > systems. > > > > > > > > I agree that the configuration should come from the device tree, but I > > > > can't test that, so I'm tempted to let someone else implement it if the > > > > driver is actually still in use (I can also write a patch if someone can > > > > test it). > > > > > > This driver was used with omap1_camera, removed from the tree a few years > > > ago by Hans, despite my attempts to refresh it. I tried to keep ov6650 > > > updated but I gave up due to lack of response to my submissions. That also > > > blocked my attempts to rework and reintroduce omap1_camera. > > > > > > I think I'm still able to update my local (v4l2, non-mc) version of > > > omap1_camera to the extent required to test any changes to ov6650. > > > However, the OMAP1 platform does not support DT, and will probably never > > > do. Then, I think that it makes sense to spend my time on that only if > > > you (media maintainers) are not going to depreciate non-DT support any > > > soon. Are you? > > > > Thank you for the quick reply, and for the offer to test this. I > > understand your frustration, and the impossibility (with reasonable > > effort) to move OMAP1 to DT. > > I forgot to mention one more limitation of OMAP1 platform: it does not > support CCF. With removal of v4l2-clk support from ov6650 a year ago, the > driver is probably no longer usable with OMAP1 boards. Ouch, that's even worse indeed :-( > > This means that we would need to add > > platform data support to the ov6650 driver, and specify the platform > > data in the corresponding board file. On the ov6650 driver side I have > > no issue with that, and while platform data is deprecated for new > > platforms, it can be kept around for older ones as long as needed. I > > however don't know if changes to board files in arch/arm/mach-omap1 > > would be accepted. > > I think that shouldn't be a problem, I could take care. > > > I also don't see any mention of ov6650 there, > > Respective i2c_board_info was removed from arch/arm/mach-omap1/board-ams- > delta.c together with removal of soc_camera support. I had a patch in my > queue that was registering the sensor info with I2C sybsystem at boot time > for v4l2_async use but that alone wouldn't help much. > > > leading > > me to believe nobody can use this driver with the mainline kernel > > without resurecting the omap1_camera driver. I'm thus wondering if this > > would be a good use of your time, or if we should just merge this patch > > as-is. > > Yes, please feel free to merge it. > > Acked-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> Thank you. > > > > > I wonder what kind of environment it is used in --- assuming it works > > > > > somewhere. > > > > > > > > > > > - if (ret) > > > > > > - return ret; > > > > > > - > > > > > > - if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW) > > > > > > - ret = ov6650_reg_rmw(client, REG_COMF, COMF_HREF_LOW, 0); > > > > > > - else if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) > > > > > > - ret = ov6650_reg_rmw(client, REG_COMF, 0, COMF_HREF_LOW); > > > > > > - if (ret) > > > > > > - return ret; > > > > > > - > > > > > > - if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) > > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_VSYNC_HIGH, 0); > > > > > > - else if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW) > > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_VSYNC_HIGH); > > > > > > - if (ret) > > > > > > - return ret; > > > > > > - > > > > > > - /* > > > > > > - * Update the configuration to report what is actually applied to > > > > > > - * the hardware. > > > > > > - */ > > > > > > - return ov6650_get_mbus_config(sd, pad, cfg); > > > > > > -} > > > > > > - > > > > > > static const struct v4l2_subdev_video_ops ov6650_video_ops = { > > > > > > .s_stream = ov6650_s_stream, > > > > > > .g_frame_interval = ov6650_g_frame_interval, > > > > > > @@ -993,7 +957,6 @@ static const struct v4l2_subdev_pad_ops ov6650_pad_ops = { > > > > > > .get_fmt = ov6650_get_fmt, > > > > > > .set_fmt = ov6650_set_fmt, > > > > > > .get_mbus_config = ov6650_get_mbus_config, > > > > > > - .set_mbus_config = ov6650_set_mbus_config, > > > > > > }; > > > > > > > > > > > > static const struct v4l2_subdev_ops ov6650_subdev_ops = {
Hi Sakari, On Friday, 7 January 2022 14:41:51 CET Sakari Ailus wrote: > Hi Janusz, > > On Wed, Jan 05, 2022 at 10:31:41PM +0100, Janusz Krzysztofik wrote: > > Hi Laurent, > > > > On Wednesday, 5 January 2022 21:19:49 CET Laurent Pinchart wrote: > > > Hi Sakari, > > > > > > On Wed, Jan 05, 2022 at 08:04:24PM +0200, Sakari Ailus wrote: > > > > On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote: > > > > > The subdev .set_mbus_config() operation is deprecated. No code in the > > > > > kernel calls it, so drop its implementation from the ov6650 driver. > > > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > > --- > > > > > drivers/media/i2c/ov6650.c | 37 ------------------------------------- > > > > > 1 file changed, 37 deletions(-) > > > > > > > > > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > > > > > index f67412150b16..455a627e35a0 100644 > > > > > --- a/drivers/media/i2c/ov6650.c > > > > > +++ b/drivers/media/i2c/ov6650.c > > > > > @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd, > > > > > return 0; > > > > > } > > > > > > > > > > -/* Alter bus settings on camera side */ > > > > > -static int ov6650_set_mbus_config(struct v4l2_subdev *sd, > > > > > - unsigned int pad, > > > > > - struct v4l2_mbus_config *cfg) > > > > > -{ > > > > > - struct i2c_client *client = v4l2_get_subdevdata(sd); > > > > > - int ret = 0; > > > > > - > > > > > - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0); > > > > > - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING); > > > > > > > > I think this configuration should come from the endpoint which the driver > > > > currently does not parse. In fact, there are no even DT bindings for the > > > > device. > > > > > > There's also no OF match table. While this isn't strictly required, it > > > may indicate that the sensor hasn't been tested much on DT-based > > > systems. > > > > > > I agree that the configuration should come from the device tree, but I > > > can't test that, so I'm tempted to let someone else implement it if the > > > driver is actually still in use (I can also write a patch if someone can > > > test it). > > > > This driver was used with omap1_camera, removed from the tree a few years > > ago by Hans, despite my attempts to refresh it. I tried to keep ov6650 > > updated but I gave up due to lack of response to my submissions. That also > > blocked my attempts to rework and reintroduce omap1_camera. > > My apologies for this --- I indeed to see a set of unreviewed ov6650 patches > from you. Please do ping me if you expect an answer but do not get one. OK, thanks. > > I think I'm still able to update my local (v4l2, non-mc) version of > > omap1_camera to the extent required to test any changes to ov6650. > > However, the OMAP1 platform does not support DT, and will probably never > > do. Then, I think that it makes sense to spend my time on that only if > > you (media maintainers) are not going to depreciate non-DT support any > > soon. Are you? > > Commenting just this and not the discussion later in this thread --- it is > possible to support such sensor drivers without DT or ACPI nowadays, > through software nodes. See e.g. drivers/media/pci/intel/ipu3/cio2-bridge.c Thanks for bringing this possibility to my awareness, I didn't know it existed. AFAICS, I2C sensor drivers like ov6650 can now be provided by board files with device properties via i2c_board_info.swnode. If I find a solution to implement a non-v4l2-clk clock device in omap1_camera driver, I'll try to get back to updating ov6650 as time permits. Thanks, Janusz
Hi Janusz, On Mon, Jan 10, 2022 at 07:11:45PM +0100, Janusz Krzysztofik wrote: > Hi Sakari, > > On Friday, 7 January 2022 14:41:51 CET Sakari Ailus wrote: > > Hi Janusz, > > > > On Wed, Jan 05, 2022 at 10:31:41PM +0100, Janusz Krzysztofik wrote: > > > Hi Laurent, > > > > > > On Wednesday, 5 January 2022 21:19:49 CET Laurent Pinchart wrote: > > > > Hi Sakari, > > > > > > > > On Wed, Jan 05, 2022 at 08:04:24PM +0200, Sakari Ailus wrote: > > > > > On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote: > > > > > > The subdev .set_mbus_config() operation is deprecated. No code in the > > > > > > kernel calls it, so drop its implementation from the ov6650 driver. > > > > > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > > > --- > > > > > > drivers/media/i2c/ov6650.c | 37 ------------------------------------- > > > > > > 1 file changed, 37 deletions(-) > > > > > > > > > > > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > > > > > > index f67412150b16..455a627e35a0 100644 > > > > > > --- a/drivers/media/i2c/ov6650.c > > > > > > +++ b/drivers/media/i2c/ov6650.c > > > > > > @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd, > > > > > > return 0; > > > > > > } > > > > > > > > > > > > -/* Alter bus settings on camera side */ > > > > > > -static int ov6650_set_mbus_config(struct v4l2_subdev *sd, > > > > > > - unsigned int pad, > > > > > > - struct v4l2_mbus_config *cfg) > > > > > > -{ > > > > > > - struct i2c_client *client = v4l2_get_subdevdata(sd); > > > > > > - int ret = 0; > > > > > > - > > > > > > - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0); > > > > > > - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) > > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING); > > > > > > > > > > I think this configuration should come from the endpoint which the driver > > > > > currently does not parse. In fact, there are no even DT bindings for the > > > > > device. > > > > > > > > There's also no OF match table. While this isn't strictly required, it > > > > may indicate that the sensor hasn't been tested much on DT-based > > > > systems. > > > > > > > > I agree that the configuration should come from the device tree, but I > > > > can't test that, so I'm tempted to let someone else implement it if the > > > > driver is actually still in use (I can also write a patch if someone can > > > > test it). > > > > > > This driver was used with omap1_camera, removed from the tree a few years > > > ago by Hans, despite my attempts to refresh it. I tried to keep ov6650 > > > updated but I gave up due to lack of response to my submissions. That also > > > blocked my attempts to rework and reintroduce omap1_camera. > > > > My apologies for this --- I indeed to see a set of unreviewed ov6650 patches > > from you. Please do ping me if you expect an answer but do not get one. > > OK, thanks. > > > > I think I'm still able to update my local (v4l2, non-mc) version of > > > omap1_camera to the extent required to test any changes to ov6650. > > > However, the OMAP1 platform does not support DT, and will probably never > > > do. Then, I think that it makes sense to spend my time on that only if > > > you (media maintainers) are not going to depreciate non-DT support any > > > soon. Are you? > > > > Commenting just this and not the discussion later in this thread --- it is > > possible to support such sensor drivers without DT or ACPI nowadays, > > through software nodes. See e.g. drivers/media/pci/intel/ipu3/cio2-bridge.c > > Thanks for bringing this possibility to my awareness, I didn't know it > existed. AFAICS, I2C sensor drivers like ov6650 can now be provided by > board files with device properties via i2c_board_info.swnode. Looking at i2c_new_client_device(), it only takes a single software node. For an endpoint at least three are required, meaning you'll need to do that separately. At least that seems to be the case at the moment. > > If I find a solution to implement a non-v4l2-clk clock device in > omap1_camera driver, I'll try to get back to updating ov6650 as time > permits. Looking forward to that. Btw. I've taken the four patches you posted earlier, rebased them a little and pushed them here: <URL:https://git.linuxtv.org/sailus/media_tree.git/log/> Let me know if they're (not) fine.
Hi Sakari, On Tuesday, 11 January 2022 12:52:38 CET Sakari Ailus wrote: > Hi Janusz, > > On Mon, Jan 10, 2022 at 07:11:45PM +0100, Janusz Krzysztofik wrote: > > Hi Sakari, > > > > On Friday, 7 January 2022 14:41:51 CET Sakari Ailus wrote: > > > Hi Janusz, > > > > > > On Wed, Jan 05, 2022 at 10:31:41PM +0100, Janusz Krzysztofik wrote: > > > > Hi Laurent, > > > > > > > > On Wednesday, 5 January 2022 21:19:49 CET Laurent Pinchart wrote: > > > > > Hi Sakari, > > > > > > > > > > On Wed, Jan 05, 2022 at 08:04:24PM +0200, Sakari Ailus wrote: > > > > > > On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote: > > > > > > > The subdev .set_mbus_config() operation is deprecated. No code in the > > > > > > > kernel calls it, so drop its implementation from the ov6650 driver. > > > > > > > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > > > > --- > > > > > > > drivers/media/i2c/ov6650.c | 37 ------------------------------------- > > > > > > > 1 file changed, 37 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c > > > > > > > index f67412150b16..455a627e35a0 100644 > > > > > > > --- a/drivers/media/i2c/ov6650.c > > > > > > > +++ b/drivers/media/i2c/ov6650.c > > > > > > > @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd, > > > > > > > return 0; > > > > > > > } > > > > > > > > > > > > > > -/* Alter bus settings on camera side */ > > > > > > > -static int ov6650_set_mbus_config(struct v4l2_subdev *sd, > > > > > > > - unsigned int pad, > > > > > > > - struct v4l2_mbus_config *cfg) > > > > > > > -{ > > > > > > > - struct i2c_client *client = v4l2_get_subdevdata(sd); > > > > > > > - int ret = 0; > > > > > > > - > > > > > > > - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING) > > > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0); > > > > > > > - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) > > > > > > > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING); > > > > > > > > > > > > I think this configuration should come from the endpoint which the driver > > > > > > currently does not parse. In fact, there are no even DT bindings for the > > > > > > device. > > > > > > > > > > There's also no OF match table. While this isn't strictly required, it > > > > > may indicate that the sensor hasn't been tested much on DT-based > > > > > systems. > > > > > > > > > > I agree that the configuration should come from the device tree, but I > > > > > can't test that, so I'm tempted to let someone else implement it if the > > > > > driver is actually still in use (I can also write a patch if someone can > > > > > test it). > > > > > > > > This driver was used with omap1_camera, removed from the tree a few years > > > > ago by Hans, despite my attempts to refresh it. I tried to keep ov6650 > > > > updated but I gave up due to lack of response to my submissions. That also > > > > blocked my attempts to rework and reintroduce omap1_camera. > > > > > > My apologies for this --- I indeed to see a set of unreviewed ov6650 patches > > > from you. Please do ping me if you expect an answer but do not get one. > > > > OK, thanks. > > > > > > I think I'm still able to update my local (v4l2, non-mc) version of > > > > omap1_camera to the extent required to test any changes to ov6650. > > > > However, the OMAP1 platform does not support DT, and will probably never > > > > do. Then, I think that it makes sense to spend my time on that only if > > > > you (media maintainers) are not going to depreciate non-DT support any > > > > soon. Are you? > > > > > > Commenting just this and not the discussion later in this thread --- it is > > > possible to support such sensor drivers without DT or ACPI nowadays, > > > through software nodes. See e.g. drivers/media/pci/intel/ipu3/cio2-bridge.c > > > > Thanks for bringing this possibility to my awareness, I didn't know it > > existed. AFAICS, I2C sensor drivers like ov6650 can now be provided by > > board files with device properties via i2c_board_info.swnode. > > Looking at i2c_new_client_device(), it only takes a single software node. > For an endpoint at least three are required, meaning you'll need to do that > separately. At least that seems to be the case at the moment. Thanks, I'll take a closer look. > > > > If I find a solution to implement a non-v4l2-clk clock device in > > omap1_camera driver, I'll try to get back to updating ov6650 as time > > permits. > > Looking forward to that. Btw. I've taken the four patches you posted > earlier, rebased them a little and pushed them here: > > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/> > > Let me know if they're (not) fine. Should be fine, thank you. Janusz > >
diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index f67412150b16..455a627e35a0 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd, return 0; } -/* Alter bus settings on camera side */ -static int ov6650_set_mbus_config(struct v4l2_subdev *sd, - unsigned int pad, - struct v4l2_mbus_config *cfg) -{ - struct i2c_client *client = v4l2_get_subdevdata(sd); - int ret = 0; - - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING) - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0); - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING); - if (ret) - return ret; - - if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW) - ret = ov6650_reg_rmw(client, REG_COMF, COMF_HREF_LOW, 0); - else if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) - ret = ov6650_reg_rmw(client, REG_COMF, 0, COMF_HREF_LOW); - if (ret) - return ret; - - if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_VSYNC_HIGH, 0); - else if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW) - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_VSYNC_HIGH); - if (ret) - return ret; - - /* - * Update the configuration to report what is actually applied to - * the hardware. - */ - return ov6650_get_mbus_config(sd, pad, cfg); -} - static const struct v4l2_subdev_video_ops ov6650_video_ops = { .s_stream = ov6650_s_stream, .g_frame_interval = ov6650_g_frame_interval, @@ -993,7 +957,6 @@ static const struct v4l2_subdev_pad_ops ov6650_pad_ops = { .get_fmt = ov6650_get_fmt, .set_fmt = ov6650_set_fmt, .get_mbus_config = ov6650_get_mbus_config, - .set_mbus_config = ov6650_set_mbus_config, }; static const struct v4l2_subdev_ops ov6650_subdev_ops = {
The subdev .set_mbus_config() operation is deprecated. No code in the kernel calls it, so drop its implementation from the ov6650 driver. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/media/i2c/ov6650.c | 37 ------------------------------------- 1 file changed, 37 deletions(-)