On Thu, Mar 28, 2024 at 09:55:48PM +0000, Sakari Ailus wrote:
> On Thu, Mar 28, 2024 at 07:54:41AM +0000, Sakari Ailus wrote:
> > Hi Fabio,
> >
> > On Thu, Mar 28, 2024 at 02:13:19AM -0300, Fabio Estevam wrote:
> > > From: Fabio Estevam <festevam@xxxxxxx>
> > >
> > > Since commit 63b0cd30b78e ("media: ov2680: Add bus-cfg / endpoint
> > > property verification") the ov2680 no longer probes on a imx7s-warp7:
> > >
> > > ov2680 1-0036: error -EINVAL: supported link freq 330000000 not found
> > > ov2680 1-0036: probe with driver ov2680 failed with error -2
> > >
> > > As the 'link-frequencies' property is not mandatory, allow the probe
> > > to succeed by skipping the link-frequency verification when the
> > > property is absent.
> > >
> > > Fixes: 63b0cd30b78e ("media: ov2680: Add bus-cfg / endpoint property verification")
> > > Signed-off-by: Fabio Estevam <festevam@xxxxxxx>
> > > ---
> > > drivers/media/i2c/ov2680.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
> > > index 39d321e2b7f9..f611ce3a749c 100644
> > > --- a/drivers/media/i2c/ov2680.c
> > > +++ b/drivers/media/i2c/ov2680.c
> > > @@ -1123,6 +1123,9 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor)
> > > goto out_free_bus_cfg;
> > > }
> > >
> > > + if (!bus_cfg.nr_of_link_frequencies)
> > > + return 0;
> > > +
> >
> > Thanks for the patch.
> >
> > I'd still rather try to avoid going to this direction as these frequencies
> > are hardware dependent. Add a new one to the driver and some boards may
> > stop working properly. For details see
> > <URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks>.
>
> While the above holds, on second thought, the driver has been around for
> quite some time and indeed the added validation does break certain boards
> (at least without adding the link frequencies there).
>
> So I'm fine with the patch.
>
> I think this should also be cc'd to stable.
And this applies to the newer version of the patch that fixes the memory
leak, of course.
>
> >
> > > for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++)
> > > if (bus_cfg.link_frequencies[i] == sensor->link_freq[0])
> > > break;
> >
>
> --
> Sakari Ailus
--
Sakari Ailus
