linux-kernel - Re: [PATCH] drm/vc4: fix infinite EPROBE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAPY8ntDqKBBt-uOb9m58jKhCn79RE26890X0EdxKiwWrypXN4Q@mail.gmail.com>
Date: Mon, 28 Apr 2025 18:47:58 +0100
From: Dave Stevenson <dave.stevenson@...pberrypi.com>
To: Gabriel Dalimonte <gabriel.dalimonte@...il.com>
Cc: Maxime Ripard <mripard@...nel.org>, Maíra Canal <mcanal@...lia.com>, 
	Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, Dmitry Baryshkov <lumag@...nel.org>
Subject: Re: [PATCH] drm/vc4: fix infinite EPROBE_DEFER loop

Hi Gabriel

On Sat, 26 Apr 2025 at 07:23, Gabriel Dalimonte
<gabriel.dalimonte@...il.com> wrote:
>
> `vc4_hdmi_audio_init` calls `devm_snd_dmaengine_pcm_register` which may
> return EPROBE_DEFER. Calling `drm_connector_hdmi_audio_init` adds a
> child device. The driver model docs[1] state that adding a child device
> prior to returning EPROBE_DEFER may result in an infinite loop.
>
> [1] https://www.kernel.org/doc/html/v6.14/driver-api/driver-model/driver.html
>
> Signed-off-by: Gabriel Dalimonte <gabriel.dalimonte@...il.com>
> ---
> Starting with v6.14, my Raspberry Pi 4B on the mainline kernel started seeing
> the vc4 driver looping during probe with:
>
> vc4-drm gpu: bound fe400000.hvs (ops vc4_hvs_ops [vc4])
> Registered IR keymap rc-cec
> rc rc0: vc4-hdmi-0 as /devices/platform/soc/fef00700.hdmi/rc/rc0
> input: vc4-hdmi-0 as /devices/platform/soc/fef00700.hdmi/rc/rc0/input3503
> vc4_hdmi fef00700.hdmi: Could not register PCM component: -517
>
> repeating several times per second.
>
> From my understanding, this happens due to the mainline kernel missing the
> patches to support audio portion of the HDMI interface. In this case, or
> other cases where the sound subsystem can't create a device, it returns
> -517 (EPROBE_DEFER). All of this is consistent with what I experienced prior
> to 6.14 as well. However, prior to 6.14 it did not try to probe infinitely.

Mainline should have all the bits for HDMI audio on Pi4.
It doesn't have the bits for Pi5 as it needs the newer DMA controller.

> Bisecting 6.13 -> 6.14, it looks like
> 9640f1437a88d8c617ff5523f1f9dc8c3ff29121 [1] moved HDMI audio connector
> initialization from audio vc4 audio initialization to vc4 connector
> initialization. If my understanding is correct, this change causes a child
> device to be added before EPROBE_DEFER is returned and queues the device probe
> to happen when a new device is added, which happens immediately because the
> audio child device was added earlier in the probe.

cc Dmitry as the author of that patch.

However I don't see an issue with moving the init back to vc4_hdmi_audio_init.
I'm not an expert on the sequencing of things around the audio side
though, so I wonder if Dmitry or Maxime could comment.

The patch could do with a Fixes: tag if 9640f1437a88 if it is
definitely the commit that breaks things.

Thanks
  Dave

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9640f1437a88d8c617ff5523f1f9dc8c3ff29121
> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index a29a6ef266f9a5952af53030a9a2d313e2ecdfce..163d092bd973bb3dfc5ea61187ec5fdf4f4f6029 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -560,12 +560,6 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
>         if (ret)
>                 return ret;
>
> -       ret = drm_connector_hdmi_audio_init(connector, dev->dev,
> -                                           &vc4_hdmi_audio_funcs,
> -                                           8, false, -1);
> -       if (ret)
> -               return ret;
> -
>         drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
>
>         /*
> @@ -2291,6 +2285,12 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
>                 return ret;
>         }
>
> +       ret = drm_connector_hdmi_audio_init(&vc4_hdmi->connector, dev,
> +                                           &vc4_hdmi_audio_funcs, 8, false,
> +                                           -1);
> +       if (ret)
> +               return ret;
> +
>         dai_link->cpus          = &vc4_hdmi->audio.cpu;
>         dai_link->codecs        = &vc4_hdmi->audio.codec;
>         dai_link->platforms     = &vc4_hdmi->audio.platform;
>
> ---
> base-commit: b60301774a8fe6c30b14a95104ec099290a2e904
> change-id: 20250426-vc4-audio-inf-probe-f67a8aa2a180
>
> Best regards,
> --
> Gabriel Dalimonte <gabriel.dalimonte@...il.com>
>