Re: [External] : Re: [PATCH] drm/i915/gmbus: fix spurious timeout on 512-byte burst reads

[samasth.norway.ananda@oracle.com]

On 2/10/26 1:23 AM, Jani Nikula wrote:
> On Fri, 06 Feb 2026, Samasth Norway Ananda <samasth.norway.ananda@oracle.com> 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 came across it while working on a different issue and was reading the 
code.

> 
> 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. ;)

Thanks. I will send out a v2 with separate patches :)

Samasth.

> 
> BR,
> Jani
> 
>> Fixes: d5dc0f43f268 ("drm/i915/gmbus: Enable burst read")
>> Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
>> ---
>>   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. */
>