* possible MXS-i2c bug
@ 2012-04-26 11:26 Marek Vasut
2012-04-26 11:41 ` Fabio Estevam
[not found] ` <201204261326.29388.marex-ynQEQJNshbs@public.gmane.org>
0 siblings, 2 replies; 18+ messages in thread
From: Marek Vasut @ 2012-04-26 11:26 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as expected.
Apparently, there might be some timing issue, when I put printk() into the at24
driver eeprom write function, it "fixed" itself. Though, replacing the i2c
driver with i2c gpio also fixed the issue, so I suspect the i2c driver has some
flaw in it.
Further investigation turned out, that the complete() isn't called in the mxs-
i2c driver irq handler unless I put the printk(). Putting the printk() in the
xfer function of the mxs-i2c driver instead of eeprom write function has the
same effect -- "fixing" it.
I didn't have time to properly investigate this matter, so consider this as a
quick and dirty bugreport (maybe?). But thie behavior I saw it quite easily
replicated, try:
dd if=/dev/zero of=/sys/bus/devices/0-00XY/eeprom bs=<sizeof eeprom> count=1
And the write will timeout.
Has anyone recently tested the driver with such eeprom? Anything from the 24xxx
series with 16 bit addresses has this problem. I tested and saw this on more
than one board, so this is not hardware issue.
If noone has any fixes already, I'll debug this tonight or so.
Thanks!
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 18+ messages in thread* possible MXS-i2c bug 2012-04-26 11:26 possible MXS-i2c bug Marek Vasut @ 2012-04-26 11:41 ` Fabio Estevam [not found] ` <201204261326.29388.marex-ynQEQJNshbs@public.gmane.org> 1 sibling, 0 replies; 18+ messages in thread From: Fabio Estevam @ 2012-04-26 11:41 UTC (permalink / raw) To: linux-arm-kernel On Thu, Apr 26, 2012 at 8:26 AM, Marek Vasut <marex@denx.de> wrote: > Hi, > > I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as expected. > > Apparently, there might be some timing issue, when I put printk() into the at24 > driver eeprom write function, it "fixed" itself. Though, replacing the i2c > driver with i2c gpio also fixed the issue, so I suspect the i2c driver has some > flaw in it. Does Wolfram's patch help? http://www.spinics.net/lists/linux-i2c/msg08002.html I don't have a I2C eeprom soldered on my mx28evk to test it. ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <201204261326.29388.marex-ynQEQJNshbs@public.gmane.org>]
* Re: possible MXS-i2c bug 2012-04-26 11:26 possible MXS-i2c bug Marek Vasut @ 2012-04-26 11:42 ` Wolfram Sang [not found] ` <201204261326.29388.marex-ynQEQJNshbs@public.gmane.org> 1 sibling, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2012-04-26 11:42 UTC (permalink / raw) To: Marek Vasut Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Fabio Estevam, Shawn Guo, linux-i2c-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 1562 bytes --] Hi Marek, you forgot the most important list, linux-i2c ;) > I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as expected. Yes, we had the same this week/last week. > Apparently, there might be some timing issue, when I put printk() into the at24 > driver eeprom write function, it "fixed" itself. Though, replacing the i2c > driver with i2c gpio also fixed the issue, so I suspect the i2c driver has some > flaw in it. Try removing the printk and use the module paramter "io_limit=24" for at24. Does this help? (Or bs<=24 with dd) > If noone has any fixes already, I'll debug this tonight or so. The write FIFO overflows, since it only can carry 8 words. There is currently no check for that. One solution would be to check the FIFO status and fill them as needed. This will probably require additional locking, so I think the proper solution would be to use DMA for transfers bigger than the write FIFO size (we would get MX23 support then, too). Sadly, we don't have DMA support yet and I don't have time to implement it. I am thinking if such transfers which would fail anyway, should currently be rejected or if the driver should depend on BROKEN or both. I am still undecided, though. Please also check the patch I sent yesterday, there is also another one coming after I get confirmation from a customer. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* possible MXS-i2c bug @ 2012-04-26 11:42 ` Wolfram Sang 0 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2012-04-26 11:42 UTC (permalink / raw) To: linux-arm-kernel Hi Marek, you forgot the most important list, linux-i2c ;) > I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as expected. Yes, we had the same this week/last week. > Apparently, there might be some timing issue, when I put printk() into the at24 > driver eeprom write function, it "fixed" itself. Though, replacing the i2c > driver with i2c gpio also fixed the issue, so I suspect the i2c driver has some > flaw in it. Try removing the printk and use the module paramter "io_limit=24" for at24. Does this help? (Or bs<=24 with dd) > If noone has any fixes already, I'll debug this tonight or so. The write FIFO overflows, since it only can carry 8 words. There is currently no check for that. One solution would be to check the FIFO status and fill them as needed. This will probably require additional locking, so I think the proper solution would be to use DMA for transfers bigger than the write FIFO size (we would get MX23 support then, too). Sadly, we don't have DMA support yet and I don't have time to implement it. I am thinking if such transfers which would fail anyway, should currently be rejected or if the driver should depend on BROKEN or both. I am still undecided, though. Please also check the patch I sent yesterday, there is also another one coming after I get confirmation from a customer. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120426/e69a67b6/attachment.sig> ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <20120426114201.GC3548-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]
* Re: possible MXS-i2c bug 2012-04-26 11:42 ` Wolfram Sang @ 2012-04-26 23:10 ` Marek Vasut -1 siblings, 0 replies; 18+ messages in thread From: Marek Vasut @ 2012-04-26 23:10 UTC (permalink / raw) To: Wolfram Sang Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Fabio Estevam, Shawn Guo, linux-i2c-u79uwXL29TY76Z2rM5mHXA Dear Wolfram Sang, > Hi Marek, > > you forgot the most important list, linux-i2c ;) > > > I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as > > expected. > > Yes, we had the same this week/last week. Ah ok ;-) > > > Apparently, there might be some timing issue, when I put printk() into > > the at24 driver eeprom write function, it "fixed" itself. Though, > > replacing the i2c driver with i2c gpio also fixed the issue, so I > > suspect the i2c driver has some flaw in it. > > Try removing the printk and use the module paramter "io_limit=24" for > at24. Does this help? (Or bs<=24 with dd) > > > If noone has any fixes already, I'll debug this tonight or so. > > The write FIFO overflows, since it only can carry 8 words. There is > currently no check for that. One solution would be to check the FIFO > status and fill them as needed. This will probably require additional > locking, so I think the proper solution would be to use DMA for > transfers bigger than the write FIFO size (we would get MX23 support > then, too). Sadly, we don't have DMA support yet and I don't have time > to implement it. > > I am thinking if such transfers which would fail anyway, should > currently be rejected or if the driver should depend on BROKEN or both. > > I am still undecided, though. I've been thinking about the DMA approach. The problem I found out is that we need to transfer all messages we're given at time in one DMA chain ... at least that's how I understand it from the FSL manual (see Fig. 27-10 in the mx28 manual). Therefore we'd have to prepare scatterlist for each message that we're given to transfer. In case we'd be given huge set of messages, we'd have to allocate these scatterlists at runtime. And maybe we'd even have to allocate buffers (to resize the ones in msg.buf for the additional one byte for i2c address). I don't like the idea of calling kmalloc() in mxs_i2c_xfer_message() at all, does anyone have any hint how to handle these? > > Please also check the patch I sent yesterday, there is also another one > coming after I get confirmation from a customer. Thanks > Regards, > > Wolfram Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 18+ messages in thread
* possible MXS-i2c bug @ 2012-04-26 23:10 ` Marek Vasut 0 siblings, 0 replies; 18+ messages in thread From: Marek Vasut @ 2012-04-26 23:10 UTC (permalink / raw) To: linux-arm-kernel Dear Wolfram Sang, > Hi Marek, > > you forgot the most important list, linux-i2c ;) > > > I recently tried mxs-i2c (on 3.4-rc4) with at24 eeprom, didn't work as > > expected. > > Yes, we had the same this week/last week. Ah ok ;-) > > > Apparently, there might be some timing issue, when I put printk() into > > the at24 driver eeprom write function, it "fixed" itself. Though, > > replacing the i2c driver with i2c gpio also fixed the issue, so I > > suspect the i2c driver has some flaw in it. > > Try removing the printk and use the module paramter "io_limit=24" for > at24. Does this help? (Or bs<=24 with dd) > > > If noone has any fixes already, I'll debug this tonight or so. > > The write FIFO overflows, since it only can carry 8 words. There is > currently no check for that. One solution would be to check the FIFO > status and fill them as needed. This will probably require additional > locking, so I think the proper solution would be to use DMA for > transfers bigger than the write FIFO size (we would get MX23 support > then, too). Sadly, we don't have DMA support yet and I don't have time > to implement it. > > I am thinking if such transfers which would fail anyway, should > currently be rejected or if the driver should depend on BROKEN or both. > > I am still undecided, though. I've been thinking about the DMA approach. The problem I found out is that we need to transfer all messages we're given at time in one DMA chain ... at least that's how I understand it from the FSL manual (see Fig. 27-10 in the mx28 manual). Therefore we'd have to prepare scatterlist for each message that we're given to transfer. In case we'd be given huge set of messages, we'd have to allocate these scatterlists at runtime. And maybe we'd even have to allocate buffers (to resize the ones in msg.buf for the additional one byte for i2c address). I don't like the idea of calling kmalloc() in mxs_i2c_xfer_message() at all, does anyone have any hint how to handle these? > > Please also check the patch I sent yesterday, there is also another one > coming after I get confirmation from a customer. Thanks > Regards, > > Wolfram Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <201204270110.21377.marex-ynQEQJNshbs@public.gmane.org>]
* Re: possible MXS-i2c bug 2012-04-26 23:10 ` Marek Vasut @ 2012-04-27 14:59 ` Wolfram Sang -1 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2012-04-27 14:59 UTC (permalink / raw) To: Marek Vasut Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Fabio Estevam, Shawn Guo, linux-i2c-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 695 bytes --] Hi, > I've been thinking about the DMA approach. The problem I found out is that we > need to transfer all messages we're given at time in one DMA chain ... at least > that's how I understand it from the FSL manual (see Fig. 27-10 in the mx28 > manual). I can't follow you: mxs_i2c_xfer() gets a list of messages, yet they are iterated over in mxs_i2c_xfer_msg(). A single I2C message is unscattered and can't be bigger than 64KB, so that should be doable? Am I missing something by already being in weekend-mode? :) -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* possible MXS-i2c bug @ 2012-04-27 14:59 ` Wolfram Sang 0 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2012-04-27 14:59 UTC (permalink / raw) To: linux-arm-kernel Hi, > I've been thinking about the DMA approach. The problem I found out is that we > need to transfer all messages we're given at time in one DMA chain ... at least > that's how I understand it from the FSL manual (see Fig. 27-10 in the mx28 > manual). I can't follow you: mxs_i2c_xfer() gets a list of messages, yet they are iterated over in mxs_i2c_xfer_msg(). A single I2C message is unscattered and can't be bigger than 64KB, so that should be doable? Am I missing something by already being in weekend-mode? :) -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120427/115b71e8/attachment.sig> ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <20120427145936.GD16504-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]
* Re: possible MXS-i2c bug 2012-04-27 14:59 ` Wolfram Sang @ 2012-04-27 15:08 ` Marek Vasut -1 siblings, 0 replies; 18+ messages in thread From: Marek Vasut @ 2012-04-27 15:08 UTC (permalink / raw) To: Wolfram Sang Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Fabio Estevam, Shawn Guo, linux-i2c-u79uwXL29TY76Z2rM5mHXA Dear Wolfram Sang, > Hi, > > > I've been thinking about the DMA approach. The problem I found out is > > that we need to transfer all messages we're given at time in one DMA > > chain ... at least that's how I understand it from the FSL manual (see > > Fig. 27-10 in the mx28 manual). > > I can't follow you: mxs_i2c_xfer() gets a list of messages, yet they > are iterated over in mxs_i2c_xfer_msg(). A single I2C message is > unscattered and can't be bigger than 64KB, so that should be doable? You can get a large list of i2c messages. In the current implementation, yes, they're iterated in mxs_i2c_xfer_msg. Correct. If you want to do DMA transfer do/from the i2c controller, you have to take all these messages and create the chain of DMA transfers according to these messages, correct? So either I missed something, or you need to do something like this to do the transfer: for_each_message { sg_init_one() dma_map_sg() dmaengine_prep_slave_sg() } and then dmaengine_submit(). Is that correct? And for each iteration of the cycle above, you need one scatterlist, you can't recycle one, correct? So to create the chain. we'd need one scatterlist per message, correct? > Am I missing something by already being in weekend-mode? :) Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 18+ messages in thread
* possible MXS-i2c bug @ 2012-04-27 15:08 ` Marek Vasut 0 siblings, 0 replies; 18+ messages in thread From: Marek Vasut @ 2012-04-27 15:08 UTC (permalink / raw) To: linux-arm-kernel Dear Wolfram Sang, > Hi, > > > I've been thinking about the DMA approach. The problem I found out is > > that we need to transfer all messages we're given at time in one DMA > > chain ... at least that's how I understand it from the FSL manual (see > > Fig. 27-10 in the mx28 manual). > > I can't follow you: mxs_i2c_xfer() gets a list of messages, yet they > are iterated over in mxs_i2c_xfer_msg(). A single I2C message is > unscattered and can't be bigger than 64KB, so that should be doable? You can get a large list of i2c messages. In the current implementation, yes, they're iterated in mxs_i2c_xfer_msg. Correct. If you want to do DMA transfer do/from the i2c controller, you have to take all these messages and create the chain of DMA transfers according to these messages, correct? So either I missed something, or you need to do something like this to do the transfer: for_each_message { sg_init_one() dma_map_sg() dmaengine_prep_slave_sg() } and then dmaengine_submit(). Is that correct? And for each iteration of the cycle above, you need one scatterlist, you can't recycle one, correct? So to create the chain. we'd need one scatterlist per message, correct? > Am I missing something by already being in weekend-mode? :) Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <201204271708.53467.marex-ynQEQJNshbs@public.gmane.org>]
* Re: possible MXS-i2c bug 2012-04-27 15:08 ` Marek Vasut @ 2012-04-27 15:41 ` Wolfram Sang -1 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2012-04-27 15:41 UTC (permalink / raw) To: Marek Vasut Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Fabio Estevam, Shawn Guo, linux-i2c-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 818 bytes --] > You can get a large list of i2c messages. In the current implementation, yes, > they're iterated in mxs_i2c_xfer_msg. Correct. > > If you want to do DMA transfer do/from the i2c controller, you have to take all > these messages and create the chain of DMA transfers according to these > messages, correct? This is what I wonder. I'd think one could work on a per message basis. Regarding Figure 27-10, the first I2C write command could be sent seperately (probably even via PIOQUEUE). The only thing to be chained is the I2C read command and the actual reading of the data. Just checked, the FSL driver does it basically this way, too. -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* possible MXS-i2c bug @ 2012-04-27 15:41 ` Wolfram Sang 0 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2012-04-27 15:41 UTC (permalink / raw) To: linux-arm-kernel > You can get a large list of i2c messages. In the current implementation, yes, > they're iterated in mxs_i2c_xfer_msg. Correct. > > If you want to do DMA transfer do/from the i2c controller, you have to take all > these messages and create the chain of DMA transfers according to these > messages, correct? This is what I wonder. I'd think one could work on a per message basis. Regarding Figure 27-10, the first I2C write command could be sent seperately (probably even via PIOQUEUE). The only thing to be chained is the I2C read command and the actual reading of the data. Just checked, the FSL driver does it basically this way, too. -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120427/db3b5ffd/attachment.sig> ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <20120427154119.GF16504-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]
* Re: possible MXS-i2c bug 2012-04-27 15:41 ` Wolfram Sang @ 2012-04-27 15:53 ` Marek Vasut -1 siblings, 0 replies; 18+ messages in thread From: Marek Vasut @ 2012-04-27 15:53 UTC (permalink / raw) To: Wolfram Sang Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Fabio Estevam, Shawn Guo, linux-i2c-u79uwXL29TY76Z2rM5mHXA Dear Wolfram Sang, > > You can get a large list of i2c messages. In the current implementation, > > yes, they're iterated in mxs_i2c_xfer_msg. Correct. > > > > If you want to do DMA transfer do/from the i2c controller, you have to > > take all these messages and create the chain of DMA transfers according > > to these messages, correct? > > This is what I wonder. I'd think one could work on a per message basis. But then you don't have the DMA chain linked. Which I wonder if the controller has any problem with or not. I tried yesterday, got wrotes working perfectly, but still had issues with reads, which is exactly what needs to be chained. I'll poke further eventually. > Regarding Figure 27-10, the first I2C write command could be sent > seperately (probably even via PIOQUEUE). I wonder if we want to combine pioqueue and DMA, that might create quite some franken-driver. > The only thing to be chained is > the I2C read command and the actual reading of the data. > > Just checked, the FSL driver does it basically this way, too. Which doesn't mean FSL driver does it correctly, but it probably worked for them and there was some bug in my DMA tinkering. Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 18+ messages in thread
* possible MXS-i2c bug @ 2012-04-27 15:53 ` Marek Vasut 0 siblings, 0 replies; 18+ messages in thread From: Marek Vasut @ 2012-04-27 15:53 UTC (permalink / raw) To: linux-arm-kernel Dear Wolfram Sang, > > You can get a large list of i2c messages. In the current implementation, > > yes, they're iterated in mxs_i2c_xfer_msg. Correct. > > > > If you want to do DMA transfer do/from the i2c controller, you have to > > take all these messages and create the chain of DMA transfers according > > to these messages, correct? > > This is what I wonder. I'd think one could work on a per message basis. But then you don't have the DMA chain linked. Which I wonder if the controller has any problem with or not. I tried yesterday, got wrotes working perfectly, but still had issues with reads, which is exactly what needs to be chained. I'll poke further eventually. > Regarding Figure 27-10, the first I2C write command could be sent > seperately (probably even via PIOQUEUE). I wonder if we want to combine pioqueue and DMA, that might create quite some franken-driver. > The only thing to be chained is > the I2C read command and the actual reading of the data. > > Just checked, the FSL driver does it basically this way, too. Which doesn't mean FSL driver does it correctly, but it probably worked for them and there was some bug in my DMA tinkering. Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <201204271753.39516.marex-ynQEQJNshbs@public.gmane.org>]
* Re: possible MXS-i2c bug 2012-04-27 15:53 ` Marek Vasut @ 2012-04-27 16:37 ` Wolfram Sang -1 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2012-04-27 16:37 UTC (permalink / raw) To: Marek Vasut Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Fabio Estevam, Shawn Guo, linux-i2c-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 1517 bytes --] > But then you don't have the DMA chain linked. Which I wonder if the controller > has any problem with or not. I tried yesterday, got wrotes working perfectly, > but still had issues with reads, which is exactly what needs to be chained. Yes, the read needs chaining of two "DMA command blocks". That should be the only chain needed, because of how the driver handles reads. It is still one I2C message, though. > I'll poke further eventually. I really hope it works out! That would be great. > > Regarding Figure 27-10, the first I2C write command could be sent > > seperately (probably even via PIOQUEUE). > > I wonder if we want to combine pioqueue and DMA, that might create quite some > franken-driver. Might be true, yet I hope it won't. Most I2C transfers tend to be very small, so PIOQEUE would have some advantage here (less overhead). > > The only thing to be chained is the I2C read command and the actual > > reading of the data. > > > > Just checked, the FSL driver does it basically this way, too. > > Which doesn't mean FSL driver does it correctly, but it probably > worked for them and there was some bug in my DMA tinkering. It's only a proof-of-concept. We both know that :) (If it works, that is, AFAICT that one will fail for transfers bigger than PAGE_SIZE, too). Thanks, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* possible MXS-i2c bug @ 2012-04-27 16:37 ` Wolfram Sang 0 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2012-04-27 16:37 UTC (permalink / raw) To: linux-arm-kernel > But then you don't have the DMA chain linked. Which I wonder if the controller > has any problem with or not. I tried yesterday, got wrotes working perfectly, > but still had issues with reads, which is exactly what needs to be chained. Yes, the read needs chaining of two "DMA command blocks". That should be the only chain needed, because of how the driver handles reads. It is still one I2C message, though. > I'll poke further eventually. I really hope it works out! That would be great. > > Regarding Figure 27-10, the first I2C write command could be sent > > seperately (probably even via PIOQUEUE). > > I wonder if we want to combine pioqueue and DMA, that might create quite some > franken-driver. Might be true, yet I hope it won't. Most I2C transfers tend to be very small, so PIOQEUE would have some advantage here (less overhead). > > The only thing to be chained is the I2C read command and the actual > > reading of the data. > > > > Just checked, the FSL driver does it basically this way, too. > > Which doesn't mean FSL driver does it correctly, but it probably > worked for them and there was some bug in my DMA tinkering. It's only a proof-of-concept. We both know that :) (If it works, that is, AFAICT that one will fail for transfers bigger than PAGE_SIZE, too). Thanks, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120427/10286e8f/attachment.sig> ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <20120427163756.GH16504-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]
* [TEST PATCH] mxs-i2c DMA support 2012-04-27 16:37 ` Wolfram Sang @ 2012-04-28 3:03 ` Marek Vasut -1 siblings, 0 replies; 18+ messages in thread From: Marek Vasut @ 2012-04-28 3:03 UTC (permalink / raw) To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: Marek Vasut, Fabio Estevam, Shawn Guo, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang This patch is by no means complete. This patch does have issues, see below. Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> Cc: Fabio Estevam <festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Cc: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> --- drivers/i2c/busses/i2c-mxs.c | 257 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 238 insertions(+), 19 deletions(-) NOTE: It's still crappy and in-the-works, but I wanted to push it out ASAP so others can play around (so don't bash me for coding style etc.). I still need to fix various things here and there until I make this official: 1) The printk("%s[%i]\n", __func__, __LINE__); in mxs_i2c_xfer_msg(), without this it craps out for some reason, I'll have to check it after I get some sleep. 2) The coexistence of DMA and PIO transfers. NOTE2: Wolfram, I merged our patch into this as I hoped it might fix my DMA/PIO coexistance issue, but obviously I'd like to see your patch go in separately. diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c index 3d471d5..eea2d8a 100644 --- a/drivers/i2c/busses/i2c-mxs.c +++ b/drivers/i2c/busses/i2c-mxs.c @@ -26,6 +26,9 @@ #include <linux/platform_device.h> #include <linux/jiffies.h> #include <linux/io.h> +#include <linux/dma-mapping.h> +#include <linux/dmaengine.h> +#include <linux/fsl/mxs-dma.h> #include <mach/common.h> @@ -109,6 +112,19 @@ struct mxs_i2c_dev { struct completion cmd_complete; u32 cmd_err; struct i2c_adapter adapter; + + bool dma; + struct resource *dmares; + struct dma_chan *dmach; + struct mxs_dma_data dma_data; + + struct mutex mutex; + + uint32_t pio_data[2]; + uint32_t addr_data; + struct scatterlist sg_io[2]; + + bool read; }; /* @@ -194,7 +210,7 @@ static int mxs_i2c_wait_for_data(struct mxs_i2c_dev *i2c) static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len) { - u32 data; + u32 data = 0; int i; for (i = 0; i < len; i++) { @@ -210,6 +226,148 @@ static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len) return 0; } +static void mxs_i2c_dma_irq_callback(void *param) +{ + struct mxs_i2c_dev *i2c = param; + struct i2c_adapter *adap = &i2c->adapter; + + if (i2c->read) { + dma_unmap_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE); + dma_unmap_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE); + } else { + dma_unmap_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE); + } + + complete(&i2c->cmd_complete); +} + +static int mxs_i2c_dma_setup_xfer(struct i2c_adapter *adap, + struct i2c_msg *msg, uint32_t flags) +{ + struct dma_async_tx_descriptor *desc; + struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap); + + if (msg->flags & I2C_M_RD) { + i2c->read = 1; + i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_READ; + + /* + * SELECT command. + */ + + /* Queue the PIO register write transfer. */ + i2c->pio_data[0] = MXS_CMD_I2C_SELECT; + desc = dmaengine_prep_slave_sg(i2c->dmach, + (struct scatterlist *)&i2c->pio_data[0], + 1, DMA_TRANS_NONE, 0); + if (!desc) { + dev_err(&adap->dev, + "Failed to get PIO reg. write descriptor.\n"); + goto select_init_pio_fail; + } + + /* Queue the DMA data transfer. */ + sg_init_one(&i2c->sg_io[0], &i2c->addr_data, 1); + dma_map_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE); + desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[0], 1, + DMA_MEM_TO_DEV, + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); + if (!desc) { + dev_err(&adap->dev, + "Failed to get DMA data write descriptor.\n"); + goto select_init_dma_fail; + } + + /* + * READ command. + */ + + /* Queue the PIO register write transfer. */ + i2c->pio_data[1] = flags | MXS_I2C_CTRL0_MASTER_MODE | + MXS_I2C_CTRL0_XFER_COUNT(msg->len); + desc = dmaengine_prep_slave_sg(i2c->dmach, + (struct scatterlist *)&i2c->pio_data[1], + 1, DMA_TRANS_NONE, DMA_PREP_INTERRUPT); + if (!desc) { + dev_err(&adap->dev, + "Failed to get PIO reg. write descriptor.\n"); + goto select_init_dma_fail; + } + + /* Queue the DMA data transfer. */ + sg_init_one(&i2c->sg_io[1], msg->buf, msg->len); + dma_map_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE); + desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[1], 1, + DMA_DEV_TO_MEM, + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); + if (!desc) { + dev_err(&adap->dev, + "Failed to get DMA data write descriptor.\n"); + goto read_init_dma_fail; + } + } else { + i2c->read = 0; + i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_WRITE; + + /* + * WRITE command. + */ + + /* Queue the PIO register write transfer. */ + i2c->pio_data[0] = flags | MXS_CMD_I2C_WRITE | + MXS_I2C_CTRL0_XFER_COUNT(msg->len + 1); + desc = dmaengine_prep_slave_sg(i2c->dmach, + (struct scatterlist *)&i2c->pio_data[0], + 1, DMA_TRANS_NONE, 0); + if (!desc) { + dev_err(&adap->dev, + "Failed to get PIO reg. write descriptor.\n"); + goto write_init_pio_fail; + } + + /* Queue the DMA data transfer. */ + sg_init_table(i2c->sg_io, 2); + sg_set_buf(&i2c->sg_io[0], &i2c->addr_data, 1); + sg_set_buf(&i2c->sg_io[1], msg->buf, msg->len); + dma_map_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE); + desc = dmaengine_prep_slave_sg(i2c->dmach, i2c->sg_io, 2, + DMA_MEM_TO_DEV, + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); + if (!desc) { + dev_err(&adap->dev, + "Failed to get DMA data write descriptor.\n"); + goto write_init_dma_fail; + } + } + + /* + * The last descriptor must have this callback, + * to finish the DMA transaction. + */ + desc->callback = mxs_i2c_dma_irq_callback; + desc->callback_param = i2c; + + /* Start the transfer. */ + dmaengine_submit(desc); + + return 0; + +/* Read failpath. */ +read_init_dma_fail: + dma_unmap_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE); +select_init_dma_fail: + dma_unmap_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE); +select_init_pio_fail: + return 1; + + +/* Write failpath. */ +write_init_dma_fail: + dma_unmap_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE); +write_init_pio_fail: + return 1; +} + /* * Low level master read/write transaction. */ @@ -220,31 +378,50 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int ret; int flags; + flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0; + dev_dbg(i2c->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n", msg->addr, msg->len, msg->flags, stop); if (msg->len == 0) return -EINVAL; + i2c->dma = 1;//(msg->len > 16); + init_completion(&i2c->cmd_complete); - flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0; + if (i2c->dma) { + writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE, + i2c->regs + MXS_I2C_QUEUECTRL_CLR); - if (msg->flags & I2C_M_RD) - mxs_i2c_pioq_setup_read(i2c, msg->addr, msg->len, flags); - else - mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf, msg->len, - flags); + ret = mxs_i2c_dma_setup_xfer(adap, msg, flags); + if (ret) + return -EINVAL; + } else { + writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE, + i2c->regs + MXS_I2C_QUEUECTRL_SET); - writel(MXS_I2C_QUEUECTRL_QUEUE_RUN, + if (msg->flags & I2C_M_RD) { + mxs_i2c_pioq_setup_read(i2c, msg->addr, + msg->len, flags); + } else { + mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf, + msg->len, flags); + } + + writel(MXS_I2C_QUEUECTRL_QUEUE_RUN, i2c->regs + MXS_I2C_QUEUECTRL_SET); + } + + printk("%s[%i]\n", __func__, __LINE__); ret = wait_for_completion_timeout(&i2c->cmd_complete, msecs_to_jiffies(1000)); + if (ret == 0) goto timeout; - if ((!i2c->cmd_err) && (msg->flags & I2C_M_RD)) { + if (!i2c->dma && (!i2c->cmd_err) && (msg->flags & I2C_M_RD)) { ret = mxs_i2c_finish_read(i2c, msg->buf, msg->len); if (ret) goto timeout; @@ -252,6 +429,9 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, if (i2c->cmd_err == -ENXIO) mxs_i2c_reset(i2c); + else + writel(MXS_I2C_QUEUECTRL_QUEUE_RUN, + i2c->regs + MXS_I2C_QUEUECTRL_CLR); dev_dbg(i2c->dev, "Done with err=%d\n", i2c->cmd_err); @@ -268,13 +448,20 @@ static int mxs_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], { int i; int err; + struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap); + + mutex_lock(&i2c->mutex); for (i = 0; i < num; i++) { err = mxs_i2c_xfer_msg(adap, &msgs[i], i == (num - 1)); - if (err) + if (err) { + mutex_unlock(&i2c->mutex); return err; + } } + mutex_unlock(&i2c->mutex); + return num; } @@ -305,7 +494,7 @@ static irqreturn_t mxs_i2c_isr(int this_irq, void *dev_id) is_last_cmd = (readl(i2c->regs + MXS_I2C_QUEUESTAT) & MXS_I2C_QUEUESTAT_WRITE_QUEUE_CNT_MASK) == 0; - if (is_last_cmd || i2c->cmd_err) + if ((is_last_cmd || i2c->cmd_err) && !i2c->dma) complete(&i2c->cmd_complete); writel(stat, i2c->regs + MXS_I2C_CTRL1_CLR); @@ -318,23 +507,45 @@ static const struct i2c_algorithm mxs_i2c_algo = { .functionality = mxs_i2c_func, }; +static bool mxs_i2c_dma_filter(struct dma_chan *chan, void *param) +{ + struct mxs_i2c_dev *i2c = param; + + if (!mxs_dma_is_apbx(chan)) + return false; + + if (chan->chan_id != i2c->dmares->start) + return false; + + chan->private = &i2c->dma_data; + + return true; +} + static int __devinit mxs_i2c_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct mxs_i2c_dev *i2c; struct i2c_adapter *adap; - struct resource *res; + struct resource *res, *dmares; resource_size_t res_size; - int err, irq; + int err, irq, dmairq; + dma_cap_mask_t mask; i2c = devm_kzalloc(dev, sizeof(struct mxs_i2c_dev), GFP_KERNEL); if (!i2c) return -ENOMEM; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!res) + dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0); + irq = platform_get_irq(pdev, 0); + dmairq = platform_get_irq(pdev, 1); + + if (!res || !dmares || irq < 0 || dmairq < 0) return -ENOENT; + mutex_init(&i2c->mutex); + res_size = resource_size(res); if (!devm_request_mem_region(dev, res->start, res_size, res->name)) return -EBUSY; @@ -343,15 +554,23 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev) if (!i2c->regs) return -EBUSY; - irq = platform_get_irq(pdev, 0); - if (irq < 0) - return irq; - err = devm_request_irq(dev, irq, mxs_i2c_isr, 0, dev_name(dev), i2c); if (err) return err; + i2c->dmares = dmares; i2c->dev = dev; + + /* Setup the DMA */ + dma_cap_zero(mask); + dma_cap_set(DMA_SLAVE, mask); + i2c->dma_data.chan_irq = dmairq; + i2c->dmach = dma_request_channel(mask, mxs_i2c_dma_filter, i2c); + if (!i2c->dmach) { + dev_err(dev, "Failed to request dma\n"); + return -ENODEV; + } + platform_set_drvdata(pdev, i2c); /* Do reset to enforce correct startup after pinmuxing */ -- 1.7.10 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [TEST PATCH] mxs-i2c DMA support @ 2012-04-28 3:03 ` Marek Vasut 0 siblings, 0 replies; 18+ messages in thread From: Marek Vasut @ 2012-04-28 3:03 UTC (permalink / raw) To: linux-arm-kernel This patch is by no means complete. This patch does have issues, see below. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Fabio Estevam <festevam@gmail.com> Cc: Shawn Guo <shawn.guo@linaro.org> Cc: linux-i2c at vger.kernel.org Cc: Wolfram Sang <w.sang@pengutronix.de> --- drivers/i2c/busses/i2c-mxs.c | 257 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 238 insertions(+), 19 deletions(-) NOTE: It's still crappy and in-the-works, but I wanted to push it out ASAP so others can play around (so don't bash me for coding style etc.). I still need to fix various things here and there until I make this official: 1) The printk("%s[%i]\n", __func__, __LINE__); in mxs_i2c_xfer_msg(), without this it craps out for some reason, I'll have to check it after I get some sleep. 2) The coexistence of DMA and PIO transfers. NOTE2: Wolfram, I merged our patch into this as I hoped it might fix my DMA/PIO coexistance issue, but obviously I'd like to see your patch go in separately. diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c index 3d471d5..eea2d8a 100644 --- a/drivers/i2c/busses/i2c-mxs.c +++ b/drivers/i2c/busses/i2c-mxs.c @@ -26,6 +26,9 @@ #include <linux/platform_device.h> #include <linux/jiffies.h> #include <linux/io.h> +#include <linux/dma-mapping.h> +#include <linux/dmaengine.h> +#include <linux/fsl/mxs-dma.h> #include <mach/common.h> @@ -109,6 +112,19 @@ struct mxs_i2c_dev { struct completion cmd_complete; u32 cmd_err; struct i2c_adapter adapter; + + bool dma; + struct resource *dmares; + struct dma_chan *dmach; + struct mxs_dma_data dma_data; + + struct mutex mutex; + + uint32_t pio_data[2]; + uint32_t addr_data; + struct scatterlist sg_io[2]; + + bool read; }; /* @@ -194,7 +210,7 @@ static int mxs_i2c_wait_for_data(struct mxs_i2c_dev *i2c) static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len) { - u32 data; + u32 data = 0; int i; for (i = 0; i < len; i++) { @@ -210,6 +226,148 @@ static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len) return 0; } +static void mxs_i2c_dma_irq_callback(void *param) +{ + struct mxs_i2c_dev *i2c = param; + struct i2c_adapter *adap = &i2c->adapter; + + if (i2c->read) { + dma_unmap_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE); + dma_unmap_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE); + } else { + dma_unmap_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE); + } + + complete(&i2c->cmd_complete); +} + +static int mxs_i2c_dma_setup_xfer(struct i2c_adapter *adap, + struct i2c_msg *msg, uint32_t flags) +{ + struct dma_async_tx_descriptor *desc; + struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap); + + if (msg->flags & I2C_M_RD) { + i2c->read = 1; + i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_READ; + + /* + * SELECT command. + */ + + /* Queue the PIO register write transfer. */ + i2c->pio_data[0] = MXS_CMD_I2C_SELECT; + desc = dmaengine_prep_slave_sg(i2c->dmach, + (struct scatterlist *)&i2c->pio_data[0], + 1, DMA_TRANS_NONE, 0); + if (!desc) { + dev_err(&adap->dev, + "Failed to get PIO reg. write descriptor.\n"); + goto select_init_pio_fail; + } + + /* Queue the DMA data transfer. */ + sg_init_one(&i2c->sg_io[0], &i2c->addr_data, 1); + dma_map_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE); + desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[0], 1, + DMA_MEM_TO_DEV, + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); + if (!desc) { + dev_err(&adap->dev, + "Failed to get DMA data write descriptor.\n"); + goto select_init_dma_fail; + } + + /* + * READ command. + */ + + /* Queue the PIO register write transfer. */ + i2c->pio_data[1] = flags | MXS_I2C_CTRL0_MASTER_MODE | + MXS_I2C_CTRL0_XFER_COUNT(msg->len); + desc = dmaengine_prep_slave_sg(i2c->dmach, + (struct scatterlist *)&i2c->pio_data[1], + 1, DMA_TRANS_NONE, DMA_PREP_INTERRUPT); + if (!desc) { + dev_err(&adap->dev, + "Failed to get PIO reg. write descriptor.\n"); + goto select_init_dma_fail; + } + + /* Queue the DMA data transfer. */ + sg_init_one(&i2c->sg_io[1], msg->buf, msg->len); + dma_map_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE); + desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[1], 1, + DMA_DEV_TO_MEM, + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); + if (!desc) { + dev_err(&adap->dev, + "Failed to get DMA data write descriptor.\n"); + goto read_init_dma_fail; + } + } else { + i2c->read = 0; + i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_WRITE; + + /* + * WRITE command. + */ + + /* Queue the PIO register write transfer. */ + i2c->pio_data[0] = flags | MXS_CMD_I2C_WRITE | + MXS_I2C_CTRL0_XFER_COUNT(msg->len + 1); + desc = dmaengine_prep_slave_sg(i2c->dmach, + (struct scatterlist *)&i2c->pio_data[0], + 1, DMA_TRANS_NONE, 0); + if (!desc) { + dev_err(&adap->dev, + "Failed to get PIO reg. write descriptor.\n"); + goto write_init_pio_fail; + } + + /* Queue the DMA data transfer. */ + sg_init_table(i2c->sg_io, 2); + sg_set_buf(&i2c->sg_io[0], &i2c->addr_data, 1); + sg_set_buf(&i2c->sg_io[1], msg->buf, msg->len); + dma_map_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE); + desc = dmaengine_prep_slave_sg(i2c->dmach, i2c->sg_io, 2, + DMA_MEM_TO_DEV, + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); + if (!desc) { + dev_err(&adap->dev, + "Failed to get DMA data write descriptor.\n"); + goto write_init_dma_fail; + } + } + + /* + * The last descriptor must have this callback, + * to finish the DMA transaction. + */ + desc->callback = mxs_i2c_dma_irq_callback; + desc->callback_param = i2c; + + /* Start the transfer. */ + dmaengine_submit(desc); + + return 0; + +/* Read failpath. */ +read_init_dma_fail: + dma_unmap_sg(&adap->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE); +select_init_dma_fail: + dma_unmap_sg(&adap->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE); +select_init_pio_fail: + return 1; + + +/* Write failpath. */ +write_init_dma_fail: + dma_unmap_sg(&adap->dev, i2c->sg_io, 2, DMA_TO_DEVICE); +write_init_pio_fail: + return 1; +} + /* * Low level master read/write transaction. */ @@ -220,31 +378,50 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int ret; int flags; + flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0; + dev_dbg(i2c->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n", msg->addr, msg->len, msg->flags, stop); if (msg->len == 0) return -EINVAL; + i2c->dma = 1;//(msg->len > 16); + init_completion(&i2c->cmd_complete); - flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0; + if (i2c->dma) { + writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE, + i2c->regs + MXS_I2C_QUEUECTRL_CLR); - if (msg->flags & I2C_M_RD) - mxs_i2c_pioq_setup_read(i2c, msg->addr, msg->len, flags); - else - mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf, msg->len, - flags); + ret = mxs_i2c_dma_setup_xfer(adap, msg, flags); + if (ret) + return -EINVAL; + } else { + writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE, + i2c->regs + MXS_I2C_QUEUECTRL_SET); - writel(MXS_I2C_QUEUECTRL_QUEUE_RUN, + if (msg->flags & I2C_M_RD) { + mxs_i2c_pioq_setup_read(i2c, msg->addr, + msg->len, flags); + } else { + mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf, + msg->len, flags); + } + + writel(MXS_I2C_QUEUECTRL_QUEUE_RUN, i2c->regs + MXS_I2C_QUEUECTRL_SET); + } + + printk("%s[%i]\n", __func__, __LINE__); ret = wait_for_completion_timeout(&i2c->cmd_complete, msecs_to_jiffies(1000)); + if (ret == 0) goto timeout; - if ((!i2c->cmd_err) && (msg->flags & I2C_M_RD)) { + if (!i2c->dma && (!i2c->cmd_err) && (msg->flags & I2C_M_RD)) { ret = mxs_i2c_finish_read(i2c, msg->buf, msg->len); if (ret) goto timeout; @@ -252,6 +429,9 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, if (i2c->cmd_err == -ENXIO) mxs_i2c_reset(i2c); + else + writel(MXS_I2C_QUEUECTRL_QUEUE_RUN, + i2c->regs + MXS_I2C_QUEUECTRL_CLR); dev_dbg(i2c->dev, "Done with err=%d\n", i2c->cmd_err); @@ -268,13 +448,20 @@ static int mxs_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], { int i; int err; + struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap); + + mutex_lock(&i2c->mutex); for (i = 0; i < num; i++) { err = mxs_i2c_xfer_msg(adap, &msgs[i], i == (num - 1)); - if (err) + if (err) { + mutex_unlock(&i2c->mutex); return err; + } } + mutex_unlock(&i2c->mutex); + return num; } @@ -305,7 +494,7 @@ static irqreturn_t mxs_i2c_isr(int this_irq, void *dev_id) is_last_cmd = (readl(i2c->regs + MXS_I2C_QUEUESTAT) & MXS_I2C_QUEUESTAT_WRITE_QUEUE_CNT_MASK) == 0; - if (is_last_cmd || i2c->cmd_err) + if ((is_last_cmd || i2c->cmd_err) && !i2c->dma) complete(&i2c->cmd_complete); writel(stat, i2c->regs + MXS_I2C_CTRL1_CLR); @@ -318,23 +507,45 @@ static const struct i2c_algorithm mxs_i2c_algo = { .functionality = mxs_i2c_func, }; +static bool mxs_i2c_dma_filter(struct dma_chan *chan, void *param) +{ + struct mxs_i2c_dev *i2c = param; + + if (!mxs_dma_is_apbx(chan)) + return false; + + if (chan->chan_id != i2c->dmares->start) + return false; + + chan->private = &i2c->dma_data; + + return true; +} + static int __devinit mxs_i2c_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct mxs_i2c_dev *i2c; struct i2c_adapter *adap; - struct resource *res; + struct resource *res, *dmares; resource_size_t res_size; - int err, irq; + int err, irq, dmairq; + dma_cap_mask_t mask; i2c = devm_kzalloc(dev, sizeof(struct mxs_i2c_dev), GFP_KERNEL); if (!i2c) return -ENOMEM; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!res) + dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0); + irq = platform_get_irq(pdev, 0); + dmairq = platform_get_irq(pdev, 1); + + if (!res || !dmares || irq < 0 || dmairq < 0) return -ENOENT; + mutex_init(&i2c->mutex); + res_size = resource_size(res); if (!devm_request_mem_region(dev, res->start, res_size, res->name)) return -EBUSY; @@ -343,15 +554,23 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev) if (!i2c->regs) return -EBUSY; - irq = platform_get_irq(pdev, 0); - if (irq < 0) - return irq; - err = devm_request_irq(dev, irq, mxs_i2c_isr, 0, dev_name(dev), i2c); if (err) return err; + i2c->dmares = dmares; i2c->dev = dev; + + /* Setup the DMA */ + dma_cap_zero(mask); + dma_cap_set(DMA_SLAVE, mask); + i2c->dma_data.chan_irq = dmairq; + i2c->dmach = dma_request_channel(mask, mxs_i2c_dma_filter, i2c); + if (!i2c->dmach) { + dev_err(dev, "Failed to request dma\n"); + return -ENODEV; + } + platform_set_drvdata(pdev, i2c); /* Do reset to enforce correct startup after pinmuxing */ -- 1.7.10 ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2012-04-28 3:03 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-26 11:26 possible MXS-i2c bug Marek Vasut
2012-04-26 11:41 ` Fabio Estevam
[not found] ` <201204261326.29388.marex-ynQEQJNshbs@public.gmane.org>
2012-04-26 11:42 ` Wolfram Sang
2012-04-26 11:42 ` Wolfram Sang
[not found] ` <20120426114201.GC3548-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-04-26 23:10 ` Marek Vasut
2012-04-26 23:10 ` Marek Vasut
[not found] ` <201204270110.21377.marex-ynQEQJNshbs@public.gmane.org>
2012-04-27 14:59 ` Wolfram Sang
2012-04-27 14:59 ` Wolfram Sang
[not found] ` <20120427145936.GD16504-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-04-27 15:08 ` Marek Vasut
2012-04-27 15:08 ` Marek Vasut
[not found] ` <201204271708.53467.marex-ynQEQJNshbs@public.gmane.org>
2012-04-27 15:41 ` Wolfram Sang
2012-04-27 15:41 ` Wolfram Sang
[not found] ` <20120427154119.GF16504-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-04-27 15:53 ` Marek Vasut
2012-04-27 15:53 ` Marek Vasut
[not found] ` <201204271753.39516.marex-ynQEQJNshbs@public.gmane.org>
2012-04-27 16:37 ` Wolfram Sang
2012-04-27 16:37 ` Wolfram Sang
[not found] ` <20120427163756.GH16504-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-04-28 3:03 ` [TEST PATCH] mxs-i2c DMA support Marek Vasut
2012-04-28 3:03 ` Marek Vasut
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.