On Tue, Mar 05, 2024 at 06:50:21PM -0800, Jesse Brandeburg wrote: > The igb driver was pre-declaring tons of functions just so that it could > have an early declaration of the pci_driver struct. > > Delete a bunch of the declarations and move the struct to the bottom of the > file, after all the functions are declared.
Reviewed-by: Maciej Fijalkowski <[email protected]> However, that's just a drop in the ocean when we look at unneeded forward declarations, isn't it? I got side-tracked while looking at some stuff and saw such forward decls in i40e_nvm.c and decided to fix this as this was rather a no-brainer that just required to move code around. Now I feel encouraged to send this, but what should we do with rest of those, though? Being a regex amateur I came up with following command: $ grep -hPonz 'static [a-z]+.+\([^()]+\);' drivers/net/ethernet/intel/igb/*.c | sed 's/1:/\n/g' in order to catch all of the forward declarations in source files and this results in very nasty list [0]...and this is only within igb! ice driver is a nice example that reviews in netdev community matter. It contains only 4 fwd decls and I believe it is very much related to times when vendor pull requests started to be actually reviewed in the upstream, not just taken as-is. [0]: static s32 igb_get_invariants_82575(struct e1000_hw *); static s32 igb_acquire_phy_82575(struct e1000_hw *); static void igb_release_phy_82575(struct e1000_hw *); static s32 igb_acquire_nvm_82575(struct e1000_hw *); static void igb_release_nvm_82575(struct e1000_hw *); static s32 igb_check_for_link_82575(struct e1000_hw *); static s32 igb_get_cfg_done_82575(struct e1000_hw *); static s32 igb_init_hw_82575(struct e1000_hw *); static s32 igb_phy_hw_reset_sgmii_82575(struct e1000_hw *); static s32 igb_read_phy_reg_sgmii_82575(struct e1000_hw *, u32, u16 *); static s32 igb_reset_hw_82575(struct e1000_hw *); static s32 igb_reset_hw_82580(struct e1000_hw *); static s32 igb_set_d0_lplu_state_82575(struct e1000_hw *, bool); static s32 igb_set_d0_lplu_state_82580(struct e1000_hw *, bool); static s32 igb_set_d3_lplu_state_82580(struct e1000_hw *, bool); static s32 igb_setup_copper_link_82575(struct e1000_hw *); static s32 igb_setup_serdes_link_82575(struct e1000_hw *); static s32 igb_write_phy_reg_sgmii_82575(struct e1000_hw *, u32, u16); static void igb_clear_hw_cntrs_82575(struct e1000_hw *); static s32 igb_acquire_swfw_sync_82575(struct e1000_hw *, u16); static s32 igb_get_pcs_speed_and_duplex_82575(struct e1000_hw *, u16 * u16 *); static s32 igb_get_phy_id_82575(struct e1000_hw *); static void igb_release_swfw_sync_82575(struct e1000_hw *, u16); static bool igb_sgmii_active_82575(struct e1000_hw *); static s32 igb_reset_init_script_82575(struct e1000_hw *); static s32 igb_read_mac_addr_82575(struct e1000_hw *); static s32 igb_set_pcie_completion_timeout(struct e1000_hw *hw); static s32 igb_reset_mdicnfg_82580(struct e1000_hw *hw); static s32 igb_validate_nvm_checksum_82580(struct e1000_hw *hw); static s32 igb_update_nvm_checksum_82580(struct e1000_hw *hw); static s32 igb_validate_nvm_checksum_i350(struct e1000_hw *hw); static s32 igb_update_nvm_checksum_i350(struct e1000_hw *hw); static s32 igb_update_flash_i210(struct e1000_hw *hw); static s32 igb_set_default_fc(struct e1000_hw *hw); static void igb_set_fc_watermarks(struct e1000_hw *hw); static s32 igb_phy_setup_autoneg(struct e1000_hw *hw); static void igb_phy_force_speed_duplex_setup(struct e1000_hw *hw u16 *phy_ctrl); static s32 igb_wait_autoneg(struct e1000_hw *hw); static s32 igb_set_master_slave_mode(struct e1000_hw *hw); static int igb_setup_all_tx_resources(struct igb_adapter *); static int igb_setup_all_rx_resources(struct igb_adapter *); static void igb_free_all_tx_resources(struct igb_adapter *); static void igb_free_all_rx_resources(struct igb_adapter *); static void igb_setup_mrqc(struct igb_adapter *); static int igb_probe(struct pci_dev *, const struct pci_device_id *); static void igb_remove(struct pci_dev *pdev); static void igb_init_queue_configuration(struct igb_adapter *adapter); static int igb_sw_init(struct igb_adapter *); static void igb_configure(struct igb_adapter *); static void igb_configure_tx(struct igb_adapter *); static void igb_configure_rx(struct igb_adapter *); static void igb_clean_all_tx_rings(struct igb_adapter *); static void igb_clean_all_rx_rings(struct igb_adapter *); static void igb_clean_tx_ring(struct igb_ring *); static void igb_clean_rx_ring(struct igb_ring *); static void igb_set_rx_mode(struct net_device *); static void igb_update_phy_info(struct timer_list *); static void igb_watchdog(struct timer_list *); static void igb_watchdog_task(struct work_struct *); static netdev_tx_t igb_xmit_frame(struct sk_buff *skb, struct net_device *); static void igb_get_stats64(struct net_device *dev struct rtnl_link_stats64 *stats); static int igb_change_mtu(struct net_device *, int); static int igb_set_mac(struct net_device *, void *); static void igb_set_uta(struct igb_adapter *adapter, bool set); static irqreturn_t igb_intr(int irq, void *); static irqreturn_t igb_intr_msi(int irq, void *); static irqreturn_t igb_msix_other(int irq, void *); static irqreturn_t igb_msix_ring(int irq, void *); static void igb_update_dca(struct igb_q_vector *); static void igb_setup_dca(struct igb_adapter *); static int igb_poll(struct napi_struct *, int); static bool igb_clean_tx_irq(struct igb_q_vector *, int); static int igb_clean_rx_irq(struct igb_q_vector *, int); static int igb_ioctl(struct net_device *, struct ifreq *, int cmd); static void igb_tx_timeout(struct net_device *, unsigned int txqueue); static void igb_reset_task(struct work_struct *); static void igb_vlan_mode(struct net_device *netdev netdev_features_t features); static int igb_vlan_rx_add_vid(struct net_device *, __be16, u16); static int igb_vlan_rx_kill_vid(struct net_device *, __be16, u16); static void igb_restore_vlan(struct igb_adapter *); static void igb_rar_set_index(struct igb_adapter *, u32); static void igb_ping_all_vfs(struct igb_adapter *); static void igb_msg_task(struct igb_adapter *); static void igb_vmm_control(struct igb_adapter *); static int igb_set_vf_mac(struct igb_adapter *, int, unsigned char *); static void igb_flush_mac_table(struct igb_adapter *); static int igb_available_rars(struct igb_adapter *, u8); static void igb_set_default_mac_filter(struct igb_adapter *); static int igb_uc_sync(struct net_device *, const unsigned char *); static int igb_uc_unsync(struct net_device *, const unsigned char *); static void igb_restore_vf_multicasts(struct igb_adapter *adapter); static int igb_ndo_set_vf_mac(struct net_device *netdev, int vf, u8 *mac); static int igb_ndo_set_vf_vlan(struct net_device *netdev int vf, u16 vlan, u8 qos, __be16 vlan_proto); static int igb_ndo_set_vf_bw(struct net_device *, int, int, int); static int igb_ndo_set_vf_spoofchk(struct net_device *netdev, int vf bool setting); static int igb_ndo_set_vf_trust(struct net_device *netdev, int vf bool setting); static int igb_ndo_get_vf_config(struct net_device *netdev, int vf struct ifla_vf_info *ivi); static void igb_check_vf_rate_limit(struct igb_adapter *); static void igb_nfc_filter_exit(struct igb_adapter *adapter); static void igb_nfc_filter_restore(struct igb_adapter *adapter); static int igb_vf_configure(struct igb_adapter *adapter, int vf); static int igb_disable_sriov(struct pci_dev *dev, bool reinit); static int igb_suspend(struct device *); static int igb_resume(struct device *); static int igb_runtime_suspend(struct device *dev); static int igb_runtime_resume(struct device *dev); static int igb_runtime_idle(struct device *dev); static void igb_shutdown(struct pci_dev *); static int igb_pci_sriov_configure(struct pci_dev *dev, int num_vfs); static int igb_notify_dca(struct notifier_block *, unsigned long, void *); static pci_ers_result_t igb_io_error_detected(struct pci_dev * pci_channel_state_t); static pci_ers_result_t igb_io_slot_reset(struct pci_dev *); static void igb_io_resume(struct pci_dev *); static void igb_init_dmac(struct igb_adapter *adapter, u32 pba); static void igb_ptp_tx_hwtstamp(struct igb_adapter *adapter); static void igb_ptp_sdp_init(struct igb_adapter *adapter); > > Reviewed-by: Alan Brady <[email protected]> > Signed-off-by: Jesse Brandeburg <[email protected]> > --- > v2: address compilation failure when CONFIG_PM=n, which is then updated > in patch 2/2, fix alignment. > changes in v1 reviewed by Simon Horman > changes in v1 reviewed by Paul Menzel > v1: original net-next posting > --- > drivers/net/ethernet/intel/igb/igb_main.c | 53 ++++++++++------------- > 1 file changed, 24 insertions(+), 29 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c > b/drivers/net/ethernet/intel/igb/igb_main.c > index 518298bbdadc..e749bf5164b8 100644 > --- a/drivers/net/ethernet/intel/igb/igb_main.c > +++ b/drivers/net/ethernet/intel/igb/igb_main.c > @@ -106,8 +106,6 @@ static int igb_setup_all_rx_resources(struct igb_adapter > *); > static void igb_free_all_tx_resources(struct igb_adapter *); > static void igb_free_all_rx_resources(struct igb_adapter *); > static void igb_setup_mrqc(struct igb_adapter *); > -static int igb_probe(struct pci_dev *, const struct pci_device_id *); > -static void igb_remove(struct pci_dev *pdev); > static void igb_init_queue_configuration(struct igb_adapter *adapter); > static int igb_sw_init(struct igb_adapter *); > int igb_open(struct net_device *); > @@ -178,20 +176,6 @@ static int igb_vf_configure(struct igb_adapter *adapter, > int vf); > static int igb_disable_sriov(struct pci_dev *dev, bool reinit); > #endif > > -static int igb_suspend(struct device *); > -static int igb_resume(struct device *); > -static int igb_runtime_suspend(struct device *dev); > -static int igb_runtime_resume(struct device *dev); > -static int igb_runtime_idle(struct device *dev); > -#ifdef CONFIG_PM > -static const struct dev_pm_ops igb_pm_ops = { > - SET_SYSTEM_SLEEP_PM_OPS(igb_suspend, igb_resume) > - SET_RUNTIME_PM_OPS(igb_runtime_suspend, igb_runtime_resume, > - igb_runtime_idle) > -}; > -#endif > -static void igb_shutdown(struct pci_dev *); > -static int igb_pci_sriov_configure(struct pci_dev *dev, int num_vfs); > #ifdef CONFIG_IGB_DCA > static int igb_notify_dca(struct notifier_block *, unsigned long, void *); > static struct notifier_block dca_notifier = { > @@ -219,19 +203,6 @@ static const struct pci_error_handlers igb_err_handler = > { > > static void igb_init_dmac(struct igb_adapter *adapter, u32 pba); > > -static struct pci_driver igb_driver = { > - .name = igb_driver_name, > - .id_table = igb_pci_tbl, > - .probe = igb_probe, > - .remove = igb_remove, > -#ifdef CONFIG_PM > - .driver.pm = &igb_pm_ops, > -#endif > - .shutdown = igb_shutdown, > - .sriov_configure = igb_pci_sriov_configure, > - .err_handler = &igb_err_handler > -}; > - > MODULE_AUTHOR("Intel Corporation, <[email protected]>"); > MODULE_DESCRIPTION("Intel(R) Gigabit Ethernet Network Driver"); > MODULE_LICENSE("GPL v2"); > @@ -647,6 +618,8 @@ struct net_device *igb_get_hw_dev(struct e1000_hw *hw) > return adapter->netdev; > } > > +static struct pci_driver igb_driver; > + > /** > * igb_init_module - Driver Registration Routine > * > @@ -10170,4 +10143,26 @@ static void igb_nfc_filter_restore(struct > igb_adapter *adapter) > > spin_unlock(&adapter->nfc_lock); > } > + > +#ifdef CONFIG_PM > +static const struct dev_pm_ops igb_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(igb_suspend, igb_resume) > + SET_RUNTIME_PM_OPS(igb_runtime_suspend, igb_runtime_resume, > + igb_runtime_idle) > +}; > +#endif > + > +static struct pci_driver igb_driver = { > + .name = igb_driver_name, > + .id_table = igb_pci_tbl, > + .probe = igb_probe, > + .remove = igb_remove, > +#ifdef CONFIG_PM > + .driver.pm = &igb_pm_ops, > +#endif > + .shutdown = igb_shutdown, > + .sriov_configure = igb_pci_sriov_configure, > + .err_handler = &igb_err_handler > +}; > + > /* igb_main.c */ > -- > 2.39.3 > >
