From: dingtianhong@huawei•com (Ding Tianhong)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH net-next v13 3/3] net: hisilicon: new hip04 ethernet driver
Date: Thu, 15 Jan 2015 18:28:42 +0800 [thread overview]
Message-ID: <54B7965A.5050507@huawei.com> (raw)
In-Reply-To: <1421296742.25598.3.camel@perches.com>
On 2015/1/15 12:39, Joe Perches wrote:
> On Wed, 2015-01-14 at 14:34 +0800, Ding Tianhong wrote:
>> Support Hisilicon hip04 ethernet driver, including 100M / 1000M controller.
>> The controller has no tx done interrupt, reclaim xmitted buffer in the poll.
>
> Mostly trivial comments:
>
>> +++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
>
> []
>
>> +#define GMAC_MAX_PKT_LEN 1516
>
> []
>
>> +static int hip04_rx_poll(struct napi_struct *napi, int budget)
>> +{
> []
>> + while (cnt && !last) {
> []
>> + desc = (struct rx_desc *)skb->data;
>> + len = be16_to_cpu(desc->pkt_len);
>> + err = be32_to_cpu(desc->pkt_err);
>> +
>> + if (0 == len) {
>> + dev_kfree_skb_any(skb);
>> + last = true;
>> + } else if ((err & RX_PKT_ERR) || (len >= GMAC_MAX_PKT_LEN)) {
>
> Is this ">=" correct? Maybe it should be ">" ?
>
> []
>
>> +static irqreturn_t hip04_mac_interrupt(int irq, void *dev_id)
>> +{
>> + struct net_device *ndev = (struct net_device *)dev_id;
>
> Unnecessary cast of void *
>
> []
>
>> +static int hip04_set_coalesce(struct net_device *netdev,
>> + struct ethtool_coalesce *ec)
>> +{
>> + struct hip04_priv *priv = netdev_priv(netdev);
>> +
>> + /* Check not supported parameters */
>> + if ((ec->rx_max_coalesced_frames) || (ec->rx_coalesce_usecs_irq) ||
>> + (ec->rx_max_coalesced_frames_irq) || (ec->tx_coalesce_usecs_irq) ||
>> + (ec->use_adaptive_rx_coalesce) || (ec->use_adaptive_tx_coalesce) ||
>> + (ec->pkt_rate_low) || (ec->rx_coalesce_usecs_low) ||
>> + (ec->rx_max_coalesced_frames_low) || (ec->tx_coalesce_usecs_high) ||
>> + (ec->tx_max_coalesced_frames_low) || (ec->pkt_rate_high) ||
>> + (ec->tx_coalesce_usecs_low) || (ec->rx_coalesce_usecs_high) ||
>> + (ec->rx_max_coalesced_frames_high) || (ec->rx_coalesce_usecs) ||
>> + (ec->tx_max_coalesced_frames_irq) ||
>> + (ec->stats_block_coalesce_usecs) ||
>> + (ec->tx_max_coalesced_frames_high) || (ec->rate_sample_interval))
>> + return -EOPNOTSUPP;
>
> Rather than a somewhat haphazard mix of these values,
> this might be simpler to read as something like:
>
> /* Check not supported parameters */
> if (ec->pkt_rate_low ||
> ec->pkt_rate_high ||
>
> ec->use_adaptive_rx_coalesce ||
> ec->rx_coalesce_usecs ||
> ec->rx_coalesce_usecs_low ||
> ec->rx_coalesce_usecs_high ||
> ec->rx_coalesce_usecs_irq ||
> ec->rx_max_coalesced_frames ||
> ec->rx_max_coalesced_frames_low ||
> ec->rx_max_coalesced_frames_high ||
> ec->rx_max_coalesced_frames_irq ||
>
> ec->use_adaptive_tx_coalesce ||
> ec->tx_coalesce_usecs_low ||
> ec->tx_coalesce_usecs_high ||
> ec->tx_max_coalesced_frames_low ||
> ec->tx_max_coalesced_frames_high ||
> ec->tx_max_coalesced_frames_irq ||
>
> ec->stats_block_coalesce_usecs ||
> ec->rate_sample_interval)
> return -EOPNOTSUPP;
>
>
>> +static void hip04_free_ring(struct net_device *ndev, struct device *d)
>> +{
>> + struct hip04_priv *priv = netdev_priv(ndev);
>> + int i;
>> +
>> + for (i = 0; i < RX_DESC_NUM; i++)
>> + if (priv->rx_buf[i])
>> + put_page(virt_to_head_page(priv->rx_buf[i]));
>
> It's generally nicer to use braces around
> for loops with single ifs.
>
>> +
>> + for (i = 0; i < TX_DESC_NUM; i++)
>> + if (priv->tx_skb[i])
>> + dev_kfree_skb_any(priv->tx_skb[i]);
>
>
>
OK, fix them later, thanks.
Ding
> .
>
next prev parent reply other threads:[~2015-01-15 10:28 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-14 6:34 [PATCH net-next v13 0/3] add hisilicon hip04 ethernet driver Ding Tianhong
2015-01-14 6:34 ` [PATCH net-next v13 1/3] Documentation: add Device tree bindings for Hisilicon hip04 ethernet Ding Tianhong
2015-01-14 6:34 ` [PATCH net-next v13 2/3] net: hisilicon: new hip04 MDIO driver Ding Tianhong
2015-01-14 6:34 ` [PATCH net-next v13 3/3] net: hisilicon: new hip04 ethernet driver Ding Tianhong
2015-01-14 8:06 ` Joe Perches
2015-01-15 2:56 ` Ding Tianhong
2015-01-14 8:53 ` Arnd Bergmann
2015-01-15 2:54 ` Ding Tianhong
2015-01-15 9:05 ` Arnd Bergmann
2015-01-14 16:34 ` Eric Dumazet
2015-01-15 10:29 ` Ding Tianhong
2015-01-15 12:30 ` Arnd Bergmann
2015-01-15 4:39 ` Joe Perches
2015-01-15 10:28 ` Ding Tianhong [this message]
2015-01-15 9:53 ` Arnd Bergmann
2015-01-19 18:11 ` Alexander Graf
2015-01-19 20:34 ` Arnd Bergmann
2015-01-20 2:15 ` Ding Tianhong
2015-01-20 12:01 ` Arnd Bergmann
2015-01-20 18:12 ` Joe Perches
2015-01-14 10:19 ` [PATCH net-next v13 0/3] add hisilicon " Alexander Graf
2015-01-15 8:37 ` Ding Tianhong
2015-01-15 9:42 ` Arnd Bergmann
2015-01-15 12:39 ` Alexander Graf
2015-01-15 12:56 ` Ding Tianhong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54B7965A.5050507@huawei.com \
--to=dingtianhong@huawei$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox