On Fri, 06 Feb 2026, Samasth Norway Ananda <[email protected]> 
wrote:
> When reading exactly 512 bytes with burst read enabled, the
> extra_byte_added path breaks out of the inner do-while without
> decrementing len. The outer while(len) then re-enters and gmbus_wait()
> times out since all data has been delivered. Decrement len before the
> break so the outer loop terminates correctly.

Nice find, and the fix looks correct. How did you figure this out? Did
you hit the issue?

I wonder if the whole extra byte thing is a workaround for some old
hardware that shouldn't be needed on modern hardware... Ville, thoughts?

> Also fix a typo in a nearby comment ("generata" -> "generate").

"Also" is a good hint that it should be a separate patch. ;)

BR,
Jani

> Fixes: d5dc0f43f268 ("drm/i915/gmbus: Enable burst read")
> Signed-off-by: Samasth Norway Ananda <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_gmbus.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c 
> b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index 2caff677600c..5fb3fee34af4 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> @@ -496,8 +496,10 @@ gmbus_xfer_read_chunk(struct intel_display *display,
>  
>               val = intel_de_read_fw(display, GMBUS3(display));
>               do {
> -                     if (extra_byte_added && len == 1)
> +                     if (extra_byte_added && len == 1) {
> +                             len--;
>                               break;
> +                     }
>  
>                       *buf++ = val & 0xff;
>                       val >>= 8;
> @@ -693,7 +695,7 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg 
> *msgs, int num,
>                       goto clear_err;
>       }
>  
> -     /* Generate a STOP condition on the bus. Note that gmbus can't generata
> +     /* Generate a STOP condition on the bus. Note that gmbus can't generate
>        * a STOP on the very first cycle. To simplify the code we
>        * unconditionally generate the STOP condition with an additional gmbus
>        * cycle. */

-- 
Jani Nikula, Intel

Reply via email to