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@xxxxxxxxxxxxxxxx>
> > > > > > > ---
> > > > > > > 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
>
>
