quoted

Auxiliary driver registration.
Net device_ops registration but open/stop are empty stubs.
tx/rx logic.

Take care for spelling: Tx/Rx; HW (just below).

Please elaborate. Spelling of what?

quoted

All major data structures of the driver are fully introduced with the
code that uses them but without their initialization code that requires
management interface with the hw.

Submitted-by: Gur Stavi [off-list ref]

this tag is not needed

quoted

Signed-off-by: Gur Stavi <redacted>

your tag should be last

quoted

Signed-off-by: Xin Guo <redacted>

no idea what Xin did, if you want to give credit to someone
you could use Co-developed-by: tag (put it just above the SoB)

quoted

Signed-off-by: gongfan <gongfan1@huawei.com>

It would be much appricieated if you will spell the full name
for gongfan, at least two parts (like you did for the rest),
especially for someone you put into the MAINTAINERS file.
OTOH, please keep in mind that maintainer should be active
on the mailing list (regarding this driver).

Ack

quoted

+
+Global function ID
+------------------
+
+Every function, PF or VF, has a unique ordinal identification within the device.
+Many commands to management (mbox or cmdq) contain this ID so HW can apply the

s/commands to management/management commands/

Ack

quoted

+L:	netdev@vger.kernel.org
+S:	Supported

there is a (relatively new) requirement that the device needs to report
to the netdev CI to have an "S" here, so for now you should use "M"

Ack

quoted

+	# Fields of HW and management structures are little endian and are
+	# currently not converted
+	depends on !CPU_BIG_ENDIAN
+	depends on X86 || ARM64 || COMPILE_TEST
+	depends on PCI_MSI && 64BIT
+	select AUXILIARY_BUS
+	help
+	  This driver supports HiNIC PCIE Ethernet cards.
+	  To compile this driver as part of the kernel, choose Y here.
+	  If unsure, choose N.
+	  The default is N.

the last 3 lines are very much obvious/redundant

Ack

quoted

diff --git a/drivers/net/ethernet/huawei/hinic3/Makefile b/drivers/net/ethernet/huawei/hinic3/Makefile
new file mode 100644
index 000000000000..02656853f629
--- /dev/null
+++ b/drivers/net/ethernet/huawei/hinic3/Makefile

@@ -0,0 +1,21 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) Huawei Technologies Co., Ltd. 2024. All rights reserved.

too late

Ack

quoted

+
+obj-$(CONFIG_HINIC3) += hinic3.o
+
+hinic3-objs := hinic3_hwdev.o \
+	       hinic3_lld.o \
+	       hinic3_common.o \
+	       hinic3_hwif.o \
+	       hinic3_hw_cfg.o \
+	       hinic3_queue_common.o \
+	       hinic3_mbox.o \
+	       hinic3_hw_comm.o \
+	       hinic3_wq.o \
+	       hinic3_nic_io.o \
+	       hinic3_nic_cfg.o \
+	       hinic3_tx.o \
+	       hinic3_rx.o \
+	       hinic3_netdev_ops.o \
+	       hinic3_rss.o \
+	       hinic3_main.o

in general, any list of random things could be sorted (to ease out git
merges in the future)

Ack

quoted

+	(((u32)(hwdev)->cfg_mgmt->svc_cap.chip_svc_type) & BIT(HINIC3_SERVICE_T_NIC))
+
+bool hinic3_support_nic(struct hinic3_hwdev *hwdev)
+{
+	if (!IS_NIC_TYPE(hwdev))
+		return false;
+
+	return true;

this is just:
	return IS_NIC_TYPE(hwdev);

Ack

quoted

+static int comm_msg_to_mgmt_sync(struct hinic3_hwdev *hwdev, u16 cmd, const void *buf_in,
+				 u32 in_size, void *buf_out, u32 *out_size)
+{
+	return hinic3_send_mbox_to_mgmt(hwdev, HINIC3_MOD_COMM, cmd, buf_in,
+					in_size, buf_out, out_size, 0);
+}
+
+int hinic3_func_reset(struct hinic3_hwdev *hwdev, u16 func_id, u64 reset_flag)
+{
+	struct comm_cmd_func_reset func_reset;

  = {}

quoted

+	u32 out_size = sizeof(func_reset);
+	int err;
+
+	memset(&func_reset, 0, sizeof(func_reset));

with "= {}" above, memset() could be eliminated

quoted

+	func_reset.func_id = func_id;
+	func_reset.reset_flag = reset_flag;

alternative would be
	struct comm_cmd_func_reset func_reset = {
		.func_id = func_id,
		.reset_flag = reset_flag,
	};

Ack

quoted

+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_hw_intf.h

@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) Huawei Technologies Co., Ltd. 2024. All rights reserved. */
+
+#ifndef HINIC3_HW_INTF_H

please add two underscores to the header guards

Ack

quoted

+#define HINIC3_HW_INTF_H