On Mon, Feb 04, 2013 at 06:19:02PM +0100, Jean Delvare wrote:
> On Mon, 4 Feb 2013 10:47:44 +0100, Jean Delvare wrote:
> > I have backported your code to kernel 3.0 and I'll get back to you when
> > I finally get a chance to test it.
>
> OK, I am done with the backport and testing. The test system I have
> access to appears to have a single slave on the second iSMT channel, at
> address 0x54. I have no idea what this device is so my testing is
> fairly limited, in particular I can't test writes. But at least I could
> test the quick command and read byte transactions. I believe the
> following fix is needed on top of your patch to make non-quick,
> non-block transactions right:
>
I've got the same device on my system here. Seth, can you elaborate on what it
is. FWIW, I took a risk, read a block from that device and wrote the same block
back to it with my v6 driver, with dyamic debug enabled, and the reported
printk's indicated it succeded. I'll test out this patch in a bit here, make
the other requested corrections and repost shortly.
Neil
> ---
> i2c-ismt.c | 35 +++++++++++++++++++++++++----------
> 1 file changed, 25 insertions(+), 10 deletions(-)
>
> --- i2c-ismt.orig/i2c-ismt.c
> +++ i2c-ismt/i2c-ismt.c
> @@ -329,10 +329,23 @@ static int ismt_process_desc(const struc
> __ismt_desc_dump(&priv->pci_dev->dev, desc);
>
> if (desc->status & ISMT_DESC_SCS) {
> - if ((size != I2C_SMBUS_QUICK) &&
> - (read_write == I2C_SMBUS_READ)) {
> + if (read_write == I2C_SMBUS_WRITE &&
> + size != I2C_SMBUS_PROC_CALL)
> + return 0;
> +
> + switch (size) {
> + case I2C_SMBUS_BYTE:
> + case I2C_SMBUS_BYTE_DATA:
> + data->byte = dma_buffer[0];
> + break;
> + case I2C_SMBUS_WORD_DATA:
> + case I2C_SMBUS_PROC_CALL:
> + data->word = dma_buffer[0] | (dma_buffer[1] << 8);
> + break;
> + case I2C_SMBUS_BLOCK_DATA:
> memcpy(&data->block[1], dma_buffer, desc->rxbytes);
> data->block[0] = desc->rxbytes;
> + break;
> }
> return 0;
> }
> @@ -426,6 +439,8 @@ static int ismt_access(struct i2c_adapte
> desc->wr_len_cmd = 2;
> dma_size = 2;
> dma_direction = DMA_TO_DEVICE;
> + priv->dma_buffer[0] = command;
> + priv->dma_buffer[1] = data->byte;
> } else {
> /* Read Byte */
> dev_dbg(dev, "I2C_SMBUS_BYTE_DATA: READ\n");
> @@ -444,6 +459,9 @@ static int ismt_access(struct i2c_adapte
> desc->wr_len_cmd = 3;
> dma_size = 3;
> dma_direction = DMA_TO_DEVICE;
> + priv->dma_buffer[0] = command;
> + priv->dma_buffer[1] = data->word & 0xff;
> + priv->dma_buffer[2] = data->word >> 8;
> } else {
> /* Read Word */
> dev_dbg(dev, "I2C_SMBUS_WORD_DATA: READ\n");
> @@ -461,6 +479,9 @@ static int ismt_access(struct i2c_adapte
> desc->rd_len = 2;
> dma_size = 3;
> dma_direction = DMA_BIDIRECTIONAL;
> + priv->dma_buffer[0] = command;
> + priv->dma_buffer[1] = data->word & 0xff;
> + priv->dma_buffer[2] = data->word >> 8;
> break;
>
> case I2C_SMBUS_BLOCK_DATA:
> @@ -471,6 +492,8 @@ static int ismt_access(struct i2c_adapte
> dma_direction = DMA_TO_DEVICE;
> desc->wr_len_cmd = dma_size;
> desc->control |= ISMT_DESC_BLK;
> + priv->dma_buffer[0] = command;
> + memcpy(&priv->dma_buffer[1], &data->block[1], dma_size);
> } else {
> /* Block Read */
> dev_dbg(dev, "I2C_SMBUS_BLOCK_DATA: READ\n");
> @@ -488,14 +511,6 @@ static int ismt_access(struct i2c_adapte
> return -EOPNOTSUPP;
> }
>
> - /* Create a temporary buffer for the DMA transaction */
> - /* and insert the command at the beginning of the buffer */
> - if ((read_write == I2C_SMBUS_WRITE) &&
> - (size > I2C_SMBUS_BYTE)) {
> - memcpy(&priv->dma_buffer[1], &data->block[1], dma_size);
> - priv->dma_buffer[0] = command;
> - }
> -
> /* map the data buffer */
> if (dma_size != 0) {
> dev_dbg(dev, " dev=%p\n", dev);
>
> Maybe that's what Seth was missing. Seth, can you please try this patch
> of mine and let us know if it helps? Warning: I was only able to test
> the quick, receive byte and read byte transactions.
>
> --
> Jean Delvare
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
