RE: [PATCH V2 2/4] i2c: tegra: Update I2C transfer using buffer

[Sowjanya Komatineni]

> >>>> +	buffer = kmalloc(ALIGN(msg->len, BYTES_PER_FIFO_WORD) +
> >>>> +					I2C_PACKET_HEADER_SIZE, GFP_KERNEL);
> >>>> +	if (!buffer)
> >>>> +		return -ENOMEM;
> >>>
> >>> Isn't it possible to avoid "buffer" allocation / copying overhead and keep code as-is for the PIO mode?
> >>>
> >>
> >> Keeping PIO mode code as is and adding DMA mode will have redundant code for header generation and msg complete timeout sections.
> >> Also in existing PIO mode implementation, TX FIFO is filled as soon as the word is ready but for DMA mode need to put all msg bytes along with header together to send thru DMA.
> >> So created common buffer to perform PIO/DMA to reduce redundancy.
> >>
> > 
> > Okay, what about something like in this sketch:
> > 
> > ...
> > 	if (msg->flags & I2C_M_RD)
> > 		xfer_size = msg->len;
> > 	else
> > 		xfer_size = ALIGN(msg->len, BYTES_PER_FIFO_WORD) +
> > 			    I2C_PACKET_HEADER_SIZE;
> > 
> > 	dma = ((xfer_size > I2C_PIO_MODE_MAX_LEN) &&
> > 		i2c_dev->tx_dma_chan && i2c_dev->rx_dma_chan);
> > 
> > 	if (dma) {
> > 		buffer = kmalloc(ALIGN(msg->len, BYTES_PER_FIFO_WORD) +
> > 				I2C_PACKET_HEADER_SIZE, GFP_KERNEL);
> > 		i2c_dev->msg_buf = (u8 *)buffer;
> > 	}
> > 
> > 	packet_header = (0 << PACKET_HEADER0_HEADER_SIZE_SHIFT) |
> > 				PACKET_HEADER0_PROTOCOL_I2C |
> > 				(i2c_dev->cont_id << PACKET_HEADER0_CONT_ID_SHIFT) |
> > 				(1 << PACKET_HEADER0_PACKET_ID_SHIFT);
> > 		i2c_writel(i2c_dev, packet_header, I2C_TX_FIFO);
> > 
> > 	if (dma)
> > 		(*buffer++) = packet_header;
> > 	else
> > 		i2c_writel(i2c_dev, packet_header, I2C_TX_FIFO);
> > 
> > ... and so on.
> > 
> > Likely that kmalloc overhead will be bigger than the above variant. Please consider to avoid kmalloc and copying for the PIO case in the next version.
> > 
>
> Also, is the intermediate kmalloc "buffer" really needed at all? Why just not to write directly into the DMA buffer?

Yes, thatâs what I am thinking too. Changing it. Will send revised patch..