[PATCH v2] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers

[PATCH v2] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers

[Alain Volmat]

In case of SMBUS byte read with PEC enabled, the whole transfer
is split into two commands.  A first write command, followed by
a read command.  The write command does not have any PEC byte
and a PEC byte is appended at the end of the read command.
(cf Read byte protocol with PEC in SMBUS specification)

Within the STM32 I2C controller, handling (either sending
or receiving) of the PEC byte is done via the PECBYTE bit in
register CR2.

Currently, the PECBYTE is set at the beginning of a transfer,
which lead to sending a PEC byte at the end of the write command
(hence losing the real last byte), and also does not check the
PEC byte received during the read command.

This patch corrects the function stm32f7_i2c_smbus_xfer_msg
in order to only set the PECBYTE during the read command.

Fixes: 9e48155f6bfe ("i2c: i2c-stm32f7: Add initial SMBus protocols support")
Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Reviewed-by: Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com>
---
v2:
  - add more details about the issue within the commit log
  - removal of blank line between Fixes and Signed-off-by

 drivers/i2c/busses/i2c-stm32f7.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 579b30581725..0d3c9a041b56 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1059,9 +1059,10 @@ static int stm32f7_i2c_smbus_xfer_msg(struct stm32f7_i2c_dev *i2c_dev,
 	/* Configure PEC */
 	if ((flags & I2C_CLIENT_PEC) && f7_msg->size != I2C_SMBUS_QUICK) {
 		cr1 |= STM32F7_I2C_CR1_PECEN;
-		cr2 |= STM32F7_I2C_CR2_PECBYTE;
-		if (!f7_msg->read_write)
+		if (!f7_msg->read_write) {
+			cr2 |= STM32F7_I2C_CR2_PECBYTE;
 			f7_msg->count++;
+		}
 	} else {
 		cr1 &= ~STM32F7_I2C_CR1_PECEN;
 		cr2 &= ~STM32F7_I2C_CR2_PECBYTE;
@@ -1149,8 +1150,10 @@ static void stm32f7_i2c_smbus_rep_start(struct stm32f7_i2c_dev *i2c_dev)
 	f7_msg->stop = true;
 
 	/* Add one byte for PEC if needed */
-	if (cr1 & STM32F7_I2C_CR1_PECEN)
+	if (cr1 & STM32F7_I2C_CR1_PECEN) {
+		cr2 |= STM32F7_I2C_CR2_PECBYTE;
 		f7_msg->count++;
+	}
 
 	/* Set number of bytes to be transferred */
 	cr2 &= ~(STM32F7_I2C_CR2_NBYTES_MASK);
-- 
2.25.1

Re: [PATCH v2] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers

[kernel test robot]

Hi,

Thanks for your patch.

FYI: kernel test robot notices the stable kernel rule is not satisfied.

The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1

Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree.
Subject: [PATCH v2] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers
Link: https://lore.kernel.org/stable/20231010084455.1718830-1-alain.volmat%40foss.st.com

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers

[Andi Shyti]

Hi Alain,

On Tue, Oct 10, 2023 at 10:44:54AM +0200, Alain Volmat wrote:
> In case of SMBUS byte read with PEC enabled, the whole transfer
> is split into two commands.  A first write command, followed by
> a read command.  The write command does not have any PEC byte
> and a PEC byte is appended at the end of the read command.
> (cf Read byte protocol with PEC in SMBUS specification)
> 
> Within the STM32 I2C controller, handling (either sending
> or receiving) of the PEC byte is done via the PECBYTE bit in
> register CR2.
> 
> Currently, the PECBYTE is set at the beginning of a transfer,
> which lead to sending a PEC byte at the end of the write command
> (hence losing the real last byte), and also does not check the
> PEC byte received during the read command.
> 
> This patch corrects the function stm32f7_i2c_smbus_xfer_msg
> in order to only set the PECBYTE during the read command.

Thanks for improving the log.

> Fixes: 9e48155f6bfe ("i2c: i2c-stm32f7: Add initial SMBus protocols support")
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
> Reviewed-by: Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com>

As this is a fix you should have also included and Cc'ed:

Cc: <stable@vger.kernel.org> # v4.18+

No need to resend.

Acked-by: Andi Shyti <andi.shyti@kernel.org> 

Thanks,
Andi

Re: [PATCH v2] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

On Tue, Oct 10, 2023 at 10:44:54AM +0200, Alain Volmat wrote:
> In case of SMBUS byte read with PEC enabled, the whole transfer
> is split into two commands.  A first write command, followed by
> a read command.  The write command does not have any PEC byte
> and a PEC byte is appended at the end of the read command.
> (cf Read byte protocol with PEC in SMBUS specification)
> 
> Within the STM32 I2C controller, handling (either sending
> or receiving) of the PEC byte is done via the PECBYTE bit in
> register CR2.
> 
> Currently, the PECBYTE is set at the beginning of a transfer,
> which lead to sending a PEC byte at the end of the write command
> (hence losing the real last byte), and also does not check the
> PEC byte received during the read command.
> 
> This patch corrects the function stm32f7_i2c_smbus_xfer_msg
> in order to only set the PECBYTE during the read command.
> 
> Fixes: 9e48155f6bfe ("i2c: i2c-stm32f7: Add initial SMBus protocols support")
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
> Reviewed-by: Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com>

Applied to for-current, thanks!

BTW, there are some patches pending from this series for stm32f4/7. Do
you have time for an ack?

http://patchwork.ozlabs.org/project/linux-i2c/list/?series=359230


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]