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