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

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

[Kunal Zodape]

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@amd.com>
---
 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;
 		}
 	}
 
-- 
2.17.1

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

[Jani Nikula]

On Mon, 01 Jun 2026, Kunal Zodape <kunal.devanandzodape@amd.com> 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@amd.com>
> ---
>  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

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

[Kunal Zodape]

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.

Suggested-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Kunal Zodape <kunal.devanandzodape@amd.com>
---
v2: Use read_poll_timeout() instead of open-coded ktime + do-while loop
    as suggested

 drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 27 +++++++++++++++-------
 1 file changed, 19 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..9dc538073bb8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
@@ -21,6 +21,7 @@
  *
  */
 
+#include <linux/iopoll.h>
 #include "amdgpu_eeprom.h"
 #include "amdgpu.h"
 
@@ -153,15 +154,25 @@ 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).
+			int ret;
+
+			/* 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);
+			ret = read_poll_timeout(i2c_transfer, r,
+						 r == 1,
+						 200, 10 * USEC_PER_MSEC,
+						 false,
+						 i2c_adap, &msgs[0], 1);
+			if (ret)
+				break;
 		}
 	}
 
-- 
2.17.1

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

[Lazar, Lijo]

On 01-Jun-26 4:53 PM, Kunal Zodape wrote:
> [Some people who received this message don't often get email from kunal.devanandzodape@amd.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> 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.

The current sleep logic may be better than sending a dummy transfter 
through firmware. That has the overhead of FW message logic and other 
clients accessing i2c bus.

The original comments in the code logic are valid for optimization only 
if driver has direct access to the i2c bus.

Thanks,
Lijo

> 
> Suggested-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Kunal Zodape <kunal.devanandzodape@amd.com>
> ---
> v2: Use read_poll_timeout() instead of open-coded ktime + do-while loop
>      as suggested
> 
>   drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 27 +++++++++++++++-------
>   1 file changed, 19 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..9dc538073bb8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> @@ -21,6 +21,7 @@
>    *
>    */
> 
> +#include <linux/iopoll.h>
>   #include "amdgpu_eeprom.h"
>   #include "amdgpu.h"
> 
> @@ -153,15 +154,25 @@ 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).
> +                       int ret;
> +
> +                       /* 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);
> +                       ret = read_poll_timeout(i2c_transfer, r,
> +                                                r == 1,
> +                                                200, 10 * USEC_PER_MSEC,
> +                                                false,
> +                                                i2c_adap, &msgs[0], 1);
> +                       if (ret)
> +                               break;
>                  }
>          }
> 
> --
> 2.17.1
> 

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

[Devanand Zodape, Kunal]

AMD General

Hi Lijo,

Thanks for the review. I want to make sure the trade-off is genuinely favourable before standing by the patch.

The case for ACK polling rests on a couple of assumptions:

- The AT24CM02 datasheet specifies a maximum write-cycle time of 10 ms, but the typical time is considerably shorter (often in the 2-5 ms range). With a fixed msleep(10), we always pay the worst-case cost, whereas ACK polling allows the write path to proceed as soon as the device is ready.

- With sleep_us = 200, the polling loop runs at most ~50 times over the full 10 ms window. If the write typically completes around ~3 ms, this reduces to ~15 polling attempts. Whether this is beneficial depends on the per-transaction FW overhead, as you pointed out.

That said, if the firmware I2C path has non-trivial per-message overhead (e.g., > ~100 us per round-trip), or if bus contention is a concern on these platforms, then the fixed delay could indeed be the better choice.

Do you have a sense of the typical FW round-trip latency on this path? That would help settle this.

Thanks,
Kunal

-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: 01 June 2026 18:35
To: Devanand Zodape, Kunal <Kunal.DevanandZodape@amd.com>; amd-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; David Airlie <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Kumar1, Rahul <Rahul.Kumar1@amd.com>; Gupta, Prateek1 <Prateek1.Gupta@amd.com>
Subject: Re: [PATCH v2] drm/amdgpu: use ACK polling for page-write completion



On 01-Jun-26 4:53 PM, Kunal Zodape wrote:
> [Some people who received this message don't often get email from
> kunal.devanandzodape@amd.com. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
>
> 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.

The current sleep logic may be better than sending a dummy transfter through firmware. That has the overhead of FW message logic and other clients accessing i2c bus.

The original comments in the code logic are valid for optimization only if driver has direct access to the i2c bus.

Thanks,
Lijo

>
> Suggested-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Kunal Zodape <kunal.devanandzodape@amd.com>
> ---
> v2: Use read_poll_timeout() instead of open-coded ktime + do-while loop
>      as suggested
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 27 +++++++++++++++-------
>   1 file changed, 19 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..9dc538073bb8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> @@ -21,6 +21,7 @@
>    *
>    */
>
> +#include <linux/iopoll.h>
>   #include "amdgpu_eeprom.h"
>   #include "amdgpu.h"
>
> @@ -153,15 +154,25 @@ 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).
> +                       int ret;
> +
> +                       /* 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);
> +                       ret = read_poll_timeout(i2c_transfer, r,
> +                                                r == 1,
> +                                                200, 10 * USEC_PER_MSEC,
> +                                                false,
> +                                                i2c_adap, &msgs[0], 1);
> +                       if (ret)
> +                               break;
>                  }
>          }
>
> --
> 2.17.1
>

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

[Lazar, Lijo]

On 02-Jun-26 11:40 AM, Devanand Zodape, Kunal wrote:
> AMD General
> 
> Hi Lijo,
> 
> Thanks for the review. I want to make sure the trade-off is genuinely favourable before standing by the patch.
> 
> The case for ACK polling rests on a couple of assumptions:
> 
> - The AT24CM02 datasheet specifies a maximum write-cycle time of 10 ms, but the typical time is considerably shorter (often in the 2-5 ms range). With a fixed msleep(10), we always pay the worst-case cost, whereas ACK polling allows the write path to proceed as soon as the device is ready.
> 
> - With sleep_us = 200, the polling loop runs at most ~50 times over the full 10 ms window. If the write typically completes around ~3 ms, this reduces to ~15 polling attempts. Whether this is beneficial depends on the per-transaction FW overhead, as you pointed out.
> 
> That said, if the firmware I2C path has non-trivial per-message overhead (e.g., > ~100 us per round-trip), or if bus contention is a concern on these platforms, then the fixed delay could indeed be the better choice.
> 

At a high level, FW protocol for i2c transfer is -
	Populate a table in device memory with the desired i2c commands.
	Send a message to the FW.
	FW reads the table and processes the commands.
	Wait for message response.
	Read the content from the buffer

This is the overhead for I2C transfer itself.

Besides,FW services only one message at a time and thus driver also 
allows only one FW message to go through. Depending on the condition, 
frequency of FW-driver interactions could vary. If the firmware happens 
to service another message, say someone using a smi tool to monitor some 
GPU metrics, then the wait may be longer to request another I2C transfer.

Now the actual HW I2C bus side, there could be more than 1 device 
connected and in some other cases (board specific) FW has to arbitrate 
master access to the bus. If the FW has to get data from some other 
device, that goes first, thus there could be more waits before this 
transfer gets through.

The max wait of 10ms is AT24CM02 specific, not sure about other EEPROM 
devs (could be less). The optimization for < 10ms thus makes sense only 
if driver has direct access to the bus and no arbitration for bus mastering.

Thanks,
Lijo

> Do you have a sense of the typical FW round-trip latency on this path? That would help settle this.
> 
> Thanks,
> Kunal
> 
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: 01 June 2026 18:35
> To: Devanand Zodape, Kunal <Kunal.DevanandZodape@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; David Airlie <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Kumar1, Rahul <Rahul.Kumar1@amd.com>; Gupta, Prateek1 <Prateek1.Gupta@amd.com>
> Subject: Re: [PATCH v2] drm/amdgpu: use ACK polling for page-write completion
> 
> 
> 
> On 01-Jun-26 4:53 PM, Kunal Zodape wrote:
>> [Some people who received this message don't often get email from
>> kunal.devanandzodape@amd.com. Learn why this is important at
>> https://aka.ms/LearnAboutSenderIdentification ]
>>
>> 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.
> 
> The current sleep logic may be better than sending a dummy transfter through firmware. That has the overhead of FW message logic and other clients accessing i2c bus.
> 
> The original comments in the code logic are valid for optimization only if driver has direct access to the i2c bus.
> 
> Thanks,
> Lijo
> 
>>
>> Suggested-by: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Kunal Zodape <kunal.devanandzodape@amd.com>
>> ---
>> v2: Use read_poll_timeout() instead of open-coded ktime + do-while loop
>>       as suggested
>>
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 27 +++++++++++++++-------
>>    1 file changed, 19 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..9dc538073bb8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
>> @@ -21,6 +21,7 @@
>>     *
>>     */
>>
>> +#include <linux/iopoll.h>
>>    #include "amdgpu_eeprom.h"
>>    #include "amdgpu.h"
>>
>> @@ -153,15 +154,25 @@ 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).
>> +                       int ret;
>> +
>> +                       /* 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);
>> +                       ret = read_poll_timeout(i2c_transfer, r,
>> +                                                r == 1,
>> +                                                200, 10 * USEC_PER_MSEC,
>> +                                                false,
>> +                                                i2c_adap, &msgs[0], 1);
>> +                       if (ret)
>> +                               break;
>>                   }
>>           }
>>
>> --
>> 2.17.1
>>
> 

Claude review: drm/amdgpu: use ACK polling for page-write completion

[Claude Code Review Bot]

Overall Series Review

Subject: drm/amdgpu: use ACK polling for page-write completion
Author: Kunal Zodape <kunal.devanandzodape@amd.com>
Patches: 6
Reviewed: 2026-06-04T14:17:38.468071

---

This mbox contains two versions (v1 and v2) of the same single patch, replacing a fixed 10 ms `msleep()` after each EEPROM page write with ACK polling. The optimization is well-motivated: EEPROM internal write cycles are typically ~5 ms but the driver was always waiting the worst-case 10 ms. ACK polling is the standard technique described in EEPROM datasheets (AT24CM02 Section 7.4) and was even marked as a TODO in the existing code.

**v1** used an open-coded `ktime` + `do/while` loop. **v2** addressed review feedback (credited to Jani Nikula) by switching to `read_poll_timeout()`, which is the right kernel idiom.

v2 is a clean, correct patch. The approach, error handling, and use of `read_poll_timeout()` are all sound. I have one minor concern about variable reuse worth calling out, but no blocking issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/amdgpu: use ACK polling for page-write completion

[Claude Code Review Bot]

Patch Review

This is the superseded version. The open-coded polling loop works correctly but was rightfully replaced in v2 with the kernel-provided `read_poll_timeout()` macro. Not much to say since v2 supersedes it.

---

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Patch Review

**Correctness: Good.** The `read_poll_timeout()` usage is correct:

```c
ret = read_poll_timeout(i2c_transfer, r,
                         r == 1,
                         200, 10 * USEC_PER_MSEC,
                         false,
                         i2c_adap, &msgs[0], 1);
```

- `op = i2c_transfer`, called as `r = i2c_transfer(i2c_adap, &msgs[0], 1)` — polls with a single-message (offset-only) write, which is the correct ACK polling probe when the adapter doesn't support zero-length transfers.
- `cond = r == 1` — correct: `i2c_transfer` returns the number of messages transferred on success.
- `sleep_us = 200` — internally uses `usleep_range(51, 200)` between polls. Reasonable: at worst ~50 polls over 10 ms.
- `timeout_us = 10 * USEC_PER_MSEC` — preserves the existing 10 ms upper bound.
- `sleep_before_read = false` — first poll is immediate. An initial sleep might save one wasted I2C transaction (the EEPROM is definitely busy right after the write), but the overhead is negligible.

**Error handling: Correct.** After timeout, `read_poll_timeout()` returns `-ETIMEDOUT`, the code breaks out of the for loop, and `r` holds the last `i2c_transfer` return value (typically `-ENXIO` from NACK), which propagates correctly through:

```c
return r < 0 ? r : eeprom_buf - p;
```

**Variable reuse — minor concern.** The macro writes into `r`, which is also the outer for-loop's transfer result variable. This works correctly because:
- On successful poll: `r == 1`, the loop continues, and the next iteration overwrites `r` with the actual two-message transfer result before checking it.
- On last page (loop exits with `buf_size == 0`): `r == 1 >= 0`, so the byte count `eeprom_buf - p` is returned, which is correct.

However, this coupling is subtle. A reader seeing `r` used for both `ARRAY_SIZE(msgs)==2` success checks and `==1` polling success needs to trace through carefully. It's not wrong, but worth being aware of. If the maintainers prefer, the `read_poll_timeout` could use the local `ret` variable instead and set `r` explicitly on error:

```c
read_poll_timeout(i2c_transfer, ret,
                   ret == 1, ...);
if (ret != 1) {
    r = ret;
    break;
}
```

But this is a style nit, not a correctness issue.

**Comment quality: Good.** The block comment explaining why an offset-only dummy write is used instead of a zero-length probe, and why the address pointer update is harmless, is helpful and well-written.

**Include addition: Correct.** `<linux/iopoll.h>` is needed for `read_poll_timeout()` and is properly placed before the local headers.

**Commit message: Good.** Clear description of what and why, appropriate `Suggested-by` tag, v2 changelog, and test information (MI200/ALDEBARAN with `ras_eeprom_reset`).

**Verdict:** v2 looks good to me. The `read_poll_timeout()` usage is correct, error paths are sound, and the approach follows standard EEPROM practice. The variable reuse is the only subtle point but works correctly as-is.

---
Generated by Claude Code Patch Reviewer