Patchwork [v2] i2c: i2c-mxs: Use DMA mode even for small transfers

login
register
mail settings
Submitter Fabio Estevam
Date July 1, 2013, 9:14 p.m.
Message ID <1372713261-20551-1-git-send-email-festevam@gmail.com>
Download mbox | patch
Permalink /patch/256225/
State Accepted
Headers show

Comments

Fabio Estevam - July 1, 2013, 9:14 p.m.
From: Fabio Estevam <fabio.estevam@freescale.com>

Recently we have been seing some reports about PIO mode not working properly.

- http://www.spinics.net/lists/linux-i2c/msg11985.html
- http://marc.info/?l=linux-i2c&m=137235593101385&w=2
- https://lkml.org/lkml/2013/6/24/430

Let's use DMA mode even for small transfers.

Without this patch, i2c reads the incorrect sgtl5000 version on a mx28evk when
touchscreen is enabled:
                                           
[    5.856270] sgtl5000 0-000a: Device with ID register 0 is not a sgtl5000     
[    9.877307] sgtl5000 0-000a: ASoC: failed to probe CODEC -19                 
[    9.883528] mxs-sgtl5000 sound.12: ASoC: failed to instantiate card -19      
[    9.892955] mxs-sgtl5000 sound.12: snd_soc_register_card failed (-19)  

Cc: <stable@vger.kernel.org>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Applies against 3.10

Changes since v1:
- Keep the PIO code

 drivers/i2c/busses/i2c-mxs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Shawn Guo - July 2, 2013, 2:23 a.m.
On Mon, Jul 01, 2013 at 06:14:21PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Recently we have been seing some reports about PIO mode not working properly.
> 
> - http://www.spinics.net/lists/linux-i2c/msg11985.html
> - http://marc.info/?l=linux-i2c&m=137235593101385&w=2
> - https://lkml.org/lkml/2013/6/24/430
> 
It also causes sgtl5000 register write failure [1].  Changing it back to
DMA mode fixes the failure.  So,

Acked-by: Shawn Guo <shawn.guo@linaro.org>

[1] http://thread.gmane.org/gmane.linux.alsa.devel/109715/focus=109747

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lucas Stach - July 2, 2013, 8:16 a.m.
Am Montag, den 01.07.2013, 18:14 -0300 schrieb Fabio Estevam:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Recently we have been seing some reports about PIO mode not working properly.
> 
> - http://www.spinics.net/lists/linux-i2c/msg11985.html
> - http://marc.info/?l=linux-i2c&m=137235593101385&w=2
> - https://lkml.org/lkml/2013/6/24/430
> 
> Let's use DMA mode even for small transfers.
> 
> Without this patch, i2c reads the incorrect sgtl5000 version on a mx28evk when
> touchscreen is enabled:
>                                            
> [    5.856270] sgtl5000 0-000a: Device with ID register 0 is not a sgtl5000     
> [    9.877307] sgtl5000 0-000a: ASoC: failed to probe CODEC -19                 
> [    9.883528] mxs-sgtl5000 sound.12: ASoC: failed to instantiate card -19      
> [    9.892955] mxs-sgtl5000 sound.12: snd_soc_register_card failed (-19)  
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> Applies against 3.10
> 
> Changes since v1:
> - Keep the PIO code
> 
>  drivers/i2c/busses/i2c-mxs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index 2039f23..6d8094d 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -494,7 +494,7 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
>  	 * based on this empirical measurement and a lot of previous frobbing.
>  	 */
>  	i2c->cmd_err = 0;
> -	if (msg->len < 8) {
> +	if (0) {	/* disable PIO mode until a proper fix is made */
>  		ret = mxs_i2c_pio_setup_xfer(adap, msg, flags);
>  		if (ret)
>  			mxs_i2c_reset(i2c);

I still plan to take another look at the PIO mode, but higher priority
things keep popping up in front of me. So this patch is:
Acked-by: Lucas Stach <l.stach@pengutronix.de>
Marek Vasut - July 3, 2013, 2:38 a.m.
Dear Lucas Stach,

> Am Montag, den 01.07.2013, 18:14 -0300 schrieb Fabio Estevam:
> > From: Fabio Estevam <fabio.estevam@freescale.com>
> > 
> > Recently we have been seing some reports about PIO mode not working
> > properly.
> > 
> > - http://www.spinics.net/lists/linux-i2c/msg11985.html
> > - http://marc.info/?l=linux-i2c&m=137235593101385&w=2
> > - https://lkml.org/lkml/2013/6/24/430
> > 
> > Let's use DMA mode even for small transfers.
> > 
> > Without this patch, i2c reads the incorrect sgtl5000 version on a mx28evk
> > when touchscreen is enabled:
> > 
> > [    5.856270] sgtl5000 0-000a: Device with ID register 0 is not a
> > sgtl5000 [    9.877307] sgtl5000 0-000a: ASoC: failed to probe CODEC -19
> > [    9.883528] mxs-sgtl5000 sound.12: ASoC: failed to instantiate card
> > -19 [    9.892955] mxs-sgtl5000 sound.12: snd_soc_register_card failed
> > (-19)
> > 
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> > ---
> > Applies against 3.10
> > 
> > Changes since v1:
> > - Keep the PIO code
> > 
> >  drivers/i2c/busses/i2c-mxs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> > index 2039f23..6d8094d 100644
> > --- a/drivers/i2c/busses/i2c-mxs.c
> > +++ b/drivers/i2c/busses/i2c-mxs.c
> > @@ -494,7 +494,7 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap,
> > struct i2c_msg *msg,
> > 
> >  	 * based on this empirical measurement and a lot of previous frobbing.
> >  	 */
> >  	
> >  	i2c->cmd_err = 0;
> > 
> > -	if (msg->len < 8) {
> > +	if (0) {	/* disable PIO mode until a proper fix is made */
> > 
> >  		ret = mxs_i2c_pio_setup_xfer(adap, msg, flags);
> >  		if (ret)
> >  		
> >  			mxs_i2c_reset(i2c);
> 
> I still plan to take another look at the PIO mode, but higher priority
> things keep popping up in front of me. So this patch is:
> Acked-by: Lucas Stach <l.stach@pengutronix.de>

Ok, update. So far I arrived at a point where I can avoid the PIO trouble.

If I only do transfers shorter or equal in length to 3 bytes via PIO, it works 
as expected. If the transfer is longer, the LA shows very long transfer 
happening actually. Therefore, the pumping of data in loop to/from PIO via the 
DATA register doesn't work.

I will update you more later, once I figure out something else. Now I need some 
sleep.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Fabio Estevam - July 15, 2013, 1:30 p.m.
Hi Wolfram,

On Mon, Jul 1, 2013 at 11:23 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Mon, Jul 01, 2013 at 06:14:21PM -0300, Fabio Estevam wrote:
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Recently we have been seing some reports about PIO mode not working properly.
>>
>> - http://www.spinics.net/lists/linux-i2c/msg11985.html
>> - http://marc.info/?l=linux-i2c&m=137235593101385&w=2
>> - https://lkml.org/lkml/2013/6/24/430
>>
> It also causes sgtl5000 register write failure [1].  Changing it back to
> DMA mode fixes the failure.  So,
>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
>
> [1] http://thread.gmane.org/gmane.linux.alsa.devel/109715/focus=109747
>

Any comment about this patch, please?

If you are happy with it, could we please have it applied into stable?

3.10 is currently broken without this patch.

Regards,

Fabio Estevam
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut - July 15, 2013, 2:12 p.m.
Dear Fabio Estevam,

> Hi Wolfram,
> 
> On Mon, Jul 1, 2013 at 11:23 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > On Mon, Jul 01, 2013 at 06:14:21PM -0300, Fabio Estevam wrote:
> >> From: Fabio Estevam <fabio.estevam@freescale.com>
> >> 
> >> Recently we have been seing some reports about PIO mode not working
> >> properly.
> >> 
> >> - http://www.spinics.net/lists/linux-i2c/msg11985.html
> >> - http://marc.info/?l=linux-i2c&m=137235593101385&w=2
> >> - https://lkml.org/lkml/2013/6/24/430
> > 
> > It also causes sgtl5000 register write failure [1].  Changing it back to
> > DMA mode fixes the failure.  So,
> > 
> > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > 
> > [1] http://thread.gmane.org/gmane.linux.alsa.devel/109715/focus=109747
> 
> Any comment about this patch, please?
> 
> If you are happy with it, could we please have it applied into stable?
> 
> 3.10 is currently broken without this patch.

Hi, either this or the PIO fix, but that's a RFC and quite a big slab of change 
which still needs some work. So please apply this and let's try the PIOfix for 
3.11 or 3.12.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Fabio Estevam - July 23, 2013, 6:15 p.m.
Wolfram,

On Mon, Jul 15, 2013 at 10:30 AM, Fabio Estevam <festevam@gmail.com> wrote:

> Any comment about this patch, please?
>
> If you are happy with it, could we please have it applied into stable?
>
> 3.10 is currently broken without this patch.

Ping?
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Vasut - July 24, 2013, 3:02 a.m.
Dear Fabio Estevam,

> Wolfram,
> 
> On Mon, Jul 15, 2013 at 10:30 AM, Fabio Estevam <festevam@gmail.com> wrote:
> > Any comment about this patch, please?
> > 
> > If you are happy with it, could we please have it applied into stable?
> > 
> > 3.10 is currently broken without this patch.

Yes, the big question is if we want to use this patch or fix the PIO. I think 
this one is much less intrusive, so I'd vouch for yours for 3.10 .

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Fabio Estevam - July 24, 2013, 3:09 a.m.
Hi Marek,

On Wed, Jul 24, 2013 at 12:02 AM, Marek Vasut <marex@denx.de> wrote:

> Yes, the big question is if we want to use this patch or fix the PIO. I think
> this one is much less intrusive, so I'd vouch for yours for 3.10 .

Correct. Also according to Documentation/stable_kernel_rules.txt :

"Rules on what kind of patches are accepted, and which ones are not, into the
"-stable" tree:

....
 - It cannot be bigger than 100 lines, with context."

Your PIO fix is really appreciated, but it surpasses 100 lines, so it
would be better if we go with this one first so that it can reach
3.10, and then we can use your PIO fix later.

Regards,

Fabio Estevam
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings - July 26, 2013, 12:46 a.m.
On Wed, 2013-07-24 at 00:09 -0300, Fabio Estevam wrote:
> Hi Marek,
> 
> On Wed, Jul 24, 2013 at 12:02 AM, Marek Vasut <marex@denx.de> wrote:
> 
> > Yes, the big question is if we want to use this patch or fix the PIO. I think
> > this one is much less intrusive, so I'd vouch for yours for 3.10 .
> 
> Correct. Also according to Documentation/stable_kernel_rules.txt :
> 
> "Rules on what kind of patches are accepted, and which ones are not, into the
> "-stable" tree:
> 
> ....
>  - It cannot be bigger than 100 lines, with context."
> 
> Your PIO fix is really appreciated, but it surpasses 100 lines, so it
> would be better if we go with this one first so that it can reach
> 3.10, and then we can use your PIO fix later.

That one is not a hard and fast rule, but small localised fixes are
preferred if possible.

Ben.
Marek Vasut - July 26, 2013, 4:20 a.m.
Dear Ben Hutchings,

> On Wed, 2013-07-24 at 00:09 -0300, Fabio Estevam wrote:
> > Hi Marek,
> > 
> > On Wed, Jul 24, 2013 at 12:02 AM, Marek Vasut <marex@denx.de> wrote:
> > > Yes, the big question is if we want to use this patch or fix the PIO. I
> > > think this one is much less intrusive, so I'd vouch for yours for 3.10
> > > .
> > 
> > Correct. Also according to Documentation/stable_kernel_rules.txt :
> > 
> > "Rules on what kind of patches are accepted, and which ones are not, into
> > the "-stable" tree:
> > 
> > ....
> > 
> >  - It cannot be bigger than 100 lines, with context."
> > 
> > Your PIO fix is really appreciated, but it surpasses 100 lines, so it
> > would be better if we go with this one first so that it can reach
> > 3.10, and then we can use your PIO fix later.
> 
> That one is not a hard and fast rule, but small localised fixes are
> preferred if possible.

Full agreement on this one, let's apply this patch.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang - Aug. 5, 2013, 8:11 a.m.
On Mon, Jul 01, 2013 at 06:14:21PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Recently we have been seing some reports about PIO mode not working properly.
> 
> - http://www.spinics.net/lists/linux-i2c/msg11985.html
> - http://marc.info/?l=linux-i2c&m=137235593101385&w=2
> - https://lkml.org/lkml/2013/6/24/430
> 
> Let's use DMA mode even for small transfers.
> 
> Without this patch, i2c reads the incorrect sgtl5000 version on a mx28evk when
> touchscreen is enabled:
>                                            
> [    5.856270] sgtl5000 0-000a: Device with ID register 0 is not a sgtl5000     
> [    9.877307] sgtl5000 0-000a: ASoC: failed to probe CODEC -19                 
> [    9.883528] mxs-sgtl5000 sound.12: ASoC: failed to instantiate card -19      
> [    9.892955] mxs-sgtl5000 sound.12: snd_soc_register_card failed (-19)  
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Applied to for-current, thanks!

Patch

diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 2039f23..6d8094d 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -494,7 +494,7 @@  static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 	 * based on this empirical measurement and a lot of previous frobbing.
 	 */
 	i2c->cmd_err = 0;
-	if (msg->len < 8) {
+	if (0) {	/* disable PIO mode until a proper fix is made */
 		ret = mxs_i2c_pio_setup_xfer(adap, msg, flags);
 		if (ret)
 			mxs_i2c_reset(i2c);