Re: possible MXS-i2c bug
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/ | signature.asc Description: Digital signature
Re: possible MXS-i2c bug
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
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: possible MXS-i2c bug
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/ | signature.asc Description: Digital signature
Re: possible MXS-i2c bug
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 -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: possible MXS-i2c bug
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/ | signature.asc Description: Digital signature
Re: possible MXS-i2c bug
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/ | signature.asc Description: Digital signature
Re: possible MXS-i2c bug
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 -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
