[PATCH 0/3] i2c: xiic: fix SMBus block read and PEC support

[PATCH 0/3] i2c: xiic: fix SMBus block read and PEC support

[Abdurrahman Hussain via B4 Relay]

This series fixes three independent bugs in the Xilinx AXI IIC driver
that together make SMBus block reads with PEC return -EBADMSG or -EIO
on otherwise clean transfers. They only surface when the client has
I2C_CLIENT_PEC set; non-PEC block reads happen to mask each issue in
turn.

The problems were uncovered driving an adm1266 PMBus device behind a
Xilinx AXI IIC FPGA block and reading its 64-byte blackbox record.

Patch 1 stops xiic_smbus_block_read_setup() from truncating rx_msg->len.
The i2c core appends a byte to msg->len when PEC is enabled, so
overwriting the length to "block size + 1" silently drops the PEC byte
and i2c_smbus_check_pec() then reads the last payload byte as the PEC.

Patch 2 raises the RX_FULL threshold so the interrupt only fires once
every remaining byte (payload plus optional PEC) is already buffered in
the FIFO. The previous threshold of rxmsg_len - 2 caused the
bytes_rem == 1 path in xiic_read_rx() to NACK a byte still on the wire.

Patch 3 stops the BNB handler from forcing tx_msg->len = 1 to signal
completion. tx_msg and rx_msg alias the same i2c_msg during a receive,
so this also clobbered rx_msg->len; and because tx_pos is already at 2
in the PEC case, the unsigned subtraction in xiic_tx_space() underflowed
and the STATE_DONE check fell through to STATE_ERROR. Advancing tx_pos
up to msg->len drives tx_space to zero without touching the length.

All three patches are pure bug fixes; non-PEC behaviour is unchanged.
Tested on real hardware -- a Xilinx AXI IIC controller talking to an
adm1266, where 64-byte PEC-checked block reads now complete cleanly.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Abdurrahman Hussain (3):
      i2c: xiic: preserve PEC byte length in SMBus block read setup
      i2c: xiic: defer RX_FULL until all trailing bytes are in FIFO
      i2c: xiic: don't clobber msg->len to signal block-read completion

 drivers/i2c/busses/i2c-xiic.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260427-i2c-xiic-2aeb501ec02a

Best regards,
--  
Abdurrahman Hussain <abdurrahman@nexthop.ai>

[PATCH 1/3] i2c: xiic: preserve PEC byte length in SMBus block read setup

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

xiic_smbus_block_read_setup() overwrites rx_msg->len based on the
announced block length byte, but the i2c core appends an extra byte to
msg->len when PEC is enabled on the client (see
__i2c_smbus_xfer_emulated in i2c-core-smbus.c). Before this handler
runs, rx_msg->len is therefore 1 + pec_len, where pec_len is 0 or 1.

Overwriting rx_msg->len to rxmsg_len + 1 drops that PEC byte from the
caller's buffer: the PEC byte is never drained from the FIFO and the
core's i2c_smbus_check_pec() reads the last payload byte instead of
the actual PEC, returning -EBADMSG even on clean transfers. Capture
pec_len before the overwrite and add it back when recomputing
rx_msg->len.

This is a pure bug fix; non-PEC behaviour is unchanged (pec_len == 0).
Tested on a Xilinx AXI IIC FPGA block driving an adm1266 PMBus
blackbox read -- with PEC enabled the full 64-byte record transfers
cleanly after this change.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/i2c/busses/i2c-xiic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 3e7735e1dae0..959a47b645a5 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -539,6 +539,8 @@ static void xiic_smbus_block_read_setup(struct xiic_i2c *i2c)
 
 	/* Check if received length is valid */
 	if (rxmsg_len <= I2C_SMBUS_BLOCK_MAX) {
+		unsigned int pec_len = i2c->rx_msg->len - 1;
+
 		/* Set Receive fifo depth */
 		if (rxmsg_len > IIC_RX_FIFO_DEPTH) {
 			/*
@@ -546,7 +548,7 @@ static void xiic_smbus_block_read_setup(struct xiic_i2c *i2c)
 			 * Receive fifo depth should set to Rx fifo capacity minus 1
 			 */
 			rfd_set = IIC_RX_FIFO_DEPTH - 1;
-			i2c->rx_msg->len = rxmsg_len + 1;
+			i2c->rx_msg->len = rxmsg_len + 1 + pec_len;
 		} else if ((rxmsg_len == 1) ||
 			(rxmsg_len == 0)) {
 			/*
@@ -562,7 +564,7 @@ static void xiic_smbus_block_read_setup(struct xiic_i2c *i2c)
 			 * Receive fifo depth should set to Rx msg len minus 2
 			 */
 			rfd_set = rxmsg_len - 2;
-			i2c->rx_msg->len = rxmsg_len + 1;
+			i2c->rx_msg->len = rxmsg_len + 1 + pec_len;
 		}
 		xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rfd_set);
 

-- 
2.53.0

[PATCH 2/3] i2c: xiic: defer RX_FULL until all trailing bytes are in FIFO

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

For the normal path of xiic_smbus_block_read_setup() (rxmsg_len less
than IIC_RX_FIFO_DEPTH), RFD was programmed to rxmsg_len - 2, which
fires the RX_FULL interrupt while the last payload byte is still in
flight. xiic_read_rx()'s bytes_rem == 1 branch then sets NACK on that
byte still on the wire, truncating the read in the PEC-enabled case.

Raise the threshold so RX_FULL fires only once every remaining byte
(payload plus optional PEC) is already buffered in the FIFO. That
routes the drain through xiic_read_rx()'s bytes_rem == 0 path, which
reads everything out and emits the stop cleanly. For the non-PEC path
the full payload is still read out through the same bytes_rem == 0
branch; the only user-visible change is that the controller waits one
extra byte-time before servicing the interrupt.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/i2c/busses/i2c-xiic.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 959a47b645a5..946e3ed2d760 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -559,11 +559,8 @@ static void xiic_smbus_block_read_setup(struct xiic_i2c *i2c)
 			rfd_set = 0;
 			i2c->rx_msg->len = SMBUS_BLOCK_READ_MIN_LEN;
 		} else {
-			/*
-			 * When Rx msg len less than Rx fifo capacity
-			 * Receive fifo depth should set to Rx msg len minus 2
-			 */
-			rfd_set = rxmsg_len - 2;
+			/* Defer RX_FULL until all trailing bytes are in FIFO. */
+			rfd_set = rxmsg_len + pec_len - 1;
 			i2c->rx_msg->len = rxmsg_len + 1 + pec_len;
 		}
 		xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rfd_set);

-- 
2.53.0

[PATCH 3/3] i2c: xiic: don't clobber msg->len to signal block-read completion

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

At the end of a SMBus block read the BNB handler force-set
tx_msg->len = 1 to push xiic_tx_space() to zero so the STATE_DONE
branch would fire. Two problems:

1. tx_msg and rx_msg alias the same i2c_msg struct during a receive
   (see xiic_start_recv), so overwriting tx_msg->len also changes
   rx_msg->len. The i2c core's i2c_smbus_check_pec() then reads the
   PEC from the wrong offset -- buf[0] instead of buf[rxmsg_len + 1]
   -- and either mis-validates or returns -EBADMSG.

2. xiic_start_recv sets tx_pos = msg->len (typically 2 when PEC is
   enabled). xiic_tx_space() is unsigned msg->len - tx_pos, so
   setting msg->len = 1 with tx_pos = 2 underflows to 0xFFFFFFFF and
   xiic_tx_space() never compares equal to 0 -- the STATE_DONE check
   falls through to STATE_ERROR, giving -EIO.

Instead, advance tx_pos up to msg->len. That drives tx_space to 0
without touching msg->len, preserving the buffer length that
xiic_smbus_block_read_setup() already grew to cover the length byte,
the payload and the optional PEC byte.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/i2c/busses/i2c-xiic.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 946e3ed2d760..27715646a9fb 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -864,8 +864,11 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
 
 		if (i2c->tx_msg && i2c->smbus_block_read) {
 			i2c->smbus_block_read = false;
-			/* Set requested message len=1 to indicate STATE_DONE */
-			i2c->tx_msg->len = 1;
+			/*
+			 * Drive xiic_tx_space() to 0 to signal STATE_DONE
+			 * without truncating the rx_msg length.
+			 */
+			i2c->tx_pos = i2c->tx_msg->len;
 		}
 
 		if (!i2c->tx_msg)

-- 
2.53.0

Re: [PATCH 0/3] i2c: xiic: fix SMBus block read and PEC support

[Shubhrajyoti Datta]

On Tue, Apr 28, 2026 at 5:48 AM Abdurrahman Hussain via B4 Relay
<devnull+abdurrahman.nexthop.ai@kernel.org> wrote:
>
> This series fixes three independent bugs in the Xilinx AXI IIC driver
> that together make SMBus block reads with PEC return -EBADMSG or -EIO
> on otherwise clean transfers. They only surface when the client has
> I2C_CLIENT_PEC set; non-PEC block reads happen to mask each issue in
> turn.
>
> The problems were uncovered driving an adm1266 PMBus device behind a
> Xilinx AXI IIC FPGA block and reading its 64-byte blackbox record.
>
> Patch 1 stops xiic_smbus_block_read_setup() from truncating rx_msg->len.
> The i2c core appends a byte to msg->len when PEC is enabled, so
> overwriting the length to "block size + 1" silently drops the PEC byte
> and i2c_smbus_check_pec() then reads the last payload byte as the PEC.
>
> Patch 2 raises the RX_FULL threshold so the interrupt only fires once
> every remaining byte (payload plus optional PEC) is already buffered in
> the FIFO. The previous threshold of rxmsg_len - 2 caused the
> bytes_rem == 1 path in xiic_read_rx() to NACK a byte still on the wire.
>
> Patch 3 stops the BNB handler from forcing tx_msg->len = 1 to signal
> completion. tx_msg and rx_msg alias the same i2c_msg during a receive,
> so this also clobbered rx_msg->len; and because tx_pos is already at 2
> in the PEC case, the unsigned subtraction in xiic_tx_space() underflowed
> and the STATE_DONE check fell through to STATE_ERROR. Advancing tx_pos
> up to msg->len drives tx_space to zero without touching the length.
>
> All three patches are pure bug fixes; non-PEC behaviour is unchanged.
> Tested on real hardware -- a Xilinx AXI IIC controller talking to an
> adm1266, where 64-byte PEC-checked block reads now complete cleanly.
>
> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>

LGTM
Reviewed-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

Re: [PATCH 0/3] i2c: xiic: fix SMBus block read and PEC support

[Michal Simek]

On 6/9/26 16:26, Shubhrajyoti Datta wrote:
> On Tue, Apr 28, 2026 at 5:48 AM Abdurrahman Hussain via B4 Relay
> <devnull+abdurrahman.nexthop.ai@kernel.org> wrote:
>>
>> This series fixes three independent bugs in the Xilinx AXI IIC driver
>> that together make SMBus block reads with PEC return -EBADMSG or -EIO
>> on otherwise clean transfers. They only surface when the client has
>> I2C_CLIENT_PEC set; non-PEC block reads happen to mask each issue in
>> turn.
>>
>> The problems were uncovered driving an adm1266 PMBus device behind a
>> Xilinx AXI IIC FPGA block and reading its 64-byte blackbox record.
>>
>> Patch 1 stops xiic_smbus_block_read_setup() from truncating rx_msg->len.
>> The i2c core appends a byte to msg->len when PEC is enabled, so
>> overwriting the length to "block size + 1" silently drops the PEC byte
>> and i2c_smbus_check_pec() then reads the last payload byte as the PEC.
>>
>> Patch 2 raises the RX_FULL threshold so the interrupt only fires once
>> every remaining byte (payload plus optional PEC) is already buffered in
>> the FIFO. The previous threshold of rxmsg_len - 2 caused the
>> bytes_rem == 1 path in xiic_read_rx() to NACK a byte still on the wire.
>>
>> Patch 3 stops the BNB handler from forcing tx_msg->len = 1 to signal
>> completion. tx_msg and rx_msg alias the same i2c_msg during a receive,
>> so this also clobbered rx_msg->len; and because tx_pos is already at 2
>> in the PEC case, the unsigned subtraction in xiic_tx_space() underflowed
>> and the STATE_DONE check fell through to STATE_ERROR. Advancing tx_pos
>> up to msg->len drives tx_space to zero without touching the length.
>>
>> All three patches are pure bug fixes; non-PEC behaviour is unchanged.
>> Tested on real hardware -- a Xilinx AXI IIC controller talking to an
>> adm1266, where 64-byte PEC-checked block reads now complete cleanly.
>>
>> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
> 
> LGTM
> Reviewed-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

Acked-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal