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,
apparently, the result seems to be for a reason that should have nothing to
do with the underlying issue.
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. 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
