[PATCH 0/3] smc911x driver updates

[PATCH 0/3] smc911x driver updates

[Catalin Marinas]

Hi,

I have three patches for the smc911x driver to allow it to be used with
the ARM RealView boards (until recently I used the out-of-tree driver
from SMSC but I need to provide a working default configuration for the
RealView boards without additional patches).


Catalin Marinas (3):
      smc911x: Make the driver safer on SMP
      smc911x: Add IRQ polarity configuration
      smc911x: Allow Kconfig dependency on ARM


 drivers/net/Kconfig     |    2 +-
 drivers/net/smc911x.c   |   36 ++++++++++++++++--------------------
 include/linux/smc911x.h |    1 +
 3 files changed, 18 insertions(+), 21 deletions(-)

-- 
Catalin

[PATCH 1/3] smc911x: Allow Kconfig dependency on ARM

[Catalin Marinas]

Since more ARM platforms use this device, it is easier to add a
dependency on ARM rather than individual platforms.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---

This patch will conflict with a patch posted two days ago -
http://marc.info/?l=linux-netdev&m=122436376803634&w=2. However,
Guennadi suggested that it is better to have the dependency on ARM
rather than individual ARM platforms.


 drivers/net/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index ad301ac..ef38f01 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -960,7 +960,7 @@ config SMC911X
 	tristate "SMSC LAN911[5678] support"
 	select CRC32
 	select MII
-	depends on ARCH_PXA || SUPERH
+	depends on ARM || SUPERH
 	help
 	  This is a driver for SMSC's LAN911x series of Ethernet chipsets
 	  including the new LAN9115, LAN9116, LAN9117, and LAN9118.

[PATCH 2/3] smc911x: Add IRQ polarity configuration

[Catalin Marinas]

Platforms like ARM Ltd's RealView require the IRQ polarity bit to be set
for the SMC9118 chip. This patch allows the dynamic configuration via
the smc911x_platdata structure.

This patch also changes the smc91x_platdata structure name to the
correct smc911x_platdata in the smc911x_drv_probe() function.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 drivers/net/smc911x.c   |   11 ++++++++---
 include/linux/smc911x.h |    1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index 8aa7460..0216440 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -180,7 +180,7 @@ static void PRINT_PKT(u_char *buf, int length)
 static void smc911x_reset(struct net_device *dev)
 {
 	struct smc911x_local *lp = netdev_priv(dev);
-	unsigned int reg, timeout=0, resets=1;
+	unsigned int reg, timeout=0, resets=1, irq_cfg;
 	unsigned long flags;
 
 	DBG(SMC_DEBUG_FUNC, "%s: --> %s\n", dev->name, __func__);
@@ -252,7 +252,12 @@ static void smc911x_reset(struct net_device *dev)
 	 * Deassert IRQ for 1*10us for edge type interrupts
 	 * and drive IRQ pin push-pull
 	 */
-	SMC_SET_IRQ_CFG(lp, (1 << 24) | INT_CFG_IRQ_EN_ | INT_CFG_IRQ_TYPE_);
+	irq_cfg = (1 << 24) | INT_CFG_IRQ_EN_ | INT_CFG_IRQ_TYPE_;
+#ifdef SMC_DYNAMIC_BUS_CONFIG
+	if (lp->cfg.irq_polarity)
+		irq_cfg |= INT_CFG_IRQ_POL_;
+#endif
+	SMC_SET_IRQ_CFG(lp, irq_cfg);
 
 	/* clear anything saved */
 	if (lp->pending_tx_skb != NULL) {
@@ -2054,7 +2059,7 @@ err_out:
  */
 static int smc911x_drv_probe(struct platform_device *pdev)
 {
-	struct smc91x_platdata *pd = pdev->dev.platform_data;
+	struct smc911x_platdata *pd = pdev->dev.platform_data;
 	struct net_device *ndev;
 	struct resource *res;
 	struct smc911x_local *lp;
diff --git a/include/linux/smc911x.h b/include/linux/smc911x.h
index b58f54c..521f371 100644
--- a/include/linux/smc911x.h
+++ b/include/linux/smc911x.h
@@ -7,6 +7,7 @@
 struct smc911x_platdata {
 	unsigned long flags;
 	unsigned long irq_flags; /* IRQF_... */
+	int irq_polarity;
 };
 
 #endif /* __SMC911X_H__ */

[PATCH 3/3] smc911x: Make the driver safer on SMP

[Catalin Marinas]

This patch extends the critical regions covered by lp->lock to make it
safer on SMP. The main failure point was the smc911x_hard_start_xmit
function being called from different CPUs. It was tested on the ARM SMP
platforms.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 drivers/net/smc911x.c |   25 ++++++++-----------------
 1 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index 0216440..89f1fa6 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -155,23 +155,17 @@ static void PRINT_PKT(u_char *buf, int length)
 /* this enables an interrupt in the interrupt mask register */
 #define SMC_ENABLE_INT(lp, x) do {			\
 	unsigned int  __mask;				\
-	unsigned long __flags;				\
-	spin_lock_irqsave(&lp->lock, __flags);		\
 	__mask = SMC_GET_INT_EN((lp));			\
 	__mask |= (x);					\
 	SMC_SET_INT_EN((lp), __mask);			\
-	spin_unlock_irqrestore(&lp->lock, __flags);	\
 } while (0)
 
 /* this disables an interrupt from the interrupt mask register */
 #define SMC_DISABLE_INT(lp, x) do {			\
 	unsigned int  __mask;				\
-	unsigned long __flags;				\
-	spin_lock_irqsave(&lp->lock, __flags);		\
 	__mask = SMC_GET_INT_EN((lp));			\
 	__mask &= ~(x);					\
 	SMC_SET_INT_EN((lp), __mask);			\
-	spin_unlock_irqrestore(&lp->lock, __flags);	\
 } while (0)
 
 /*
@@ -279,6 +273,8 @@ static void smc911x_enable(struct net_device *dev)
 
 	DBG(SMC_DEBUG_FUNC, "%s: --> %s\n", dev->name, __func__);
 
+	spin_lock_irqsave(&lp->lock, flags);
+
 	SMC_SET_MAC_ADDR(lp, dev->dev_addr);
 
 	/* Enable TX */
@@ -291,12 +287,10 @@ static void smc911x_enable(struct net_device *dev)
 	SMC_SET_FIFO_TSL(lp, 64);
 	SMC_SET_GPT_CFG(lp, GPT_CFG_TIMER_EN_ | 10000);
 
-	spin_lock_irqsave(&lp->lock, flags);
 	SMC_GET_MAC_CR(lp, cr);
 	cr |= MAC_CR_TXEN_ | MAC_CR_HBDIS_;
 	SMC_SET_MAC_CR(lp, cr);
 	SMC_SET_TX_CFG(lp, TX_CFG_TX_ON_);
-	spin_unlock_irqrestore(&lp->lock, flags);
 
 	/* Add 2 byte padding to start of packets */
 	SMC_SET_RX_CFG(lp, (2<<8) & RX_CFG_RXDOFF_);
@@ -305,9 +299,7 @@ static void smc911x_enable(struct net_device *dev)
 	if (cr & MAC_CR_RXEN_)
 		DBG(SMC_DEBUG_RX, "%s: Receiver already enabled\n", dev->name);
 
-	spin_lock_irqsave(&lp->lock, flags);
 	SMC_SET_MAC_CR(lp, cr | MAC_CR_RXEN_);
-	spin_unlock_irqrestore(&lp->lock, flags);
 
 	/* Interrupt on every received packet */
 	SMC_SET_FIFO_RSA(lp, 0x01);
@@ -323,6 +315,8 @@ static void smc911x_enable(struct net_device *dev)
 		mask|=INT_EN_RDFO_EN_;
 	}
 	SMC_ENABLE_INT(lp, mask);
+
+	spin_unlock_irqrestore(&lp->lock, flags);
 }
 
 /*
@@ -463,7 +457,6 @@ static void smc911x_hardware_send_pkt(struct net_device *dev)
 	struct sk_buff *skb;
 	unsigned int cmdA, cmdB, len;
 	unsigned char *buf;
-	unsigned long flags;
 
 	DBG(SMC_DEBUG_FUNC | SMC_DEBUG_TX, "%s: --> %s\n", dev->name, __func__);
 	BUG_ON(lp->pending_tx_skb == NULL);
@@ -508,11 +501,9 @@ static void smc911x_hardware_send_pkt(struct net_device *dev)
 	dev->trans_start = jiffies;
 	dev_kfree_skb(skb);
 #endif
-	spin_lock_irqsave(&lp->lock, flags);
 	if (!lp->tx_throttle) {
 		netif_wake_queue(dev);
 	}
-	spin_unlock_irqrestore(&lp->lock, flags);
 	SMC_ENABLE_INT(lp, INT_EN_TDFA_EN_ | INT_EN_TSFL_EN_);
 }
 
@@ -531,6 +522,8 @@ static int smc911x_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	DBG(SMC_DEBUG_FUNC | SMC_DEBUG_TX, "%s: --> %s\n",
 		dev->name, __func__);
 
+	spin_lock_irqsave(&lp->lock, flags);
+
 	BUG_ON(lp->pending_tx_skb != NULL);
 
 	free = SMC_GET_TX_FIFO_INF(lp) & TX_FIFO_INF_TDFREE_;
@@ -540,12 +533,10 @@ static int smc911x_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (free <= SMC911X_TX_FIFO_LOW_THRESHOLD) {
 		DBG(SMC_DEBUG_TX, "%s: Disabling data flow due to low FIFO space (%d)\n",
 			dev->name, free);
-		spin_lock_irqsave(&lp->lock, flags);
 		/* Reenable when at least 1 packet of size MTU present */
 		SMC_SET_FIFO_TDA(lp, (SMC911X_TX_FIFO_LOW_THRESHOLD)/64);
 		lp->tx_throttle = 1;
 		netif_stop_queue(dev);
-		spin_unlock_irqrestore(&lp->lock, flags);
 	}
 
 	/* Drop packets when we run out of space in TX FIFO
@@ -561,6 +552,7 @@ static int smc911x_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		lp->pending_tx_skb = NULL;
 		dev->stats.tx_errors++;
 		dev->stats.tx_dropped++;
+		spin_unlock_irqrestore(&lp->lock, flags);
 		dev_kfree_skb(skb);
 		return 0;
 	}
@@ -570,7 +562,6 @@ static int smc911x_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		/* If the DMA is already running then defer this packet Tx until
 		 * the DMA IRQ starts it
 		 */
-		spin_lock_irqsave(&lp->lock, flags);
 		if (lp->txdma_active) {
 			DBG(SMC_DEBUG_TX | SMC_DEBUG_DMA, "%s: Tx DMA running, deferring packet\n", dev->name);
 			lp->pending_tx_skb = skb;
@@ -581,11 +572,11 @@ static int smc911x_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			DBG(SMC_DEBUG_TX | SMC_DEBUG_DMA, "%s: Activating Tx DMA\n", dev->name);
 			lp->txdma_active = 1;
 		}
-		spin_unlock_irqrestore(&lp->lock, flags);
 	}
 #endif
 	lp->pending_tx_skb = skb;
 	smc911x_hardware_send_pkt(dev);
+	spin_unlock_irqrestore(&lp->lock, flags);
 
 	return 0;
 }

Re: [PATCH 3/3] smc911x: Make the driver safer on SMP

[Jeff Garzik]

Catalin Marinas wrote:
> This patch extends the critical regions covered by lp->lock to make it
> safer on SMP. The main failure point was the smc911x_hard_start_xmit
> function being called from different CPUs. It was tested on the ARM SMP
> platforms.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  drivers/net/smc911x.c |   25 ++++++++-----------------
>  1 files changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> index 0216440..89f1fa6 100644
> --- a/drivers/net/smc911x.c
> +++ b/drivers/net/smc911x.c
> @@ -155,23 +155,17 @@ static void PRINT_PKT(u_char *buf, int length)
>  /* this enables an interrupt in the interrupt mask register */
>  #define SMC_ENABLE_INT(lp, x) do {			\
>  	unsigned int  __mask;				\
> -	unsigned long __flags;				\
> -	spin_lock_irqsave(&lp->lock, __flags);		\
>  	__mask = SMC_GET_INT_EN((lp));			\
>  	__mask |= (x);					\
>  	SMC_SET_INT_EN((lp), __mask);			\
> -	spin_unlock_irqrestore(&lp->lock, __flags);	\
>  } while (0)
>  
>  /* this disables an interrupt from the interrupt mask register */
>  #define SMC_DISABLE_INT(lp, x) do {			\
>  	unsigned int  __mask;				\
> -	unsigned long __flags;				\
> -	spin_lock_irqsave(&lp->lock, __flags);		\
>  	__mask = SMC_GET_INT_EN((lp));			\
>  	__mask &= ~(x);					\
>  	SMC_SET_INT_EN((lp), __mask);			\
> -	spin_unlock_irqrestore(&lp->lock, __flags);	\
>  } while (0)

Very nice... this has the added benefit of making the locking more 
obvious to the reviewer, rather than burying the locking in a macro as 
the previous code did.

	Jeff

Re: [PATCH 3/3] smc911x: Make the driver safer on SMP

[Jeff Garzik]

Catalin Marinas wrote:
> This patch extends the critical regions covered by lp->lock to make it
> safer on SMP. The main failure point was the smc911x_hard_start_xmit
> function being called from different CPUs. It was tested on the ARM SMP
> platforms.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  drivers/net/smc911x.c |   25 ++++++++-----------------
>  1 files changed, 8 insertions(+), 17 deletions(-)

applied 1-3

Re: [PATCH 3/3] smc911x: Make the driver safer on SMP

[Catalin Marinas]

On Wed, 2008-10-22 at 07:01 -0400, Jeff Garzik wrote:
> Catalin Marinas wrote:
> > This patch extends the critical regions covered by lp->lock to make it
> > safer on SMP. The main failure point was the smc911x_hard_start_xmit
> > function being called from different CPUs. It was tested on the ARM SMP
> > platforms.
> > 
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> >  drivers/net/smc911x.c |   25 ++++++++-----------------
> >  1 files changed, 8 insertions(+), 17 deletions(-)
> 
> applied 1-3

Thanks.

-- 
Catalin