RE: [PATCH V3 net-next 1/8] net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC
Hi Bo Yu, > -Original Message- > From: Bo Yu [mailto:tsu.y...@gmail.com] > Sent: Monday, June 19, 2017 1:57 AM > To: Salil Mehta > Cc: da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y); > mehta.salil@gmail.com; netdev@vger.kernel.org; linux- > ker...@vger.kernel.org; Linuxarm > Subject: Re: [PATCH V3 net-next 1/8] net: hns3: Add support of HNS3 > Ethernet Driver for hip08 SoC > > Hi, > On Sat, Jun 17, 2017 at 06:24:24PM +0100, Salil Mehta wrote: > >+struct notifier_block notifier_block; > >+/* Vxlan/Geneve information */ > >+struct hns3_udp_tunnel udp_tnl[HNS3_UDP_TNL_MAX]; > >+}; > >+ > >+/* the distance between [begin, end) in a ring buffer > >+ * note: there is a unuse slot between the begin and the end > >+ */ > >+static inline int ring_dist(struct hns3_enet_ring *ring, int begin, > int end) > >+{ > >+return (end - begin + ring->desc_num) % ring->desc_num; > >+} > >+ > >+static inline int ring_space(struct hns3_enet_ring *ring) > >+{ > >+return ring->desc_num - > >+ring_dist(ring, ring->next_to_clean, ring->next_to_use) - > 1; > >+} > >+ > >+static inline int is_ring_empty(struct hns3_enet_ring *ring) > >+{ > >+return ring->next_to_use == ring->next_to_clean; > >+} > >+ > >+static inline void hns3_write_reg(void __iomem *base, u32 reg, u32 > value) > >+{ > >+u8 __iomem *reg_addr = READ_ONCE(base); > >+ > >+writel(value, reg_addr + reg); > >+} > >+ > >+#define hns3_write_dev(a, reg, value) \ > >+hns3_write_reg((a)->io_base, (reg), (value)) > >+ > >+#define hnae_queue_xmit(tqp, buf_num) writel_relaxed(buf_num, \ > >+(tqp)->io_base + HNS3_RING_TX_RING_TAIL_REG) > >+ > >+#define ring_to_dev(ring) (&(ring)->tqp->handle->pdev->dev) > >+ > >+#define ring_to_dma_dir(ring) (HNAE3_IS_TX_RING(ring) ? \ > >+DMA_TO_DEVICE : DMA_FROM_DEVICE) > >+ > >+#define tx_ring_data(priv, idx) ((priv)->ring_data[idx]) > >+ > >+#define hnae_buf_size(_ring) ((_ring)->buf_size) > >+#define hnae_page_order(_ring) (get_order(hnae_buf_size(_ring))) > >+#define hnae_page_size(_ring) (PAGE_SIZE << hnae_page_order(_ring)) > >+ > >+/* iterator for handling rings in ring group */ > >+#define hns3_for_each_ring(pos, head) \ > >+for (pos = (head).ring; pos != NULL; pos = pos->next) > > Only a pos? Comparsion to NULL could be written "pos" noticed by > checkpatch. Fixed in patch V4. Thanks! Salil > > > >+ > >+void hns3_ethtool_set_ops(struct net_device *ndev); > >+ > >+int hns3_nic_net_xmit_hw( > >+struct net_device *ndev, > >+struct sk_buff *skb, > >+struct hns3_nic_ring_data *ring_data); > >+int hns3_clean_tx_ring(struct hns3_enet_ring *ring, int budget); > >+int hns3_clean_rx_ring_ex( > >+struct hns3_enet_ring *ring, > >+struct sk_buff **skb_ex, > >+int budget); > >+#endif > >-- > >2.7.4 > > > >
RE: [PATCH V3 net-next 1/8] net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC
Hi Bo Yu, > -Original Message- > From: Bo Yu [mailto:tsu.y...@gmail.com] > Sent: Monday, June 19, 2017 1:18 AM > To: Salil Mehta > Cc: da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y); > mehta.salil@gmail.com; netdev@vger.kernel.org; linux- > ker...@vger.kernel.org; Linuxarm > Subject: Re: [PATCH V3 net-next 1/8] net: hns3: Add support of HNS3 > Ethernet Driver for hip08 SoC > > Hi, > On Sat, Jun 17, 2017 at 06:24:24PM +0100, Salil Mehta wrote: > >+static int hns3_fill_desc(struct hns3_enet_ring *ring, void *priv, > >+ int size, dma_addr_t dma, int frag_end, > >+ enum hns_desc_type type) > >+{ > >+struct hns3_desc_cb *desc_cb = &ring->desc_cb[ring->next_to_use]; > >+struct hns3_desc *desc = &ring->desc[ring->next_to_use]; > >+u32 ol_type_vlan_len_msec = 0; > >+u16 bdtp_fe_sc_vld_ra_ri = 0; > >+u32 type_cs_vlan_tso = 0; > >+struct sk_buff *skb; > >+u32 paylen = 0; > >+u16 mss = 0; > >+__be16 protocol; > >+u8 ol4_proto; > >+u8 il4_proto; > >+int ret; > >+ > >+/* The txbd's baseinfo of DESC_TYPE_PAGE & DESC_TYPE_SKB */ > >+desc_cb->priv = priv; > >+desc_cb->length = size; > >+desc_cb->dma = dma; > >+desc_cb->type = type; > >+ > >+/* now, fill the descriptor */ > >+desc->addr = cpu_to_le64(dma); > >+desc->tx.send_size = cpu_to_le16((u16)size); > >+hns3_set_txbd_baseinfo(&bdtp_fe_sc_vld_ra_ri, frag_end); > >+desc->tx.bdtp_fe_sc_vld_ra_ri = > cpu_to_le16(bdtp_fe_sc_vld_ra_ri); > >+ > >+if (type == DESC_TYPE_SKB) { > >+skb = (struct sk_buff *)priv; > >+paylen = cpu_to_le16(skb->len); > >+ > >+if (skb->ip_summed == CHECKSUM_PARTIAL) { > >+skb_reset_mac_len(skb); > >+protocol = skb->protocol; > >+ > >+/* vlan packe t*/ > > Just a spealling: /* vlan packet */ Fixed in V4 patch. Thanks! Salil > > >+if (protocol == htons(ETH_P_8021Q)) { > >+protocol = vlan_get_protocol(skb); > >+skb->protocol = protocol; > >+} > >+hns3_get_l4_protocol(skb, &ol4_proto, &il4_proto); > >+hns3_set_l2l3l4_len(skb, ol4_proto, il4_proto, > >+&type_cs_vlan_tso, > >+&ol_type_vlan_len_msec); > >+ret = hns3_set_l3l4_type_csum(skb, ol4_proto, > il4_proto, > >+ &type_cs_vlan_tso, > >+ &ol_type_vlan_len_msec); > >+if (ret) > >+return ret; > >+ > >+ret = hns3_set_tso(skb, &paylen, &mss, > >+ &type_cs_vlan_tso); > >+if (ret) > >+return ret; > >+} > >+ > >+/* Set txbd */ > >+desc->tx.ol_type_vlan_len_msec = > >+cpu_to_le32(ol_type_vlan_len_msec); > >+desc->tx.type_cs_vlan_tso_len = > >+cpu_to_le32(type_cs_vlan_tso); > >+desc->tx.paylen = cpu_to_le16(paylen); > >+desc->tx.mss = cpu_to_le16(mss); > >+} > >+ > >+/* move ring pointer to next.*/ > >+ring_ptr_move_fw(ring, next_to_use); > >+ > >+return 0; > >+} > >+ > >+static int hns3_fill_desc_tso(struct hns3_enet_ring *ring, void > *priv, > >+ int size, dma_addr_t dma, int frag_end, > >+ enum hns_desc_type type) > >+{ > >+int frag_buf_num; > >+int sizeoflast; > >+int ret, k; > >+ > >+frag_buf_num = (size + HNS3_MAX_BD_SIZE - 1) / HNS3_MAX_BD_SIZE; > >+sizeoflast = size % HNS3_MAX_BD_SIZE; > >+sizeoflast = sizeoflast ? sizeoflast : HNS3_MAX_BD_SIZE; > >+ > >+/* When the frag size is bigger than hardware, split this frag */ > >+for (k = 0; k < frag_buf_num; k++) { > >+ret = hns3_fill_desc(ring, priv, > >+ (k == frag_buf_num - 1) ? > >+sizeoflast : HNS3_MAX_BD_SIZE, > >+
RE: [PATCH V3 net-next 1/8] net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC
Hi Andrew > -Original Message- > From: Andrew Lunn [mailto:and...@lunn.ch] > Sent: Saturday, June 17, 2017 6:54 PM > To: Salil Mehta > Cc: da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y); > mehta.salil@gmail.com; netdev@vger.kernel.org; linux- > ker...@vger.kernel.org; Linuxarm > Subject: Re: [PATCH V3 net-next 1/8] net: hns3: Add support of HNS3 > Ethernet Driver for hip08 SoC > > > +static int hns3_nic_net_up(struct net_device *ndev) > > +{ > > + struct hns3_nic_priv *priv = netdev_priv(ndev); > > + struct hnae3_handle *h = priv->ae_handle; > > + int i, j; > > + int ret; > > + > > + ret = hns3_nic_init_irq(priv); > > + if (ret != 0) { > > if (ret) > > No need to compare with zero. Sure, changed in V4 patch. Thanks Salil > > > + netdev_err(ndev, "hns init irq failed! ret=%d\n", ret); > > + return ret; > > > +static int hns3_nic_net_open(struct net_device *ndev) > > +{ > > + struct hns3_nic_priv *priv = netdev_priv(ndev); > > + struct hnae3_handle *h = priv->ae_handle; > > + int ret; > > + > > + netif_carrier_off(ndev); > > + > > + ret = netif_set_real_num_tx_queues(ndev, h->kinfo.num_tqps); > > + if (ret < 0) { > > + netdev_err(ndev, "netif_set_real_num_tx_queues fail, > ret=%d!\n", > > + ret); > > + return ret; > > + } > > In general, functions return 0 for success, and something else for an > error. So there is no need to do a comparison. Please remove all > comparisons, unless it is really needed. It also makes the code look > consistent. At the moment you sometime have < 0, sometime !=0, and > sometimes no comparison at all. Acknowledged, scanned and have changed in V4 patch. Please have a look. Thanks Salil > > Andrew
Re: [PATCH V3 net-next 1/8] net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC
Hi, On Sat, Jun 17, 2017 at 06:24:24PM +0100, Salil Mehta wrote: + struct notifier_block notifier_block; + /* Vxlan/Geneve information */ + struct hns3_udp_tunnel udp_tnl[HNS3_UDP_TNL_MAX]; +}; + +/* the distance between [begin, end) in a ring buffer + * note: there is a unuse slot between the begin and the end + */ +static inline int ring_dist(struct hns3_enet_ring *ring, int begin, int end) +{ + return (end - begin + ring->desc_num) % ring->desc_num; +} + +static inline int ring_space(struct hns3_enet_ring *ring) +{ + return ring->desc_num - + ring_dist(ring, ring->next_to_clean, ring->next_to_use) - 1; +} + +static inline int is_ring_empty(struct hns3_enet_ring *ring) +{ + return ring->next_to_use == ring->next_to_clean; +} + +static inline void hns3_write_reg(void __iomem *base, u32 reg, u32 value) +{ + u8 __iomem *reg_addr = READ_ONCE(base); + + writel(value, reg_addr + reg); +} + +#define hns3_write_dev(a, reg, value) \ + hns3_write_reg((a)->io_base, (reg), (value)) + +#define hnae_queue_xmit(tqp, buf_num) writel_relaxed(buf_num, \ + (tqp)->io_base + HNS3_RING_TX_RING_TAIL_REG) + +#define ring_to_dev(ring) (&(ring)->tqp->handle->pdev->dev) + +#define ring_to_dma_dir(ring) (HNAE3_IS_TX_RING(ring) ? \ + DMA_TO_DEVICE : DMA_FROM_DEVICE) + +#define tx_ring_data(priv, idx) ((priv)->ring_data[idx]) + +#define hnae_buf_size(_ring) ((_ring)->buf_size) +#define hnae_page_order(_ring) (get_order(hnae_buf_size(_ring))) +#define hnae_page_size(_ring) (PAGE_SIZE << hnae_page_order(_ring)) + +/* iterator for handling rings in ring group */ +#define hns3_for_each_ring(pos, head) \ + for (pos = (head).ring; pos != NULL; pos = pos->next) Only a pos? Comparsion to NULL could be written "pos" noticed by checkpatch. + +void hns3_ethtool_set_ops(struct net_device *ndev); + +int hns3_nic_net_xmit_hw( + struct net_device *ndev, + struct sk_buff *skb, + struct hns3_nic_ring_data *ring_data); +int hns3_clean_tx_ring(struct hns3_enet_ring *ring, int budget); +int hns3_clean_rx_ring_ex( + struct hns3_enet_ring *ring, + struct sk_buff **skb_ex, + int budget); +#endif -- 2.7.4
Re: [PATCH V3 net-next 1/8] net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC
Hi, On Sat, Jun 17, 2017 at 06:24:24PM +0100, Salil Mehta wrote: +static int hns3_fill_desc(struct hns3_enet_ring *ring, void *priv, + int size, dma_addr_t dma, int frag_end, + enum hns_desc_type type) +{ + struct hns3_desc_cb *desc_cb = &ring->desc_cb[ring->next_to_use]; + struct hns3_desc *desc = &ring->desc[ring->next_to_use]; + u32 ol_type_vlan_len_msec = 0; + u16 bdtp_fe_sc_vld_ra_ri = 0; + u32 type_cs_vlan_tso = 0; + struct sk_buff *skb; + u32 paylen = 0; + u16 mss = 0; + __be16 protocol; + u8 ol4_proto; + u8 il4_proto; + int ret; + + /* The txbd's baseinfo of DESC_TYPE_PAGE & DESC_TYPE_SKB */ + desc_cb->priv = priv; + desc_cb->length = size; + desc_cb->dma = dma; + desc_cb->type = type; + + /* now, fill the descriptor */ + desc->addr = cpu_to_le64(dma); + desc->tx.send_size = cpu_to_le16((u16)size); + hns3_set_txbd_baseinfo(&bdtp_fe_sc_vld_ra_ri, frag_end); + desc->tx.bdtp_fe_sc_vld_ra_ri = cpu_to_le16(bdtp_fe_sc_vld_ra_ri); + + if (type == DESC_TYPE_SKB) { + skb = (struct sk_buff *)priv; + paylen = cpu_to_le16(skb->len); + + if (skb->ip_summed == CHECKSUM_PARTIAL) { + skb_reset_mac_len(skb); + protocol = skb->protocol; + + /* vlan packe t*/ Just a spealling: /* vlan packet */ + if (protocol == htons(ETH_P_8021Q)) { + protocol = vlan_get_protocol(skb); + skb->protocol = protocol; + } + hns3_get_l4_protocol(skb, &ol4_proto, &il4_proto); + hns3_set_l2l3l4_len(skb, ol4_proto, il4_proto, + &type_cs_vlan_tso, + &ol_type_vlan_len_msec); + ret = hns3_set_l3l4_type_csum(skb, ol4_proto, il4_proto, + &type_cs_vlan_tso, + &ol_type_vlan_len_msec); + if (ret) + return ret; + + ret = hns3_set_tso(skb, &paylen, &mss, + &type_cs_vlan_tso); + if (ret) + return ret; + } + + /* Set txbd */ + desc->tx.ol_type_vlan_len_msec = + cpu_to_le32(ol_type_vlan_len_msec); + desc->tx.type_cs_vlan_tso_len = + cpu_to_le32(type_cs_vlan_tso); + desc->tx.paylen = cpu_to_le16(paylen); + desc->tx.mss = cpu_to_le16(mss); + } + + /* move ring pointer to next.*/ + ring_ptr_move_fw(ring, next_to_use); + + return 0; +} + +static int hns3_fill_desc_tso(struct hns3_enet_ring *ring, void *priv, + int size, dma_addr_t dma, int frag_end, + enum hns_desc_type type) +{ + int frag_buf_num; + int sizeoflast; + int ret, k; + + frag_buf_num = (size + HNS3_MAX_BD_SIZE - 1) / HNS3_MAX_BD_SIZE; + sizeoflast = size % HNS3_MAX_BD_SIZE; + sizeoflast = sizeoflast ? sizeoflast : HNS3_MAX_BD_SIZE; + + /* When the frag size is bigger than hardware, split this frag */ + for (k = 0; k < frag_buf_num; k++) { + ret = hns3_fill_desc(ring, priv, +(k == frag_buf_num - 1) ? + sizeoflast : HNS3_MAX_BD_SIZE, + dma + HNS3_MAX_BD_SIZE * k, + frag_end && (k == frag_buf_num - 1) ? 1 : 0, + (type == DESC_TYPE_SKB && !k) ? + DESC_TYPE_SKB : DESC_TYPE_PAGE); + if (ret) + return ret; + } + + return 0; +} + +static int hns3_nic_maybe_stop_tso(struct sk_buff **out_skb, int *bnum, + struct hns3_enet_ring *ring) +{ + struct sk_buff *skb = *out_skb; + struct skb_frag_struct *frag; + int bdnum_for_frag; + int frag_num; + int buf_num; + int size; + int i; + + size = skb_headlen(skb); + buf_num = (size + HNS3_MAX_BD_SIZE - 1) / HNS3_MAX_BD_SIZE; + + frag_num = skb_shinfo(skb)->nr_frags; + for (i = 0; i < frag_num; i++) { + frag = &skb_shinfo(skb)->frags[i]; + size = skb_frag_size(frag); + bdnum_for_frag = + (size + HNS3_MAX_BD_SIZE - 1) / HNS3_MAX_BD_SIZE; + if (bdnum_for_frag > HNS3_MAX_BD_PER_FRAG) + return -ENOMEM; + + buf_num += bdnum_for_frag; + } + +
Re: [PATCH V3 net-next 1/8] net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC
> + > + for (i = 0; i < priv->vector_num; i++) { > + tqp_vectors = &priv->tqp_vector[i]; > + > + if (tqp_vectors->irq_init_flag == HNS3_VEVTOR_INITED) Should VEVTOR be VECTOR? > +static void hns3_set_vector_gl(struct hns3_enet_tqp_vector *tqp_vector, > +u32 gl_value) > +{ > + writel(gl_value, tqp_vector->mask_addr + HNS3_VECTOR_GL0_OFFSET); > + writel(gl_value, tqp_vector->mask_addr + HNS3_VECTOR_GL1_OFFSET); > + writel(gl_value, tqp_vector->mask_addr + HNS3_VECTOR_GL2_OFFSET); > +} > + > +static void hns3_set_vector_rl(struct hns3_enet_tqp_vector *tqp_vector, > +u32 rl_value) Could you use more informative names. What does gl and rl mean? > +{ > + writel(rl_value, tqp_vector->mask_addr + HNS3_VECTOR_RL_OFFSET); > +} > + > +static void hns3_vector_gl_rl_init(struct hns3_enet_tqp_vector *tqp_vector) > +{ > + /* Default :enable interrupt coalesce */ > + tqp_vector->rx_group.int_gl = HNS3_INT_GL_50K; > + tqp_vector->tx_group.int_gl = HNS3_INT_GL_50K; > + hns3_set_vector_gl(tqp_vector, HNS3_INT_GL_50K); > + hns3_set_vector_rl(tqp_vector, 0); > + tqp_vector->rx_group.flow_level = HNS3_FLOW_LOW; > + tqp_vector->tx_group.flow_level = HNS3_FLOW_LOW; > +} > + > +static int hns3_nic_net_up(struct net_device *ndev) > +{ > + struct hns3_nic_priv *priv = netdev_priv(ndev); > + struct hnae3_handle *h = priv->ae_handle; > + int i, j; > + int ret; > + > + ret = hns3_nic_init_irq(priv); > + if (ret != 0) { > + netdev_err(ndev, "hns init irq failed! ret=%d\n", ret); > + return ret; > + } > + > + for (i = 0; i < priv->vector_num; i++) > + hns3_vector_enable(&priv->tqp_vector[i]); > + > + ret = h->ae_algo->ops->start ? h->ae_algo->ops->start(h) : 0; > + if (ret) > + goto out_start_err; > + > + return 0; > + > +out_start_err: > + netif_stop_queue(ndev); This seems asymmetric. Where is the netif_start_queue()? > + > + for (j = i - 1; j >= 0; j--) > + hns3_vector_disable(&priv->tqp_vector[j]); > + > + return ret; > +} > + > +static int hns3_nic_net_open(struct net_device *ndev) > +{ > + struct hns3_nic_priv *priv = netdev_priv(ndev); > + struct hnae3_handle *h = priv->ae_handle; > + int ret; > + > + netif_carrier_off(ndev); > + > + ret = netif_set_real_num_tx_queues(ndev, h->kinfo.num_tqps); > + if (ret < 0) { > + netdev_err(ndev, "netif_set_real_num_tx_queues fail, ret=%d!\n", > +ret); > + return ret; > + } > + > + ret = netif_set_real_num_rx_queues(ndev, h->kinfo.num_tqps); > + if (ret < 0) { > + netdev_err(ndev, > +"netif_set_real_num_rx_queues fail, ret=%d!\n", ret); > + return ret; > + } > + > + ret = hns3_nic_net_up(ndev); > + if (ret) { > + netdev_err(ndev, > +"hns net up fail, ret=%d!\n", ret); > + return ret; > + } > + > + netif_carrier_on(ndev); Carrier on should be performed when the PHY says there is link. > + netif_tx_wake_all_queues(ndev); > + > + return 0; > +} > + > +static void hns3_nic_net_down(struct net_device *ndev) > +{ > + struct hns3_nic_priv *priv = netdev_priv(ndev); > + struct hnae3_ae_ops *ops; > + int i; > + > + netif_tx_stop_all_queues(ndev); > + netif_carrier_off(ndev); > + > + ops = priv->ae_handle->ae_algo->ops; > + > + if (ops->stop) > + ops->stop(priv->ae_handle); > + > + for (i = 0; i < priv->vector_num; i++) > + hns3_vector_disable(&priv->tqp_vector[i]); > +} > + > +static int hns3_nic_net_stop(struct net_device *ndev) > +{ > + hns3_nic_net_down(ndev); > + > + return 0; > +} > + > +void hns3_set_multicast_list(struct net_device *ndev) > +{ > + struct hns3_nic_priv *priv = netdev_priv(ndev); > + struct hnae3_handle *h = priv->ae_handle; > + struct netdev_hw_addr *ha = NULL; > + > + if (!h) { > + netdev_err(ndev, "hnae handle is null\n"); > + return; > + } > + > + if (h->ae_algo->ops->set_mc_addr) { > + netdev_for_each_mc_addr(ha, ndev) > + if (h->ae_algo->ops->set_mc_addr(h, ha->addr)) > + netdev_err(ndev, "set multicast fail\n"); > + } > +} > + > +static int hns3_nic_uc_sync(struct net_device *netdev, > + const unsigned char *addr) > +{ > + struct hns3_nic_priv *priv = netdev_priv(netdev); > + struct hnae3_handle *h = priv->ae_handle; > + > + if (h->ae_algo->ops->add_uc_addr) > + return h->ae_algo->ops->add_uc_addr(h, addr); > + > + return 0; > +} > + > +static int hns3_nic_uc_unsync(struct net_device *netdev, > + const unsigned char *addr) > +{ > + struct hns3_nic_priv *priv = netdev_pr
Re: [PATCH V3 net-next 1/8] net: hns3: Add support of HNS3 Ethernet Driver for hip08 SoC
> +static int hns3_nic_net_up(struct net_device *ndev) > +{ > + struct hns3_nic_priv *priv = netdev_priv(ndev); > + struct hnae3_handle *h = priv->ae_handle; > + int i, j; > + int ret; > + > + ret = hns3_nic_init_irq(priv); > + if (ret != 0) { if (ret) No need to compare with zero. > + netdev_err(ndev, "hns init irq failed! ret=%d\n", ret); > + return ret; > +static int hns3_nic_net_open(struct net_device *ndev) > +{ > + struct hns3_nic_priv *priv = netdev_priv(ndev); > + struct hnae3_handle *h = priv->ae_handle; > + int ret; > + > + netif_carrier_off(ndev); > + > + ret = netif_set_real_num_tx_queues(ndev, h->kinfo.num_tqps); > + if (ret < 0) { > + netdev_err(ndev, "netif_set_real_num_tx_queues fail, ret=%d!\n", > +ret); > + return ret; > + } In general, functions return 0 for success, and something else for an error. So there is no need to do a comparison. Please remove all comparisons, unless it is really needed. It also makes the code look consistent. At the moment you sometime have < 0, sometime !=0, and sometimes no comparison at all. Andrew