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 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
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, &hnae3_client_list, node) { > > + if (client_tmp->type == client->type) > > + return 0; > > + } > > + > > + hnae3_list_add(&hnae3_list_client_lock, &client->node, > > + &hnae3_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
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
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
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
> +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, &hnae3_client_list, node) { > + if (client_tmp->type == client->type) > + return 0; > + } > + > + hnae3_list_add(&hnae3_list_client_lock, &client->node, > +&hnae3_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
> +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
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); +