Re: [PATCH v3 4/4] arm64: dts: rockchip: enable HDMI RX audio capture on Orange Pi 5 Plus

[Igor Paunovic]

> Does exposing this codec to the ALSA subsystem trigger a suspend
> regression in the hdmirx driver?

Good catch -- this one is real. The audio worker missed the pattern
the driver already applies to its hotplug and res_change works,
which hdmirx_disable_irq() cancels on the suspend path. Fixed for
v4: suspend cancels audio_work before the controller clocks are
gated, and resume re-arms it when a capture stream is active; the
worker's FIFO error path then resynchronizes the clock and the FIFO
on its first run after resume.

> Should swab32() be used here instead to ensure the byte swap happens
> unconditionally on all architectures?

Yes -- readl() already abstracts the bus endianness, so what this
code wants is an unconditional byte reversal, not an endian
conversion. Switched to swab32() for v4 (no functional change on
arm64, where this controller lives, but the right primitive
regardless).

Both changes are in my tree, compile-tested, and will be part of v4
together with whatever comes out of human review of the series.

Igor

On Sat, Jul 18, 2026 at 11:12 AM <sashiko-bot@xxxxxxxxxx> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The audio_work delayed work is not cancelled during system suspend, leading to a fatal bus fault.
> - [Medium] Incorrect use of be32_to_cpu() for unconditional byte swapping breaks audio capture on big-endian architectures.
> --
>
> --- Patch [4]: [PATCH v3 4/4] arm64: dts: rockchip: enable HDMI RX audio capture on Orange Pi 5 Plus ---
> commit 030cd8fd5334b79e1751dec57b1de1f5fa4e3df3
> Author: Igor Paunovic <royalnet026@xxxxxxxxx>
>
> arm64: dts: rockchip: enable HDMI RX audio capture on Orange Pi 5 Plus
>
> This patch routes the HDMI receiver audio to the i2s7_8ch interface. It creates
> a simple-audio-card configuration to expose a capture-only ALSA card driven
> by the HDMI input. The setup uses the receiver as the bitclock and frame master.
>
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> > index 145986695720a..2a68610cddbc3 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> > @@ -37,6 +37,22 @@ hdmi1_con_in: endpoint {
> >               };
> >       };
> >
> > +     hdmiin-sound {
> > +             compatible = "simple-audio-card";
> > +             simple-audio-card,name = "rockchip,hdmiin";
> > +             simple-audio-card,format = "i2s";
> > +             simple-audio-card,bitclock-master = <&hdmiin_codec>;
> > +             simple-audio-card,frame-master = <&hdmiin_codec>;
> > +
> > +             simple-audio-card,cpu {
> > +                     sound-dai = <&i2s7_8ch>;
> > +             };
> > +
> > +             hdmiin_codec: simple-audio-card,codec {
> > +                     sound-dai = <&hdmi_receiver 0>;
>
> [Severity: High]
> Does exposing this codec to the ALSA subsystem trigger a suspend regression in
> the hdmirx driver?
>
> Looking at the underlying codec driver hdmirx_suspend() in snps_hdmirx.c:
>
> drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c:hdmirx_suspend()
> {
>         struct snps_hdmirx_dev *hdmirx_dev = dev_get_drvdata(dev);
>
>         hdmirx_disable_irq(dev);
>
>         /* TODO store CEC HW state */
>         disable_irq(hdmirx_dev->cec->irq);
>
>         return hdmirx_disable(dev);
> }
>
> The audio_work delayed work is queued on system_unbound_wq, which is not
> freezable, but this suspend callback doesn't cancel it.
>
> If system suspend is initiated while audio capture is active, the worker
> could fire after clocks are disabled via hdmirx_disable(), resulting in
> a bus fault when it performs MMIO reads.
>
> [Severity: Medium]
> Will this audio capture work correctly on big-endian architectures?
>
> The underlying hdmirx_audio_fs() driver function uses be32_to_cpu() for an
> unconditional byte swap when calculating the ACR subpacket:
>
> drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c:hdmirx_audio_fs()
> {
> ...
>         acr_cts = be32_to_cpu((__force __be32)acr_pb3_0) & 0xfffff;
>         acr_n = (be32_to_cpu((__force __be32)acr_pb7_4) & 0x0fffff00) >> 8;
> ...
> }
>
> Since be32_to_cpu() behaves as a no-op on big-endian CPUs, the bytes remain
> unswapped, causing the bitwise masking to extract the wrong bytes for the
> ACR CTS and N fields.
>
> Should swab32() be used here instead to ensure the byte swap happens
> unconditionally on all architectures?
>
> > +             };
> > +     };
> > +
>
> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260718085728.6797-1-royalnet026@xxxxxxxxx?part=4