Re: [PATCH v2 2/2] media: synopsys: hdmirx: add HDMI audio capture support

[Igor Paunovic]

Hello Dmitry,

> Audio capture should be stopped when the codec removed and
> hdmirx_audio_shutdown() stops the audio_work. The v1 was good here,
> Sashiko bot should be wrong.

You're right -- the worker is only armed while a capture stream is
active (from hw_params()), and hdmirx_audio_shutdown() already stops
it with cancel_delayed_work_sync() when the stream closes. The extra
flag clear and final cancel in remove() were redundant, so I'll
restore the v1 teardown in v3.

> I'd like to see rest of the patches (I2S and etc) to test this patch
> before giving ack.

The enablement I test with on the Orange Pi 5 Plus is small: enable
i2s7_8ch (the RX-only I2S interface dedicated to HDMI RX) and add a
simple-audio-card that links it to the HDMI RX codec DAI 0, with the
codec as bitclock/frame master. I originally planned to send the dts
part separately through the rockchip tree, but for testing it belongs
with the series -- I'll include it in v3.

If Rock 5B is more convenient for you to test on, the same two
additions apply there; I can add that variant as well, compile-tested
only since I don't have the board.
v3 will then contain: the remove() revert, the ACR local renames plus
a byte-packing comment and the get_dai_id stub removal (both from the
bot's v2 review, already discussed in that subthread), and the dts
enablement.

Thanks for taking the time to test!

Igor

On Fri, Jul 17, 2026 at 1:08 AM Dmitry Osipenko
<dmitry.osipenko@xxxxxxxxxxxxx> wrote:
>
> 15.07.2026 23:08, Igor Paunovic пишет:
> > @@ -2788,6 +3058,16 @@ static void hdmirx_remove(struct platform_device *pdev)
> >       struct device *dev = &pdev->dev;
> >       struct snps_hdmirx_dev *hdmirx_dev = dev_get_drvdata(dev);
> >
> > +     /*
> > +      * Clear the streaming flag first so a running worker cannot re-arm
> > +      * itself, then tear down the codec (which stops any active stream)
> > +      * before the final cancel.
> > +      */
> > +     WRITE_ONCE(hdmirx_dev->audio_streaming, false);
> > +     if (hdmirx_dev->audio_pdev)
> > +             platform_device_unregister(hdmirx_dev->audio_pdev);
> > +     cancel_delayed_work_sync(&hdmirx_dev->audio_work);
> > +
>
> Audio capture should be stopped when the codec removed and
> hdmirx_audio_shutdown() stops the audio_work. The v1 was good here,
> Sashiko bot should be wrong.
>
> I'd like to see rest of the patches (I2S and etc) to test this patch
> before giving ack.
>
> --
> Best regards,
> Dmitry