On Tue, Jul 22, 2025 at 02:19:11PM +0100, Simon Horman wrote:

On Mon, Jul 21, 2025 at 07:32:27PM +0800, Dong Yibo wrote:

quoted

Initialize get hw capability from mbx_fw ops.

Signed-off-by: Dong Yibo <dong100@mucse.com>

...

quoted

diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx_fw.h

...

quoted

+struct hw_abilities {
+	u8 link_stat;
+	u8 lane_mask;
+	__le32 speed;
+	__le16 phy_type;
+	__le16 nic_mode;
+	__le16 pfnum;
+	__le32 fw_version;
+	__le32 axi_mhz;
+	union {
+		u8 port_id[4];
+		__le32 port_ids;
+	};
+	__le32 bd_uid;
+	__le32 phy_id;
+	__le32 wol_status;
+	union {
+		__le32 ext_ability;
+		struct {
+			__le32 valid : 1; /* 0 */
+			__le32 wol_en : 1; /* 1 */
+			__le32 pci_preset_runtime_en : 1; /* 2 */
+			__le32 smbus_en : 1; /* 3 */
+			__le32 ncsi_en : 1; /* 4 */
+			__le32 rpu_en : 1; /* 5 */
+			__le32 v2 : 1; /* 6 */
+			__le32 pxe_en : 1; /* 7 */
+			__le32 mctp_en : 1; /* 8 */
+			__le32 yt8614 : 1; /* 9 */
+			__le32 pci_ext_reset : 1; /* 10 */
+			__le32 rpu_availble : 1; /* 11 */
+			__le32 fw_lldp_ability : 1; /* 12 */
+			__le32 lldp_enabled : 1; /* 13 */
+			__le32 only_1g : 1; /* 14 */
+			__le32 force_down_en: 1; /* 15 */
+		} e;

I am not sure how __le32 bitfields work on big endian hosts. Do they?

I would suggest using some combination of BIT/GENMASK,
FIELD_PREP/FIELT_GET, and le32_from_cpu/cpu_from_le32 instead.

Flagged by Sparse.

You are right, '__le32 bitfields' is error. I will fix it. 

quoted

+		struct {
+			u32 valid : 1; /* 0 */
+			u32 wol_en : 1; /* 1 */
+			u32 pci_preset_runtime_en : 1; /* 2 */
+			u32 smbus_en : 1; /* 3 */
+			u32 ncsi_en : 1; /* 4 */
+			u32 rpu_en : 1; /* 5 */
+			u32 v2 : 1; /* 6 */
+			u32 pxe_en : 1; /* 7 */
+			u32 mctp_en : 1; /* 8 */
+			u32 yt8614 : 1; /* 9 */
+			u32 pci_ext_reset : 1; /* 10 */
+			u32 rpu_availble : 1; /* 11 */
+			u32 fw_lldp_ability : 1; /* 12 */
+			u32 lldp_enabled : 1; /* 13 */
+			u32 only_1g : 1; /* 14 */
+			u32 force_down_en: 1; /* 15 */
+		} e_host;
+	};
+} __packed;

...

quoted

+/* req is little endian. bigendian should be conserened */
+struct mbx_fw_cmd_req {

...

quoted

+		struct {
+			__le32 lane;
+			__le32 op;
+			__le32 enable;
+			__le32 inteval;

interval

Flagged by checkpatch.pl --codespell

...

Got it, I will also use checkpatch.pl to check other patches.

quoted

+/* firmware -> driver */
+struct mbx_fw_cmd_reply {
+	/* fw must set: DD, CMP, Error(if error), copy value */
+	__le16 flags;
+	/* from command: LB,RD,VFC,BUF,SI,EI,FE */
+	__le16 opcode; /* 2-3: copy from req */
+	__le16 error_code; /* 4-5: 0 if no error */
+	__le16 datalen; /* 6-7: */
+	union {
+		struct {
+			__le32 cookie_lo; /* 8-11: */
+			__le32 cookie_hi; /* 12-15: */
+		};
+		void *cookie;
+	};
+	/* ===== data ==== [16-64] */
+	union {
+		u8 data[40];
+
+		struct version {
+			__le32 major;
+			__le32 sub;
+			__le32 modify;
+		} version;
+
+		struct {
+			__le32 value[4];
+		} r_reg;
+
+		struct {
+			__le32 new_value;
+		} modify_reg;
+
+		struct get_temp {
+			__le32 temp;
+			__le32 volatage;

voltage

Got it, I will fix this.

quoted

+		} get_temp;
+
+		struct {
+#define MBX_SFP_READ_MAX_CNT 32
+			u8 value[MBX_SFP_READ_MAX_CNT];
+		} sfp_read;
+
+		struct mac_addr {
+			__le32 lanes;
+			struct _addr {
+				/*
+				 * for macaddr:01:02:03:04:05:06
+				 * mac-hi=0x01020304 mac-lo=0x05060000
+				 */
+				u8 mac[8];
+			} addrs[4];
+		} mac_addr;
+
+		struct get_dump_reply {
+			__le32 flags;
+			__le32 version;
+			__le32 bytes;
+			__le32 data[4];
+		} get_dump;
+
+		struct get_lldp_reply {
+			__le32 value;
+			__le32 inteval;

interval

Got it, I will fix this.

quoted

+		} get_lldp;
+
+		struct rnpgbe_eee_cap phy_eee_abilities;
+		struct lane_stat_data lanestat;
+		struct hw_abilities hw_abilities;
+		struct phy_statistics phy_statistics;
+	};
+} __packed;

...

Thanks for your feedback.