diff mbox series

i2c: microchip-core: fix erroneous late ack send

Message ID 20220805074346.4123650-1-conor.dooley@microchip.com
State Accepted
Headers show
Series i2c: microchip-core: fix erroneous late ack send | expand

Commit Message

Conor Dooley Aug. 5, 2022, 7:43 a.m. UTC
A late ack is currently being sent at the end of a transfer due to
incorrect logic in mchp_corei2c_empty_rx(). Currently the Assert Ack
bit is being written to the controller's control reg after the last
byte has been received, causing it to sent another byte with the ack.
Instead, the AA flag should be written to to the contol register when
the penultimate byte is read so it is sent out for the last byte.

Reported-by: Andreas Buerkler <andreas.buerkler@enclustra.com>
Fixes: 64a6f1c4987e ("i2c: add support for microchip fpga i2c controllers")
Tested-by: Lewis Hanly <lewis.hanly@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---

FYI Wolfram, I am still sitting on the MAINTAINERS update as the
SoC updates have not been pulled yet for 6.0 (AFAICT)

 drivers/i2c/busses/i2c-microchip-corei2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Wolfram Sang Aug. 11, 2022, 12:28 p.m. UTC | #1
On Fri, Aug 05, 2022 at 08:43:46AM +0100, Conor Dooley wrote:
> A late ack is currently being sent at the end of a transfer due to
> incorrect logic in mchp_corei2c_empty_rx(). Currently the Assert Ack
> bit is being written to the controller's control reg after the last
> byte has been received, causing it to sent another byte with the ack.
> Instead, the AA flag should be written to to the contol register when
> the penultimate byte is read so it is sent out for the last byte.
> 
> Reported-by: Andreas Buerkler <andreas.buerkler@enclustra.com>
> Fixes: 64a6f1c4987e ("i2c: add support for microchip fpga i2c controllers")
> Tested-by: Lewis Hanly <lewis.hanly@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Fixed these checkpatch warnings:

WARNING: 'contol' may be misspelled - perhaps 'control'?
WARNING: Possible repeated word: 'to'

and applied to for-current, thanks!
Conor Dooley Aug. 11, 2022, 12:38 p.m. UTC | #2
On 11/08/2022 13:28, Wolfram Sang wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> On Fri, Aug 05, 2022 at 08:43:46AM +0100, Conor Dooley wrote:
>> A late ack is currently being sent at the end of a transfer due to
>> incorrect logic in mchp_corei2c_empty_rx(). Currently the Assert Ack
>> bit is being written to the controller's control reg after the last
>> byte has been received, causing it to sent another byte with the ack.
>> Instead, the AA flag should be written to to the contol register when
>> the penultimate byte is read so it is sent out for the last byte.
>>
>> Reported-by: Andreas Buerkler <andreas.buerkler@enclustra.com>
>> Fixes: 64a6f1c4987e ("i2c: add support for microchip fpga i2c controllers")
>> Tested-by: Lewis Hanly <lewis.hanly@microchip.com>
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>
> Fixed these checkpatch warnings:
>
> WARNING: 'contol' may be misspelled - perhaps 'control'?
> WARNING: Possible repeated word: 'to'

Heh, that's what I get for editing the commit in send-email..

>
> and applied to for-current, thanks!

Thanks :)

>
>
Wolfram Sang Aug. 11, 2022, 12:46 p.m. UTC | #3
> Heh, that's what I get for editing the commit in send-email..

I know that feeling... :)
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-microchip-corei2c.c b/drivers/i2c/busses/i2c-microchip-corei2c.c
index 6df0f1c33278..4d7e9b25f018 100644
--- a/drivers/i2c/busses/i2c-microchip-corei2c.c
+++ b/drivers/i2c/busses/i2c-microchip-corei2c.c
@@ -206,7 +206,7 @@  static void mchp_corei2c_empty_rx(struct mchp_corei2c_dev *idev)
 		idev->msg_len--;
 	}
 
-	if (idev->msg_len == 0) {
+	if (idev->msg_len <= 1) {
 		ctrl = readb(idev->base + CORE_I2C_CTRL);
 		ctrl &= ~CTRL_AA;
 		writeb(ctrl, idev->base + CORE_I2C_CTRL);