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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 11 Dec 2014 20:31:03 +0100
From:	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
To:	Cyrille Pitchen <cyrille.pitchen@...el.com>
Cc:	<nicolas.ferre@...el.com>, <davem@...emloft.net>,
	<linux-arm-kernel@...ts.infradead.org>, <netdev@...r.kernel.org>,
	<soren.brinkmann@...inx.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] net/macb: add TX multiqueue support for gem

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html