On 10/9/2018 10:31 AM, Igor Russkikh wrote:
> From: Pavel Belous <pavel.bel...@aquantia.com>
>
> Start, stop and reset are all done via hw_atl layer.
> Link interrupt configuration is also done here.
>
> Signed-off-by: Igor Russkikh <igor.russk...@aquantia.com>
> Signed-off-by: Pavel Belous <pavel.bel...@aquantia.com>
> Signed-off-by: Pavel Belous <pavel.bel...@aquantia.com>
<...>
> +static void
> +atl_print_adapter_info(struct aq_hw_s *hw __rte_unused)
"hw" is used below, you can drop __rte_unused
<...>
> + /* For secondary processes, the primary process has done all the work */
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> + return 0;
> +
> + rte_eth_copy_pci_info(eth_dev, pci_dev);
This shouldn't be required, done in rte_eth_dev_pci_allocate() just before
calling this function, can you please double check?
<...>
> + /* Copy the permanent MAC address */
> + if (hw->aq_fw_ops->get_mac_permanent(hw,
> + (u8 *)ð_dev->data->mac_addrs[0]) != 0)
You can use "eth_dev->data->mac_addrs->addr_bytes" to prevent casting but both
same eventually
<...>
> +#ifdef RTE_LIBRTE_SECURITY
> + rte_free(eth_dev->security_ctx);
> +#endif
I think "eth_dev->security_ctx" should be allocated in the driver, if you are
not allocating it, you don't need to free it.
<...>
> static void
> -atl_dev_stop(struct rte_eth_dev *dev __rte_unused)
> +atl_dev_stop(struct rte_eth_dev *dev)
> {
> + struct aq_hw_s *hw =
> + ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
> + /* reset the NIC */
> + atl_reset_hw(hw);
> + hw->adapter_stopped = 0;
Just to double check, below in close() there is an "atl_dev_stop(dev);" called,
which look like for stop, should it be called here too?
And the state "hw->adapter_stopped", should it be "0" or "1" when device
stopped?
<...>
> +static int
> +atl_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
> +{
> + struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> + uint32_t fw_ver = 0;
> + unsigned int ret = 0;
> +
> + ret = hw_atl_utils_get_fw_version(hw, &fw_ver);
> + if (ret)
> + return 0;
Why not return error for this case?
