Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH firmware 1/4] rtl_nic: update firmware for RTL8168F
From: Ben Hutchings @ 2012-07-09 23:58 UTC (permalink / raw)
  To: Hayes Wang; +Cc: dwmw2, romieu, netdev, linux-kernel
In-Reply-To: <1341803512-1309-1-git-send-email-hayeswang@realtek.com>

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

On Mon, 2012-07-09 at 11:11 +0800, Hayes Wang wrote:
> File: rtl_nic/rtl8168f-1.fw
> Version: 0.0.5
[...]

Applied.

Ben.

-- 
Ben Hutchings
The generation of random numbers is too important to be left to chance.
                                                            - Robert Coveyou

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

^ permalink raw reply

* Re: [PATCH firmware 2/4] rtl_nic: update firmware for RTL8411
From: Ben Hutchings @ 2012-07-09 23:58 UTC (permalink / raw)
  To: Hayes Wang; +Cc: dwmw2, romieu, netdev, linux-kernel
In-Reply-To: <1341803512-1309-2-git-send-email-hayeswang@realtek.com>

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

On Mon, 2012-07-09 at 11:11 +0800, Hayes Wang wrote:
> File: rtl_nic/rtl8411-1.fw
> Version: 0.0.3
[...]

Applied.

Ben.

-- 
Ben Hutchings
The generation of random numbers is too important to be left to chance.
                                                            - Robert Coveyou

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

^ permalink raw reply

* [PATCH] vxge/s2io: remove dead URLs
From: Jon Mason @ 2012-07-10  0:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev

URLs to neterion.com and s2io.com no longer resolve.  Remove all references to
these URLs in the driver source and documentation.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
 Documentation/networking/s2io.txt              |   14 ++------------
 Documentation/networking/vxge.txt              |    7 -------
 MAINTAINERS                                    |    2 --
 drivers/net/ethernet/neterion/vxge/vxge-main.c |    4 +---
 4 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/Documentation/networking/s2io.txt b/Documentation/networking/s2io.txt
index 4be0c03..6b5cbfa 100644
--- a/Documentation/networking/s2io.txt
+++ b/Documentation/networking/s2io.txt
@@ -136,16 +136,6 @@ For more information, please review the AMD8131 errata at
 http://vip.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/
 26310_AMD-8131_HyperTransport_PCI-X_Tunnel_Revision_Guide_rev_3_18.pdf
 
-6.  Available Downloads
-Neterion "s2io" driver in Red Hat and Suse 2.6-based distributions is kept up 
-to date, also the latest "s2io" code (including support for 2.4 kernels) is 
-available via "Support" link on the Neterion site:  http://www.neterion.com.
-
-For Xframe User Guide (Programming manual), visit ftp site ns1.s2io.com,
-user: linuxdocs password: HALdocs
-
-7. Support 
+6. Support 
 For further support please contact either your 10GbE Xframe NIC vendor (IBM, 
-HP, SGI etc.) or click on the "Support" link on the Neterion site:  
-http://www.neterion.com.
-
+HP, SGI etc.)
diff --git a/Documentation/networking/vxge.txt b/Documentation/networking/vxge.txt
index d2e2997..bb76c66 100644
--- a/Documentation/networking/vxge.txt
+++ b/Documentation/networking/vxge.txt
@@ -91,10 +91,3 @@ v)  addr_learn_en
        virtualization environment.
        Valid range: 0,1 (disabled, enabled respectively)
        Default: 0
-
-4) Troubleshooting:
--------------------
-
-To resolve an issue with the source code or X3100 series adapter, please collect
-the statistics, register dumps using ethool, relevant logs and email them to
-support@neterion.com.
diff --git a/MAINTAINERS b/MAINTAINERS
index 8da1373..ce7398e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4638,8 +4638,6 @@ F:	net/sched/sch_netem.c
 NETERION 10GbE DRIVERS (s2io/vxge)
 M:	Jon Mason <jdmason@kudzu.us>
 L:	netdev@vger.kernel.org
-W:	http://trac.neterion.com/cgi-bin/trac.cgi/wiki/Linux?Anonymous
-W:	http://trac.neterion.com/cgi-bin/trac.cgi/wiki/X3100Linux?Anonymous
 S:	Supported
 F:	Documentation/networking/s2io.txt
 F:	Documentation/networking/vxge.txt
diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c b/drivers/net/ethernet/neterion/vxge/vxge-main.c
index 2fd1edb..24824fcc 100644
--- a/drivers/net/ethernet/neterion/vxge/vxge-main.c
+++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c
@@ -4261,9 +4261,7 @@ static int vxge_probe_fw_update(struct vxgedev *vdev)
 	if (VXGE_FW_VER(VXGE_CERT_FW_VER_MAJOR, VXGE_CERT_FW_VER_MINOR, 0) >
 	    VXGE_FW_VER(maj, min, 0)) {
 		vxge_debug_init(VXGE_ERR, "%s: Firmware %d.%d.%d is too old to"
-				" be used with this driver.\n"
-				"Please get the latest version from "
-				"ftp://ftp.s2io.com/pub/X3100-Drivers/FIRMWARE",
+				" be used with this driver."
 				VXGE_DRIVER_NAME, maj, min, bld);
 		return -EINVAL;
 	}
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH firmware 3/4] rtl_nic: add new firmware for RTL8106E
From: Ben Hutchings @ 2012-07-10  0:08 UTC (permalink / raw)
  To: Hayes Wang; +Cc: dwmw2, romieu, netdev, linux-kernel
In-Reply-To: <1341803512-1309-3-git-send-email-hayeswang@realtek.com>

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

On Mon, 2012-07-09 at 11:11 +0800, Hayes Wang wrote:
> File: rtl_nic/rtl8106e-1.fw
> Version: 0.0.1
[...]

As someone already pointed out, this message was not correctly encoded.
The character encoding was specified as an empty string
(Content-Encoding header) or whitespace (Subject):

> Content-Type: text/plain; charset=""
[...]
> Subject: =?
> 	?q?=5BPATCH=20firmware=203/4=5D=20rtl=5Fnic=3A=20add=20new=20firmware=20for=20RTL8106E?=

I fixed that up just this once, but please fix whatever causes that
because I don't wish to have to do so again.

Ben.

-- 
Ben Hutchings
The generation of random numbers is too important to be left to chance.
                                                            - Robert Coveyou

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

^ permalink raw reply

* Re: [PATCH firmware 4/4] rtl_nic: add new firmware for RTL8168G
From: Ben Hutchings @ 2012-07-10  0:08 UTC (permalink / raw)
  To: Hayes Wang; +Cc: dwmw2, romieu, netdev, linux-kernel
In-Reply-To: <1341803512-1309-4-git-send-email-hayeswang@realtek.com>

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

On Mon, 2012-07-09 at 11:11 +0800, Hayes Wang wrote:
> File: rtl_nic/rtl8168g-1.fw
> Version: 0.0.1
[...]

Also had the encoding problem, but also applied.

Ben.

-- 
Ben Hutchings
The generation of random numbers is too important to be left to chance.
                                                            - Robert Coveyou

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

^ permalink raw reply

* [PATCH 01/11] lance: remove unnecessary setting of skb->dev
From: Jon Mason @ 2012-07-10  0:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev

skb->dev is being unnecessarily set during ring init.  It is already being set
to the proper value when eth_type_trans is called on packet receive, and the
skb->dev is not referenced anywhere else in the code.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
 drivers/net/ethernet/amd/lance.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/amd/lance.c b/drivers/net/ethernet/amd/lance.c
index a6e2e84..5c72843 100644
--- a/drivers/net/ethernet/amd/lance.c
+++ b/drivers/net/ethernet/amd/lance.c
@@ -873,10 +873,9 @@ lance_init_ring(struct net_device *dev, gfp_t gfp)
 
 		skb = alloc_skb(PKT_BUF_SZ, GFP_DMA | gfp);
 		lp->rx_skbuff[i] = skb;
-		if (skb) {
-			skb->dev = dev;
+		if (skb)
 			rx_buff = skb->data;
-		} else
+		else
 			rx_buff = kmalloc(PKT_BUF_SZ, GFP_DMA | gfp);
 		if (rx_buff == NULL)
 			lp->rx_ring[i].base = 0;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 02/11] enic: remove unnecessary setting of skb->dev
From: Jon Mason @ 2012-07-10  0:09 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Christian Benvenuti, Roopa Prabhu, Neel Patel,
	Nishank Trivedi
In-Reply-To: <1341878975-10577-1-git-send-email-jdmason@kudzu.us>

skb->dev is being unnecessarily set after calling eth_type_trans.
eth_type_trans already sets skb->dev to the proper value, thus making this
unnecessary.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
Cc: Christian Benvenuti <benve@cisco.com>
Cc: Roopa Prabhu <roprabhu@cisco.com>
Cc: Neel Patel <neepatel@cisco.com>
Cc: Nishank Trivedi <nistrive@cisco.com>
---
 drivers/net/ethernet/cisco/enic/enic_main.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 8132c78..ad1468b 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -1300,8 +1300,6 @@ static void enic_rq_indicate_buf(struct vnic_rq *rq,
 			skb->ip_summed = CHECKSUM_COMPLETE;
 		}
 
-		skb->dev = netdev;
-
 		if (vlan_stripped)
 			__vlan_hwaccel_put_tag(skb, vlan_tci);
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 03/11] netxen: remove unnecessary setting of skb->dev
From: Jon Mason @ 2012-07-10  0:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Sony Chacko, Rajesh Borundia
In-Reply-To: <1341878975-10577-1-git-send-email-jdmason@kudzu.us>

skb->dev is being unnecessarily set by the driver on packet recieve.
eth_type_trans already sets skb->dev to the proper value and it is not
referenced anywhere else in the dirver, thus making its setting unnecessary.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
Cc: Sony Chacko <sony.chacko@qlogic.com>
Cc: Rajesh Borundia <rajesh.borundia@qlogic.com>
---
 .../net/ethernet/qlogic/netxen/netxen_nic_init.c   |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index 8694124..b2c1b676 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -1532,8 +1532,6 @@ static struct sk_buff *netxen_process_rxbuf(struct netxen_adapter *adapter,
 	} else
 		skb->ip_summed = CHECKSUM_NONE;
 
-	skb->dev = adapter->netdev;
-
 	buffer->skb = NULL;
 no_skb:
 	buffer->state = NETXEN_BUFFER_FREE;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 04/11] lantiq_etop: remove unnecessary setting of skb->dev
From: Jon Mason @ 2012-07-10  0:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev
In-Reply-To: <1341878975-10577-1-git-send-email-jdmason@kudzu.us>

skb->dev is being unnecessarily set before calling eth_type_trans.
eth_type_trans already sets skb->dev to the proper value, thus making this
unnecessary.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
 drivers/net/ethernet/lantiq_etop.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
index 5dc9cbd..9fa39eb 100644
--- a/drivers/net/ethernet/lantiq_etop.c
+++ b/drivers/net/ethernet/lantiq_etop.c
@@ -149,7 +149,6 @@ ltq_etop_hw_receive(struct ltq_etop_chan *ch)
 	spin_unlock_irqrestore(&priv->lock, flags);
 
 	skb_put(skb, len);
-	skb->dev = ch->netdev;
 	skb->protocol = eth_type_trans(skb, ch->netdev);
 	netif_receive_skb(skb);
 }
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 05/11] ksz884x: remove unnecessary setting of skb->dev
From: Jon Mason @ 2012-07-10  0:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev
In-Reply-To: <1341878975-10577-1-git-send-email-jdmason@kudzu.us>

skb->dev is being unnecessarily set during ring init.  It is already being set
to the proper value when eth_type_trans is called on packet receive, and the
skb->dev is not referenced anywhere else in the code.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
 drivers/net/ethernet/micrel/ksz884x.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ksz884x.c b/drivers/net/ethernet/micrel/ksz884x.c
index eaf9ff0..2b59e39 100644
--- a/drivers/net/ethernet/micrel/ksz884x.c
+++ b/drivers/net/ethernet/micrel/ksz884x.c
@@ -4480,14 +4480,12 @@ static void ksz_init_rx_buffers(struct dev_info *adapter)
 		dma_buf->len = adapter->mtu;
 		if (!dma_buf->skb)
 			dma_buf->skb = alloc_skb(dma_buf->len, GFP_ATOMIC);
-		if (dma_buf->skb && !dma_buf->dma) {
-			dma_buf->skb->dev = adapter->dev;
+		if (dma_buf->skb && !dma_buf->dma)
 			dma_buf->dma = pci_map_single(
 				adapter->pdev,
 				skb_tail_pointer(dma_buf->skb),
 				dma_buf->len,
 				PCI_DMA_FROMDEVICE);
-		}
 
 		/* Set descriptor. */
 		set_rx_buf(desc, dma_buf->dma);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 06/11] qlcnic: remove unnecessary setting of skb->dev
From: Jon Mason @ 2012-07-10  0:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Anirban Chakraborty, Sony Chacko, linux-driver
In-Reply-To: <1341878975-10577-1-git-send-email-jdmason@kudzu.us>

skb->dev is being unnecessarily set before calling eth_type_trans.
eth_type_trans already sets skb->dev to the proper value, thus making this
unnecessary.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
Cc: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
Cc: Sony Chacko <sony.chacko@qlogic.com>
Cc: linux-driver@qlogic.com
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
index 8620b69..0bcda9c 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
@@ -1488,8 +1488,6 @@ static struct sk_buff *qlcnic_process_rxbuf(struct qlcnic_adapter *adapter,
 		skb_checksum_none_assert(skb);
 	}
 
-	skb->dev = adapter->netdev;
-
 	buffer->skb = NULL;
 
 	return skb;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 07/11] qlge: remove unnecessary setting of skb->dev
From: Jon Mason @ 2012-07-10  0:09 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Anirban Chakraborty, Jitendra Kalsaria, Ron Mercer,
	linux-driver
In-Reply-To: <1341878975-10577-1-git-send-email-jdmason@kudzu.us>

skb->dev is being unnecessarily set by the driver on packet recieve.
eth_type_trans already sets skb->dev to the proper value and it is not
referenced anywhere else in the dirver, thus making its setting unnecessary.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
Cc: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
Cc: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Cc: Ron Mercer <ron.mercer@qlogic.com>
Cc: linux-driver@qlogic.com
---
 drivers/net/ethernet/qlogic/qlge/qlge_main.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index 3c3499d..ca427eb 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -1619,7 +1619,6 @@ static void ql_process_mac_rx_skb(struct ql_adapter *qdev,
 	}
 
 	prefetch(skb->data);
-	skb->dev = ndev;
 	if (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) {
 		netif_printk(qdev, rx_status, KERN_DEBUG, qdev->ndev,
 			     "%s Multicast.\n",
@@ -1934,7 +1933,6 @@ static void ql_process_mac_split_rx_intr(struct ql_adapter *qdev,
 	}
 
 	prefetch(skb->data);
-	skb->dev = ndev;
 	if (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) {
 		netif_printk(qdev, rx_status, KERN_DEBUG, qdev->ndev, "%s Multicast.\n",
 			     (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) ==
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 08/11] sunbmac: remove unnecessary setting of skb->dev
From: Jon Mason @ 2012-07-10  0:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev
In-Reply-To: <1341878975-10577-1-git-send-email-jdmason@kudzu.us>

skb->dev is being unnecessarily set during ring init and skb alloc in rx.  It is
already being set to the proper value when eth_type_trans is called on packet
receive, and the skb->dev is not referenced anywhere else in the code.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
 drivers/net/ethernet/sun/sunbmac.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunbmac.c b/drivers/net/ethernet/sun/sunbmac.c
index 2a83fc5..967fe8c 100644
--- a/drivers/net/ethernet/sun/sunbmac.c
+++ b/drivers/net/ethernet/sun/sunbmac.c
@@ -233,7 +233,6 @@ static void bigmac_init_rings(struct bigmac *bp, int from_irq)
 			continue;
 
 		bp->rx_skbs[i] = skb;
-		skb->dev = dev;
 
 		/* Because we reserve afterwards. */
 		skb_put(skb, ETH_FRAME_LEN);
@@ -838,7 +837,6 @@ static void bigmac_rx(struct bigmac *bp)
 					 RX_BUF_ALLOC_SIZE - 34,
 					 DMA_FROM_DEVICE);
 			bp->rx_skbs[elem] = new_skb;
-			new_skb->dev = bp->dev;
 			skb_put(new_skb, ETH_FRAME_LEN);
 			skb_reserve(new_skb, 34);
 			this->rx_addr =
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 09/11] sungem: remove unnecessary setting of skb->dev
From: Jon Mason @ 2012-07-10  0:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev
In-Reply-To: <1341878975-10577-1-git-send-email-jdmason@kudzu.us>

skb->dev is being unnecessarily set by the driver's skb alloc routine (which is
called in init and during rx).  It is already being set to the proper value when
eth_type_trans is called on packet receive, and the skb->dev is not referenced
anywhere else in the code.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
 drivers/net/ethernet/sun/sungem.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/sun/sungem.c b/drivers/net/ethernet/sun/sungem.c
index 3cf4ab7..9ae12d0 100644
--- a/drivers/net/ethernet/sun/sungem.c
+++ b/drivers/net/ethernet/sun/sungem.c
@@ -752,7 +752,6 @@ static __inline__ struct sk_buff *gem_alloc_skb(struct net_device *dev, int size
 	if (likely(skb)) {
 		unsigned long offset = ALIGNED_RX_SKB_ADDR(skb->data);
 		skb_reserve(skb, offset);
-		skb->dev = dev;
 	}
 	return skb;
 }
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 10/11] sunhme: remove unnecessary setting of skb->dev
From: Jon Mason @ 2012-07-10  0:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev
In-Reply-To: <1341878975-10577-1-git-send-email-jdmason@kudzu.us>

skb->dev is being unnecessarily set during ring init and skb alloc in rx.  It is
already being set to the proper value when eth_type_trans is called on packet
receive, and the skb->dev is not referenced anywhere else in the code.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
 drivers/net/ethernet/sun/sunhme.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c
index dfc00c4..73f341b 100644
--- a/drivers/net/ethernet/sun/sunhme.c
+++ b/drivers/net/ethernet/sun/sunhme.c
@@ -1249,7 +1249,6 @@ static void happy_meal_clean_rings(struct happy_meal *hp)
 static void happy_meal_init_rings(struct happy_meal *hp)
 {
 	struct hmeal_init_block *hb = hp->happy_block;
-	struct net_device *dev = hp->dev;
 	int i;
 
 	HMD(("happy_meal_init_rings: counters to zero, "));
@@ -1270,7 +1269,6 @@ static void happy_meal_init_rings(struct happy_meal *hp)
 			continue;
 		}
 		hp->rx_skbs[i] = skb;
-		skb->dev = dev;
 
 		/* Because we reserve afterwards. */
 		skb_put(skb, (ETH_FRAME_LEN + RX_OFFSET + 4));
@@ -2031,7 +2029,6 @@ static void happy_meal_rx(struct happy_meal *hp, struct net_device *dev)
 			}
 			dma_unmap_single(hp->dma_dev, dma_addr, RX_BUF_ALLOC_SIZE, DMA_FROM_DEVICE);
 			hp->rx_skbs[elem] = new_skb;
-			new_skb->dev = dev;
 			skb_put(new_skb, (ETH_FRAME_LEN + RX_OFFSET + 4));
 			hme_write_rxd(hp, this,
 				      (RXFLAG_OWN|((RX_BUF_ALLOC_SIZE-RX_OFFSET)<<16)),
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 11/11] ll_temac: remove unnecessary setting of skb->dev
From: Jon Mason @ 2012-07-10  0:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev
In-Reply-To: <1341878975-10577-1-git-send-email-jdmason@kudzu.us>

skb->dev is being unnecessarily set by the driver on packet recieve.
eth_type_trans already sets skb->dev to the proper value and it is not
referenced anywhere else in the dirver, thus making its setting unnecessary.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
 drivers/net/ethernet/xilinx/ll_temac_main.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 1eaf712..705146d 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -768,7 +768,6 @@ static void ll_temac_recv(struct net_device *ndev)
 				 DMA_FROM_DEVICE);
 
 		skb_put(skb, length);
-		skb->dev = ndev;
 		skb->protocol = eth_type_trans(skb, ndev);
 		skb_checksum_none_assert(skb);
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 0/4] Calxeda xgmac fixes and performance improvements
From: Rob Herring @ 2012-07-10  0:16 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: David S. Miller, Rob Herring

From: Rob Herring <rob.herring@calxeda.com>

A few fixes and performance improvements for the Calxeda xgmac driver for
3.6. It would be nice to get the 2 fixes into 3.5, but since it is a bit
late in the cycle they can wait.

Rob

Rob Herring (4):
  net: calxedaxgmac: fix net timeout recovery
  net: calxedaxgmac: fix hang on rx refill
  net: calxedaxgmac: set outstanding AXI bus transactions to 8
  net: calxedaxgmac: enable rx cut-thru mode

 drivers/net/ethernet/calxeda/xgmac.c |   35 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 18 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH 1/4] net: calxedaxgmac: fix net timeout recovery
From: Rob Herring @ 2012-07-10  0:16 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: David S. Miller, Rob Herring
In-Reply-To: <1341879370-23385-1-git-send-email-robherring2@gmail.com>

From: Rob Herring <rob.herring@calxeda.com>

Fix net tx watchdog timeout recovery. The descriptor ring was reset,
but the DMA engine was not reset to the beginning of the ring.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/net/ethernet/calxeda/xgmac.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 11f667f..c4fd2e3 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -933,6 +933,7 @@ static void xgmac_tx_err(struct xgmac_priv *priv)
 	desc_init_tx_desc(priv->dma_tx, DMA_TX_RING_SZ);
 	priv->tx_tail = 0;
 	priv->tx_head = 0;
+	writel(priv->dma_tx_phy, priv->base + XGMAC_DMA_TX_BASE_ADDR);
 	writel(reg | DMA_CONTROL_ST, priv->base + XGMAC_DMA_CONTROL);
 
 	writel(DMA_STATUS_TU | DMA_STATUS_TPS | DMA_STATUS_NIS | DMA_STATUS_AIS,
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 2/4] net: calxedaxgmac: fix hang on rx refill
From: Rob Herring @ 2012-07-10  0:16 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: David S. Miller, Rob Herring
In-Reply-To: <1341879370-23385-1-git-send-email-robherring2@gmail.com>

From: Rob Herring <rob.herring@calxeda.com>

Fix intermittent hangs in xgmac_rx_refill. If a ring buffer entry already
had an skb allocated, then xgmac_rx_refill would get stuck in a loop. This
can happen on a rx error when we just leave the skb allocated to the entry.

[ 7884.510000] INFO: rcu_preempt detected stall on CPU 0 (t=727315 jiffies)
[ 7884.510000] [<c0010a59>] (unwind_backtrace+0x1/0x98) from [<c006fd93>] (__rcu_pending+0x11b/0x2c4)
[ 7884.510000] [<c006fd93>] (__rcu_pending+0x11b/0x2c4) from [<c0070b95>] (rcu_check_callbacks+0xed/0x1a8)
[ 7884.510000] [<c0070b95>] (rcu_check_callbacks+0xed/0x1a8) from [<c0036abb>] (update_process_times+0x2b/0x48)
[ 7884.510000] [<c0036abb>] (update_process_times+0x2b/0x48) from [<c004e8fd>] (tick_sched_timer+0x51/0x94)
[ 7884.510000] [<c004e8fd>] (tick_sched_timer+0x51/0x94) from [<c0045527>] (__run_hrtimer+0x4f/0x1e8)
[ 7884.510000] [<c0045527>] (__run_hrtimer+0x4f/0x1e8) from [<c0046003>] (hrtimer_interrupt+0xd7/0x1e4)
[ 7884.510000] [<c0046003>] (hrtimer_interrupt+0xd7/0x1e4) from [<c00101d3>] (twd_handler+0x17/0x24)
[ 7884.510000] [<c00101d3>] (twd_handler+0x17/0x24) from [<c006be39>] (handle_percpu_devid_irq+0x59/0x114)
[ 7884.510000] [<c006be39>] (handle_percpu_devid_irq+0x59/0x114) from [<c0069aab>] (generic_handle_irq+0x17/0x2c)
[ 7884.510000] [<c0069aab>] (generic_handle_irq+0x17/0x2c) from [<c000cc8d>] (handle_IRQ+0x35/0x7c)
[ 7884.510000] [<c000cc8d>] (handle_IRQ+0x35/0x7c) from [<c033b153>] (__irq_svc+0x33/0xb8)
[ 7884.510000] [<c033b153>] (__irq_svc+0x33/0xb8) from [<c0244b06>] (xgmac_rx_refill+0x3a/0x140)
[ 7884.510000] [<c0244b06>] (xgmac_rx_refill+0x3a/0x140) from [<c02458ed>] (xgmac_poll+0x265/0x3bc)
[ 7884.510000] [<c02458ed>] (xgmac_poll+0x265/0x3bc) from [<c029fcbf>] (net_rx_action+0xc3/0x200)
[ 7884.510000] [<c029fcbf>] (net_rx_action+0xc3/0x200) from [<c0030cab>] (__do_softirq+0xa3/0x1bc)

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/net/ethernet/calxeda/xgmac.c |   27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index c4fd2e3..3ca1d79 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -671,26 +671,23 @@ static void xgmac_rx_refill(struct xgmac_priv *priv)
 
 		p = priv->dma_rx + entry;
 
-		if (priv->rx_skbuff[entry] != NULL)
-			continue;
-
-		skb = __skb_dequeue(&priv->rx_recycle);
-		if (skb == NULL)
-			skb = netdev_alloc_skb(priv->dev, priv->dma_buf_sz);
-		if (unlikely(skb == NULL))
-			break;
-
-		priv->rx_skbuff[entry] = skb;
-		paddr = dma_map_single(priv->device, skb->data,
-					 priv->dma_buf_sz, DMA_FROM_DEVICE);
-		desc_set_buf_addr(p, paddr, priv->dma_buf_sz);
+		if (priv->rx_skbuff[entry] == NULL) {
+			skb = __skb_dequeue(&priv->rx_recycle);
+			if (skb == NULL)
+				skb = netdev_alloc_skb(priv->dev, priv->dma_buf_sz);
+			if (unlikely(skb == NULL))
+				break;
+
+			priv->rx_skbuff[entry] = skb;
+			paddr = dma_map_single(priv->device, skb->data,
+					       priv->dma_buf_sz, DMA_FROM_DEVICE);
+			desc_set_buf_addr(p, paddr, priv->dma_buf_sz);
+		}
 
 		netdev_dbg(priv->dev, "rx ring: head %d, tail %d\n",
 			priv->rx_head, priv->rx_tail);
 
 		priv->rx_head = dma_ring_incr(priv->rx_head, DMA_RX_RING_SZ);
-		/* Ensure descriptor is in memory before handing to h/w */
-		wmb();
 		desc_set_rx_owner(p);
 	}
 }
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 4/4] net: calxedaxgmac: enable rx cut-thru mode
From: Rob Herring @ 2012-07-10  0:16 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: David S. Miller, Rob Herring
In-Reply-To: <1341879370-23385-1-git-send-email-robherring2@gmail.com>

From: Rob Herring <rob.herring@calxeda.com>

Enabling RX cut-thru mode yields better performance as received frames
start getting written to memory before a whole frame is received.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/net/ethernet/calxeda/xgmac.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index abb8f40..2b4b4f5 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -264,7 +264,7 @@
 #define XGMAC_OMR_FEF		0x00000080	/* Forward Error Frames */
 #define XGMAC_OMR_DT		0x00000040	/* Drop TCP/IP csum Errors */
 #define XGMAC_OMR_RSF		0x00000020	/* RX FIFO Store and Forward */
-#define XGMAC_OMR_RTC		0x00000010	/* RX Threshhold Ctrl */
+#define XGMAC_OMR_RTC_256	0x00000018	/* RX Threshhold Ctrl */
 #define XGMAC_OMR_RTC_MASK	0x00000018	/* RX Threshhold Ctrl MASK */
 
 /* XGMAC HW Features Register */
@@ -982,7 +982,8 @@ static int xgmac_hw_init(struct net_device *dev)
 	writel(value, ioaddr + XGMAC_DMA_CONTROL);
 
 	/* Set the HW DMA mode and the COE */
-	writel(XGMAC_OMR_TSF | XGMAC_OMR_RSF | XGMAC_OMR_RFD | XGMAC_OMR_RFA,
+	writel(XGMAC_OMR_TSF | XGMAC_OMR_RFD | XGMAC_OMR_RFA |
+		XGMAC_OMR_RTC_256,
 		ioaddr + XGMAC_OMR);
 
 	/* Reset the MMC counters */
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 3/4] net: calxedaxgmac: set outstanding AXI bus transactions to 8
From: Rob Herring @ 2012-07-10  0:16 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: David S. Miller, Rob Herring
In-Reply-To: <1341879370-23385-1-git-send-email-robherring2@gmail.com>

From: Rob Herring <rob.herring@calxeda.com>

Increase the number of outstanding read and write AXI transactions from 1
to 8 for better performance.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/net/ethernet/calxeda/xgmac.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 3ca1d79..abb8f40 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -970,7 +970,7 @@ static int xgmac_hw_init(struct net_device *dev)
 	writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_INTR_ENA);
 
 	/* XGMAC requires AXI bus init. This is a 'magic number' for now */
-	writel(0x000100E, ioaddr + XGMAC_DMA_AXI_BUS);
+	writel(0x0077000E, ioaddr + XGMAC_DMA_AXI_BUS);
 
 	ctrl |= XGMAC_CONTROL_DDIC | XGMAC_CONTROL_JE | XGMAC_CONTROL_ACS |
 		XGMAC_CONTROL_CAR;
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH v2] fail dentry revalidation after namespace change
From: Eric W. Biederman @ 2012-07-10  0:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Glauber Costa, linux-kernel, netdev, Greg Thelen, Serge Hallyn,
	Tejun Heo, Greg Kroah-Hartman
In-Reply-To: <20120709161336.0ec23592.akpm@linux-foundation.org>

Andrew Morton <akpm@linux-foundation.org> writes:

>>  {
>>  	struct sysfs_dirent *sd;
>>  	int is_dir;
>> +	int type;
>>  
>>  	if (nd->flags & LOOKUP_RCU)
>>  		return -ECHILD;
>> @@ -326,6 +327,13 @@ static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd)
>>  	if (strcmp(dentry->d_name.name, sd->s_name) != 0)
>>  		goto out_bad;
>>  
>> +	/* The sysfs dirent has been moved to a different namespace */
>> +	type = KOBJ_NS_TYPE_NONE;
>> +	if (sd->s_parent)
>> +		type = sysfs_ns_type(sd->s_parent);
>> +	if (type && (sysfs_info(dentry->d_sb)->ns[type] != sd->s_ns))
>
> eww, the code is assuming that KOBJ_NS_TYPE_NONE has a value of zero. 
> Don't do that; it smells bad.

Gag.  An incomplete change in idiom.

KOBJ_NS_TYPE_NONE is explicitly defined as 0 so that it can be used
this way, and every where else in fs/sysfs/dir.c uses this idiom.

Furthermore your change below takes one line of readable code and turns
it into something inappropriate to talk about in polite company.

If you want the code to be perfect type should be defined as
"enum kobj_ns_type type" instead of "int kobj_ns_type".

Of course the truly perfect solution is to rework the sysfs
code in a manner similar to proc, with magic internal symlinks
and multiple parallel tress for the different namespaces.
For the users of sysfs semantically there would be no changes
but in the implementation there would many fewer special cases
for namespaces.   The only special case would be reduced to
the internal sysfs symlink that lookup would have to know
about.

> @@ -329,10 +329,12 @@ static int sysfs_dentry_revalidate(struc
>  
>  	/* The sysfs dirent has been moved to a different namespace */
>  	type = KOBJ_NS_TYPE_NONE;
> -	if (sd->s_parent)
> +	if (sd->s_parent) {
>  		type = sysfs_ns_type(sd->s_parent);
> -	if (type && (sysfs_info(dentry->d_sb)->ns[type] != sd->s_ns))
> -		goto out_bad;
> +		if (type != KOBJ_NS_TYPE_NONE &&
> +				sysfs_info(dentry->d_sb)->ns[type] != sd->s_ns)
> +			goto out_bad;
> +	}

Pray tell in what parallel universe is that monstrosity above more
readable than the line it replaces?

Eric

^ permalink raw reply

* Re: [PATCH v2] fail dentry revalidation after namespace change
From: Andrew Morton @ 2012-07-10  0:47 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Glauber Costa, linux-kernel, netdev, Greg Thelen, Serge Hallyn,
	Tejun Heo, Greg Kroah-Hartman
In-Reply-To: <87txxgxxs7.fsf@xmission.com>

On Mon, 09 Jul 2012 17:30:48 -0700 ebiederm@xmission.com (Eric W. Biederman) wrote:

> Andrew Morton <akpm@linux-foundation.org> writes:
> 
> >>  {
> >>  	struct sysfs_dirent *sd;
> >>  	int is_dir;
> >> +	int type;
> >>  
> >>  	if (nd->flags & LOOKUP_RCU)
> >>  		return -ECHILD;
> >> @@ -326,6 +327,13 @@ static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd)
> >>  	if (strcmp(dentry->d_name.name, sd->s_name) != 0)
> >>  		goto out_bad;
> >>  
> >> +	/* The sysfs dirent has been moved to a different namespace */
> >> +	type = KOBJ_NS_TYPE_NONE;
> >> +	if (sd->s_parent)
> >> +		type = sysfs_ns_type(sd->s_parent);
> >> +	if (type && (sysfs_info(dentry->d_sb)->ns[type] != sd->s_ns))
> >
> > eww, the code is assuming that KOBJ_NS_TYPE_NONE has a value of zero. 
> > Don't do that; it smells bad.
> 
> Gag.  An incomplete change in idiom.
> 
> KOBJ_NS_TYPE_NONE is explicitly defined as 0 so that it can be used
> this way, and every where else in fs/sysfs/dir.c uses this idiom.

One man's idiom is another man's idiocy.

Seriously.  What sort of idea is that?  Create an enumerated type and
then just ignore it?

> Pray tell in what parallel universe is that monstrosity above more
> readable than the line it replaces?

Don't be silly, it is not a "monstrosity".  The code it is modifying
contains an unneeded test-and-branch.  It's a test and branch which the
compiler might be able to avoid.  If we can demonstrate that the
compiler does indeed optimise it, or if we can find a less monstrous
way of implementing it then fine.  Otherwise, efficiency wins.

^ permalink raw reply

* Re: [PATCH v2] fail dentry revalidation after namespace change
From: Eric W. Biederman @ 2012-07-10  1:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Glauber Costa, linux-kernel, netdev, Greg Thelen, Serge Hallyn,
	Tejun Heo, Greg Kroah-Hartman
In-Reply-To: <20120709174705.0e2078c8.akpm@linux-foundation.org>

Andrew Morton <akpm@linux-foundation.org> writes:

> On Mon, 09 Jul 2012 17:30:48 -0700 ebiederm@xmission.com (Eric W. Biederman) wrote:
>
>> Andrew Morton <akpm@linux-foundation.org> writes:
>> 
>> >>  {
>> >>  	struct sysfs_dirent *sd;
>> >>  	int is_dir;
>> >> +	int type;
>> >>  
>> >>  	if (nd->flags & LOOKUP_RCU)
>> >>  		return -ECHILD;
>> >> @@ -326,6 +327,13 @@ static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd)
>> >>  	if (strcmp(dentry->d_name.name, sd->s_name) != 0)
>> >>  		goto out_bad;
>> >>  
>> >> +	/* The sysfs dirent has been moved to a different namespace */
>> >> +	type = KOBJ_NS_TYPE_NONE;
>> >> +	if (sd->s_parent)
>> >> +		type = sysfs_ns_type(sd->s_parent);
>> >> +	if (type && (sysfs_info(dentry->d_sb)->ns[type] != sd->s_ns))
>> >
>> > eww, the code is assuming that KOBJ_NS_TYPE_NONE has a value of zero. 
>> > Don't do that; it smells bad.
>> 
>> Gag.  An incomplete change in idiom.
>> 
>> KOBJ_NS_TYPE_NONE is explicitly defined as 0 so that it can be used
>> this way, and every where else in fs/sysfs/dir.c uses this idiom.
>
> One man's idiom is another man's idiocy.

And code that uses inconsistent idioms is even harder to read.

A half assed cleanup is worse than no cleanup.

> Seriously.  What sort of idea is that?  Create an enumerated type and
> then just ignore it?

It isn't ignored.  It just has a well defined NULL value. That is hardly
controversial.

>> Pray tell in what parallel universe is that monstrosity above more
>> readable than the line it replaces?
>
> Don't be silly, it is not a "monstrosity".  The code it is modifying
> contains an unneeded test-and-branch.  It's a test and branch which the
> compiler might be able to avoid.  If we can demonstrate that the
> compiler does indeed optimise it, or if we can find a less monstrous
> way of implementing it then fine.  Otherwise, efficiency wins.

Efficiency wins?  In a rarely used function?  Which kernel are you
working on?

Readable maintainable code wins.  Unreadable code causes regressions.

Your addition while it may not be monstrous is most definitely less
readable.

Eric

^ permalink raw reply

* Re: [PATCH v2] fail dentry revalidation after namespace change
From: Andrew Morton @ 2012-07-10  2:15 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Glauber Costa, linux-kernel, netdev, Greg Thelen, Serge Hallyn,
	Tejun Heo, Greg Kroah-Hartman
In-Reply-To: <87629wxu1i.fsf@xmission.com>

On Mon, 09 Jul 2012 18:51:37 -0700 ebiederm@xmission.com (Eric W. Biederman) wrote:

> Andrew Morton <akpm@linux-foundation.org> writes:
> 
> > On Mon, 09 Jul 2012 17:30:48 -0700 ebiederm@xmission.com (Eric W. Biederman) wrote:
> >
> >> Andrew Morton <akpm@linux-foundation.org> writes:
> >> 
> >> >>  {
> >> >>  	struct sysfs_dirent *sd;
> >> >>  	int is_dir;
> >> >> +	int type;
> >> >>  
> >> >>  	if (nd->flags & LOOKUP_RCU)
> >> >>  		return -ECHILD;
> >> >> @@ -326,6 +327,13 @@ static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd)
> >> >>  	if (strcmp(dentry->d_name.name, sd->s_name) != 0)
> >> >>  		goto out_bad;
> >> >>  
> >> >> +	/* The sysfs dirent has been moved to a different namespace */
> >> >> +	type = KOBJ_NS_TYPE_NONE;
> >> >> +	if (sd->s_parent)
> >> >> +		type = sysfs_ns_type(sd->s_parent);
> >> >> +	if (type && (sysfs_info(dentry->d_sb)->ns[type] != sd->s_ns))
> >> >
> >> > eww, the code is assuming that KOBJ_NS_TYPE_NONE has a value of zero. 
> >> > Don't do that; it smells bad.
> >> 
> >> Gag.  An incomplete change in idiom.
> >> 
> >> KOBJ_NS_TYPE_NONE is explicitly defined as 0 so that it can be used
> >> this way, and every where else in fs/sysfs/dir.c uses this idiom.
> >
> > One man's idiom is another man's idiocy.
> 
> And code that uses inconsistent idioms is even harder to read.

Not true.  That patch is more readable when it is changed to use
correct types.  If only because readers don't need to go in and check
that KOBJ_NS_TYPE_NONE has value zero.

> > Seriously.  What sort of idea is that?  Create an enumerated type and
> > then just ignore it?
> 
> It isn't ignored.  It just has a well defined NULL value. That is hardly
> controversial.

If it's uncontroversial, why are we talking about it?  Why did I, an
experienced C and kernel developer, think that it looked stupid and
possibly buggy?

I'm uncomfortable with propagating this idiotic and unnecessary trick
any further.  It's better to fix it.

> >> Pray tell in what parallel universe is that monstrosity above more
> >> readable than the line it replaces?
> >
> > Don't be silly, it is not a "monstrosity".  The code it is modifying
> > contains an unneeded test-and-branch.  It's a test and branch which the
> > compiler might be able to avoid.  If we can demonstrate that the
> > compiler does indeed optimise it, or if we can find a less monstrous
> > way of implementing it then fine.  Otherwise, efficiency wins.
> 
> Efficiency wins?  In a rarely used function?  Which kernel are you
> working on?

One in which we frequently optimise uncommon code paths.

> Readable maintainable code wins.  Unreadable code causes regressions.

Dude, the whole reason for having enums and enumerated types is for
readability and maintainability.  If we didn't care about that, we'd
use literal constants everywhere.  And here you are arguing against
that readability and maintainability.

If you want to say "yes, the sysfs code is bad but I can't be bothered
fixing it all" then grumble, but OK.  But for heavens sake, don't go
and *defend* what that code is doing.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox