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
| ||
|
Date: Wed, 4 Jul 2012 16:18:25 +1000 From: Greg Ungerer <gerg@...pgear.com> To: Joe Perches <joe@...ches.com> CC: <netdev@...r.kernel.org>, <linux-m68k@...r.kernel.org>, Greg Ungerer <gerg@...inux.org> Subject: Re: [PATCH 2/2] net: add support for NS8390 based eth controllers on some ColdFire CPU boards Hi Joe, On 04/07/12 15:18, Joe Perches wrote: > On Wed, 2012-07-04 at 14:56 +1000, gerg@...pgear.com wrote: >> From: Greg Ungerer <gerg@...inux.org> >> >> A number of older ColdFire CPU based boards use NS8390 based network >> controllers. Most use the Davicom 9008F or the UMC 9008F. This driver >> provides the support code to get these devices working on these platforms. > > Hi Greg, just some trivia: Thanks for the quick feedback! > [] > >> diff --git a/drivers/net/ethernet/8390/mcf8390.c b/drivers/net/ethernet/8390/mcf8390.c > > [] > >> +#ifdef NE2000_ODDOFFSET >> +/* >> + * A lot of the ColdFire boards use a separate address region for odd offset >> + * register addresses. The following macros and functions convert and map >> + * as required. Note that the data port accesses are treated a little >> + * differently, and always accessed via the insX/outsX functions. >> + */ >> +#define NE_PTR(a) (((a) & 0x1) ? (NE2000_ODDOFFSET + (a) - 1) : (a)) > > Maybe use static inlines instead of macros? > > static inline void *NE_PTR(void *ptr) > { > if ((unsigned long)ptr & 1) > return ptr - 1 + NE2000_ODDOFFSET; > return ptr; > } Ok, that looks better. Though I might make the arg u32, which matches the way that the address is passed to the IO functions currently. > [] > >> +static void mcf8390_get_8390_hdr(struct net_device *dev, >> + struct e8390_pkt_hdr *hdr, int ring_page) >> +{ > [] >> + /* >> + * This *shouldn't* happen. >> + * If it does, it's the last thing you'll see >> + */ >> + if (ei_status.dmaing) { >> + netdev_err(dev, >> + "%s: DMAing conflict [DMAstat:%d][irqlock:%d]\n", >> + __func__, ei_local->dmaing, ei_local->irqlock); > > This message seems to get repeated a few times. > Maybe another function/macro or maybe a BUG? > > some_dma_err(dev, __func__, ei_local); Yep, sure thing. I copied that part "as is", and a few of the other drivers seem to have it done this way too. No excuses though :-) >> +static void mcf8390_block_input(struct net_device *dev, int count, >> + struct sk_buff *skb, int ring_offset) >> +{ > [] >> + /* >> + * This *shouldn't* happen. >> + * If it does, it's the last thing you'll see >> + */ >> + if (ei_local->dmaing) { >> + netdev_err(dev, >> + "%s: DMAing conflict [DMAstat:%d][irqlock:%d]\n", >> + __func__, ei_local->dmaing, ei_local->irqlock); >> + return; >> + } > >> +static void mcf8390_block_output(struct net_device *dev, int count, >> + const unsigned char *buf, >> + const int start_page) >> +{ > [] >> + /* >> + *This *shouldn't* happen. >> + * If it does, it's the last thing you'll see >> + */ >> + if (ei_local->dmaing) { >> + netdev_err(dev, >> + "%s: DMAing conflict [DMAstat:%d][irqlock:%d]\n", >> + __func__, ei_local->dmaing, ei_local->irqlock); >> + return; >> + } > >> +static int mcf8390_init(struct net_device *dev) >> +{ >> + static u32 offsets[] = { >> + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, >> + 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, >> + }; > > const? u8? That is assigned to the "reg_offset" field of "struct ei_device" (defined in the existing 8390.h) and that is: u32 *reg_offset; /* Register mapping table */ So I can't change this. >> + pr_debug("Found ethernet address: %pM\n", dev->dev_addr); > > netdev_dbg ? Sure, will change it. Thanks for the quick feedback, much appreciated. I'll send a revised patch in 24 hours or so. Regards Greg ------------------------------------------------------------------------ Greg Ungerer -- Principal Engineer EMAIL: gerg@...pgear.com SnapGear Group, McAfee PHONE: +61 7 3435 2888 8 Gardner Close FAX: +61 7 3217 5323 Milton, QLD, 4064, Australia WEB: http://www.SnapGear.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
Powered by blists - more mailing lists