Following are the minor changes for [PATCH 1/9] in accordance with the
given suggestions.

Regards,
pradeep

diff -u linux-2.6.16.18/drivers/net/netxen/netxen_nic_ethtool.c
linux-2.6.16.18/drivers/net/netxen/netxen_nic_ethtool.c
--- linux-2.6.16.18/drivers/net/netxen/netxen_nic_ethtool.c
2006-05-25 02:43:22.000000000 -0700
+++ linux-2.6.16.18/drivers/net/netxen/netxen_nic_ethtool.c
2006-05-26 04:05:34.000000000 -0700
@@ -145,7 +145,7 @@

                ecmd->port = PORT_TP;

-               if (port->state) {
+               if (dev->flags & IFF_UP) {
                        ecmd->speed = port->link_speed;
                        ecmd->duplex = port->link_duplex;
                } else
@@ -512,16 +512,6 @@
        ring->rx_jumbo_pending = 0;
 }

-/*
- * Note: This change will be reflected in all the four ports as there
is
- * only one common adapter.
- */
-static int
-netxen_nic_set_ringparam(struct net_device *dev, struct
ethtool_ringparam *ring)
-{
-       return 0;
-}
-
 static void
 netxen_nic_get_pauseparam(struct net_device *dev,
                          struct ethtool_pauseparam *pause)
@@ -781,7 +771,6 @@
        .get_eeprom_len = netxen_nic_get_eeprom_len,
        .get_eeprom = netxen_nic_get_eeprom,
        .get_ringparam = netxen_nic_get_ringparam,
-       .set_ringparam = netxen_nic_set_ringparam,
        .get_pauseparam = netxen_nic_get_pauseparam,
        .set_pauseparam = netxen_nic_set_pauseparam,
        .get_rx_csum = netxen_nic_get_rx_csum,


On Thu, 2006-05-25 at 09:33 -0700, Stephen Hemminger wrote:
> Minor nits.
> 
> On Thu, 25 May 2006 03:48:38 -0700 (PDT)
> "Linsys Contractor Amit S. Kale" <[EMAIL PROTECTED]> wrote:
> 
> > +/*
> > + * Note: This change will be reflected in all the four ports as there is 
> > + * only one common adapter.
> > + */
> > +static int
> > +netxen_nic_set_ringparam(struct net_device *dev, struct ethtool_ringparam 
> > *ring)
> > +{
> > +   return 0;
> > +}
> 
> Why not just return have no hook if you can't set parameters.  Then the ioctl
> will return not supported -EOPNOTSUPP
> 
> >
> > +static u32 netxen_nic_get_rx_csum(struct net_device *dev)
> > +{
> > +   return (dev->features & NETIF_F_HW_CSUM);
> > +}
> 
> 
> You got receive and transmit checksum confused.  You need to separate
> checksumming on output (dev->features & NETIF_F_HW_CSUM) versus receive
> checksum (controlled by hardware and usually a flag in private data 
> structure).
> 
> > +static int netxen_nic_set_rx_csum(struct net_device *dev, u32 data)
> > +{
> > +   if (data)
> > +           dev->features |= NETIF_F_HW_CSUM;
> > +   else
> > +           dev->features &= (~NETIF_F_HW_CSUM);
> > +
> > +   if (netif_running(dev)) {
> > +           dev->stop(dev); /* verify */
> > +           dev->open(dev);
> 
> What if open fail fails?  Then you have an "interesting" recovery
> situation.  
> > +   }
> > +   return 0;
> > +}
> > 
> 

-- 
pradeep

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

Reply via email to