Re: [net-next v2 70/71] tile: Move the Tilera driver

[Jeff Kirsher <jeffrey.t.kirsher@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5455 bytes --]

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@tilera.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.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@tilera.com>
> 

This definitely makes sense, I was not sure if there is ever a
possibility of another driver from Tilera for future silicon on TILE
kernels, so I would want to lean towards the Kconfig format I am using
(which got a minor tweak since I sent this out) because it provides a
easy what to add additional drivers.

I can modify the current Kconfig so that it defaults to yes on Tile
kernels by doing this:

+config NET_VENDOR_TILERA
+       bool "Tilera devices"
+       depends on TILE
+	default y if 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.
+
+if NET_VENDOR_TILERA
+
+config TILE_NET
+       tristate "Tilera GBE/XGBE network driver support"
+       depends on 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.
+
+endif # NET_VENDOR_TILERA

This Kconfig would automatically default to y for TILE kernels and would
allow you to easily add additional future drivers for Tilera silicon,
and if you expand to other architectures/systems in the future it would
also allow for that as well.  Your thoughts?

Otherwise I can have it just as this:
+config TILE_NET
+       tristate "Tilera GBE/XGBE network driver support"
+       depends on 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.

Because you only have 1 driver, there is no large need to add the
NET_VENDOR_<blah> and it can always be added in the future if the need
arises.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]