Hello Sakari,
On Mon, Nov 04, 2024 at 03:13:53PM +0000, Sakari Ailus wrote:
> Hi Stanislaw,
>
> On Mon, Nov 04, 2024 at 09:50:55AM +0100, Stanislaw Gruszka wrote:
> > On some Lenovo platforms, the patch workarounds problems with ov2740
> > sensor initialization, which manifest themself like below:
> >
> > [ 4.540476] ov2740 i2c-INT3474:01: error -EIO: failed to find sensor
> > [ 4.542066] ov2740 i2c-INT3474:01: probe with driver ov2740 failed with error -5
> >
> > or
> >
> > [ 7.742633] ov2740 i2c-INT3474:01: chip id mismatch: 2740 != 0
> > [ 7.742638] ov2740 i2c-INT3474:01: error -ENXIO: failed to find sensor
> >
> > and also by random failures of video stream start.
> >
> > Issue can be reproduced by this script:
> >
> > n=0
> > k=0
> > while [ $n -lt 50 ] ; do
> > sudo modprobe -r ov2740
> > sleep `expr $RANDOM % 5`
> > sudo modprobe ov2740
> > if media-ctl -p | grep -q ov2740 ; then
> > let k++
> > fi
> > let n++
> > done
> > echo Success rate $k/$n
> >
> > Without the patch, success rate is approximately 15 or 50 tries.
> > With the patch it does not fail.
> >
> > This problem is some hardware or firmware malfunction, that can not be
> > easy debug and fix. While setting small autosuspend delay is not perfect
> > workaround as user can configure it to any value, it will prevent
> > the failures by default.
> >
> > Additionally setting small autosuspend delay should have positive effect
> > on power consumption as for most ljca workloads device is used for just
> > a few milliseconds flowed by long periods of at least 100ms of inactivity
> > (usually more).
>
> I'm not very happy about this patch. While it makes the problem go away,
I'm not very happy having unreliable camera on my laptop ;-)
> apparently, the result seems to be for a reason that should have nothing to
> do with the underlying issue.
I can not be certain here, but I think when we do suspend ljca device
it either resets part of HW internally or do some latch of output gpio pins.
I think that is related to the root of the problem.
> I'm still not saying no to the patch as it hides the problem or at least,
> but we should properly describe the problem in the driver.
Ok, I can add comment before setting the delay.
Regards
Stanislaw
> It may well be
> that after an unrelated update elsewhere in the kernel the problem
> reappears again.
>
> >
> > Fixes: acd6199f195d ("usb: Add support for Intel LJCA device")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@xxxxxxxxxxxxxxx>
> > ---
> > drivers/usb/misc/usb-ljca.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c
> > index dcb3c5d248ac..062b7fb47114 100644
> > --- a/drivers/usb/misc/usb-ljca.c
> > +++ b/drivers/usb/misc/usb-ljca.c
> > @@ -810,6 +810,7 @@ static int ljca_probe(struct usb_interface *interface,
> > if (ret)
> > goto err_free;
> >
> > + pm_runtime_set_autosuspend_delay(&usb_dev->dev, 10);
> > usb_enable_autosuspend(usb_dev);
> >
> > return 0;
>
> --
> Kind regards,
>
> Sakari Ailus
