From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-out.m-online.net (mail-out.m-online.net [212.18.0.10]) by ozlabs.org (Postfix) with ESMTP id A8D35B7D6F for ; Wed, 10 Feb 2010 20:17:14 +1100 (EST) Message-ID: <4B72794A.9010805@grandegger.com> Date: Wed, 10 Feb 2010 10:15:54 +0100 From: Wolfgang Grandegger MIME-Version: 1.0 To: David Miller Subject: Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver References: <4B5871F2.9090005@grandegger.com> <20100121.180311.228791894.davem@davemloft.net> <20100209152317.5303b1b3@wker> <20100209.121356.216640014.davem@davemloft.net> In-Reply-To: <20100209.121356.216640014.davem@davemloft.net> Content-Type: text/plain; charset=ISO-8859-1 Cc: wd@denx.de, dzu@denx.de, netdev@vger.kernel.org, linuxppc-dev@ozlabs.org, agust@denx.de, kosmo@semihalf.com List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Hi David, David Miller wrote: > From: Anatolij Gustschin > Date: Tue, 9 Feb 2010 15:23:17 +0100 > >> In my understanding, in the ESP scsi driver the set of defines for >> the register offsets is common for all chip drivers. The chip driver >> methods for register access translate the offsets because the >> registers on some chips are at different intervals (4-byte, 1-byte, >> 16-byte for mac_esp.c). But the register order is the same for >> different chips. >> >> In our case non only the register order is not the same for 8xx >> FEC and 5121 FEC, but there are also other differences, different >> reserved areas between several registers, some registers are >> available only on 8xx and some only on 5121. > > That only means you would need to use a table based register address > translation scheme, rather than a simple calculation. Something > like: > > static unsigned int chip_xxx_table[] = > { > [GENERIC_REG_FOO] = CHIP_XXX_FOO, > ... > }; > > static u32 chip_xxx_read_reg(struct chip *p, unsigned int reg) > { > unsigned int reg_off = chip_xxx_table[reg]; > > return readl(p->regs + reg_off); > } > > And this table can have special tokens in entries for > registers which do not exist on a chip, so you can trap > attempted access to them in these read/write handlers. Yes, that could be done, but to honest, I do not see any improvement in respect to the previous patch where the register offset were defined via pointers within a structure. > Please stop looking for excuses to fork this driver, a > unified driver I think can be done cleanly. Other people suggested to fork the driver because it's getting too ugly. Wolfgang. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Grandegger Subject: Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver Date: Wed, 10 Feb 2010 10:15:54 +0100 Message-ID: <4B72794A.9010805@grandegger.com> References: <4B5871F2.9090005@grandegger.com> <20100121.180311.228791894.davem@davemloft.net> <20100209152317.5303b1b3@wker> <20100209.121356.216640014.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: agust@denx.de, netdev@vger.kernel.org, dzu@denx.de, wd@denx.de, jcrigby@gmail.com, kosmo@semihalf.com, linuxppc-dev@ozlabs.org, grant.likely@secretlab.ca To: David Miller Return-path: Received: from mail-out.m-online.net ([212.18.0.10]:46664 "EHLO mail-out.m-online.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752130Ab0BJJRO (ORCPT ); Wed, 10 Feb 2010 04:17:14 -0500 In-Reply-To: <20100209.121356.216640014.davem@davemloft.net> Sender: netdev-owner@vger.kernel.org List-ID: Hi David, David Miller wrote: > From: Anatolij Gustschin > Date: Tue, 9 Feb 2010 15:23:17 +0100 > >> In my understanding, in the ESP scsi driver the set of defines for >> the register offsets is common for all chip drivers. The chip driver >> methods for register access translate the offsets because the >> registers on some chips are at different intervals (4-byte, 1-byte, >> 16-byte for mac_esp.c). But the register order is the same for >> different chips. >> >> In our case non only the register order is not the same for 8xx >> FEC and 5121 FEC, but there are also other differences, different >> reserved areas between several registers, some registers are >> available only on 8xx and some only on 5121. > > That only means you would need to use a table based register address > translation scheme, rather than a simple calculation. Something > like: > > static unsigned int chip_xxx_table[] = > { > [GENERIC_REG_FOO] = CHIP_XXX_FOO, > ... > }; > > static u32 chip_xxx_read_reg(struct chip *p, unsigned int reg) > { > unsigned int reg_off = chip_xxx_table[reg]; > > return readl(p->regs + reg_off); > } > > And this table can have special tokens in entries for > registers which do not exist on a chip, so you can trap > attempted access to them in these read/write handlers. Yes, that could be done, but to honest, I do not see any improvement in respect to the previous patch where the register offset were defined via pointers within a structure. > Please stop looking for excuses to fork this driver, a > unified driver I think can be done cleanly. Other people suggested to fork the driver because it's getting too ugly. Wolfgang.