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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 ?

> > > >  	for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++)
> > > >  		if (bus_cfg.link_frequencies[i] == sensor->link_freq[0])
> > > >  			break;

-- 
Regards,

Laurent Pinchart




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux