Re: [PATCH v2 2/2] can: m_can: Batch FIFO writes during CAN transmit

[Matt Kline <matt@bitbashing.io>]

On Wed, Aug 04, 2021 at 11:18:58AM +0200, Marc Kleine-Budde wrote:
> >  
> > -	cdev->ops->write_fifo(cdev, addr_offset, val);
> > +	result = cdev->ops->write_fifo(cdev, addr_offset, val, val_count);
> > +	WARN_ON(result != 0);
> 
> What about converting all read/write functions to return an error, and
> handle the error in the caller?

Yeah, that would be cleaner.

> >  	/* acknowledge rx fifo 0 */
> > @@ -1546,8 +1548,8 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
> >  	struct net_device *dev = cdev->net;
> >  	struct sk_buff *skb = cdev->tx_skb;
> >  	u32 id, cccr, fdflags;
> > -	int i;
> >  	int putidx;
> > +	u32 id_and_dlc[2];
> 
> Can you create a struct for this?

Ditto, sure!

> >  
> >  	cdev->tx_skb = NULL;
> >  
> > @@ -1563,18 +1565,16 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
> >  	if (cf->can_id & CAN_RTR_FLAG)
> >  		id |= TX_BUF_RTR;
> >  
> > +	id_and_dlc[0] = id;
> > +
> >  	if (cdev->version == 30) {
> >  		netif_stop_queue(dev);
> >  
> > -		/* message ram configuration */
> > -		m_can_fifo_write(cdev, 0, M_CAN_FIFO_ID, id);
> > -		m_can_fifo_write(cdev, 0, M_CAN_FIFO_DLC,
> > -				 can_fd_len2dlc(cf->len) << 16);
> > +		id_and_dlc[1] = can_fd_len2dlc(cf->len) << 16;
> >  
> > -		for (i = 0; i < cf->len; i += 4)
> > -			m_can_fifo_write(cdev, 0,
> > -					 M_CAN_FIFO_DATA(i / 4),
> > -					 *(u32 *)(cf->data + i));
> > +		/* Write the frame ID, DLC, and payload to the FIFO element. */
> > +		m_can_fifo_write(cdev, 0, M_CAN_FIFO_ID, id_and_dlc, ARRAY_SIZE(id_and_dlc));
> > +		m_can_fifo_write(cdev, 0, M_CAN_FIFO_DATA, cf->data, DIV_ROUND_UP(cf->len, 4));
> 
> Does it make sense to combine these, too? Same for the v3.1 variant.

I think that's the eventual goal, but since the ID, DLC, and frame data would
have to be contiguous for a single m_can_fifo_write(), you'd end up copying
things around. I wanted to start with this smaller, simpler patch first.
Is that alright?

I'll try to send a v3 up shortly.

Best,
Matt