Re: [PATCH v8 08/14] drm/bridge: it6505: avoid division by zero in audio FS debug print

[Luca Ceresoli]

> REG_AUDIO_INPUT_FREQ reads back 0 when no audio clock has been
> latched, and it6505_read() returns a negative errno on i2c failure.
> The debug print divides by the readback, and its arguments are
> evaluated even with debug output disabled. Only print when the value
> is usable as a divisor.
>
> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
> Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
>
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index cf2cd59625d5..0d2101432416 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -1623,8 +1623,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);
> -	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);
>  }
>

By just looking at your patch and the current driver code I'm failing to
understand what's going on with the REG_AUDIO_INPUT_FREQ register:

 * it is read, but never written
 * its value is used by:
   - it6505_enable_audio() only for logging: is that logging really useful?
   - it6505_audio_input(), just checking whether it is 0xFF or not; what's
     that 0xFF? Is it documented? A macro hinting at what 0xFF means would
     be better than just '0xFF'

If you could share what that register is for, it would probably answer some
of those questions.

Best regards,
Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com