Re: [PATCH v9 22/22] drm/bridge: synopsys: dw-dp: Add audio support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The result of regmap_read() is used without checking its return value, potentially leading to the use of uninitialized stack memory.
--

--- Patch [22]: [PATCH v9 22/22] drm/bridge: synopsys: dw-dp: Add audio support ---
commit f36ed9436eafc188ce4405c443eb47147a26dbd1
Author: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/[email protected]>
drm/bridge: synopsys: dw-dp: Add audio support

This commit implements audio support for the Synopsys DesignWare DisplayPort
controller.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c

[ ... ]

> @@ -1690,6 +1721,254 @@ static int dw_dp_link_enable(struct dw_dp *dp)

[ ... ]

> +static int dw_dp_audio_prepare(struct drm_bridge *bridge,
> +			       struct drm_connector *connector,
> +			       struct hdmi_codec_daifmt *daifmt,
> +			       struct hdmi_codec_params *params)
> +{
> +	struct dw_dp *dp = bridge_to_dp(bridge);
> +	u8 supported_audio_interfaces;
> +	enum dw_dp_audio_interface audio_interface;
> +	u32 cfg1;
> +	int ret;

[ ... ]

> +	switch (daifmt->fmt) {
> +	case HDMI_SPDIF:
> +		audio_interface = DW_DP_AUDIO_SPDIF;
> +		break;
> +	case HDMI_I2S:
> +		/*
> +		 * It is recommended to use SPDIF instead of I2S, since I2S mode requires
> +		 * manually inserting PCUV control bits from userspace and this is done
> +		 * automatically in hardware for SPDIF mode.
> +		 */
> +		audio_interface = DW_DP_AUDIO_I2S;
> +		break;
> +	default:
> +		dev_err(dp->dev, "invalid DAI format %d\n", daifmt->fmt);
> +		return -EINVAL;
> +	}
> +
> +	regmap_read(dp->regmap, DW_DP_CONFIG_REG1, &cfg1);
> +	supported_audio_interfaces = FIELD_GET(AUDIO_SELECT, cfg1);

[Severity: Medium]
Is it possible for regmap_read() to fail here? If the read operation fails,
cfg1 would remain uninitialized, causing FIELD_GET() to operate on
uninitialized stack memory. 

Should the return value of regmap_read() be checked before accessing cfg1?

> +
> +	if (supported_audio_interfaces != DW_DP_AUDIO_I2S_AND_SPDIF &&
> +	    supported_audio_interfaces != audio_interface) {
> +		dev_err(dp->dev, "unsupported DAI %d\n", daifmt->fmt);
> +		return -EINVAL;
> +	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-synopsys-dw-dp-improvements-v9-0-2e6e46135312-ZGY8ohtN/[email protected]?part=22