Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Missing PM runtime reference in audio callbacks leads to TOCTOU race with device power-off. --
--- Patch [12]: [PATCH v6 12/13] drm/bridge: it6505: Add audio support --- commit 10341e11c958ca412de2f96980c0f7ee9f5854c5 Author: Jiaxin Yu <[email protected]> drm/bridge: it6505: Add audio support Add audio support for it6505 by bridging to the hdmi-codec: register an "hdmi-audio-codec" platform device from probe and wire up the previously unused audio helpers via hdmi_codec_ops. This unblocks the mt8186-mt6366 sound card which references it6505 as the I2S3 codec. Link: https://lore.kernel.org/all/[email protected]/ > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c > b/drivers/gpu/drm/bridge/ite-it6505.c > index aeaa7b5ecf5aa..c560f1258f6d7 100644 > --- a/drivers/gpu/drm/bridge/ite-it6505.c > +++ b/drivers/gpu/drm/bridge/ite-it6505.c [ ... ] > -static void __maybe_unused it6505_audio_shutdown(struct device *dev, void > *data) > +static void it6505_audio_shutdown(struct device *dev, void *data) > { > struct it6505 *it6505 = dev_get_drvdata(dev); > > + mutex_lock(&it6505->audio_lock); > + it6505->audio.mute = true; > if (it6505->powered) > - it6505_disable_audio(it6505); > + __it6505_disable_audio(it6505); > + mutex_unlock(&it6505->audio_lock); > + cancel_delayed_work_sync(&it6505->delayed_audio); > +} [ ... ] > +static int it6505_audio_mute(struct device *dev, void *data, > + bool enable, int direction) > +{ > + struct it6505 *it6505 = dev_get_drvdata(dev); > + > + DRM_DEV_DEBUG_DRIVER(dev, "mute: %d", enable); > + > + /* > + * Delay enabling audio until the stream is unmuted; InfoFrames > + * without audio data upset some DP-to-HDMI dongles. > + */ > + if (enable) { > + mutex_lock(&it6505->audio_lock); > + it6505->audio.mute = true; > + if (it6505->powered) > + __it6505_disable_audio(it6505); > + mutex_unlock(&it6505->audio_lock); > + cancel_delayed_work_sync(&it6505->delayed_audio); > + } else { > + mutex_lock(&it6505->audio_lock); > + it6505->audio.mute = false; > + mutex_unlock(&it6505->audio_lock); > + queue_delayed_work(system_wq, &it6505->delayed_audio, > + msecs_to_jiffies(180)); > + } > + > + return 0; > +} [Severity: High] Does this code race with device power-off? If the audio stream is unmuted via ALSA, it6505_audio_mute() queues it6505->delayed_audio to run after 180ms. If a user unplugs the DP cable during this delay, it6505_extcon_work() calls pm_runtime_put_sync() on the parent device, invoking the runtime suspend callback it6505_poweroff(). it6505_poweroff() disables regulators and asserts the reset GPIO before clearing the it6505->powered flag at the very end of the function. It does not cancel the delayed work. If it6505_delayed_audio() (or a concurrent it6505_audio_mute() / it6505_audio_shutdown()) checks it6505->powered before it6505_poweroff() updates the flag, they could proceed to issue I2C writes while the device's power is physically cut. This can result in I2C bus lockups since unpowered chips may pull SDA/SCL lines low. Should these functions use a PM runtime reference (e.g., via pm_runtime_get_if_active()) to safely access the hardware? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=12
