Re: [PATCH 1/2] media: ov2680: Allow probing if link-frequencies is absent

[Sakari Ailus <sakari.ailus@linux.intel.com>]

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@denx.de>
> > > > > 
> > > > > 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@denx.de>
> > > > > ---
> > > > >  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