diff mbox

skge: convert to hw_features

Message ID 20110331110135.9625513A68@rere.qmqm.pl
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Michał Mirosław March 31, 2011, 11:01 a.m. UTC 
>From the code it looks like the hardware might support full HW_CSUM not
just IP_CSUM.  This needs testing and so is not changed here.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/skge.c |   53 ++++-----------------------------------------------
 drivers/net/skge.h |    1 -
 2 files changed, 5 insertions(+), 49 deletions(-)

[Simple conversion to hw_features, where constraints can be
detected at device init time.]

Comments

Stephen Hemminger March 31, 2011, 3:41 p.m. UTC | #1 
On Thu, 31 Mar 2011 13:01:35 +0200 (CEST)
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> >From the code it looks like the hardware might support full HW_CSUM not
> just IP_CSUM.  This needs testing and so is not changed here.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

I don't think this will work right. The old code did not allow setting
tx/rx csum on the older GENESIS chip,
Michał Mirosław March 31, 2011, 8:48 p.m. UTC | #2 
On Thu, Mar 31, 2011 at 08:41:02AM -0700, Stephen Hemminger wrote:
> On Thu, 31 Mar 2011 13:01:35 +0200 (CEST)
> Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > >From the code it looks like the hardware might support full HW_CSUM not
> > just IP_CSUM.  This needs testing and so is not changed here.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> I don't think this will work right. The old code did not allow setting
> tx/rx csum on the older GENESIS chip, 

If you look at the changes near the end of patche, the hw_features are set
(and so features are made changeable) only when hw->chip_id != CHIP_ID_GENESIS.

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Hemminger March 31, 2011, 8:56 p.m. UTC | #3 
On Thu, 31 Mar 2011 22:48:03 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> On Thu, Mar 31, 2011 at 08:41:02AM -0700, Stephen Hemminger wrote:
> > On Thu, 31 Mar 2011 13:01:35 +0200 (CEST)
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > > >From the code it looks like the hardware might support full HW_CSUM not
> > > just IP_CSUM.  This needs testing and so is not changed here.
> > > 
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > I don't think this will work right. The old code did not allow setting
> > tx/rx csum on the older GENESIS chip, 
> 
> If you look at the changes near the end of patche, the hw_features are set
> (and so features are made changeable) only when hw->chip_id != CHIP_ID_GENESIS.
> 
> Best Regards,
> Michał Mirosław

I actually have one of those old boards, let me test it.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller April 6, 2011, 7:48 p.m. UTC | #4 
From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST)

>>From the code it looks like the hardware might support full HW_CSUM not
> just IP_CSUM.  This needs testing and so is not changed here.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Stephen, did you get a chance to test this patch like you said you
would?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller April 8, 2011, 3:17 a.m. UTC | #5 
From: David Miller <davem@davemloft.net>
Date: Wed, 06 Apr 2011 12:48:43 -0700 (PDT)

> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST)
> 
>>>From the code it looks like the hardware might support full HW_CSUM not
>> just IP_CSUM.  This needs testing and so is not changed here.
>> 
>> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 
> Stephen, did you get a chance to test this patch like you said you
> would?

I don't see any value in waiting any longer, so I've applied this patch.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/skge.c b/drivers/net/skge.c
index 35b28f4..0082808 100644
--- a/drivers/net/skge.c
+++ b/drivers/net/skge.c
@@ -537,46 +537,6 @@  static int skge_nway_reset(struct net_device *dev)
 	return 0;
 }
 
-static int skge_set_sg(struct net_device *dev, u32 data)
-{
-	struct skge_port *skge = netdev_priv(dev);
-	struct skge_hw *hw = skge->hw;
-
-	if (hw->chip_id == CHIP_ID_GENESIS && data)
-		return -EOPNOTSUPP;
-	return ethtool_op_set_sg(dev, data);
-}
-
-static int skge_set_tx_csum(struct net_device *dev, u32 data)
-{
-	struct skge_port *skge = netdev_priv(dev);
-	struct skge_hw *hw = skge->hw;
-
-	if (hw->chip_id == CHIP_ID_GENESIS && data)
-		return -EOPNOTSUPP;
-
-	return ethtool_op_set_tx_csum(dev, data);
-}
-
-static u32 skge_get_rx_csum(struct net_device *dev)
-{
-	struct skge_port *skge = netdev_priv(dev);
-
-	return skge->rx_csum;
-}
-
-/* Only Yukon supports checksum offload. */
-static int skge_set_rx_csum(struct net_device *dev, u32 data)
-{
-	struct skge_port *skge = netdev_priv(dev);
-
-	if (skge->hw->chip_id == CHIP_ID_GENESIS && data)
-		return -EOPNOTSUPP;
-
-	skge->rx_csum = data;
-	return 0;
-}
-
 static void skge_get_pauseparam(struct net_device *dev,
 				struct ethtool_pauseparam *ecmd)
 {
@@ -925,10 +885,6 @@  static const struct ethtool_ops skge_ethtool_ops = {
 	.set_pauseparam = skge_set_pauseparam,
 	.get_coalesce	= skge_get_coalesce,
 	.set_coalesce	= skge_set_coalesce,
-	.set_sg		= skge_set_sg,
-	.set_tx_csum	= skge_set_tx_csum,
-	.get_rx_csum	= skge_get_rx_csum,
-	.set_rx_csum	= skge_set_rx_csum,
 	.get_strings	= skge_get_strings,
 	.phys_id	= skge_phys_id,
 	.get_sset_count = skge_get_sset_count,
@@ -3085,7 +3041,8 @@  static struct sk_buff *skge_rx_get(struct net_device *dev,
 	}
 
 	skb_put(skb, len);
-	if (skge->rx_csum) {
+
+	if (dev->features & NETIF_F_RXCSUM) {
 		skb->csum = csum;
 		skb->ip_summed = CHECKSUM_COMPLETE;
 	}
@@ -3847,10 +3804,10 @@  static struct net_device *skge_devinit(struct skge_hw *hw, int port,
 	setup_timer(&skge->link_timer, xm_link_timer, (unsigned long) skge);
 
 	if (hw->chip_id != CHIP_ID_GENESIS) {
-		dev->features |= NETIF_F_IP_CSUM | NETIF_F_SG;
-		skge->rx_csum = 1;
+		dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
+		                   NETIF_F_RXCSUM;
+		dev->features |= dev->hw_features;
 	}
-	dev->features |= NETIF_F_GRO;
 
 	/* read the mac address */
 	memcpy_fromio(dev->dev_addr, hw->regs + B2_MAC_1 + port*8, ETH_ALEN);
diff --git a/drivers/net/skge.h b/drivers/net/skge.h
index 507addc..f055c47 100644
--- a/drivers/net/skge.h
+++ b/drivers/net/skge.h
@@ -2460,7 +2460,6 @@  struct skge_port {
 	struct timer_list    link_timer;
 	enum pause_control   flow_control;
 	enum pause_status    flow_status;
-	u8		     rx_csum;
 	u8		     blink_on;
 	u8		     wol;
 	u8		     autoneg;	/* AUTONEG_ENABLE, AUTONEG_DISABLE */