Re: [PATCH v2 1/1] net/macb: add TX multiqueue support for gem

[Cyrille Pitchen <cyrille.pitchen@atmel.com>]

Le 11/12/2014 20:31, Thomas Petazzoni a écrit :
> Dear Cyrille Pitchen,
> 
> On Thu, 11 Dec 2014 11:16:51 +0100, Cyrille Pitchen wrote:
> 
>> +#define GEM_ISR1				0x0400
>> +#define GEM_ISR2				0x0404
>> +#define GEM_ISR3				0x0408
>> +#define GEM_ISR4				0x040c
>> +#define GEM_ISR5				0x0410
>> +#define GEM_ISR6				0x0414
>> +#define GEM_ISR7				0x0418
> 
> What about doing instead:
> 
> #define GEM_ISR(q)				((q) == 0 ? MACB_ISR : 0x400 + (q) << 2)
> 
> And ditto for all other registers, which will save a lot of boring repeated code.
> 
> If you do that, then you can avoid the following fields in the
> macb_queue structure:
> 
> +	unsigned int		ISR;
> +	unsigned int		IER;
> +	unsigned int		IDR;
> +	unsigned int		IMR;
> +	unsigned int		TBQP;
> 
> And the not very pleasant calculation of those offsets:
> 
> +	bp->queues[0].bp = bp;
> +	bp->queues[0].ISR  = MACB_ISR;
> +	bp->queues[0].IER  = MACB_IER;
> +	bp->queues[0].IDR  = MACB_IDR;
> +	bp->queues[0].IMR  = MACB_IMR;
> +	bp->queues[0].TBQP = MACB_TBQP;
> +	for (q = 1, queue = &bp->queues[1]; q < MACB_MAX_QUEUES; ++q) {
> +		if (!(queue_mask & (1 << q)))
> +			continue;
> +
> +		queue->bp = bp;
> +		queue->ISR  = (q-1) * sizeof(u32) + GEM_ISR1;
> +		queue->IER  = (q-1) * sizeof(u32) + GEM_IER1;
> +		queue->IDR  = (q-1) * sizeof(u32) + GEM_IDR1;
> +		queue->IMR  = (q-1) * sizeof(u32) + GEM_IMR1;
> +		queue->TBQP = (q-1) * sizeof(u32) + GEM_TBQP1;
> +		queue++;
> +	}
> 
> You replace the ISR, IER, IDR, IMR and TBQP by an "id" field in
> macb_queue, which contains the queue number, and then change your:
> 
> +#define queue_readl(queue, reg)				\
> +	__raw_readl((queue)->bp->regs + queue->reg)
> +#define queue_writel(queue, reg, value)			\
> +	__raw_writel((value), (queue)->bp->regs + queue->reg)
> 
> to
> 
> +#define queue_readl(queue, reg)				\
> +	__raw_readl((queue)->bp->regs + reg((queue)->id))
> +#define queue_writel(queue, reg, value)			\
> +	__raw_writel((value), (queue)->bp->regs + reg((queue)->id)
> 
> Best regards,
> 
> Thomas
> 
Good idea, I'm working on it.

Thanks,

Cyrille