On Wed, Aug 08, 2007 at 01:16:43PM +1000, Tony Breeds wrote: > On Tue, Aug 07, 2007 at 04:22:31PM +1000, David Gibson wrote: > > Based on BenH's earlier work, this is a new version of the EMAC driver > > for the built-in ethernet found on PowerPC 4xx embedded CPUs. The > > same ASIC is also found in the Axon bridge chip. This new version is > > designed to work in the arch/powerpc tree, using the device tree to > > probe the device, rather than the old and ugly arch/ppc OCP layer. > > <snip> > > Hi David, > I had a look over the patch FWIW, asside from the points below > looks good. > > Minor nits: > * Shouldn't ndev->priv accesses, use netdev_priv() ? if not a comment > somewhere about why not will keep the janators off your back.
netdev_priv() hey? Hadn't heard of that before. I guess so... Changed. > * c++ style comments Eh, all the c++ comments are FIXMEs anyway. I'm inclined to leave them. > * in emac_probe(): > + /* Wait for dependent devices */ > + err = -ENODEV; > + err = emac_wait_deps(dev); > The initialisation to -ENODEV is pointless right? Yes, I think so. Removed. > * s/get_property/of_get_property/g or you'll make sfr grumpy ;P Oops, yes. > * In drivers/net/ibm_newemac/Makefile, I think the preferred method is > to use ibm_newemac-y rather than ibm_newemac-objs. > > Is there any reason to leave config IBM_NEW_EMAC (and IBM_EMAC) in > drivers/net/Kconfig ? It seems to me they'd be nicer to in the > appropriate drivers/net/*Kconfig. Yes, and that also seems to be how it's done for tulip. Moved. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev