Re: [PATCH 2.6] Natsemi - remove compilation warnings

[Jeff Garzik]

Linus Torvalds wrote:
> On Thu, 30 Sep 2004, Franz Pletz wrote:
>
> > It seems like your patch unfortunately went into 2.6.9-rc2-mm[3,4] and 
> > 2.6.9-rc3.
>
>
> It's definitely not in _my_ -rc3. Which kernel are you looking at?
>
>
> > My Natsemi network card stops working with 2.6.9-rc3. After succesfully 
> > revoking your patch from 
> > http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.9-rc2/2.6.9-rc2-mm3/broken-out/natsemi-remove-compilation-warnings.patch
> > everything works fine.
>
>
> That patch does indeed look totally bogus. The reason a lot of network

<blink> <blink> <blink>  This patch is so bogus its laffable.  I think 
akpm forgot his coffee, I know he's smarter than that :)


> drivers complain about readl/writel is that "struct net_device" is very
> confused about what the IO addresses mean, and they mean different things
> for different users. Which makes type safety basically disappear, and now
> that we check it more carefully, things break.
>
> This patch should clean up natsemi.c a bit, and makes the warnings go 
> away. Does it work for you? (It really should, it's just a basic 
> search-and-replace fix).
>
> This is bigger than the broken patch, but that's really pretty
> unavoidable, unless "struct net_device" is fixed. And the way it's
> structured, if "net_device" ever _is_ fixed, this driver will now be
> trivially updated.
>
> 		Linus
>
> ----
> ===== drivers/net/natsemi.c 1.68 vs edited =====
> --- 1.68/drivers/net/natsemi.c	2004-07-27 11:18:53 -07:00
> +++ edited/drivers/net/natsemi.c	2004-09-30 10:22:44 -07:00
> @@ -719,7 +719,7 @@
>  };
>  
>  static void move_int_phy(struct net_device *dev, int addr);
> -static int eeprom_read(long ioaddr, int location);
> +static int eeprom_read(void __iomem *ioaddr, int location);
>  static int mdio_read(struct net_device *dev, int reg);
>  static void mdio_write(struct net_device *dev, int reg, u16 data);
>  static void init_phy_fixup(struct net_device *dev);
> @@ -769,9 +769,15 @@
>  static int netdev_get_regs(struct net_device *dev, u8 *buf);
>  static int netdev_get_eeprom(struct net_device *dev, u8 *buf);
>  
> +static inline void __iomem *ns_ioaddr(struct net_device *dev)
> +{
> +	return (void __iomem *) dev->base_addr;
> +}
> +

hmmmm.  Since dev->base_addr gets exported to userspace, I don't think 
it's that quick/easy to change.

Wouldn't it be better to just phase out the base of dev->base_addr 
completely?  I tend to prefer adding a "void __iomem *regs" to struct 
netdev_private, and ignore dev->base_addr completely.

	Jeff


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/