RE: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Salil Mehta]

Hi Stephen,

> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Monday, June 19, 2017 5:59 PM
> To: Salil Mehta
> Cc: da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the
> HNAE3 framework
> 
> On Sat, 17 Jun 2017 18:24:25 +0100
> Salil Mehta <salil.me...@huawei.com> wrote:
> 
> > +
> > +/* This struct defines the operation on the handle.
> > + *
> > + * init_ae_dev(): (mandatory)
> > + *   Get PF configure from pci_dev and initialize PF hardware
> > + * uninit_ae_dev()
> > + *   Disable PF device and release PF resource
> > + * register_client
> > + *   Register client to ae_dev
> > + * unregister_client()
> > + *   Unregister client from ae_dev
> > + * start()
> > + *   Enable the hardware
> > + * stop()
> > + *   Disable the hardware
> > + * get_status()
> > + *   Get the carrier state of the back channel of the handle, 1 for
> ok, 0 for
> > + *   non-ok
> > + * get_ksettings_an_result()
> > + *   Get negotiation status,speed and duplex
> > + * update_speed_duplex_h()
> > + *   Update hardware speed and duplex
> > + * get_media_type()
> > + *   Get media type of MAC
> > + * adjust_link()
> > + *   Adjust link status
> > + * set_loopback()
> > + *   Set loopback
> > + * set_promisc_mode
> > + *   Set promisc mode
> > + * set_mtu()
> > + *   set mtu
> > + * get_pauseparam()
> > + *   get tx and rx of pause frame use
> > + * set_pauseparam()
> > + *   set tx and rx of pause frame use
> > + * set_autoneg()
> > + *   set auto autonegotiation of pause frame use
> > + * get_autoneg()
> > + *   get auto autonegotiation of pause frame use
> > + * get_coalesce_usecs()
> > + *   get usecs to delay a TX interrupt after a packet is sent
> > + * get_rx_max_coalesced_frames()
> > + *   get Maximum number of packets to be sent before a TX interrupt.
> > + * set_coalesce_usecs()
> > + *   set usecs to delay a TX interrupt after a packet is sent
> > + * set_coalesce_frames()
> > + *   set Maximum number of packets to be sent before a TX interrupt.
> > + * get_mac_addr()
> > + *   get mac address
> > + * set_mac_addr()
> > + *   set mac address
> > + * add_uc_addr
> > + *   Add unicast addr to mac table
> > + * rm_uc_addr
> > + *   Remove unicast addr from mac table
> > + * set_mc_addr()
> > + *   Set multicast address
> > + * add_mc_addr
> > + *   Add multicast address to mac table
> > + * rm_mc_addr
> > + *   Remove multicast address from mac table
> > + * update_stats()
> > + *   Update Old network device statistics
> > + * get_ethtool_stats()
> > + *   Get ethtool network device statistics
> > + * get_strings()
> > + *   Get a set of strings that describe the requested objects
> > + * get_sset_count()
> > + *   Get number of strings that @get_strings will write
> > + * update_led_status()
> > + *   Update the led status
> > + * set_led_id()
> > + *   Set led id
> > + * get_regs()
> > + *   Get regs dump
> > + * get_regs_len()
> > + *   Get the len of the regs dump
> > + * get_rss_key_size()
> > + *   Get rss key size
> > + * get_rss_indir_size()
> > + *   Get rss indirection table size
> > + * get_rss()
> > + *   Get rss table
> > + * set_rss()
> > + *   Set rss table
> > + * get_tc_size()
> > + *   Get tc size of handle
> > + * get_vector()
> > + *   Get vector number and vector infomation
> > + * map_ring_to_vector()
> > + *   Map rings to vector
> > + * unmap_ring_from_vector()
> > + *   Unmap rings from vector
> > + * add_tunnel_udp()
> > + *   Add tunnel information to hardware
> > + * del_tunnel_udp()
> > + *   Delete tunnel information from hardware
> > + * reset_queue()
> > + *   Reset queue
> > + * get_fw_version()
> > + *   Get firmware version
> > + * get_mdix_mode()
> > + *   Get media typr of phy
> > + * set_vlan_filter()
> > + *   Set vlan filter config of Ports
> > + * set_vf_vlan_filter()
> > + *   Set vlan filter config of vf
> > + */
> > +struct hnae3_ae_ops {
> > +   int (*init_ae_dev)(struct hnae3_ae_dev *ae_dev);
> > +   void (*uninit_ae_dev)(struct hnae3_ae_dev *ae_dev);
> > +
> > +   int (*register_client)(struct hnae3_client *client,
> > +

RE: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Salil Mehta]

Hi Stephen,

> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Monday, June 19, 2017 5:59 PM
> To: Salil Mehta
> Cc: da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the
> HNAE3 framework
> 
> On Sat, 17 Jun 2017 18:24:25 +0100
> Salil Mehta  wrote:
> 
> > +
> > +/* This struct defines the operation on the handle.
> > + *
> > + * init_ae_dev(): (mandatory)
> > + *   Get PF configure from pci_dev and initialize PF hardware
> > + * uninit_ae_dev()
> > + *   Disable PF device and release PF resource
> > + * register_client
> > + *   Register client to ae_dev
> > + * unregister_client()
> > + *   Unregister client from ae_dev
> > + * start()
> > + *   Enable the hardware
> > + * stop()
> > + *   Disable the hardware
> > + * get_status()
> > + *   Get the carrier state of the back channel of the handle, 1 for
> ok, 0 for
> > + *   non-ok
> > + * get_ksettings_an_result()
> > + *   Get negotiation status,speed and duplex
> > + * update_speed_duplex_h()
> > + *   Update hardware speed and duplex
> > + * get_media_type()
> > + *   Get media type of MAC
> > + * adjust_link()
> > + *   Adjust link status
> > + * set_loopback()
> > + *   Set loopback
> > + * set_promisc_mode
> > + *   Set promisc mode
> > + * set_mtu()
> > + *   set mtu
> > + * get_pauseparam()
> > + *   get tx and rx of pause frame use
> > + * set_pauseparam()
> > + *   set tx and rx of pause frame use
> > + * set_autoneg()
> > + *   set auto autonegotiation of pause frame use
> > + * get_autoneg()
> > + *   get auto autonegotiation of pause frame use
> > + * get_coalesce_usecs()
> > + *   get usecs to delay a TX interrupt after a packet is sent
> > + * get_rx_max_coalesced_frames()
> > + *   get Maximum number of packets to be sent before a TX interrupt.
> > + * set_coalesce_usecs()
> > + *   set usecs to delay a TX interrupt after a packet is sent
> > + * set_coalesce_frames()
> > + *   set Maximum number of packets to be sent before a TX interrupt.
> > + * get_mac_addr()
> > + *   get mac address
> > + * set_mac_addr()
> > + *   set mac address
> > + * add_uc_addr
> > + *   Add unicast addr to mac table
> > + * rm_uc_addr
> > + *   Remove unicast addr from mac table
> > + * set_mc_addr()
> > + *   Set multicast address
> > + * add_mc_addr
> > + *   Add multicast address to mac table
> > + * rm_mc_addr
> > + *   Remove multicast address from mac table
> > + * update_stats()
> > + *   Update Old network device statistics
> > + * get_ethtool_stats()
> > + *   Get ethtool network device statistics
> > + * get_strings()
> > + *   Get a set of strings that describe the requested objects
> > + * get_sset_count()
> > + *   Get number of strings that @get_strings will write
> > + * update_led_status()
> > + *   Update the led status
> > + * set_led_id()
> > + *   Set led id
> > + * get_regs()
> > + *   Get regs dump
> > + * get_regs_len()
> > + *   Get the len of the regs dump
> > + * get_rss_key_size()
> > + *   Get rss key size
> > + * get_rss_indir_size()
> > + *   Get rss indirection table size
> > + * get_rss()
> > + *   Get rss table
> > + * set_rss()
> > + *   Set rss table
> > + * get_tc_size()
> > + *   Get tc size of handle
> > + * get_vector()
> > + *   Get vector number and vector infomation
> > + * map_ring_to_vector()
> > + *   Map rings to vector
> > + * unmap_ring_from_vector()
> > + *   Unmap rings from vector
> > + * add_tunnel_udp()
> > + *   Add tunnel information to hardware
> > + * del_tunnel_udp()
> > + *   Delete tunnel information from hardware
> > + * reset_queue()
> > + *   Reset queue
> > + * get_fw_version()
> > + *   Get firmware version
> > + * get_mdix_mode()
> > + *   Get media typr of phy
> > + * set_vlan_filter()
> > + *   Set vlan filter config of Ports
> > + * set_vf_vlan_filter()
> > + *   Set vlan filter config of vf
> > + */
> > +struct hnae3_ae_ops {
> > +   int (*init_ae_dev)(struct hnae3_ae_dev *ae_dev);
> > +   void (*uninit_ae_dev)(struct hnae3_ae_dev *ae_dev);
> > +
> > +   int (*register_client)(struct hnae3_client *client,
> > +  struct hna

RE: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Salil Mehta]

Hi Bo Yu,

> -Original Message-
> From: Bo Yu [mailto:tsu.y...@gmail.com]
> Sent: Monday, June 19, 2017 1:40 AM
> To: Salil Mehta
> Cc: da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the
> HNAE3 framework
> 
> Hi,
> On Sat, Jun 17, 2017 at 06:24:25PM +0100, Salil Mehta wrote:
> >+ *   Unregister client from ae_dev
> >+ * start()
> >+ *   Enable the hardware
> >+ * stop()
> >+ *   Disable the hardware
> >+ * get_status()
> >+ *   Get the carrier state of the back channel of the handle, 1 for
> ok, 0 for
> >+ *   non-ok
> >+ * get_ksettings_an_result()
> >+ *   Get negotiation status,speed and duplex
> >+ * update_speed_duplex_h()
> >+ *   Update hardware speed and duplex
> >+ * get_media_type()
> >+ *   Get media type of MAC
> >+ * adjust_link()
> >+ *   Adjust link status
> >+ * set_loopback()
> >+ *   Set loopback
> >+ * set_promisc_mode
> >+ *   Set promisc mode
> >+ * set_mtu()
> >+ *   set mtu
> >+ * get_pauseparam()
> >+ *   get tx and rx of pause frame use
> >+ * set_pauseparam()
> >+ *   set tx and rx of pause frame use
> >+ * set_autoneg()
> >+ *   set auto autonegotiation of pause frame use
> >+ * get_autoneg()
> >+ *   get auto autonegotiation of pause frame use
> >+ * get_coalesce_usecs()
> >+ *   get usecs to delay a TX interrupt after a packet is sent
> >+ * get_rx_max_coalesced_frames()
> >+ *   get Maximum number of packets to be sent before a TX interrupt.
> >+ * set_coalesce_usecs()
> >+ *   set usecs to delay a TX interrupt after a packet is sent
> >+ * set_coalesce_frames()
> >+ *   set Maximum number of packets to be sent before a TX interrupt.
> >+ * get_mac_addr()
> >+ *   get mac address
> >+ * set_mac_addr()
> >+ *   set mac address
> >+ * add_uc_addr
> >+ *   Add unicast addr to mac table
> >+ * rm_uc_addr
> >+ *   Remove unicast addr from mac table
> >+ * set_mc_addr()
> >+ *   Set multicast address
> >+ * add_mc_addr
> >+ *   Add multicast address to mac table
> >+ * rm_mc_addr
> >+ *   Remove multicast address from mac table
> >+ * update_stats()
> >+ *   Update Old network device statistics
> >+ * get_ethtool_stats()
> >+ *   Get ethtool network device statistics
> >+ * get_strings()
> >+ *   Get a set of strings that describe the requested objects
> >+ * get_sset_count()
> >+ *   Get number of strings that @get_strings will write
> >+ * update_led_status()
> >+ *   Update the led status
> >+ * set_led_id()
> >+ *   Set led id
> >+ * get_regs()
> >+ *   Get regs dump
> >+ * get_regs_len()
> >+ *   Get the len of the regs dump
> >+ * get_rss_key_size()
> >+ *   Get rss key size
> >+ * get_rss_indir_size()
> >+ *   Get rss indirection table size
> >+ * get_rss()
> >+ *   Get rss table
> >+ * set_rss()
> >+ *   Set rss table
> >+ * get_tc_size()
> >+ *   Get tc size of handle
> >+ * get_vector()
> >+ *   Get vector number and vector infomation
> 
> Just another spealling : information
> 
> Checkpatch will report it also.
Fixed it. As far as I know chechkpatch.pl depends upon its dictionary
for it to be able to catch such mistakes. Have you prepared your own?

Thanks
Salil
> 
> >+ * map_ring_to_vector()
> >+ *   Map rings to vector
> >+ * unmap_ring_from_vector()
> >+ *   Unmap rings from vector
> >+ * add_tunnel_udp()
> >+ *   Add tunnel information to hardware
> >+ * del_tunnel_udp()
> >+ *   Delete tunnel information from hardware
> >+ * reset_queue()
> >+ *   Reset queue
> >+ * get_fw_version()
> >+ *   Get firmware version
> >+ * get_mdix_mode()
> >+ *   Get media typr of phy
> >+ * set_vlan_filter()
> >+ *   Set vlan filter config of Ports
> >+ * set_vf_vlan_filter()
> >+ *   Set vlan filter config of vf
> >+ */
> >+struct hnae3_ae_ops {
> >+int (*init_ae_dev)(struct hnae3_ae_dev *ae_dev);
> >+void (*uninit_ae_dev)(struct hnae3_ae_dev *ae_dev);
> >+
> >+int (*register_client)(struct hnae3_client *client,
> >+   struct hnae3_ae_dev *ae_dev);
> >+void (*unregister_client)(struct hnae3_client *client,
> >+  struct hnae3_ae_dev *ae_dev);
> >+int (*start)(struct hnae3_handle *handle);
> >+void (*stop)(struct h

RE: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Salil Mehta]

Hi Bo Yu,

> -Original Message-
> From: Bo Yu [mailto:tsu.y...@gmail.com]
> Sent: Monday, June 19, 2017 1:40 AM
> To: Salil Mehta
> Cc: da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the
> HNAE3 framework
> 
> Hi,
> On Sat, Jun 17, 2017 at 06:24:25PM +0100, Salil Mehta wrote:
> >+ *   Unregister client from ae_dev
> >+ * start()
> >+ *   Enable the hardware
> >+ * stop()
> >+ *   Disable the hardware
> >+ * get_status()
> >+ *   Get the carrier state of the back channel of the handle, 1 for
> ok, 0 for
> >+ *   non-ok
> >+ * get_ksettings_an_result()
> >+ *   Get negotiation status,speed and duplex
> >+ * update_speed_duplex_h()
> >+ *   Update hardware speed and duplex
> >+ * get_media_type()
> >+ *   Get media type of MAC
> >+ * adjust_link()
> >+ *   Adjust link status
> >+ * set_loopback()
> >+ *   Set loopback
> >+ * set_promisc_mode
> >+ *   Set promisc mode
> >+ * set_mtu()
> >+ *   set mtu
> >+ * get_pauseparam()
> >+ *   get tx and rx of pause frame use
> >+ * set_pauseparam()
> >+ *   set tx and rx of pause frame use
> >+ * set_autoneg()
> >+ *   set auto autonegotiation of pause frame use
> >+ * get_autoneg()
> >+ *   get auto autonegotiation of pause frame use
> >+ * get_coalesce_usecs()
> >+ *   get usecs to delay a TX interrupt after a packet is sent
> >+ * get_rx_max_coalesced_frames()
> >+ *   get Maximum number of packets to be sent before a TX interrupt.
> >+ * set_coalesce_usecs()
> >+ *   set usecs to delay a TX interrupt after a packet is sent
> >+ * set_coalesce_frames()
> >+ *   set Maximum number of packets to be sent before a TX interrupt.
> >+ * get_mac_addr()
> >+ *   get mac address
> >+ * set_mac_addr()
> >+ *   set mac address
> >+ * add_uc_addr
> >+ *   Add unicast addr to mac table
> >+ * rm_uc_addr
> >+ *   Remove unicast addr from mac table
> >+ * set_mc_addr()
> >+ *   Set multicast address
> >+ * add_mc_addr
> >+ *   Add multicast address to mac table
> >+ * rm_mc_addr
> >+ *   Remove multicast address from mac table
> >+ * update_stats()
> >+ *   Update Old network device statistics
> >+ * get_ethtool_stats()
> >+ *   Get ethtool network device statistics
> >+ * get_strings()
> >+ *   Get a set of strings that describe the requested objects
> >+ * get_sset_count()
> >+ *   Get number of strings that @get_strings will write
> >+ * update_led_status()
> >+ *   Update the led status
> >+ * set_led_id()
> >+ *   Set led id
> >+ * get_regs()
> >+ *   Get regs dump
> >+ * get_regs_len()
> >+ *   Get the len of the regs dump
> >+ * get_rss_key_size()
> >+ *   Get rss key size
> >+ * get_rss_indir_size()
> >+ *   Get rss indirection table size
> >+ * get_rss()
> >+ *   Get rss table
> >+ * set_rss()
> >+ *   Set rss table
> >+ * get_tc_size()
> >+ *   Get tc size of handle
> >+ * get_vector()
> >+ *   Get vector number and vector infomation
> 
> Just another spealling : information
> 
> Checkpatch will report it also.
Fixed it. As far as I know chechkpatch.pl depends upon its dictionary
for it to be able to catch such mistakes. Have you prepared your own?

Thanks
Salil
> 
> >+ * map_ring_to_vector()
> >+ *   Map rings to vector
> >+ * unmap_ring_from_vector()
> >+ *   Unmap rings from vector
> >+ * add_tunnel_udp()
> >+ *   Add tunnel information to hardware
> >+ * del_tunnel_udp()
> >+ *   Delete tunnel information from hardware
> >+ * reset_queue()
> >+ *   Reset queue
> >+ * get_fw_version()
> >+ *   Get firmware version
> >+ * get_mdix_mode()
> >+ *   Get media typr of phy
> >+ * set_vlan_filter()
> >+ *   Set vlan filter config of Ports
> >+ * set_vf_vlan_filter()
> >+ *   Set vlan filter config of vf
> >+ */
> >+struct hnae3_ae_ops {
> >+int (*init_ae_dev)(struct hnae3_ae_dev *ae_dev);
> >+void (*uninit_ae_dev)(struct hnae3_ae_dev *ae_dev);
> >+
> >+int (*register_client)(struct hnae3_client *client,
> >+   struct hnae3_ae_dev *ae_dev);
> >+void (*unregister_client)(struct hnae3_client *client,
> >+  struct hnae3_ae_dev *ae_dev);
> >+int (*start)(struct hnae3_handle *handle);
> >+void (*stop)(struct h

RE: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Salil Mehta]

Hi Andrew,
> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Sunday, June 18, 2017 4:02 PM
> To: Salil Mehta
> Cc: da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the
> HNAE3 framework
> 
> > +static int __init hnae3_init(void)
> > +{
> > +   return 0;
> > +}
> > +
> > +static void __exit hnae3_exit(void)
> > +{
> > +}
> > +
> > +module_init(hnae3_init);
> > +module_exit(hnae3_exit);
> 
> I think init and exit functions are optional. Since your's don't do
> anything useful, please try without them.
Yes, you were right. Removed in V4 patch. 

Thanks
Salil
> 
>Andrew

RE: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Salil Mehta]

Hi Andrew,
> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Sunday, June 18, 2017 4:02 PM
> To: Salil Mehta
> Cc: da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the
> HNAE3 framework
> 
> > +static int __init hnae3_init(void)
> > +{
> > +   return 0;
> > +}
> > +
> > +static void __exit hnae3_exit(void)
> > +{
> > +}
> > +
> > +module_init(hnae3_init);
> > +module_exit(hnae3_exit);
> 
> I think init and exit functions are optional. Since your's don't do
> anything useful, please try without them.
Yes, you were right. Removed in V4 patch. 

Thanks
Salil
> 
>Andrew

RE: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Salil Mehta]

Hi Andrew,

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Saturday, June 17, 2017 8:46 PM
> To: Salil Mehta
> Cc: da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the
> HNAE3 framework
> 
> > +static void hnae3_list_add(spinlock_t *lock, struct list_head *node,
> > +  struct list_head *head)
> > +{
> > +   unsigned long flags;
> > +
> > +   spin_lock_irqsave(lock, flags);
> > +   list_add_tail(node, head);
> > +   spin_unlock_irqrestore(lock, flags);
> > +}
> > +
> > +static void hnae3_list_del(spinlock_t *lock, struct list_head *node)
> > +{
> > +   unsigned long flags;
> > +
> > +   spin_lock_irqsave(lock, flags);
> > +   list_del(node);
> > +   spin_unlock_irqrestore(lock, flags);
> > +}
> > +
> 
> > +int hnae3_register_client(struct hnae3_client *client)
> > +{
> > +   struct hnae3_client *client_tmp;
> > +   struct hnae3_ae_dev *ae_dev;
> > +   int ret;
> > +
> > +   /* One system should only have one client for every type */
> > +   list_for_each_entry(client_tmp, _client_list, node) {
> > +   if (client_tmp->type == client->type)
> > +   return 0;
> > +   }
> > +
> > +   hnae3_list_add(_list_client_lock, >node,
> > +  _client_list);
> 
> Please could you explain your locking scheme. I don't get it.
> 
>Thanks
>   Andrew
Locking scheme has been fixed in the V4 patch. Please review it.

Thanks
Salil

RE: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Salil Mehta]

Hi Andrew,

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Saturday, June 17, 2017 8:46 PM
> To: Salil Mehta
> Cc: da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the
> HNAE3 framework
> 
> > +static void hnae3_list_add(spinlock_t *lock, struct list_head *node,
> > +  struct list_head *head)
> > +{
> > +   unsigned long flags;
> > +
> > +   spin_lock_irqsave(lock, flags);
> > +   list_add_tail(node, head);
> > +   spin_unlock_irqrestore(lock, flags);
> > +}
> > +
> > +static void hnae3_list_del(spinlock_t *lock, struct list_head *node)
> > +{
> > +   unsigned long flags;
> > +
> > +   spin_lock_irqsave(lock, flags);
> > +   list_del(node);
> > +   spin_unlock_irqrestore(lock, flags);
> > +}
> > +
> 
> > +int hnae3_register_client(struct hnae3_client *client)
> > +{
> > +   struct hnae3_client *client_tmp;
> > +   struct hnae3_ae_dev *ae_dev;
> > +   int ret;
> > +
> > +   /* One system should only have one client for every type */
> > +   list_for_each_entry(client_tmp, _client_list, node) {
> > +   if (client_tmp->type == client->type)
> > +   return 0;
> > +   }
> > +
> > +   hnae3_list_add(_list_client_lock, >node,
> > +  _client_list);
> 
> Please could you explain your locking scheme. I don't get it.
> 
>Thanks
>   Andrew
Locking scheme has been fixed in the V4 patch. Please review it.

Thanks
Salil

Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Stephen Hemminger]

On Sat, 17 Jun 2017 18:24:25 +0100
Salil Mehta  wrote:

> +
> +/* This struct defines the operation on the handle.
> + *
> + * init_ae_dev(): (mandatory)
> + *   Get PF configure from pci_dev and initialize PF hardware
> + * uninit_ae_dev()
> + *   Disable PF device and release PF resource
> + * register_client
> + *   Register client to ae_dev
> + * unregister_client()
> + *   Unregister client from ae_dev
> + * start()
> + *   Enable the hardware
> + * stop()
> + *   Disable the hardware
> + * get_status()
> + *   Get the carrier state of the back channel of the handle, 1 for ok, 0 for
> + *   non-ok
> + * get_ksettings_an_result()
> + *   Get negotiation status,speed and duplex
> + * update_speed_duplex_h()
> + *   Update hardware speed and duplex
> + * get_media_type()
> + *   Get media type of MAC
> + * adjust_link()
> + *   Adjust link status
> + * set_loopback()
> + *   Set loopback
> + * set_promisc_mode
> + *   Set promisc mode
> + * set_mtu()
> + *   set mtu
> + * get_pauseparam()
> + *   get tx and rx of pause frame use
> + * set_pauseparam()
> + *   set tx and rx of pause frame use
> + * set_autoneg()
> + *   set auto autonegotiation of pause frame use
> + * get_autoneg()
> + *   get auto autonegotiation of pause frame use
> + * get_coalesce_usecs()
> + *   get usecs to delay a TX interrupt after a packet is sent
> + * get_rx_max_coalesced_frames()
> + *   get Maximum number of packets to be sent before a TX interrupt.
> + * set_coalesce_usecs()
> + *   set usecs to delay a TX interrupt after a packet is sent
> + * set_coalesce_frames()
> + *   set Maximum number of packets to be sent before a TX interrupt.
> + * get_mac_addr()
> + *   get mac address
> + * set_mac_addr()
> + *   set mac address
> + * add_uc_addr
> + *   Add unicast addr to mac table
> + * rm_uc_addr
> + *   Remove unicast addr from mac table
> + * set_mc_addr()
> + *   Set multicast address
> + * add_mc_addr
> + *   Add multicast address to mac table
> + * rm_mc_addr
> + *   Remove multicast address from mac table
> + * update_stats()
> + *   Update Old network device statistics
> + * get_ethtool_stats()
> + *   Get ethtool network device statistics
> + * get_strings()
> + *   Get a set of strings that describe the requested objects
> + * get_sset_count()
> + *   Get number of strings that @get_strings will write
> + * update_led_status()
> + *   Update the led status
> + * set_led_id()
> + *   Set led id
> + * get_regs()
> + *   Get regs dump
> + * get_regs_len()
> + *   Get the len of the regs dump
> + * get_rss_key_size()
> + *   Get rss key size
> + * get_rss_indir_size()
> + *   Get rss indirection table size
> + * get_rss()
> + *   Get rss table
> + * set_rss()
> + *   Set rss table
> + * get_tc_size()
> + *   Get tc size of handle
> + * get_vector()
> + *   Get vector number and vector infomation
> + * map_ring_to_vector()
> + *   Map rings to vector
> + * unmap_ring_from_vector()
> + *   Unmap rings from vector
> + * add_tunnel_udp()
> + *   Add tunnel information to hardware
> + * del_tunnel_udp()
> + *   Delete tunnel information from hardware
> + * reset_queue()
> + *   Reset queue
> + * get_fw_version()
> + *   Get firmware version
> + * get_mdix_mode()
> + *   Get media typr of phy
> + * set_vlan_filter()
> + *   Set vlan filter config of Ports
> + * set_vf_vlan_filter()
> + *   Set vlan filter config of vf
> + */
> +struct hnae3_ae_ops {
> + int (*init_ae_dev)(struct hnae3_ae_dev *ae_dev);
> + void (*uninit_ae_dev)(struct hnae3_ae_dev *ae_dev);
> +
> + int (*register_client)(struct hnae3_client *client,
> +struct hnae3_ae_dev *ae_dev);
> + void (*unregister_client)(struct hnae3_client *client,
> +   struct hnae3_ae_dev *ae_dev);
> + int (*start)(struct hnae3_handle *handle);
> + void (*stop)(struct hnae3_handle *handle);
> + int (*get_status)(struct hnae3_handle *handle);
> + void (*get_ksettings_an_result)(struct hnae3_handle *handle,
> + u8 *auto_neg, u32 *speed, u8 *duplex);
> +
> + int (*update_speed_duplex_h)(struct hnae3_handle *handle);
> + int (*cfg_mac_speed_dup_h)(struct hnae3_handle *handle, int speed,
> +u8 duplex);
> +
> + void (*get_media_type)(struct hnae3_handle *handle, u8 *media_type);
> + void (*adjust_link)(struct hnae3_handle *handle, int speed, int duplex);
> + int (*set_loopback)(struct hnae3_handle *handle,
> + enum hnae3_loop loop_mode, bool en);
> +
> + void (*set_promisc_mode)(struct hnae3_handle *handle, u32 en);
> + int (*set_mtu)(struct hnae3_handle *handle, int new_mtu);
> +
> + void (*get_pauseparam)(struct hnae3_handle *handle,
> +u32 *auto_neg, u32 *rx_en, u32 *tx_en);
> + int (*set_pauseparam)(struct hnae3_handle *handle,
> +   u32 auto_neg, u32 rx_en, u32 tx_en);
> +
> + int 

Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Stephen Hemminger]

On Sat, 17 Jun 2017 18:24:25 +0100
Salil Mehta  wrote:

> +
> +/* This struct defines the operation on the handle.
> + *
> + * init_ae_dev(): (mandatory)
> + *   Get PF configure from pci_dev and initialize PF hardware
> + * uninit_ae_dev()
> + *   Disable PF device and release PF resource
> + * register_client
> + *   Register client to ae_dev
> + * unregister_client()
> + *   Unregister client from ae_dev
> + * start()
> + *   Enable the hardware
> + * stop()
> + *   Disable the hardware
> + * get_status()
> + *   Get the carrier state of the back channel of the handle, 1 for ok, 0 for
> + *   non-ok
> + * get_ksettings_an_result()
> + *   Get negotiation status,speed and duplex
> + * update_speed_duplex_h()
> + *   Update hardware speed and duplex
> + * get_media_type()
> + *   Get media type of MAC
> + * adjust_link()
> + *   Adjust link status
> + * set_loopback()
> + *   Set loopback
> + * set_promisc_mode
> + *   Set promisc mode
> + * set_mtu()
> + *   set mtu
> + * get_pauseparam()
> + *   get tx and rx of pause frame use
> + * set_pauseparam()
> + *   set tx and rx of pause frame use
> + * set_autoneg()
> + *   set auto autonegotiation of pause frame use
> + * get_autoneg()
> + *   get auto autonegotiation of pause frame use
> + * get_coalesce_usecs()
> + *   get usecs to delay a TX interrupt after a packet is sent
> + * get_rx_max_coalesced_frames()
> + *   get Maximum number of packets to be sent before a TX interrupt.
> + * set_coalesce_usecs()
> + *   set usecs to delay a TX interrupt after a packet is sent
> + * set_coalesce_frames()
> + *   set Maximum number of packets to be sent before a TX interrupt.
> + * get_mac_addr()
> + *   get mac address
> + * set_mac_addr()
> + *   set mac address
> + * add_uc_addr
> + *   Add unicast addr to mac table
> + * rm_uc_addr
> + *   Remove unicast addr from mac table
> + * set_mc_addr()
> + *   Set multicast address
> + * add_mc_addr
> + *   Add multicast address to mac table
> + * rm_mc_addr
> + *   Remove multicast address from mac table
> + * update_stats()
> + *   Update Old network device statistics
> + * get_ethtool_stats()
> + *   Get ethtool network device statistics
> + * get_strings()
> + *   Get a set of strings that describe the requested objects
> + * get_sset_count()
> + *   Get number of strings that @get_strings will write
> + * update_led_status()
> + *   Update the led status
> + * set_led_id()
> + *   Set led id
> + * get_regs()
> + *   Get regs dump
> + * get_regs_len()
> + *   Get the len of the regs dump
> + * get_rss_key_size()
> + *   Get rss key size
> + * get_rss_indir_size()
> + *   Get rss indirection table size
> + * get_rss()
> + *   Get rss table
> + * set_rss()
> + *   Set rss table
> + * get_tc_size()
> + *   Get tc size of handle
> + * get_vector()
> + *   Get vector number and vector infomation
> + * map_ring_to_vector()
> + *   Map rings to vector
> + * unmap_ring_from_vector()
> + *   Unmap rings from vector
> + * add_tunnel_udp()
> + *   Add tunnel information to hardware
> + * del_tunnel_udp()
> + *   Delete tunnel information from hardware
> + * reset_queue()
> + *   Reset queue
> + * get_fw_version()
> + *   Get firmware version
> + * get_mdix_mode()
> + *   Get media typr of phy
> + * set_vlan_filter()
> + *   Set vlan filter config of Ports
> + * set_vf_vlan_filter()
> + *   Set vlan filter config of vf
> + */
> +struct hnae3_ae_ops {
> + int (*init_ae_dev)(struct hnae3_ae_dev *ae_dev);
> + void (*uninit_ae_dev)(struct hnae3_ae_dev *ae_dev);
> +
> + int (*register_client)(struct hnae3_client *client,
> +struct hnae3_ae_dev *ae_dev);
> + void (*unregister_client)(struct hnae3_client *client,
> +   struct hnae3_ae_dev *ae_dev);
> + int (*start)(struct hnae3_handle *handle);
> + void (*stop)(struct hnae3_handle *handle);
> + int (*get_status)(struct hnae3_handle *handle);
> + void (*get_ksettings_an_result)(struct hnae3_handle *handle,
> + u8 *auto_neg, u32 *speed, u8 *duplex);
> +
> + int (*update_speed_duplex_h)(struct hnae3_handle *handle);
> + int (*cfg_mac_speed_dup_h)(struct hnae3_handle *handle, int speed,
> +u8 duplex);
> +
> + void (*get_media_type)(struct hnae3_handle *handle, u8 *media_type);
> + void (*adjust_link)(struct hnae3_handle *handle, int speed, int duplex);
> + int (*set_loopback)(struct hnae3_handle *handle,
> + enum hnae3_loop loop_mode, bool en);
> +
> + void (*set_promisc_mode)(struct hnae3_handle *handle, u32 en);
> + int (*set_mtu)(struct hnae3_handle *handle, int new_mtu);
> +
> + void (*get_pauseparam)(struct hnae3_handle *handle,
> +u32 *auto_neg, u32 *rx_en, u32 *tx_en);
> + int (*set_pauseparam)(struct hnae3_handle *handle,
> +   u32 auto_neg, u32 rx_en, u32 tx_en);
> +
> + int (*set_autoneg)(struct hnae3_handle 

Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Bo Yu]

Hi,
On Sat, Jun 17, 2017 at 06:24:25PM +0100, Salil Mehta wrote:

+ *   Unregister client from ae_dev
+ * start()
+ *   Enable the hardware
+ * stop()
+ *   Disable the hardware
+ * get_status()
+ *   Get the carrier state of the back channel of the handle, 1 for ok, 0 for
+ *   non-ok
+ * get_ksettings_an_result()
+ *   Get negotiation status,speed and duplex
+ * update_speed_duplex_h()
+ *   Update hardware speed and duplex
+ * get_media_type()
+ *   Get media type of MAC
+ * adjust_link()
+ *   Adjust link status
+ * set_loopback()
+ *   Set loopback
+ * set_promisc_mode
+ *   Set promisc mode
+ * set_mtu()
+ *   set mtu
+ * get_pauseparam()
+ *   get tx and rx of pause frame use
+ * set_pauseparam()
+ *   set tx and rx of pause frame use
+ * set_autoneg()
+ *   set auto autonegotiation of pause frame use
+ * get_autoneg()
+ *   get auto autonegotiation of pause frame use
+ * get_coalesce_usecs()
+ *   get usecs to delay a TX interrupt after a packet is sent
+ * get_rx_max_coalesced_frames()
+ *   get Maximum number of packets to be sent before a TX interrupt.
+ * set_coalesce_usecs()
+ *   set usecs to delay a TX interrupt after a packet is sent
+ * set_coalesce_frames()
+ *   set Maximum number of packets to be sent before a TX interrupt.
+ * get_mac_addr()
+ *   get mac address
+ * set_mac_addr()
+ *   set mac address
+ * add_uc_addr
+ *   Add unicast addr to mac table
+ * rm_uc_addr
+ *   Remove unicast addr from mac table
+ * set_mc_addr()
+ *   Set multicast address
+ * add_mc_addr
+ *   Add multicast address to mac table
+ * rm_mc_addr
+ *   Remove multicast address from mac table
+ * update_stats()
+ *   Update Old network device statistics
+ * get_ethtool_stats()
+ *   Get ethtool network device statistics
+ * get_strings()
+ *   Get a set of strings that describe the requested objects
+ * get_sset_count()
+ *   Get number of strings that @get_strings will write
+ * update_led_status()
+ *   Update the led status
+ * set_led_id()
+ *   Set led id
+ * get_regs()
+ *   Get regs dump
+ * get_regs_len()
+ *   Get the len of the regs dump
+ * get_rss_key_size()
+ *   Get rss key size
+ * get_rss_indir_size()
+ *   Get rss indirection table size
+ * get_rss()
+ *   Get rss table
+ * set_rss()
+ *   Set rss table
+ * get_tc_size()
+ *   Get tc size of handle
+ * get_vector()
+ *   Get vector number and vector infomation


Just another spealling : information

Checkpatch will report it also.


+ * map_ring_to_vector()
+ *   Map rings to vector
+ * unmap_ring_from_vector()
+ *   Unmap rings from vector
+ * add_tunnel_udp()
+ *   Add tunnel information to hardware
+ * del_tunnel_udp()
+ *   Delete tunnel information from hardware
+ * reset_queue()
+ *   Reset queue
+ * get_fw_version()
+ *   Get firmware version
+ * get_mdix_mode()
+ *   Get media typr of phy
+ * set_vlan_filter()
+ *   Set vlan filter config of Ports
+ * set_vf_vlan_filter()
+ *   Set vlan filter config of vf
+ */
+struct hnae3_ae_ops {
+   int (*init_ae_dev)(struct hnae3_ae_dev *ae_dev);
+   void (*uninit_ae_dev)(struct hnae3_ae_dev *ae_dev);
+
+   int (*register_client)(struct hnae3_client *client,
+  struct hnae3_ae_dev *ae_dev);
+   void (*unregister_client)(struct hnae3_client *client,
+ struct hnae3_ae_dev *ae_dev);
+   int (*start)(struct hnae3_handle *handle);
+   void (*stop)(struct hnae3_handle *handle);
+   int (*get_status)(struct hnae3_handle *handle);
+   void (*get_ksettings_an_result)(struct hnae3_handle *handle,
+   u8 *auto_neg, u32 *speed, u8 *duplex);
+
+   int (*update_speed_duplex_h)(struct hnae3_handle *handle);
+   int (*cfg_mac_speed_dup_h)(struct hnae3_handle *handle, int speed,
+  u8 duplex);
+
+   void (*get_media_type)(struct hnae3_handle *handle, u8 *media_type);
+   void (*adjust_link)(struct hnae3_handle *handle, int speed, int duplex);
+   int (*set_loopback)(struct hnae3_handle *handle,
+   enum hnae3_loop loop_mode, bool en);
+
+   void (*set_promisc_mode)(struct hnae3_handle *handle, u32 en);
+   int (*set_mtu)(struct hnae3_handle *handle, int new_mtu);
+
+   void (*get_pauseparam)(struct hnae3_handle *handle,
+  u32 *auto_neg, u32 *rx_en, u32 *tx_en);
+   int (*set_pauseparam)(struct hnae3_handle *handle,
+ u32 auto_neg, u32 rx_en, u32 tx_en);
+
+   int (*set_autoneg)(struct hnae3_handle *handle, bool enable);
+   int (*get_autoneg)(struct hnae3_handle *handle);
+
+   void (*get_coalesce_usecs)(struct hnae3_handle *handle,
+  u32 *tx_usecs, u32 *rx_usecs);
+   void (*get_rx_max_coalesced_frames)(struct hnae3_handle *handle,
+   u32 *tx_frames, u32 *rx_frames);
+   int (*set_coalesce_usecs)(struct hnae3_handle *handle, u32 timeout);
+ 

Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Bo Yu]

Hi,
On Sat, Jun 17, 2017 at 06:24:25PM +0100, Salil Mehta wrote:

+ *   Unregister client from ae_dev
+ * start()
+ *   Enable the hardware
+ * stop()
+ *   Disable the hardware
+ * get_status()
+ *   Get the carrier state of the back channel of the handle, 1 for ok, 0 for
+ *   non-ok
+ * get_ksettings_an_result()
+ *   Get negotiation status,speed and duplex
+ * update_speed_duplex_h()
+ *   Update hardware speed and duplex
+ * get_media_type()
+ *   Get media type of MAC
+ * adjust_link()
+ *   Adjust link status
+ * set_loopback()
+ *   Set loopback
+ * set_promisc_mode
+ *   Set promisc mode
+ * set_mtu()
+ *   set mtu
+ * get_pauseparam()
+ *   get tx and rx of pause frame use
+ * set_pauseparam()
+ *   set tx and rx of pause frame use
+ * set_autoneg()
+ *   set auto autonegotiation of pause frame use
+ * get_autoneg()
+ *   get auto autonegotiation of pause frame use
+ * get_coalesce_usecs()
+ *   get usecs to delay a TX interrupt after a packet is sent
+ * get_rx_max_coalesced_frames()
+ *   get Maximum number of packets to be sent before a TX interrupt.
+ * set_coalesce_usecs()
+ *   set usecs to delay a TX interrupt after a packet is sent
+ * set_coalesce_frames()
+ *   set Maximum number of packets to be sent before a TX interrupt.
+ * get_mac_addr()
+ *   get mac address
+ * set_mac_addr()
+ *   set mac address
+ * add_uc_addr
+ *   Add unicast addr to mac table
+ * rm_uc_addr
+ *   Remove unicast addr from mac table
+ * set_mc_addr()
+ *   Set multicast address
+ * add_mc_addr
+ *   Add multicast address to mac table
+ * rm_mc_addr
+ *   Remove multicast address from mac table
+ * update_stats()
+ *   Update Old network device statistics
+ * get_ethtool_stats()
+ *   Get ethtool network device statistics
+ * get_strings()
+ *   Get a set of strings that describe the requested objects
+ * get_sset_count()
+ *   Get number of strings that @get_strings will write
+ * update_led_status()
+ *   Update the led status
+ * set_led_id()
+ *   Set led id
+ * get_regs()
+ *   Get regs dump
+ * get_regs_len()
+ *   Get the len of the regs dump
+ * get_rss_key_size()
+ *   Get rss key size
+ * get_rss_indir_size()
+ *   Get rss indirection table size
+ * get_rss()
+ *   Get rss table
+ * set_rss()
+ *   Set rss table
+ * get_tc_size()
+ *   Get tc size of handle
+ * get_vector()
+ *   Get vector number and vector infomation


Just another spealling : information

Checkpatch will report it also.


+ * map_ring_to_vector()
+ *   Map rings to vector
+ * unmap_ring_from_vector()
+ *   Unmap rings from vector
+ * add_tunnel_udp()
+ *   Add tunnel information to hardware
+ * del_tunnel_udp()
+ *   Delete tunnel information from hardware
+ * reset_queue()
+ *   Reset queue
+ * get_fw_version()
+ *   Get firmware version
+ * get_mdix_mode()
+ *   Get media typr of phy
+ * set_vlan_filter()
+ *   Set vlan filter config of Ports
+ * set_vf_vlan_filter()
+ *   Set vlan filter config of vf
+ */
+struct hnae3_ae_ops {
+   int (*init_ae_dev)(struct hnae3_ae_dev *ae_dev);
+   void (*uninit_ae_dev)(struct hnae3_ae_dev *ae_dev);
+
+   int (*register_client)(struct hnae3_client *client,
+  struct hnae3_ae_dev *ae_dev);
+   void (*unregister_client)(struct hnae3_client *client,
+ struct hnae3_ae_dev *ae_dev);
+   int (*start)(struct hnae3_handle *handle);
+   void (*stop)(struct hnae3_handle *handle);
+   int (*get_status)(struct hnae3_handle *handle);
+   void (*get_ksettings_an_result)(struct hnae3_handle *handle,
+   u8 *auto_neg, u32 *speed, u8 *duplex);
+
+   int (*update_speed_duplex_h)(struct hnae3_handle *handle);
+   int (*cfg_mac_speed_dup_h)(struct hnae3_handle *handle, int speed,
+  u8 duplex);
+
+   void (*get_media_type)(struct hnae3_handle *handle, u8 *media_type);
+   void (*adjust_link)(struct hnae3_handle *handle, int speed, int duplex);
+   int (*set_loopback)(struct hnae3_handle *handle,
+   enum hnae3_loop loop_mode, bool en);
+
+   void (*set_promisc_mode)(struct hnae3_handle *handle, u32 en);
+   int (*set_mtu)(struct hnae3_handle *handle, int new_mtu);
+
+   void (*get_pauseparam)(struct hnae3_handle *handle,
+  u32 *auto_neg, u32 *rx_en, u32 *tx_en);
+   int (*set_pauseparam)(struct hnae3_handle *handle,
+ u32 auto_neg, u32 rx_en, u32 tx_en);
+
+   int (*set_autoneg)(struct hnae3_handle *handle, bool enable);
+   int (*get_autoneg)(struct hnae3_handle *handle);
+
+   void (*get_coalesce_usecs)(struct hnae3_handle *handle,
+  u32 *tx_usecs, u32 *rx_usecs);
+   void (*get_rx_max_coalesced_frames)(struct hnae3_handle *handle,
+   u32 *tx_frames, u32 *rx_frames);
+   int (*set_coalesce_usecs)(struct hnae3_handle *handle, u32 timeout);
+ 

Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Andrew Lunn]

> +static int __init hnae3_init(void)
> +{
> + return 0;
> +}
> +
> +static void __exit hnae3_exit(void)
> +{
> +}
> +
> +module_init(hnae3_init);
> +module_exit(hnae3_exit);

I think init and exit functions are optional. Since your's don't do
anything useful, please try without them.

 Andrew

Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Andrew Lunn]

> +static int __init hnae3_init(void)
> +{
> + return 0;
> +}
> +
> +static void __exit hnae3_exit(void)
> +{
> +}
> +
> +module_init(hnae3_init);
> +module_exit(hnae3_exit);

I think init and exit functions are optional. Since your's don't do
anything useful, please try without them.

 Andrew

Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Andrew Lunn]

> +static void hnae3_list_add(spinlock_t *lock, struct list_head *node,
> +struct list_head *head)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(lock, flags);
> + list_add_tail(node, head);
> + spin_unlock_irqrestore(lock, flags);
> +}
> +
> +static void hnae3_list_del(spinlock_t *lock, struct list_head *node)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(lock, flags);
> + list_del(node);
> + spin_unlock_irqrestore(lock, flags);
> +}
> +

> +int hnae3_register_client(struct hnae3_client *client)
> +{
> + struct hnae3_client *client_tmp;
> + struct hnae3_ae_dev *ae_dev;
> + int ret;
> +
> + /* One system should only have one client for every type */
> + list_for_each_entry(client_tmp, _client_list, node) {
> + if (client_tmp->type == client->type)
> + return 0;
> + }
> +
> + hnae3_list_add(_list_client_lock, >node,
> +_client_list);

Please could you explain your locking scheme. I don't get it.

   Thanks
Andrew

Re: [PATCH V3 net-next 2/8] net: hns3: Add support of the HNAE3 framework

[Andrew Lunn]

> +static void hnae3_list_add(spinlock_t *lock, struct list_head *node,
> +struct list_head *head)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(lock, flags);
> + list_add_tail(node, head);
> + spin_unlock_irqrestore(lock, flags);
> +}
> +
> +static void hnae3_list_del(spinlock_t *lock, struct list_head *node)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(lock, flags);
> + list_del(node);
> + spin_unlock_irqrestore(lock, flags);
> +}
> +

> +int hnae3_register_client(struct hnae3_client *client)
> +{
> + struct hnae3_client *client_tmp;
> + struct hnae3_ae_dev *ae_dev;
> + int ret;
> +
> + /* One system should only have one client for every type */
> + list_for_each_entry(client_tmp, _client_list, node) {
> + if (client_tmp->type == client->type)
> + return 0;
> + }
> +
> + hnae3_list_add(_list_client_lock, >node,
> +_client_list);

Please could you explain your locking scheme. I don't get it.

   Thanks
Andrew