diff mbox series

[v4,4/4] i2c: stm32f4: Fix stmpe811 get xyz data timeout issue

Message ID 1591709203-12106-5-git-send-email-dillon.minfei@gmail.com
State Superseded
Headers show
Series [v4,1/4] ARM: dts: stm32: add I2C3 support on STM32F429 SoC | expand

Commit Message

Dillon Min June 9, 2020, 1:26 p.m. UTC
From: dillon min <dillon.minfei@gmail.com>

as stm32f429's internal flash is 2Mbytes and compiled kernel
image bigger than 2Mbytes, so we have to load kernel image
to sdram on stm32f429-disco board which has 8Mbytes sdram space.

based on above context, as you knows kernel running on external
sdram is more slower than internal flash. besides, we need read 4
bytes to get touch screen xyz(x, y, pressure) coordinate data in
stmpe811 interrupt.

so, in stm32f4_i2c_handle_rx_done, as i2c read slower than running
in xip mode, have to adjust 'STOP/START bit set position' from last
two bytes to last one bytes. else, will get i2c timeout in reading
touch screen coordinate.

to not take side effect, introduce IIC_LAST_BYTE_POS to support xip
kernel or has mmu platform.

Signed-off-by: dillon min <dillon.minfei@gmail.com>
---

V4: indroduce 'IIC_LAST_BYTE_POS' to compatible with xipkernel boot

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

Comments

Dillon Min March 15, 2021, 12:43 p.m. UTC | #1
Hi All,

Just a gentle ping.

Regards.

On Tue, Jun 9, 2020 at 9:27 PM <dillon.minfei@gmail.com> wrote:
>

> From: dillon min <dillon.minfei@gmail.com>

>

> as stm32f429's internal flash is 2Mbytes and compiled kernel

> image bigger than 2Mbytes, so we have to load kernel image

> to sdram on stm32f429-disco board which has 8Mbytes sdram space.

>

> based on above context, as you knows kernel running on external

> sdram is more slower than internal flash. besides, we need read 4

> bytes to get touch screen xyz(x, y, pressure) coordinate data in

> stmpe811 interrupt.

>

> so, in stm32f4_i2c_handle_rx_done, as i2c read slower than running

> in xip mode, have to adjust 'STOP/START bit set position' from last

> two bytes to last one bytes. else, will get i2c timeout in reading

> touch screen coordinate.

>

> to not take side effect, introduce IIC_LAST_BYTE_POS to support xip

> kernel or has mmu platform.

>

> Signed-off-by: dillon min <dillon.minfei@gmail.com>

> ---

>

> V4: indroduce 'IIC_LAST_BYTE_POS' to compatible with xipkernel boot

>

>  drivers/i2c/busses/i2c-stm32f4.c | 12 +++++++++---

>  1 file changed, 9 insertions(+), 3 deletions(-)

>

> diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c

> index d6a69dfcac3f..97cf42ae7fa0 100644

> --- a/drivers/i2c/busses/i2c-stm32f4.c

> +++ b/drivers/i2c/busses/i2c-stm32f4.c

> @@ -93,6 +93,12 @@

>  #define STM32F4_I2C_MAX_FREQ           46U

>  #define HZ_TO_MHZ                      1000000

>

> +#if !defined(CONFIG_MMU) && !defined(CONFIG_XIP_KERNEL)

> +#define IIC_LAST_BYTE_POS 1

> +#else

> +#define IIC_LAST_BYTE_POS 2

> +#endif

> +

>  /**

>   * struct stm32f4_i2c_msg - client specific data

>   * @addr: 8-bit slave addr, including r/w bit

> @@ -439,7 +445,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

>         int i;

>

>         switch (msg->count) {

> -       case 2:

> +       case IIC_LAST_BYTE_POS:

>                 /*

>                  * In order to correctly send the Stop or Repeated Start

>                  * condition on the I2C bus, the STOP/START bit has to be set

> @@ -454,7 +460,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

>                 else

>                         stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);

>

> -               for (i = 2; i > 0; i--)

> +               for (i = IIC_LAST_BYTE_POS; i > 0; i--)

>                         stm32f4_i2c_read_msg(i2c_dev);

>

>                 reg = i2c_dev->base + STM32F4_I2C_CR2;

> @@ -463,7 +469,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

>

>                 complete(&i2c_dev->complete);

>                 break;

> -       case 3:

> +       case (IIC_LAST_BYTE_POS+1):

>                 /*

>                  * In order to correctly generate the NACK pulse after the last

>                  * received data byte, we have to enable NACK before reading N-2

> --

> 2.7.4

>
Dillon Min May 7, 2021, 2:54 a.m. UTC | #2
Hi Pierre-Yves, Alain

Could you help to take a look?
i really appreciate it.

Thanks,

Best Regards
Dillon

On Mon, Mar 15, 2021 at 9:00 PM Wolfram Sang <wsa@kernel.org> wrote:
>

> On Mon, Mar 15, 2021 at 08:43:54PM +0800, dillon min wrote:

> > Hi All,

> >

> > Just a gentle ping.

>

> Pierre-Yves?

>

> >

> > Regards.

> >

> > On Tue, Jun 9, 2020 at 9:27 PM <dillon.minfei@gmail.com> wrote:

> > >

> > > From: dillon min <dillon.minfei@gmail.com>

> > >

> > > as stm32f429's internal flash is 2Mbytes and compiled kernel

> > > image bigger than 2Mbytes, so we have to load kernel image

> > > to sdram on stm32f429-disco board which has 8Mbytes sdram space.

> > >

> > > based on above context, as you knows kernel running on external

> > > sdram is more slower than internal flash. besides, we need read 4

> > > bytes to get touch screen xyz(x, y, pressure) coordinate data in

> > > stmpe811 interrupt.

> > >

> > > so, in stm32f4_i2c_handle_rx_done, as i2c read slower than running

> > > in xip mode, have to adjust 'STOP/START bit set position' from last

> > > two bytes to last one bytes. else, will get i2c timeout in reading

> > > touch screen coordinate.

> > >

> > > to not take side effect, introduce IIC_LAST_BYTE_POS to support xip

> > > kernel or has mmu platform.

> > >

> > > Signed-off-by: dillon min <dillon.minfei@gmail.com>

> > > ---

> > >

> > > V4: indroduce 'IIC_LAST_BYTE_POS' to compatible with xipkernel boot

> > >

> > >  drivers/i2c/busses/i2c-stm32f4.c | 12 +++++++++---

> > >  1 file changed, 9 insertions(+), 3 deletions(-)

> > >

> > > diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c

> > > index d6a69dfcac3f..97cf42ae7fa0 100644

> > > --- a/drivers/i2c/busses/i2c-stm32f4.c

> > > +++ b/drivers/i2c/busses/i2c-stm32f4.c

> > > @@ -93,6 +93,12 @@

> > >  #define STM32F4_I2C_MAX_FREQ           46U

> > >  #define HZ_TO_MHZ                      1000000

> > >

> > > +#if !defined(CONFIG_MMU) && !defined(CONFIG_XIP_KERNEL)

> > > +#define IIC_LAST_BYTE_POS 1

> > > +#else

> > > +#define IIC_LAST_BYTE_POS 2

> > > +#endif

> > > +

> > >  /**

> > >   * struct stm32f4_i2c_msg - client specific data

> > >   * @addr: 8-bit slave addr, including r/w bit

> > > @@ -439,7 +445,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

> > >         int i;

> > >

> > >         switch (msg->count) {

> > > -       case 2:

> > > +       case IIC_LAST_BYTE_POS:

> > >                 /*

> > >                  * In order to correctly send the Stop or Repeated Start

> > >                  * condition on the I2C bus, the STOP/START bit has to be set

> > > @@ -454,7 +460,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

> > >                 else

> > >                         stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);

> > >

> > > -               for (i = 2; i > 0; i--)

> > > +               for (i = IIC_LAST_BYTE_POS; i > 0; i--)

> > >                         stm32f4_i2c_read_msg(i2c_dev);

> > >

> > >                 reg = i2c_dev->base + STM32F4_I2C_CR2;

> > > @@ -463,7 +469,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

> > >

> > >                 complete(&i2c_dev->complete);

> > >                 break;

> > > -       case 3:

> > > +       case (IIC_LAST_BYTE_POS+1):

> > >                 /*

> > >                  * In order to correctly generate the NACK pulse after the last

> > >                  * received data byte, we have to enable NACK before reading N-2

> > > --

> > > 2.7.4

> > >
Patrice CHOTARD May 7, 2021, 12:29 p.m. UTC | #3
Hi Dillon

In order to test this patch, it's tricky to make a recent kernel running 
on stm32f429-disco as this board embeds only 8MB of SDRAM and 2MB of flash.

Can you indicates us which kernel version you are using and also the kernel config please ?

Thanks

Patrice

On 5/7/21 4:54 AM, dillon min wrote:
> Hi Pierre-Yves, Alain

> 

> Could you help to take a look?

> i really appreciate it.

> 

> Thanks,

> 

> Best Regards

> Dillon

> 

> On Mon, Mar 15, 2021 at 9:00 PM Wolfram Sang <wsa@kernel.org> wrote:

>>

>> On Mon, Mar 15, 2021 at 08:43:54PM +0800, dillon min wrote:

>>> Hi All,

>>>

>>> Just a gentle ping.

>>

>> Pierre-Yves?

>>

>>>

>>> Regards.

>>>

>>> On Tue, Jun 9, 2020 at 9:27 PM <dillon.minfei@gmail.com> wrote:

>>>>

>>>> From: dillon min <dillon.minfei@gmail.com>

>>>>

>>>> as stm32f429's internal flash is 2Mbytes and compiled kernel

>>>> image bigger than 2Mbytes, so we have to load kernel image

>>>> to sdram on stm32f429-disco board which has 8Mbytes sdram space.

>>>>

>>>> based on above context, as you knows kernel running on external

>>>> sdram is more slower than internal flash. besides, we need read 4

>>>> bytes to get touch screen xyz(x, y, pressure) coordinate data in

>>>> stmpe811 interrupt.

>>>>

>>>> so, in stm32f4_i2c_handle_rx_done, as i2c read slower than running

>>>> in xip mode, have to adjust 'STOP/START bit set position' from last

>>>> two bytes to last one bytes. else, will get i2c timeout in reading

>>>> touch screen coordinate.

>>>>

>>>> to not take side effect, introduce IIC_LAST_BYTE_POS to support xip

>>>> kernel or has mmu platform.

>>>>

>>>> Signed-off-by: dillon min <dillon.minfei@gmail.com>

>>>> ---

>>>>

>>>> V4: indroduce 'IIC_LAST_BYTE_POS' to compatible with xipkernel boot

>>>>

>>>>  drivers/i2c/busses/i2c-stm32f4.c | 12 +++++++++---

>>>>  1 file changed, 9 insertions(+), 3 deletions(-)

>>>>

>>>> diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c

>>>> index d6a69dfcac3f..97cf42ae7fa0 100644

>>>> --- a/drivers/i2c/busses/i2c-stm32f4.c

>>>> +++ b/drivers/i2c/busses/i2c-stm32f4.c

>>>> @@ -93,6 +93,12 @@

>>>>  #define STM32F4_I2C_MAX_FREQ           46U

>>>>  #define HZ_TO_MHZ                      1000000

>>>>

>>>> +#if !defined(CONFIG_MMU) && !defined(CONFIG_XIP_KERNEL)

>>>> +#define IIC_LAST_BYTE_POS 1

>>>> +#else

>>>> +#define IIC_LAST_BYTE_POS 2

>>>> +#endif

>>>> +

>>>>  /**

>>>>   * struct stm32f4_i2c_msg - client specific data

>>>>   * @addr: 8-bit slave addr, including r/w bit

>>>> @@ -439,7 +445,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

>>>>         int i;

>>>>

>>>>         switch (msg->count) {

>>>> -       case 2:

>>>> +       case IIC_LAST_BYTE_POS:

>>>>                 /*

>>>>                  * In order to correctly send the Stop or Repeated Start

>>>>                  * condition on the I2C bus, the STOP/START bit has to be set

>>>> @@ -454,7 +460,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

>>>>                 else

>>>>                         stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);

>>>>

>>>> -               for (i = 2; i > 0; i--)

>>>> +               for (i = IIC_LAST_BYTE_POS; i > 0; i--)

>>>>                         stm32f4_i2c_read_msg(i2c_dev);

>>>>

>>>>                 reg = i2c_dev->base + STM32F4_I2C_CR2;

>>>> @@ -463,7 +469,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

>>>>

>>>>                 complete(&i2c_dev->complete);

>>>>                 break;

>>>> -       case 3:

>>>> +       case (IIC_LAST_BYTE_POS+1):

>>>>                 /*

>>>>                  * In order to correctly generate the NACK pulse after the last

>>>>                  * received data byte, we have to enable NACK before reading N-2

>>>> --

>>>> 2.7.4

>>>>

> _______________________________________________

> Linux-stm32 mailing list

> Linux-stm32@st-md-mailman.stormreply.com

> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32

>
Dillon Min May 7, 2021, 12:47 p.m. UTC | #4
Hi Patrice

Thanks for the reply.

On Fri, May 7, 2021 at 8:29 PM Patrice CHOTARD
<patrice.chotard@foss.st.com> wrote:
>

> Hi Dillon

>

> In order to test this patch, it's tricky to make a recent kernel running

> on stm32f429-disco as this board embeds only 8MB of SDRAM and 2MB of flash.

>

> Can you indicates us which kernel version you are using and also the kernel config please ?


Since this patch was submitted in last year, might it be 5.0?  not for sure
Anyway, I will resubmit it in the recent kernel version with the kernel config
and a detailed test step for you.

Thanks for your time.

>

> Thanks

>

> Patrice

>

> On 5/7/21 4:54 AM, dillon min wrote:

> > Hi Pierre-Yves, Alain

> >

> > Could you help to take a look?

> > i really appreciate it.

> >

> > Thanks,

> >

> > Best Regards

> > Dillon

> >

> > On Mon, Mar 15, 2021 at 9:00 PM Wolfram Sang <wsa@kernel.org> wrote:

> >>

> >> On Mon, Mar 15, 2021 at 08:43:54PM +0800, dillon min wrote:

> >>> Hi All,

> >>>

> >>> Just a gentle ping.

> >>

> >> Pierre-Yves?

> >>

> >>>

> >>> Regards.

> >>>

> >>> On Tue, Jun 9, 2020 at 9:27 PM <dillon.minfei@gmail.com> wrote:

> >>>>

> >>>> From: dillon min <dillon.minfei@gmail.com>

> >>>>

> >>>> as stm32f429's internal flash is 2Mbytes and compiled kernel

> >>>> image bigger than 2Mbytes, so we have to load kernel image

> >>>> to sdram on stm32f429-disco board which has 8Mbytes sdram space.

> >>>>

> >>>> based on above context, as you knows kernel running on external

> >>>> sdram is more slower than internal flash. besides, we need read 4

> >>>> bytes to get touch screen xyz(x, y, pressure) coordinate data in

> >>>> stmpe811 interrupt.

> >>>>

> >>>> so, in stm32f4_i2c_handle_rx_done, as i2c read slower than running

> >>>> in xip mode, have to adjust 'STOP/START bit set position' from last

> >>>> two bytes to last one bytes. else, will get i2c timeout in reading

> >>>> touch screen coordinate.

> >>>>

> >>>> to not take side effect, introduce IIC_LAST_BYTE_POS to support xip

> >>>> kernel or has mmu platform.

> >>>>

> >>>> Signed-off-by: dillon min <dillon.minfei@gmail.com>

> >>>> ---

> >>>>

> >>>> V4: indroduce 'IIC_LAST_BYTE_POS' to compatible with xipkernel boot

> >>>>

> >>>>  drivers/i2c/busses/i2c-stm32f4.c | 12 +++++++++---

> >>>>  1 file changed, 9 insertions(+), 3 deletions(-)

> >>>>

> >>>> diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c

> >>>> index d6a69dfcac3f..97cf42ae7fa0 100644

> >>>> --- a/drivers/i2c/busses/i2c-stm32f4.c

> >>>> +++ b/drivers/i2c/busses/i2c-stm32f4.c

> >>>> @@ -93,6 +93,12 @@

> >>>>  #define STM32F4_I2C_MAX_FREQ           46U

> >>>>  #define HZ_TO_MHZ                      1000000

> >>>>

> >>>> +#if !defined(CONFIG_MMU) && !defined(CONFIG_XIP_KERNEL)

> >>>> +#define IIC_LAST_BYTE_POS 1

> >>>> +#else

> >>>> +#define IIC_LAST_BYTE_POS 2

> >>>> +#endif

> >>>> +

> >>>>  /**

> >>>>   * struct stm32f4_i2c_msg - client specific data

> >>>>   * @addr: 8-bit slave addr, including r/w bit

> >>>> @@ -439,7 +445,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

> >>>>         int i;

> >>>>

> >>>>         switch (msg->count) {

> >>>> -       case 2:

> >>>> +       case IIC_LAST_BYTE_POS:

> >>>>                 /*

> >>>>                  * In order to correctly send the Stop or Repeated Start

> >>>>                  * condition on the I2C bus, the STOP/START bit has to be set

> >>>> @@ -454,7 +460,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

> >>>>                 else

> >>>>                         stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);

> >>>>

> >>>> -               for (i = 2; i > 0; i--)

> >>>> +               for (i = IIC_LAST_BYTE_POS; i > 0; i--)

> >>>>                         stm32f4_i2c_read_msg(i2c_dev);

> >>>>

> >>>>                 reg = i2c_dev->base + STM32F4_I2C_CR2;

> >>>> @@ -463,7 +469,7 @@ static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)

> >>>>

> >>>>                 complete(&i2c_dev->complete);

> >>>>                 break;

> >>>> -       case 3:

> >>>> +       case (IIC_LAST_BYTE_POS+1):

> >>>>                 /*

> >>>>                  * In order to correctly generate the NACK pulse after the last

> >>>>                  * received data byte, we have to enable NACK before reading N-2

> >>>> --

> >>>> 2.7.4

> >>>>

> > _______________________________________________

> > Linux-stm32 mailing list

> > Linux-stm32@st-md-mailman.stormreply.com

> > https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32

> >


---
Best regards
Dillon,
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c
index d6a69dfcac3f..97cf42ae7fa0 100644
--- a/drivers/i2c/busses/i2c-stm32f4.c
+++ b/drivers/i2c/busses/i2c-stm32f4.c
@@ -93,6 +93,12 @@ 
 #define STM32F4_I2C_MAX_FREQ		46U
 #define HZ_TO_MHZ			1000000
 
+#if !defined(CONFIG_MMU) && !defined(CONFIG_XIP_KERNEL)
+#define IIC_LAST_BYTE_POS 1
+#else
+#define IIC_LAST_BYTE_POS 2
+#endif
+
 /**
  * struct stm32f4_i2c_msg - client specific data
  * @addr: 8-bit slave addr, including r/w bit
@@ -439,7 +445,7 @@  static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)
 	int i;
 
 	switch (msg->count) {
-	case 2:
+	case IIC_LAST_BYTE_POS:
 		/*
 		 * In order to correctly send the Stop or Repeated Start
 		 * condition on the I2C bus, the STOP/START bit has to be set
@@ -454,7 +460,7 @@  static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)
 		else
 			stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
 
-		for (i = 2; i > 0; i--)
+		for (i = IIC_LAST_BYTE_POS; i > 0; i--)
 			stm32f4_i2c_read_msg(i2c_dev);
 
 		reg = i2c_dev->base + STM32F4_I2C_CR2;
@@ -463,7 +469,7 @@  static void stm32f4_i2c_handle_rx_done(struct stm32f4_i2c_dev *i2c_dev)
 
 		complete(&i2c_dev->complete);
 		break;
-	case 3:
+	case (IIC_LAST_BYTE_POS+1):
 		/*
 		 * In order to correctly generate the NACK pulse after the last
 		 * received data byte, we have to enable NACK before reading N-2