Subject: Re: [PATCH] dm9000: add checksum offload support

From: Mike Rapoport <[email-masked]>
Date: Thu, 02 Jul 2009 10:37:27 +0300

Ben Dooks wrote:
> Mike Rapoport wrote:
>> From: Yeasah Pell <[email-masked]>
> 
> Not a very informative message here. At least add a copy
> of the subject, better to elaborate on the patch contents
> here, such as the versions of the DM9000 that this will
> work on.

Sorry, my fault.

>> Signed-off-by: Yeasah Pell <[email-masked]>
>> Signed-off-by: Mike Rapoport <[email-masked]>
>> ---
>>  drivers/net/dm9000.c |  105
>> +++++++++++++++++++++++++++++++++++++++++--------
>>  drivers/net/dm9000.h |   18 +++++++++
>>  2 files changed, 106 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/net/dm9000.c b/drivers/net/dm9000.c
>> index dd771de..52b02d6 100644
>> --- a/drivers/net/dm9000.c
>> +++ b/drivers/net/dm9000.c
>> @@ -92,6 +92,7 @@ typedef struct board_info {
>>      u16        tx_pkt_cnt;
>>      u16        queue_pkt_len;
>>      u16        queue_start_addr;
>> +    u16        queue_ip_summed;
>>      u16        dbug_cnt;
>>      u8        io_mode;        /* 0:word, 2:byte */
>>      u8        phy_addr;
>> @@ -124,6 +125,9 @@ typedef struct board_info {
>>  
>>      struct mii_if_info mii;
>>      u32        msg_enable;
>> +
>> +    int        rx_csum;
>> +    int        can_csum;
>>  } board_info_t;
>>  
>>  /* debug code */
>> @@ -460,6 +464,40 @@ static int dm9000_nway_reset(struct net_device *dev)
>>      return mii_nway_restart(&dm->mii);
>>  }
>>  
>> +static uint32_t dm9000_get_rx_csum(struct net_device *dev)
>> +{
>> +    board_info_t *dm = to_dm9000_board(dev);
>> +    return dm->rx_csum;
>> +}
>> +
>> +static int dm9000_set_rx_csum(struct net_device *dev, uint32_t data)
>> +{
>> +    board_info_t *dm = to_dm9000_board(dev);
>> +    unsigned long flags;
>> +
>> +    if (dm->can_csum) {
>> +        dm->rx_csum = data;
>> +
>> +        spin_lock_irqsave(&dm->lock, flags);
>> +        iow(dm, DM9000_RCSR, dm->rx_csum ? RCSR_CSUM : 0);
>> +        spin_unlock_irqrestore(&dm->lock, flags);
>> +
>> +        return 0;
>> +    }
>> +
>> +    return -EOPNOTSUPP;
>> +}
>> +
>> +static int dm9000_set_tx_csum(struct net_device *dev, uint32_t data)
>> +{
>> +    board_info_t *dm = to_dm9000_board(dev);
>> +    int ret = -EOPNOTSUPP;
>> +
>> +    if (dm->can_csum)
>> +        ret = ethtool_op_set_tx_csum(dev, data);
>> +    return ret;
>> +}
>> +
>>  static u32 dm9000_get_link(struct net_device *dev)
>>  {
>>      board_info_t *dm = to_dm9000_board(dev);
>> @@ -540,6 +578,10 @@ static const struct ethtool_ops
>> dm9000_ethtool_ops = {
>>       .get_eeprom_len        = dm9000_get_eeprom_len,
>>       .get_eeprom        = dm9000_get_eeprom,
>>       .set_eeprom        = dm9000_set_eeprom,
>> +    .get_rx_csum        = dm9000_get_rx_csum,
>> +    .set_rx_csum        = dm9000_set_rx_csum,
>> +    .get_tx_csum        = ethtool_op_get_tx_csum,
>> +    .set_tx_csum        = dm9000_set_tx_csum,
>>  };
>>  
>>  static void dm9000_show_carrier(board_info_t *db,
>> @@ -685,6 +727,9 @@ dm9000_init_dm9000(struct net_device *dev)
>>      /* I/O mode */
>>      db->io_mode = ior(db, DM9000_ISR) >> 6;    /* ISR bit7:6 keeps
>> I/O mode */
>>  
>> +    /* Checksum mode */
>> +    dm9000_set_rx_csum(dev, db->rx_csum);
>> +
>>      /* GPIO0 on pre-activate PHY */
>>      iow(db, DM9000_GPR, 0);    /* REG_1F bit0 activate phyxcer */
>>      iow(db, DM9000_GPCR, GPCR_GEP_CNTL);    /* Let GPIO0 output */
>> @@ -743,6 +788,28 @@ static void dm9000_timeout(struct net_device *dev)
>>      spin_unlock_irqrestore(&db->lock, flags);
>>  }
>>  
>> +static void dm9000_send_packet(struct net_device *dev,
>> +                   int ip_summed,
>> +                   u16 pkt_len)
>> +{
>> +    board_info_t *dm = to_dm9000_board(dev);
>> +
>> +    /* The DM9000 is not smart enough to leave fragmented packets
>> alone. */
>> +    if (ip_summed == CHECKSUM_NONE)
>> +        iow(dm, DM9000_TCCR, 0);
>> +    else
>> +        iow(dm, DM9000_TCCR, TCCR_IP | TCCR_UDP | TCCR_TCP);
> 
> Is it worth keeping a local copy of this and
> only writing on change, since a write to the device is
> at-least two IO cycles?

Ok.

>> +    /* Set TX length to DM9000 */
>> +    iow(dm, DM9000_TXPLL, pkt_len);
>> +    iow(dm, DM9000_TXPLH, pkt_len >> 8);
>> +
>> +    /* Issue TX polling command */
>> +    iow(dm, DM9000_TCR, TCR_TXREQ);    /* Cleared after TX complete */
>> +
>> +    dev->trans_start = jiffies;    /* save the time stamp */
>> +}
>> +
>>  /*
>>   *  Hardware start transmission.
>>   *  Send a packet to media from the upper layer.
>> @@ -769,17 +836,11 @@ dm9000_start_xmit(struct sk_buff *skb, struct
>> net_device *dev)
>>      db->tx_pkt_cnt++;
>>      /* TX control: First packet immediately send, second packet queue */
>>      if (db->tx_pkt_cnt == 1) {
>> -        /* Set TX length to DM9000 */
>> -        iow(db, DM9000_TXPLL, skb->len);
>> -        iow(db, DM9000_TXPLH, skb->len >> 8);
>> -
>> -        /* Issue TX polling command */
>> -        iow(db, DM9000_TCR, TCR_TXREQ);    /* Cleared after TX
>> complete */
>> -
>> -        dev->trans_start = jiffies;    /* save the time stamp */
>> +        dm9000_send_packet(dev, skb->ip_summed, skb->len);
>>      } else {
>>          /* Second packet */
>>          db->queue_pkt_len = skb->len;
>> +        db->queue_ip_summed = skb->ip_summed;
>>          netif_stop_queue(dev);
>>      }
>>  
>> @@ -809,12 +870,9 @@ static void dm9000_tx_done(struct net_device
>> *dev, board_info_t *db)
>>              dev_dbg(db->dev, "tx done, NSR %02x\n", tx_status);
>>  
>>          /* Queue packet check & send */
>> -        if (db->tx_pkt_cnt > 0) {
>> -            iow(db, DM9000_TXPLL, db->queue_pkt_len);
>> -            iow(db, DM9000_TXPLH, db->queue_pkt_len >> 8);
>> -            iow(db, DM9000_TCR, TCR_TXREQ);
>> -            dev->trans_start = jiffies;
>> -        }
>> +        if (db->tx_pkt_cnt > 0)
>> +            dm9000_send_packet(dev, db->queue_ip_summed,
>> +                       db->queue_pkt_len);
>>          netif_wake_queue(dev);
>>      }
>>  }
>> @@ -846,14 +904,14 @@ dm9000_rx(struct net_device *dev)
>>          rxbyte = readb(db->io_data);
>>  
>>          /* Status check: this byte must be 0 or 1 */
>> -        if (rxbyte > DM9000_PKT_RDY) {
>> +        if (rxbyte & DM9000_PKT_ERR) {
>>              dev_warn(db->dev, "status check fail: %d\n", rxbyte);
>>              iow(db, DM9000_RCR, 0x00);    /* Stop Device */
>>              iow(db, DM9000_ISR, IMR_PAR);    /* Stop INT request */
>>              return;
>>          }
>>  
>> -        if (rxbyte != DM9000_PKT_RDY)
>> +        if (!(rxbyte & DM9000_PKT_RDY))
>>              return;
>>  
>>          /* A packet ready now  & Get status/length */
>> @@ -914,6 +972,12 @@ dm9000_rx(struct net_device *dev)
>>  
>>              /* Pass to upper layer */
>>              skb->protocol = eth_type_trans(skb, dev);
>> +            if (db->rx_csum) {
>> +                if ((((rxbyte & 0x1c) << 3) & rxbyte) == 0)
>> +                    skb->ip_summed = CHECKSUM_UNNECESSARY;
>> +                else
>> +                    skb->ip_summed = CHECKSUM_NONE;
>> +            }
>>              netif_rx(skb);
>>              dev->stats.rx_packets++;
>>  
>> @@ -922,7 +986,7 @@ dm9000_rx(struct net_device *dev)
>>  
>>              (db->dumpblk)(db->io_data, RxLen);
>>          }
>> -    } while (rxbyte == DM9000_PKT_RDY);
>> +    } while (rxbyte & DM9000_PKT_RDY);
>>  }
>>  
>>  static irqreturn_t dm9000_interrupt(int irq, void *dev_id)
>> @@ -1349,6 +1413,13 @@ dm9000_probe(struct platform_device *pdev)
>>          db->type = TYPE_DM9000E;
>>      }
>>  
>> +    /* dm9000a/b are capable of hardware checksum offload */
>> +    if (db->type == TYPE_DM9000A || db->type == TYPE_DM9000B) {
>> +        db->can_csum = 1;
>> +        db->rx_csum = 1;
>> +        ndev->features |= NETIF_F_IP_CSUM;
> 
> can ndev->features be checked and db->can_csum be removed?

ethtool_op_set_tx_csum modifies ndev->features, so disabling tx checksumming
will cause set_rx_csum to return -EOPNOTSUPP.
> 
>> +    }
>> +
>>      /* from this point we assume that we have found a DM9000 */
>>  
>>      /* driver system function */
>> diff --git a/drivers/net/dm9000.h b/drivers/net/dm9000.h
>> index ba25cf5..80817c2 100644
>> --- a/drivers/net/dm9000.h
>> +++ b/drivers/net/dm9000.h
>> @@ -45,6 +45,10 @@
>>  #define DM9000_CHIPR           0x2C
>>  #define DM9000_SMCR            0x2F
>>  
>> +#define DM9000_ETXCSR          0x30
>> +#define DM9000_TCCR           0x31
>> +#define DM9000_RCSR           0x32
>> +
>>  #define CHIPR_DM9000A           0x19
>>  #define CHIPR_DM9000B           0x1B
>>  
>> @@ -131,7 +135,21 @@
>>  
>>  #define GPCR_GEP_CNTL       (1<<0)
>>  
>> +#define TCCR_IP            (1<<0)
>> +#define TCCR_TCP        (1<<1)
>> +#define TCCR_UDP        (1<<2)
>> +
>> +#define RCSR_UDP_BAD        (1<<7)
>> +#define RCSR_TCP_BAD        (1<<6)
>> +#define RCSR_IP_BAD        (1<<5)
>> +#define RCSR_UDP        (1<<4)
>> +#define RCSR_TCP        (1<<3)
>> +#define RCSR_IP            (1<<2)
>> +#define RCSR_CSUM        (1<<1)
>> +#define RCSR_DISCARD        (1<<0)
>> +
>>  #define DM9000_PKT_RDY        0x01    /* Packet ready to receive */
>> +#define DM9000_PKT_ERR        0x02
>>  #define DM9000_PKT_MAX        1536    /* Received packet max size */
>>  
>>  /* DM9000A / DM9000B definitions */
> 
> 

-- 
Sincerely yours,
Mike.