Message-ID: <1312257199.2294.11.camel@jtkirshe-mobl>
Date:	Mon, 01 Aug 2011 20:53:18 -0700
From:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
To:	Chris Metcalf <cmetcalf@...era.com>,
	H Hartley Sweeten <hartleys@...ionengravers.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>
Subject: Re: [net-next v2 70/71] tile: Move the Tilera driver

On Mon, 2011-08-01 at 10:21 -0700, Chris Metcalf wrote:
> On 7/30/2011 11:27 PM, Jeff Kirsher wrote:
> > Move the Tilera driver into drivers/net/ethernet/tile and
> > make the necessary Kconfig and Makefile changes.
> >
> > CC: Chris Metcalf <cmetcalf@...era.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> > [...]
> > +++ b/drivers/net/ethernet/tile/Kconfig
> > @@ -0,0 +1,28 @@
> > +#
> > +# Tilera network device configuration
> > +#
> > +
> > +config NET_VENDOR_TILERA
> > +	bool "Tilera devices"
> > +	depends on TILE
> > +	---help---
> > +	  If you have a network (Ethernet) card belonging to this class, say Y
> > +	  and read the Ethernet-HOWTO, available from
> > +	  <http://www.tldp.org/docs.html#howto>.
> > +
> > +	  Note that the answer to this question doesn't directly affect the
> > +	  kernel: saying N will just cause the configurator to skip all
> > +	  the questions about Tilera cards. If you say Y, you will be asked for
> > +	  your specific card in the following questions.
> > +
> > +config TILE_NET
> > +	tristate "Tilera GBE/XGBE network driver support"
> > +	depends on NET_VENDOR_TILERA && TILE
> > +	default y
> > +	select CRC32
> > +	---help---
> > +	  This is a standard Linux network device driver for the
> > +	  on-chip Tilera Gigabit Ethernet and XAUI interfaces.
> > +
> > +	  To compile this driver as a module, choose M here: the module
> > +	  will be called tile_net.
> 
> Overall, this seems fine, since the Tilera drivers get grouped more
> appropriately as a result.  However, the drivers in question are not
> Ethernet cards (and Tilera is not an Ethernet card vendor and has no plans
> to become one).  Instead, this is the driver support for the built-in
> networking hardware on the Tilera multicore CPU chip.  I'm happy to group
> this support under drivers/net/ethernet/tile/, but I think it's appropriate
> to default it to "Y" if you are building a TILE kernel (since you are
> guaranteed to have the networking hardware available).
> 
> I suspect for now the cleanest thing to do is to fold the two config
> options together, using NET_VENDOR_TILERA for consistency with other
> NET_VENDOR_xxx symbols, and defaulting it to "Y" via "depends on TILE".  I
> don't think the Ethernet-HOWO reference is particularly helpful since it
> mostly tackles all the various card issues, kernel boot param issues, etc.,
> none of which are relevant to this driver.  Something like:
> 
> +config NET_VENDOR_TILERA
> +	bool "Tilera devices"
> +	depends on TILE
> +	default y
> +	select CRC32
> +	---help---
> +	  This is a standard Linux network device driver for the arch/tile
> +	  on-chip Gigabit Ethernet and XAUI interfaces.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called tile_net.
> 
> 
> Obviously you'd also need to tweak the TILE_NET symbol in the Makefile to
> be VENDOR_TILERA.  If this makes sense to you, go ahead and make the
> change, and feel free to use my
> 
> Acked-by: Chris Metcalf <cmetcalf@...era.com>
> 

Sounds fine, I will make the necessary changes.

Also based on feedback from Hartley Sweeten and Geert Uytterhoeven, I
will make a change to the Kconifg's to make it less a maintenance issue
going forward by wrapping the drivers in the Kconfig with "if
NET_VENDOR_<blah> ... endif.  This way new drivers will not have to add
the dependency of NET_VENDOR_<blah>.

Download attachment "signature.asc" of type "application/pgp-signature" (491 bytes)

Powered by blists - more mailing lists