On Fri, Mar 29, 2024 at 12:04:03AM +0200, Laurent Pinchart wrote:
> On Thu, Mar 28, 2024 at 09:57:46PM +0000, Sakari Ailus wrote:
> > 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.
>
> Should we add least log a warning when link frequencies are not present
> in DT ?
Sounds reasonable.
>
> > > > > for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++)
> > > > > if (bus_cfg.link_frequencies[i] == sensor->link_freq[0])
> > > > > break;
--
Sakari Ailus
