Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S968328AbXEHOxp (ORCPT ); Tue, 8 May 2007 10:53:45 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S968249AbXEHOxn (ORCPT ); Tue, 8 May 2007 10:53:43 -0400 Received: from alephnull.demon.nl ([83.160.184.112]:50236 "EHLO xi.wantstofly.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S968104AbXEHOxm (ORCPT ); Tue, 8 May 2007 10:53:42 -0400 DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=1148133259; d=wantstofly.org; h=date:from:to:cc:subject:message-id:mime-version:content-type: content-disposition:in-reply-to:user-agent; b=dTE49yhbMkNd/6oURVmwAZBw4llmYaIsH3l+as30WlZRkqKdhx0uBFEnm0K68 hFz6oy2X2mxvP1KpGLstRIN7A== Date: Tue, 8 May 2007 16:53:40 +0200 From: Lennert Buytenhek To: Krzysztof Halasa Cc: Michael-Luke Jones , Jeff Garzik , netdev@vger.kernel.org, lkml , Russell King , ARM Linux Mailing List Subject: Re: [PATCH] Intel IXP4xx network drivers v.2 - Ethernet and HSS Message-ID: <20070508145340.GD387@xi.wantstofly.org> References: <5BB7E1AB-5CE1-43C8-8CE3-E0DE0236BD09@cam.ac.uk> <86D26EBE-5899-468F-9C79-23E83E0DE04B@cam.ac.uk> <20070508113735.GB32055@xi.wantstofly.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.1i Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2936 Lines: 81 On Tue, May 08, 2007 at 04:31:12PM +0200, Krzysztof Halasa wrote: > >> +/* Built-in 10/100 Ethernet MAC interfaces */ > >> +static struct mac_plat_info ixdp425_plat_mac[] = { > >> + { > >> + .phy = 0, > >> + .rxq = 3, > >> + }, { > >> + .phy = 1, > >> + .rxq = 4, > >> + } > >> +}; > > > > As with Christian's driver (I'm feeling like a bit of a broken record > > here :-), putting knowledge of which queue to use (which is firmware- > > specific) in the _board_ support file is almost certainly wrong. > > > > I would just put the port number in there, and let the ethernet > > driver map the port number to the hardware queue number. After all, > > the ethernet driver knows which queues the firmware uses, while the > > board support code doesn't. > > No, quite the opposite. The board code knows its set of hardware > interfaces etc. and can let Ethernet driver use, say, HSS queues. > The driver can't know that. You are attacking a point that I did not make. The board support code knows such things as that the "front ethernet port" on the board is connected to the CPU's MII port number #2, but the board support code does _not_ know that MII port number #2 corresponds to "ixp4xx hardware queue #5." If Intel puts out a firmware update next month, and your ethernet driver is modified to take advantage of the new features in that firmware and starts depending on the newer version of that firmware, we will have to modify every ixp4xx board support file in case the firmware update modifies the ixp4xx queue numbers in use. The mapping from hardware ports (MII port #0, MII port #6, HSS port #42, whatever) to ixp4xx hardware queue numbers (0-63) should _not_ be put in every single ixp4xx board support file. Even if you only change the (in board support file) .rxq = 4, line to something like this instead: (in some ixp4xx-specific or driver-specific header file) #define IXP4XX_MII_PORT_1_RX_QUEUE 4 (in board support file) .rxq = IXP4XX_MII_PORT_1_RX_QUEUE, then you have remved this dependency, and then you only have to update one place if you move to a newer firmware version. > > I generally discourage the use of such wrappers, as it often makes > > people forget that the set and clear operations are not atomic, and > > it ignores the fact that some of the other bits in the register you > > are modifying might have side-effects. > > Without them the code in question is hardly readable, You can read Polish, how can you complain about code readability. :-)) *runs* > I pick the need to remember about non-atomicity and possible side > effects instead :-) Sure, point taken, it's just that the person after you might not remember.. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/