Re: [PATCH v5 07/11] drm/bridge: it6505: avoid division by zero in audio FS debug print

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing I2C read error handling in `it6505_audio_input()` leads to logic reset and false positive audio enable.
--

--- Patch [7]: [PATCH v5 07/11] drm/bridge: it6505: avoid division by zero in audio FS debug print ---
commit ebf71cd14ba5699a1da0c18fc4c670be657e122d
Author: Daniel Golle <daniel@makrotopia.org>

drm/bridge: it6505: avoid division by zero in audio FS debug print

This commit adds a check for regbe > 0 before printing debug information
about the audio input frequency in it6505_enable_audio(). This prevents a
division by zero when regbe is 0 or negative due to an I2C read failure.

> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index fc95f8d4f4129..81d2c96445318 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -1622,8 +1622,11 @@ static void it6505_enable_audio(struct it6505 *it6505)
>  	it6505_set_bits(it6505, REG_AUDIO_SRC_CTRL, AUDIO_FIFO_RESET, 0x00);
>  	it6505_set_bits(it6505, REG_RESET_CTRL, AUDIO_RESET, 0x00);
>  	regbe = it6505_read(it6505, REG_AUDIO_INPUT_FREQ);

[Severity: High]
This is a pre-existing issue, but since we are handling I2C read failures
for REG_AUDIO_INPUT_FREQ here, I noticed a similar problem in
it6505_audio_input().

If it6505_read() fails, it returns a negative error code. In
it6505_audio_input():

	reg05 = it6505_read(it6505, REG_RESET_CTRL);
	it6505_set_bits(it6505, REG_RESET_CTRL, AUDIO_RESET, 0x00);
	usleep_range(3000, 4000);
	regbe = it6505_read(it6505, REG_AUDIO_INPUT_FREQ);
	it6505_write(it6505, REG_RESET_CTRL, reg05);

The negative error code in reg05 is stored without validation and passed
directly to it6505_write(). Because the regmap uses 8-bit values, the integer
is truncated (e.g., -5 becomes 0xFB). Will this inadvertently assert critical
hardware reset bits such as ALL_LOGIC_RESET and VIDEO_RESET?

Additionally, at the end of it6505_audio_input():

	return regbe != 0xFF;

Since regbe can be a negative error code if the second read fails, this check
evaluates to true. Could this falsely signal that audio should be enabled,
leading to unexpected behavior?

> -	DRM_DEV_DEBUG_DRIVER(dev, "regbe:0x%02x audio input fs: %d.%d kHz",
> -			     regbe, 6750 / regbe, (6750 % regbe) * 10 / regbe);
> +	if (regbe > 0)
> +		DRM_DEV_DEBUG_DRIVER(dev,
> +				     "regbe:0x%02x audio input fs: %d.%d kHz",
> +				     regbe, 6750 / regbe,
> +				     (6750 % regbe) * 10 / regbe);
>  	it6505_set_bits(it6505, REG_DATA_MUTE_CTRL, EN_AUD_MUTE, 0x00);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784717129.git.daniel@makrotopia.org?part=7