Re: [PATCH net 01/20] net/hinic: Initialize hw interface
Hi Andrew,
Now is the merge window and we need to resubmit.
We will fix it when we will resubmit.
The version number was used for the ethtool information and module version and
will be removed.
devm_kzalloc - will be used for the allocation of the memory at initialization.
We used pr_ for messages that point on errors or information in module,
and dev_ for errors in device and netif_ for errors in network device
operations.
netif_ will be used for network device ops and dev_ for the others.
Aviad
On 7/12/2017 6:29 PM, Andrew Lunn wrote:
>> +
>> +#define HINIC_DRV_NAME "HiNIC"
>> +#define HINIC_DRV_VERSION "1.0"
>
> Hi Aviad
>
> Please don't add a driver version. There was a discussion about this
> recently, how pointless it is.
>
>> +/**
>> + * hinic_init_hwdev - Initialize the NIC HW
>> + * @hwdev: the NIC HW device that is returned from the initialization
>> + * @pdev: the NIC pci device
>> + *
>> + * Return 0 - Success, negative - Failure
>> + *
>> + * Initialize the NIC HW device and return a pointer to it in the first arg
>> + **/
>> +int hinic_init_hwdev(struct hinic_hwdev **hwdev, struct pci_dev *pdev)
>> +{
>> +struct hinic_pfhwdev *pfhwdev;
>> +struct hinic_hwif *hwif;
>> +int err;
>> +
>> +hwif = kzalloc(sizeof(*hwif), GFP_KERNEL);
>
> Using the devm_ functions makes your cleanup code simpler when
> handling memory.
>
>> +/**
>> + * nic_dev_init - Initialize the NIC device
>> + * @pdev: the NIC pci device
>> + *
>> + * Return 0 - Success, negative - Failure
>> + **/
>> +static int nic_dev_init(struct pci_dev *pdev)
>> +{
>> +struct hinic_dev *nic_dev;
>> +struct net_device *netdev;
>> +struct hinic_hwdev *hwdev;
>> +int err, num_qps;
>> +
>> +err = hinic_init_hwdev(&hwdev, pdev);
>> +if (err) {
>> +dev_err(&pdev->dev, "Failed to initialize HW device\n");
>> +return err;
>> +}
>> +
>> +num_qps = hinic_hwdev_num_qps(hwdev);
>> +if (num_qps <= 0) {
>> +dev_err(&pdev->dev, "Invalid number of QPS\n");
>> +err = -EINVAL;
>> +goto num_qps_err;
>> +}
>> +
>> +netdev = alloc_etherdev_mq(sizeof(*nic_dev), num_qps);
>> +if (!netdev) {
>> +pr_err("Failed to allocate Ethernet device\n");
>
> Above you used dev_err, here you used pr_err(). Please be consistent.
>
>> +err = -ENOMEM;
>> +goto alloc_etherdev_err;
>> +}
>> +
>> +netdev->netdev_ops = &hinic_netdev_ops;
>> +
>> +nic_dev = (struct hinic_dev *)netdev_priv(netdev);
>> +nic_dev->hwdev = hwdev;
>> +nic_dev->netdev = netdev;
>> +nic_dev->msg_enable = MSG_ENABLE_DEFAULT;
>> +
>> +pci_set_drvdata(pdev, netdev);
>> +
>> +netif_carrier_off(netdev);
>> +
>> +err = register_netdev(netdev);
>> +if (err) {
>> +netif_err(nic_dev, probe, netdev, "Failed to register
>> netdev\n");
>
> probably not a good idea to use netif_err, if register_netdev just
> failed. dev_err() would be better.
>
>
> .
>
Re: [PATCH net 01/20] net/hinic: Initialize hw interface
> +
> +#define HINIC_DRV_NAME "HiNIC"
> +#define HINIC_DRV_VERSION"1.0"
Hi Aviad
Please don't add a driver version. There was a discussion about this
recently, how pointless it is.
> +/**
> + * hinic_init_hwdev - Initialize the NIC HW
> + * @hwdev: the NIC HW device that is returned from the initialization
> + * @pdev: the NIC pci device
> + *
> + * Return 0 - Success, negative - Failure
> + *
> + * Initialize the NIC HW device and return a pointer to it in the first arg
> + **/
> +int hinic_init_hwdev(struct hinic_hwdev **hwdev, struct pci_dev *pdev)
> +{
> + struct hinic_pfhwdev *pfhwdev;
> + struct hinic_hwif *hwif;
> + int err;
> +
> + hwif = kzalloc(sizeof(*hwif), GFP_KERNEL);
Using the devm_ functions makes your cleanup code simpler when
handling memory.
> +/**
> + * nic_dev_init - Initialize the NIC device
> + * @pdev: the NIC pci device
> + *
> + * Return 0 - Success, negative - Failure
> + **/
> +static int nic_dev_init(struct pci_dev *pdev)
> +{
> + struct hinic_dev *nic_dev;
> + struct net_device *netdev;
> + struct hinic_hwdev *hwdev;
> + int err, num_qps;
> +
> + err = hinic_init_hwdev(&hwdev, pdev);
> + if (err) {
> + dev_err(&pdev->dev, "Failed to initialize HW device\n");
> + return err;
> + }
> +
> + num_qps = hinic_hwdev_num_qps(hwdev);
> + if (num_qps <= 0) {
> + dev_err(&pdev->dev, "Invalid number of QPS\n");
> + err = -EINVAL;
> + goto num_qps_err;
> + }
> +
> + netdev = alloc_etherdev_mq(sizeof(*nic_dev), num_qps);
> + if (!netdev) {
> + pr_err("Failed to allocate Ethernet device\n");
Above you used dev_err, here you used pr_err(). Please be consistent.
> + err = -ENOMEM;
> + goto alloc_etherdev_err;
> + }
> +
> + netdev->netdev_ops = &hinic_netdev_ops;
> +
> + nic_dev = (struct hinic_dev *)netdev_priv(netdev);
> + nic_dev->hwdev = hwdev;
> + nic_dev->netdev = netdev;
> + nic_dev->msg_enable = MSG_ENABLE_DEFAULT;
> +
> + pci_set_drvdata(pdev, netdev);
> +
> + netif_carrier_off(netdev);
> +
> + err = register_netdev(netdev);
> + if (err) {
> + netif_err(nic_dev, probe, netdev, "Failed to register
> netdev\n");
probably not a good idea to use netif_err, if register_netdev just
failed. dev_err() would be better.
[PATCH net 01/20] net/hinic: Initialize hw interface
Initialize hw interface as part of the nic initialization for accessing hw. Signed-off-by: Aviad Krawczyk Signed-off-by: Zhaochen --- Documentation/networking/hinic.txt | 125 drivers/net/ethernet/Kconfig | 1 + drivers/net/ethernet/Makefile | 1 + drivers/net/ethernet/huawei/Kconfig| 19 ++ drivers/net/ethernet/huawei/Makefile | 5 + drivers/net/ethernet/huawei/hinic/Kconfig | 13 ++ drivers/net/ethernet/huawei/hinic/Makefile | 3 + drivers/net/ethernet/huawei/hinic/hinic_dev.h | 34 drivers/net/ethernet/huawei/hinic/hinic_hw_csr.h | 36 drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c | 220 + drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h | 42 drivers/net/ethernet/huawei/hinic/hinic_hw_if.c| 208 +++ drivers/net/ethernet/huawei/hinic/hinic_hw_if.h| 160 +++ drivers/net/ethernet/huawei/hinic/hinic_main.c | 212 .../net/ethernet/huawei/hinic/hinic_pci_id_tbl.h | 27 +++ 15 files changed, 1106 insertions(+) create mode 100644 Documentation/networking/hinic.txt create mode 100644 drivers/net/ethernet/huawei/Kconfig create mode 100644 drivers/net/ethernet/huawei/Makefile create mode 100644 drivers/net/ethernet/huawei/hinic/Kconfig create mode 100644 drivers/net/ethernet/huawei/hinic/Makefile create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_dev.h create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_hw_csr.h create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_hw_if.c create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_hw_if.h create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_main.c create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_pci_id_tbl.h diff --git a/Documentation/networking/hinic.txt b/Documentation/networking/hinic.txt new file mode 100644 index 000..c826660 --- /dev/null +++ b/Documentation/networking/hinic.txt @@ -0,0 +1,125 @@ +Linux Kernel Driver for Huawei Intelligent NIC(HiNIC) family + + +Overview: += +HiNIC is a network interface card for the Data Center Area. + +The driver supports a range of link-speed devices (10GbE, 25GbE, 40GbE, etc.). +The driver supports also a negotiated and extendable feature set. + +Some HiNIC devices support SR-IOV. This driver is used for Physical Function +(PF). + +HiNIC devices support MSI-X interrupt vector for each Tx/Rx queue and +adaptive interrupt moderation. + +HiNIC devices support also various offload features such as checksum offload, +TCP Transmit Segmentation Offload(TSO), Receive-Side Scaling(RSS) and +LRO(Large Receive Offload). + + +Supported PCI vendor ID/device IDs: +=== + +19e5:1822 - HiNIC PF + + +Driver Architecture and Source Code: + + +hinic_dev - Implement a Logical Network device that is independent from +specific HW details about HW data structure formats. + +hinic_hwdev - Implement the HW details of the device and include the components +for accessing the PCI NIC. + +hinic_hwdev contains the following components: +=== + +HW Interface: += + +The interface for accessing the pci device (DMA memory and PCI BARs). +(hinic_hw_if.c, hinic_hw_if.h) + +Configuration Status Registers Area that describes the HW Registers on the +configuration and status BAR0. (hinic_hw_csr.h) + +MGMT components: + + +Asynchronous Event Queues(AEQs) - The event queues for receiving messages from +the MGMT modules on the cards. (hinic_hw_eqs.c, hinic_hw_eqs.h) + +Application Programmable Interface commands(API CMD) - Interface for sending +MGMT commands to the card. (hinic_hw_api_cmd.c, hinic_hw_api_cmd.h) + +Management (MGMT) - the PF to MGMT channel that uses API CMD for sending MGMT +commands to the card and receives notifications from the MGMT modules on the +card by AEQs. Also set the addresses of the IO CMDQs in HW. +(hinic_hw_mgmt.c, hinic_hw_mgmt.h) + +IO components: +== + +Completion Event Queues(CEQs) - The completion Event Queues that describe IO +tasks that are finished. (hinic_hw_eqs.c, hinic_hw_eqs.h) + +Work Queues(WQ) - Contain the memory and operations for use by CMD queues and +the Queue Pairs. The WQ is a Memory Block in a Page. The Block contains +pointers to Memory Areas that are the Memory for the Work Queue Elements(WQEs). +(hinic_hw_wq.c, hinic_hw_wq.h) + +Command Queues(CMDQ) - The queues for sending commands for IO management and is +used to set the QPs addresses in HW. The commands completion events are +accumulated on the CEQ that is configured to receive the CMDQ completion events
