| Message ID | 6ecf8164-2cef-4a5a-9a18-5c1ba2c41f7c@CMEXHTCAS2.ad.emulex.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 2013-04-30 at 17:07 +0530, Somnath Kotur wrote: > From: Kalesh AP <kalesh.purayil@emulex.com> > > Cannot issue any other mailbox command while firmware download is in progress. > The get_drvinfo ethtool hook can be invoked without acquiring any rtnl_lock. [...] Although I documented that as being possible in ethtool.h, that was only because of one particular caller that has since been removed (commit e52ac3398c3d 'net: Use device model to get driver name in skb_gso_segment()'). So unless I missed another case, you could change the documentation. Ben.
From: Ben Hutchings <bhutchings@solarflare.com> Date: Tue, 30 Apr 2013 14:22:11 +0100 > On Tue, 2013-04-30 at 17:07 +0530, Somnath Kotur wrote: >> From: Kalesh AP <kalesh.purayil@emulex.com> >> >> Cannot issue any other mailbox command while firmware download is in progress. >> The get_drvinfo ethtool hook can be invoked without acquiring any rtnl_lock. > [...] > > Although I documented that as being possible in ethtool.h, that was only > because of one particular caller that has since been removed (commit > e52ac3398c3d 'net: Use device model to get driver name in > skb_gso_segment()'). So unless I missed another case, you could change > the documentation. Please address Ben's feedback and resubmit this series, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Although I documented that as being possible in ethtool.h, that was only > because of one particular caller that has since been removed (commit > e52ac3398c3d 'net: Use device model to get driver name in > skb_gso_segment()'). So unless I missed another case, you could change the > documentation. Yes, will do so though I guess with that, it would no longer qualify as a bug-fix ,but would be more of a cleanup(net-next) patch. Thanks Som
> Although I documented that as being possible in ethtool.h, that was only > because of one particular caller that has since been removed (commit > e52ac3398c3d 'net: Use device model to get driver name in > skb_gso_segment()'). So unless I missed another case, you could change the > documentation. > Ben, pls correct me if my understanding is wrong here , but I do see one more case where it might have been missed and that Is in the function - is_cnic_dev() in cnic.c , I believe this was the case that we were trying to work-around with. Thanks Som
On Tue, 2013-05-07 at 06:27 +0000, Kotur, Somnath wrote: > > Although I documented that as being possible in ethtool.h, that was only > > because of one particular caller that has since been removed (commit > > e52ac3398c3d 'net: Use device model to get driver name in > > skb_gso_segment()'). So unless I missed another case, you could change the > > documentation. > > > Ben, pls correct me if my understanding is wrong here , but I do see > one more case where it might have been missed and that > Is in the function - is_cnic_dev() in cnic.c , I believe this was the > case that we were trying to work-around with. It's called from a netdev notifier, which holds the RTNL lock. Ben.
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h index 941aa1f..362efec 100644 --- a/drivers/net/ethernet/emulex/benet/be.h +++ b/drivers/net/ethernet/emulex/benet/be.h @@ -395,6 +395,7 @@ struct be_adapter { u32 cmd_privileges; /* Ethtool knobs and info */ char fw_ver[FW_VER_LEN]; + char fw_on_flash[FW_VER_LEN]; int if_handle; /* Used to configure filtering */ u32 *pmac_id; /* MAC addr handle used by BE card */ u32 beacon_state; /* for set_phys_id */ diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c index 76b302f..abe471f 100644 --- a/drivers/net/ethernet/emulex/benet/be_ethtool.c +++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c @@ -176,19 +176,15 @@ static void be_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo) { struct be_adapter *adapter = netdev_priv(netdev); - char fw_on_flash[FW_VER_LEN]; - - memset(fw_on_flash, 0 , sizeof(fw_on_flash)); - be_cmd_get_fw_ver(adapter, adapter->fw_ver, fw_on_flash); strlcpy(drvinfo->driver, DRV_NAME, sizeof(drvinfo->driver)); strlcpy(drvinfo->version, DRV_VER, sizeof(drvinfo->version)); - if (!memcmp(adapter->fw_ver, fw_on_flash, FW_VER_LEN)) + if (!memcmp(adapter->fw_ver, adapter->fw_on_flash, FW_VER_LEN)) strlcpy(drvinfo->fw_version, adapter->fw_ver, sizeof(drvinfo->fw_version)); else snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version), - "%s [%s]", adapter->fw_ver, fw_on_flash); + "%s [%s]", adapter->fw_ver, adapter->fw_on_flash); strlcpy(drvinfo->bus_info, pci_name(adapter->pdev), sizeof(drvinfo->bus_info)); diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c index 1232e91..5544354 100644 --- a/drivers/net/ethernet/emulex/benet/be_main.c +++ b/drivers/net/ethernet/emulex/benet/be_main.c @@ -3138,7 +3138,7 @@ static int be_setup(struct be_adapter *adapter) if (status) goto err; - be_cmd_get_fw_ver(adapter, adapter->fw_ver, NULL); + be_cmd_get_fw_ver(adapter, adapter->fw_ver, adapter->fw_on_flash); if (adapter->vlans_added) be_vid_config(adapter); @@ -3717,6 +3717,10 @@ int be_load_fw(struct be_adapter *adapter, u8 *fw_file) else status = be_fw_download(adapter, fw); + if (!status) + be_cmd_get_fw_ver(adapter, adapter->fw_ver, + adapter->fw_on_flash); + fw_exit: release_firmware(fw); return status;