Re: [PATCH] drm/amdgpu: use ACK polling for page-write completion

[Jani Nikula]

On Mon, 01 Jun 2026, Kunal Zodape <kunal.devanandzodape@xxxxxxx> wrote:
> The EEPROM write path currently waits a fixed 10 ms after each page
> write to cover the maximum write-cycle time.
>
> Replace the fixed delay with ACK polling so the driver can continue as
> soon as the EEPROM finishes its internal write cycle. Since the SMU I2C
> adapter used for these EEPROM accesses does not support zero-length
> transfers, poll readiness with an offset-only dummy write.
>
> Keep the existing 10 ms timeout as the upper bound for the polling loop.
>
> Tested on MI200 (ALDEBARAN) with ras_eeprom_reset confirming clean
> write/read-back with no I2C errors.
>
> Signed-off-by: Kunal Zodape <kunal.devanandzodape@xxxxxxx>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 28 +++++++++++++++-------
>  1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> index 8cd69836dd99..53be5a31c40c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> @@ -153,15 +153,27 @@ static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr,
>  			break;
>  
>  		if (!read) {
> -			/* According to EEPROM specs the length of the
> -			 * self-writing cycle, tWR (tW), is 10 ms.
> -			 *
> -			 * TODO: Use polling on ACK, aka Acknowledge
> -			 * Polling, to minimize waiting for the
> -			 * internal write cycle to complete, as it is
> -			 * usually smaller than tWR (tW).
> +			ktime_t timeout = ktime_add_ms(ktime_get(), 10);
> +
> +			/* Poll for ACK to detect when the self-timed
> +			 * internal write cycle has completed, as per
> +			 * Acknowledge Polling described in the AT24CM02
> +			 * datasheet, Section 7.4. The SMU I2C adapter
> +			 * used by these EEPROM paths does not support
> +			 * zero-length messages, so use an offset-only
> +			 * dummy write to probe for the ACK. The address
> +			 * pointer update is harmless because each real
> +			 * transfer reprograms it before use.
>  			 */
> -			msleep(10);
> +			do {
> +				r = i2c_transfer(i2c_adap, &msgs[0], 1);
> +				if (r == 1)
> +					break;
> +				usleep_range(100, 200);
> +			} while (ktime_before(ktime_get(), timeout));
> +
> +			if (r != 1)
> +				break;

See poll_timeout_us().


BR,
Jani.


>  		}
>  	}

-- 
Jani Nikula, Intel