Re: [PATCH] SMSC LAN911x and LAN921x vendor driver

[Peter Korsgaard <jacmet@sunsite.dk>]

>>>>> "Steve" == Steve Glendinning <steve.glendinning@smsc.com> writes:

Hi,

 Steve> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
 Steve> index 9f6cc8a..8f12a8e 100644
 Steve> --- a/drivers/net/Kconfig
 Steve> +++ b/drivers/net/Kconfig
 Steve> @@ -967,6 +967,19 @@ config SMC911X
 Steve>  	  called smc911x.  If you want to compile it as a module, say M 
 Steve>  	  here and read <file:Documentation/kbuild/modules.txt>
 
 Steve> +config SMSC911X
 Steve> +	tristate "SMSC LAN911x/LAN921x families embedded ethernet support"
 Steve> +	depends on NET_ETHERNET
 Steve> +	select CRC32
 Steve> +	select MII
 Steve> +	---help---
 Steve> +	  Say Y here if you want support for SMSC LAN911x and LAN921x families
 Steve> +	  of ethernet controllers.
 Steve> +
 Steve> +	  To compile this driver as a module, choose M here and read
 Steve> +	  <file:Documentation/networking/net-modules.txt>. The module
 Steve> +	  will be called smsc911x.
 Steve> +

What is the point of 2 drivers for the same hardware? Either fix the
existing driver or remove the old one and reuse the Kconfig symbols
for the new one so the existing defconfig continues to work.

Like I said last year, I would really prefer the first option:

http://thread.gmane.org/gmane.linux.network/66703/focus=67812

You also don't seem to have added the big endian test I mentioned
further in the thread.

 Steve> +static void smsc911x_mdio_write(struct net_device *dev, int phy_id,
 Steve> +				int location, int val)
 Steve> +{
 Steve> +	struct smsc911x_data *pdata = netdev_priv(dev);
 Steve> +	unsigned long flags;
 Steve> +
 Steve> +	spin_lock_irqsave(&pdata->phy_lock, flags);
 Steve> +	smsc911x_phy_write(pdata, location, val);
 Steve> +	spin_unlock_irqrestore(&pdata->phy_lock, flags);
 Steve> +}
 Steve> +
 Steve> +/* Autodetects and initialises external phy for SMSC9115 and SMSC9117 flavors.
 Steve> + * If something goes wrong, returns -ENODEV to revert back to internal phy.
 Steve> + * Performed at initialisation only, so interrupts are enabled */
 Steve> +static int smsc911x_phy_initialise_external(struct smsc911x_data *pdata)

Any reason why you don't use phylib?

-- 
Bye, Peter Korsgaard