* [PATCH] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers
@ 2023-10-02 8:42 Alain Volmat
2023-10-03 7:50 ` Pierre Yves MORDRET
2023-10-03 17:42 ` Andi Shyti
0 siblings, 2 replies; 4+ messages in thread
From: Alain Volmat @ 2023-10-02 8:42 UTC (permalink / raw
To: Pierre-Yves MORDRET, Alain Volmat, Andi Shyti, Maxime Coquelin,
Alexandre Torgue, M'boumba Cedric Madianga, Wolfram Sang
Cc: Pierre-Yves MORDRET, linux-i2c, linux-stm32, linux-arm-kernel,
linux-kernel
The PECBYTE bit allows to generate (in case of write) or
compute/compare the PEC byte (in case of read). In case
of reading a value (performed by first sending a write
command, then followed by a read command) the PECBYTE should
only be set before starting the read command and not before
the first write command.
Fixes: 9e48155f6bfe ("i2c: i2c-stm32f7: Add initial SMBus protocols support")
Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers
2023-10-02 8:42 [PATCH] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers Alain Volmat
@ 2023-10-03 7:50 ` Pierre Yves MORDRET
2023-10-03 17:42 ` Andi Shyti
1 sibling, 0 replies; 4+ messages in thread
From: Pierre Yves MORDRET @ 2023-10-03 7:50 UTC (permalink / raw
To: Alain Volmat, Andi Shyti, Maxime Coquelin, Alexandre Torgue,
M'boumba Cedric Madianga, Wolfram Sang
Cc: Pierre-Yves MORDRET, linux-i2c, linux-stm32, linux-arm-kernel,
linux-kernel
Hi Alain,
Sounds good to me
Reviewed-by: Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com>
Regards
On 10/2/23 10:42, Alain Volmat wrote:
> The PECBYTE bit allows to generate (in case of write) or
> compute/compare the PEC byte (in case of read). In case
> of reading a value (performed by first sending a write
> command, then followed by a read command) the PECBYTE should
> only be set before starting the read command and not before
> the first write command.
>
> Fixes: 9e48155f6bfe ("i2c: i2c-stm32f7: Add initial SMBus protocols support")
>
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
> ---
> 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);
--
--
~ Py MORDRET
--
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers
2023-10-02 8:42 [PATCH] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers Alain Volmat
2023-10-03 7:50 ` Pierre Yves MORDRET
@ 2023-10-03 17:42 ` Andi Shyti
2023-10-05 7:19 ` Alain Volmat
1 sibling, 1 reply; 4+ messages in thread
From: Andi Shyti @ 2023-10-03 17:42 UTC (permalink / raw
To: Alain Volmat
Cc: Pierre-Yves MORDRET, Maxime Coquelin, Alexandre Torgue,
M'boumba Cedric Madianga, Wolfram Sang, Pierre-Yves MORDRET,
linux-i2c, linux-stm32, linux-arm-kernel, linux-kernel
Hi Alain,
On Mon, Oct 02, 2023 at 10:42:10AM +0200, Alain Volmat wrote:
> The PECBYTE bit allows to generate (in case of write) or
> compute/compare the PEC byte (in case of read). In case
> of reading a value (performed by first sending a write
> command, then followed by a read command) the PECBYTE should
> only be set before starting the read command and not before
> the first write command.
What is this patch fixing?
Can you please point this detail in the documentation, I haven't
found it[*]
> Fixes: 9e48155f6bfe ("i2c: i2c-stm32f7: Add initial SMBus protocols support")
>
> Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
please, don't leave blank lines between tags.
Thanks,
Andi
[*] Hope this is the correct one:
https://www.st.com/resource/en/reference_manual/rm0385-stm32f75xxx-and-stm32f74xxx-advanced-armbased-32bit-mcus-stmicroelectronics.pdf
> ---
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers
2023-10-03 17:42 ` Andi Shyti
@ 2023-10-05 7:19 ` Alain Volmat
0 siblings, 0 replies; 4+ messages in thread
From: Alain Volmat @ 2023-10-05 7:19 UTC (permalink / raw
To: Andi Shyti
Cc: Pierre-Yves MORDRET, Maxime Coquelin, Alexandre Torgue,
M'boumba Cedric Madianga, Wolfram Sang, Pierre-Yves MORDRET,
linux-i2c, linux-stm32, linux-arm-kernel, linux-kernel
Hi Andi,
Thanks for the review.
On Tue, Oct 03, 2023 at 07:42:46PM +0200, Andi Shyti wrote:
> Hi Alain,
>
> On Mon, Oct 02, 2023 at 10:42:10AM +0200, Alain Volmat wrote:
> > The PECBYTE bit allows to generate (in case of write) or
> > compute/compare the PEC byte (in case of read). In case
> > of reading a value (performed by first sending a write
> > command, then followed by a read command) the PECBYTE should
> > only be set before starting the read command and not before
> > the first write command.
>
> What is this patch fixing?
>
> Can you please point this detail in the documentation, I haven't
> found it[*]
This is about the handling of the PECBYTE bit of the I2C_CR2 register
(cf page 1010 of the spec you pointed). There were no issue in case
of performing SMBUS write (with PEC), however read was not working.
PECBYTE was set from the very beginning of the transaction, but since
SMBUS read is first made of a write transfer, followed by a read transfer,
the PECBYTE was appended to the end of the write transfer (instead of the read
transfer), leading to lose of the last byte of the write transfer.
(in addition to the fact that the PEC byte should NOT be placed at the
end of the write transfer).
(cf Figure 30 of SMBUS specification [1]).
I could add more information within the commit log if you prefer.
[1] http://www.smbus.org/specs/SMBus_3_2_20220112.pdf
>
> > Fixes: 9e48155f6bfe ("i2c: i2c-stm32f7: Add initial SMBus protocols support")
> >
> > Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
>
> please, don't leave blank lines between tags.
Ok, will remove this blank line within a v2.
Thanks,
Alain
>
> Thanks,
> Andi
>
> [*] Hope this is the correct one:
> https://www.st.com/resource/en/reference_manual/rm0385-stm32f75xxx-and-stm32f74xxx-advanced-armbased-32bit-mcus-stmicroelectronics.pdf
>
> > ---
> > 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
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-05 14:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-02 8:42 [PATCH] i2c: stm32f7: Fix PEC handling in case of SMBUS transfers Alain Volmat
2023-10-03 7:50 ` Pierre Yves MORDRET
2023-10-03 17:42 ` Andi Shyti
2023-10-05 7:19 ` Alain Volmat
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).