All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] net: marvell: prestera: fix hw structure laid out
@ 2021-11-03  9:54 Volodymyr Mytnyk
  2021-11-03 10:30 ` Denis Kirjanov
  2021-11-04  9:12 ` Geert Uytterhoeven
  0 siblings, 2 replies; 4+ messages in thread
From: Volodymyr Mytnyk @ 2021-11-03  9:54 UTC (permalink / raw)
  To: kuba, andrew
  Cc: mickeyr, serhiy.pshyk, taras.chornyi, Volodymyr Mytnyk,
	Taras Chornyi, David S. Miller, Vadym Kochan, Yevhen Orlov,
	netdev, linux-kernel

From: Volodymyr Mytnyk <vmytnyk@marvell.com>

- fix structure laid out discussed in:
    [PATCH net-next v4] net: marvell: prestera: add firmware v4.0 support
    https://www.spinics.net/lists/kernel/msg4127689.html

- fix review comments discussed in:
    [PATCH] [-next] net: marvell: prestera: Add explicit padding
    https://www.spinics.net/lists/kernel/msg4130293.html

- fix patchwork issues
- rebase on net master

Reported-by: kernel test robot <lkp@intel.com>
Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
Signed-off-by: Volodymyr Mytnyk <vmytnyk@marvell.com>
---
 .../ethernet/marvell/prestera/prestera_ethtool.c   |   3 +-
 .../net/ethernet/marvell/prestera/prestera_hw.c    | 129 +++++++++++----------
 .../net/ethernet/marvell/prestera/prestera_main.c  |   6 +-
 .../net/ethernet/marvell/prestera/prestera_pci.c   |   3 +-
 4 files changed, 75 insertions(+), 66 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
index 6011454dba71..40d5b89573bb 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
@@ -499,7 +499,8 @@ static void prestera_port_mdix_get(struct ethtool_link_ksettings *ecmd,
 {
 	struct prestera_port_phy_state *state = &port->state_phy;
 
-	if (prestera_hw_port_phy_mode_get(port, &state->mdix, NULL, NULL, NULL)) {
+	if (prestera_hw_port_phy_mode_get(port,
+					  &state->mdix, NULL, NULL, NULL)) {
 		netdev_warn(port->dev, "MDIX params get failed");
 		state->mdix = ETH_TP_MDI_INVALID;
 	}
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
index 4f5f52dcdd9d..fb0f17c9352f 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
@@ -180,109 +180,113 @@ struct prestera_msg_common_resp {
 	struct prestera_msg_ret ret;
 };
 
-union prestera_msg_switch_param {
-	u8 mac[ETH_ALEN];
-	__le32 ageing_timeout_ms;
-} __packed;
-
 struct prestera_msg_switch_attr_req {
 	struct prestera_msg_cmd cmd;
 	__le32 attr;
-	union prestera_msg_switch_param param;
-	u8 pad[2];
+	union {
+		__le32 ageing_timeout_ms;
+		struct {
+			u8 mac[ETH_ALEN];
+			u8 __pad[2];
+		};
+	} param;
 };
 
 struct prestera_msg_switch_init_resp {
 	struct prestera_msg_ret ret;
 	__le32 port_count;
 	__le32 mtu_max;
-	u8  switch_id;
-	u8  lag_max;
-	u8  lag_member_max;
 	__le32 size_tbl_router_nexthop;
-} __packed __aligned(4);
+	u8 switch_id;
+	u8 lag_max;
+	u8 lag_member_max;
+};
 
 struct prestera_msg_event_port_param {
 	union {
 		struct {
-			u8 oper;
 			__le32 mode;
 			__le32 speed;
+			u8 oper;
 			u8 duplex;
 			u8 fc;
 			u8 fec;
-		} __packed mac;
+		} mac;
 		struct {
-			u8 mdix;
 			__le64 lmode_bmap;
+			u8 mdix;
 			u8 fc;
+			u8 __pad[2];
 		} __packed phy;
 	} __packed;
-} __packed __aligned(4);
+} __packed;
 
 struct prestera_msg_port_cap_param {
 	__le64 link_mode;
-	u8  type;
-	u8  fec;
-	u8  fc;
-	u8  transceiver;
-};
+	u8 type;
+	u8 fec;
+	u8 fc;
+	u8 transceiver;
+} __packed;
 
 struct prestera_msg_port_flood_param {
 	u8 type;
 	u8 enable;
-};
+	u8 __pad[2];
+} __packed;
 
 union prestera_msg_port_param {
+	__le32 mtu;
+	__le32 speed;
+	__le32 link_mode;
 	u8 admin_state;
 	u8 oper_state;
-	__le32 mtu;
 	u8 mac[ETH_ALEN];
 	u8 accept_frm_type;
-	__le32 speed;
 	u8 learning;
 	u8 flood;
-	__le32 link_mode;
 	u8 type;
 	u8 duplex;
 	u8 fec;
 	u8 fc;
-
 	union {
 		struct {
-			u8 admin:1;
+			u8 admin;
 			u8 fc;
 			u8 ap_enable;
+			u8 __reserved;
 			union {
 				struct {
 					__le32 mode;
-					u8  inband:1;
 					__le32 speed;
-					u8  duplex;
-					u8  fec;
-					u8  fec_supp;
-				} __packed reg_mode;
+					u8 inband;
+					u8 duplex;
+					u8 fec;
+					u8 fec_supp;
+				} reg_mode;
 				struct {
 					__le32 mode;
 					__le32 speed;
-					u8  fec;
-					u8  fec_supp;
-				} __packed ap_modes[PRESTERA_AP_PORT_MAX];
-			} __packed;
-		} __packed mac;
+					u8 fec;
+					u8 fec_supp;
+					u8 __pad[2];
+				} ap_modes[PRESTERA_AP_PORT_MAX];
+			};
+		} mac;
 		struct {
-			u8 admin:1;
-			u8 adv_enable;
 			__le64 modes;
 			__le32 mode;
+			u8 admin;
+			u8 adv_enable;
 			u8 mdix;
-		} __packed phy;
+			u8 __pad;
+		} phy;
 	} __packed link;
 
 	struct prestera_msg_port_cap_param cap;
 	struct prestera_msg_port_flood_param flood_ext;
 	struct prestera_msg_event_port_param link_evt;
-} __packed;
+};
 
 struct prestera_msg_port_attr_req {
 	struct prestera_msg_cmd cmd;
@@ -290,14 +294,12 @@ struct prestera_msg_port_attr_req {
 	__le32 port;
 	__le32 dev;
 	union prestera_msg_port_param param;
-} __packed __aligned(4);
-
+};
 
 struct prestera_msg_port_attr_resp {
 	struct prestera_msg_ret ret;
 	union prestera_msg_port_param param;
-} __packed __aligned(4);
-
+};
 
 struct prestera_msg_port_stats_resp {
 	struct prestera_msg_ret ret;
@@ -322,13 +324,13 @@ struct prestera_msg_vlan_req {
 	__le32 port;
 	__le32 dev;
 	__le16 vid;
-	u8  is_member;
-	u8  is_tagged;
+	u8 is_member;
+	u8 is_tagged;
 };
 
 struct prestera_msg_fdb_req {
 	struct prestera_msg_cmd cmd;
-	u8 dest_type;
+	__le32 flush_mode;
 	union {
 		struct {
 			__le32 port;
@@ -336,11 +338,12 @@ struct prestera_msg_fdb_req {
 		};
 		__le16 lag_id;
 	} dest;
-	u8  mac[ETH_ALEN];
 	__le16 vid;
-	u8  dynamic;
-	__le32 flush_mode;
-} __packed __aligned(4);
+	u8 dest_type;
+	u8 dynamic;
+	u8 mac[ETH_ALEN];
+	u8 __pad[2];
+};
 
 struct prestera_msg_bridge_req {
 	struct prestera_msg_cmd cmd;
@@ -383,7 +386,7 @@ struct prestera_msg_acl_match {
 		struct {
 			u8 key[ETH_ALEN];
 			u8 mask[ETH_ALEN];
-		} __packed mac;
+		} mac;
 	} keymask;
 };
 
@@ -446,7 +449,8 @@ struct prestera_msg_stp_req {
 	__le32 port;
 	__le32 dev;
 	__le16 vid;
-	u8  state;
+	u8 state;
+	u8 __pad;
 };
 
 struct prestera_msg_rxtx_req {
@@ -497,21 +501,21 @@ union prestera_msg_event_fdb_param {
 
 struct prestera_msg_event_fdb {
 	struct prestera_msg_event id;
-	u8 dest_type;
+	__le32 vid;
 	union {
 		__le32 port_id;
 		__le16 lag_id;
 	} dest;
-	__le32 vid;
 	union prestera_msg_event_fdb_param param;
-} __packed __aligned(4);
+	u8 dest_type;
+};
 
-static inline void prestera_hw_build_tests(void)
+static void prestera_hw_build_tests(void)
 {
 	/* check requests */
 	BUILD_BUG_ON(sizeof(struct prestera_msg_common_req) != 4);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_switch_attr_req) != 16);
-	BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_req) != 120);
+	BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_req) != 140);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_port_info_req) != 8);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_vlan_req) != 16);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_fdb_req) != 28);
@@ -528,7 +532,7 @@ static inline void prestera_hw_build_tests(void)
 	/* check responses */
 	BUILD_BUG_ON(sizeof(struct prestera_msg_common_resp) != 8);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_switch_init_resp) != 24);
-	BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_resp) != 112);
+	BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_resp) != 132);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_port_stats_resp) != 248);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_port_info_resp) != 20);
 	BUILD_BUG_ON(sizeof(struct prestera_msg_bridge_resp) != 12);
@@ -561,9 +565,9 @@ static int __prestera_cmd_ret(struct prestera_switch *sw,
 	if (err)
 		return err;
 
-	if (__le32_to_cpu(ret->cmd.type) != PRESTERA_CMD_TYPE_ACK)
+	if (ret->cmd.type != __cpu_to_le32(PRESTERA_CMD_TYPE_ACK))
 		return -EBADE;
-	if (__le32_to_cpu(ret->status) != PRESTERA_CMD_ACK_OK)
+	if (ret->status != __cpu_to_le32(PRESTERA_CMD_ACK_OK))
 		return -EINVAL;
 
 	return 0;
@@ -1356,7 +1360,8 @@ int prestera_hw_port_speed_get(const struct prestera_port *port, u32 *speed)
 int prestera_hw_port_autoneg_restart(struct prestera_port *port)
 {
 	struct prestera_msg_port_attr_req req = {
-		.attr = __cpu_to_le32(PRESTERA_CMD_PORT_ATTR_PHY_AUTONEG_RESTART),
+		.attr =
+		    __cpu_to_le32(PRESTERA_CMD_PORT_ATTR_PHY_AUTONEG_RESTART),
 		.port = __cpu_to_le32(port->hw_id),
 		.dev = __cpu_to_le32(port->dev_id),
 	};
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index 625b40149fac..4369a3ffad45 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
@@ -405,7 +405,8 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id)
 
 	err = prestera_port_cfg_mac_write(port, &cfg_mac);
 	if (err) {
-		dev_err(prestera_dev(sw), "Failed to set port(%u) mac mode\n", id);
+		dev_err(prestera_dev(sw),
+			"Failed to set port(%u) mac mode\n", id);
 		goto err_port_init;
 	}
 
@@ -418,7 +419,8 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id)
 						    false, 0, 0,
 						    port->cfg_phy.mdix);
 		if (err) {
-			dev_err(prestera_dev(sw), "Failed to set port(%u) phy mode\n", id);
+			dev_err(prestera_dev(sw),
+				"Failed to set port(%u) phy mode\n", id);
 			goto err_port_init;
 		}
 	}
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
index 5d4d410b07c8..461259b3655a 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
@@ -411,7 +411,8 @@ static int prestera_fw_cmd_send(struct prestera_fw *fw, int qid,
 		goto cmd_exit;
 	}
 
-	memcpy_fromio(out_msg, prestera_fw_cmdq_buf(fw, qid) + in_size, ret_size);
+	memcpy_fromio(out_msg,
+		      prestera_fw_cmdq_buf(fw, qid) + in_size, ret_size);
 
 cmd_exit:
 	prestera_fw_write(fw, PRESTERA_CMDQ_REQ_CTL_REG(qid),
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH net v2] net: marvell: prestera: fix hw structure laid out
  2021-11-03  9:54 [PATCH net v2] net: marvell: prestera: fix hw structure laid out Volodymyr Mytnyk
@ 2021-11-03 10:30 ` Denis Kirjanov
  2021-11-04  9:12 ` Geert Uytterhoeven
  1 sibling, 0 replies; 4+ messages in thread
From: Denis Kirjanov @ 2021-11-03 10:30 UTC (permalink / raw)
  To: Volodymyr Mytnyk, kuba, andrew
  Cc: mickeyr, serhiy.pshyk, taras.chornyi, Volodymyr Mytnyk,
	Taras Chornyi, David S. Miller, Vadym Kochan, Yevhen Orlov,
	netdev, linux-kernel



11/3/21 12:54 PM, Volodymyr Mytnyk пишет:
> From: Volodymyr Mytnyk <vmytnyk@marvell.com>
> 
> - fix structure laid out discussed in:
>      [PATCH net-next v4] net: marvell: prestera: add firmware v4.0 support
>      https://www.spinics.net/lists/kernel/msg4127689.html
> 
> - fix review comments discussed in:
>      [PATCH] [-next] net: marvell: prestera: Add explicit padding
>      https://www.spinics.net/lists/kernel/msg4130293.html
> 
> - fix patchwork issues
> - rebase on net master
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
> Signed-off-by: Volodymyr Mytnyk <vmytnyk@marvell.com>
> ---
>   .../ethernet/marvell/prestera/prestera_ethtool.c   |   3 +-
>   .../net/ethernet/marvell/prestera/prestera_hw.c    | 129 +++++++++++----------
>   .../net/ethernet/marvell/prestera/prestera_main.c  |   6 +-
>   .../net/ethernet/marvell/prestera/prestera_pci.c   |   3 +-
>   4 files changed, 75 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
> index 6011454dba71..40d5b89573bb 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
> @@ -499,7 +499,8 @@ static void prestera_port_mdix_get(struct ethtool_link_ksettings *ecmd,
>   {
>   	struct prestera_port_phy_state *state = &port->state_phy;
>   
> -	if (prestera_hw_port_phy_mode_get(port, &state->mdix, NULL, NULL, NULL)) {
> +	if (prestera_hw_port_phy_mode_get(port,
> +					  &state->mdix, NULL, NULL, NULL)) {
>   		netdev_warn(port->dev, "MDIX params get failed");
>   		state->mdix = ETH_TP_MDI_INVALID;
>   	}
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> index 4f5f52dcdd9d..fb0f17c9352f 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> @@ -180,109 +180,113 @@ struct prestera_msg_common_resp {
>   	struct prestera_msg_ret ret;
>   };
>   
> -union prestera_msg_switch_param {
> -	u8 mac[ETH_ALEN];
> -	__le32 ageing_timeout_ms;
> -} __packed;
> -
>   struct prestera_msg_switch_attr_req {
>   	struct prestera_msg_cmd cmd;
>   	__le32 attr;
> -	union prestera_msg_switch_param param;
> -	u8 pad[2];
> +	union {
> +		__le32 ageing_timeout_ms;
> +		struct {
> +			u8 mac[ETH_ALEN];
> +			u8 __pad[2];
> +		};
> +	} param;
>   };
>   
>   struct prestera_msg_switch_init_resp {
>   	struct prestera_msg_ret ret;
>   	__le32 port_count;
>   	__le32 mtu_max;
> -	u8  switch_id;
> -	u8  lag_max;
> -	u8  lag_member_max;
>   	__le32 size_tbl_router_nexthop;
> -} __packed __aligned(4);
> +	u8 switch_id;
> +	u8 lag_max;
> +	u8 lag_member_max;
> +};
>   
>   struct prestera_msg_event_port_param {
>   	union {
>   		struct {
> -			u8 oper;
>   			__le32 mode;
>   			__le32 speed;
> +			u8 oper;
>   			u8 duplex;
>   			u8 fc;
>   			u8 fec;
> -		} __packed mac;
> +		} mac;
>   		struct {
> -			u8 mdix;
>   			__le64 lmode_bmap;
> +			u8 mdix;
>   			u8 fc;
> +			u8 __pad[2];
>   		} __packed phy;
>   	} __packed;
> -} __packed __aligned(4);
> +} __packed;
>   
>   struct prestera_msg_port_cap_param {
>   	__le64 link_mode;
> -	u8  type;
> -	u8  fec;
> -	u8  fc;
> -	u8  transceiver;
> -};
> +	u8 type;
> +	u8 fec;
> +	u8 fc;
> +	u8 transceiver;
> +} __packed;
>   
>   struct prestera_msg_port_flood_param {
>   	u8 type;
>   	u8 enable;
> -};
> +	u8 __pad[2];
> +} __packed;
>   
>   union prestera_msg_port_param {
> +	__le32 mtu;
> +	__le32 speed;
> +	__le32 link_mode;
>   	u8 admin_state;
>   	u8 oper_state;
> -	__le32 mtu;
>   	u8 mac[ETH_ALEN];
>   	u8 accept_frm_type;
> -	__le32 speed;
>   	u8 learning;
>   	u8 flood;
> -	__le32 link_mode;
>   	u8 type;
>   	u8 duplex;
>   	u8 fec;
>   	u8 fc;
> -
>   	union {
>   		struct {
> -			u8 admin:1;
> +			u8 admin;
>   			u8 fc;
>   			u8 ap_enable;
> +			u8 __reserved;
>   			union {
>   				struct {
>   					__le32 mode;
> -					u8  inband:1;
>   					__le32 speed;
> -					u8  duplex;
> -					u8  fec;
> -					u8  fec_supp;
> -				} __packed reg_mode;
> +					u8 inband;
> +					u8 duplex;
> +					u8 fec;
> +					u8 fec_supp;
> +				} reg_mode;
>   				struct {
>   					__le32 mode;
>   					__le32 speed;
> -					u8  fec;
> -					u8  fec_supp;
> -				} __packed ap_modes[PRESTERA_AP_PORT_MAX];
> -			} __packed;
> -		} __packed mac;
> +					u8 fec;
> +					u8 fec_supp;
> +					u8 __pad[2];
> +				} ap_modes[PRESTERA_AP_PORT_MAX];
> +			};
> +		} mac;
>   		struct {
> -			u8 admin:1;
> -			u8 adv_enable;
>   			__le64 modes;
>   			__le32 mode;
> +			u8 admin;
> +			u8 adv_enable;
>   			u8 mdix;
> -		} __packed phy;
> +			u8 __pad;
> +		} phy;
>   	} __packed link;
>   
>   	struct prestera_msg_port_cap_param cap;
>   	struct prestera_msg_port_flood_param flood_ext;
>   	struct prestera_msg_event_port_param link_evt;
> -} __packed;
> +};
>   
>   struct prestera_msg_port_attr_req {
>   	struct prestera_msg_cmd cmd;
> @@ -290,14 +294,12 @@ struct prestera_msg_port_attr_req {
>   	__le32 port;
>   	__le32 dev;
>   	union prestera_msg_port_param param;
> -} __packed __aligned(4);
> -
> +};
>   
>   struct prestera_msg_port_attr_resp {
>   	struct prestera_msg_ret ret;
>   	union prestera_msg_port_param param;
> -} __packed __aligned(4);
> -
> +};
>   
>   struct prestera_msg_port_stats_resp {
>   	struct prestera_msg_ret ret;
> @@ -322,13 +324,13 @@ struct prestera_msg_vlan_req {
>   	__le32 port;
>   	__le32 dev;
>   	__le16 vid;
> -	u8  is_member;
> -	u8  is_tagged;
> +	u8 is_member;
> +	u8 is_tagged;
>   };
>   
>   struct prestera_msg_fdb_req {
>   	struct prestera_msg_cmd cmd;
> -	u8 dest_type;
> +	__le32 flush_mode;
>   	union {
>   		struct {
>   			__le32 port;
> @@ -336,11 +338,12 @@ struct prestera_msg_fdb_req {
>   		};
>   		__le16 lag_id;
>   	} dest;
> -	u8  mac[ETH_ALEN];
>   	__le16 vid;
> -	u8  dynamic;
> -	__le32 flush_mode;
> -} __packed __aligned(4);
> +	u8 dest_type;
> +	u8 dynamic;
> +	u8 mac[ETH_ALEN];
> +	u8 __pad[2];
> +};
>   
>   struct prestera_msg_bridge_req {
>   	struct prestera_msg_cmd cmd;
> @@ -383,7 +386,7 @@ struct prestera_msg_acl_match {
>   		struct {
>   			u8 key[ETH_ALEN];
>   			u8 mask[ETH_ALEN];
> -		} __packed mac;
> +		} mac;
>   	} keymask;
>   };
>   
> @@ -446,7 +449,8 @@ struct prestera_msg_stp_req {
>   	__le32 port;
>   	__le32 dev;
>   	__le16 vid;
> -	u8  state;
> +	u8 state;
> +	u8 __pad;
>   };
>   
>   struct prestera_msg_rxtx_req {
> @@ -497,21 +501,21 @@ union prestera_msg_event_fdb_param {
>   
>   struct prestera_msg_event_fdb {
>   	struct prestera_msg_event id;
> -	u8 dest_type;
> +	__le32 vid;
>   	union {
>   		__le32 port_id;
>   		__le16 lag_id;
>   	} dest;
> -	__le32 vid;
>   	union prestera_msg_event_fdb_param param;
> -} __packed __aligned(4);
> +	u8 dest_type;
> +};
>   
> -static inline void prestera_hw_build_tests(void)
> +static void prestera_hw_build_tests(void)
>   {
>   	/* check requests */
>   	BUILD_BUG_ON(sizeof(struct prestera_msg_common_req) != 4);
>   	BUILD_BUG_ON(sizeof(struct prestera_msg_switch_attr_req) != 16);
> -	BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_req) != 120);
> +	BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_req) != 140);
>   	BUILD_BUG_ON(sizeof(struct prestera_msg_port_info_req) != 8);
>   	BUILD_BUG_ON(sizeof(struct prestera_msg_vlan_req) != 16);
>   	BUILD_BUG_ON(sizeof(struct prestera_msg_fdb_req) != 28);
> @@ -528,7 +532,7 @@ static inline void prestera_hw_build_tests(void)
>   	/* check responses */
>   	BUILD_BUG_ON(sizeof(struct prestera_msg_common_resp) != 8);
>   	BUILD_BUG_ON(sizeof(struct prestera_msg_switch_init_resp) != 24);
> -	BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_resp) != 112);
> +	BUILD_BUG_ON(sizeof(struct prestera_msg_port_attr_resp) != 132);
>   	BUILD_BUG_ON(sizeof(struct prestera_msg_port_stats_resp) != 248);
>   	BUILD_BUG_ON(sizeof(struct prestera_msg_port_info_resp) != 20);
>   	BUILD_BUG_ON(sizeof(struct prestera_msg_bridge_resp) != 12);
> @@ -561,9 +565,9 @@ static int __prestera_cmd_ret(struct prestera_switch *sw,
>   	if (err)
>   		return err;
>   
> -	if (__le32_to_cpu(ret->cmd.type) != PRESTERA_CMD_TYPE_ACK)
> +	if (ret->cmd.type != __cpu_to_le32(PRESTERA_CMD_TYPE_ACK))
>   		return -EBADE;
> -	if (__le32_to_cpu(ret->status) != PRESTERA_CMD_ACK_OK)
> +	if (ret->status != __cpu_to_le32(PRESTERA_CMD_ACK_OK))
>   		return -EINVAL;
>   
>   	return 0;
> @@ -1356,7 +1360,8 @@ int prestera_hw_port_speed_get(const struct prestera_port *port, u32 *speed)
>   int prestera_hw_port_autoneg_restart(struct prestera_port *port)
>   {
>   	struct prestera_msg_port_attr_req req = {
> -		.attr = __cpu_to_le32(PRESTERA_CMD_PORT_ATTR_PHY_AUTONEG_RESTART),
> +		.attr =
> +		    __cpu_to_le32(PRESTERA_CMD_PORT_ATTR_PHY_AUTONEG_RESTART),

All the newline changes are unrelated to the structure layout chnages.
Send them as a separate patch.


>   		.port = __cpu_to_le32(port->hw_id),
>   		.dev = __cpu_to_le32(port->dev_id),
>   	};
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> index 625b40149fac..4369a3ffad45 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> @@ -405,7 +405,8 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id)
>   
>   	err = prestera_port_cfg_mac_write(port, &cfg_mac);
>   	if (err) {
> -		dev_err(prestera_dev(sw), "Failed to set port(%u) mac mode\n", id);
> +		dev_err(prestera_dev(sw),
> +			"Failed to set port(%u) mac mode\n", id);
>   		goto err_port_init;
>   	}
>   
> @@ -418,7 +419,8 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id)
>   						    false, 0, 0,
>   						    port->cfg_phy.mdix);
>   		if (err) {
> -			dev_err(prestera_dev(sw), "Failed to set port(%u) phy mode\n", id);
> +			dev_err(prestera_dev(sw),
> +				"Failed to set port(%u) phy mode\n", id);
>   			goto err_port_init;
>   		}
>   	}
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> index 5d4d410b07c8..461259b3655a 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> @@ -411,7 +411,8 @@ static int prestera_fw_cmd_send(struct prestera_fw *fw, int qid,
>   		goto cmd_exit;
>   	}
>   
> -	memcpy_fromio(out_msg, prestera_fw_cmdq_buf(fw, qid) + in_size, ret_size);
> +	memcpy_fromio(out_msg,
> +		      prestera_fw_cmdq_buf(fw, qid) + in_size, ret_size);
>   
>   cmd_exit:
>   	prestera_fw_write(fw, PRESTERA_CMDQ_REQ_CTL_REG(qid),
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH net v2] net: marvell: prestera: fix hw structure laid out
  2021-11-03  9:54 [PATCH net v2] net: marvell: prestera: fix hw structure laid out Volodymyr Mytnyk
  2021-11-03 10:30 ` Denis Kirjanov
@ 2021-11-04  9:12 ` Geert Uytterhoeven
  2021-11-04  9:45   ` Volodymyr Mytnyk [C]
  1 sibling, 1 reply; 4+ messages in thread
From: Geert Uytterhoeven @ 2021-11-04  9:12 UTC (permalink / raw)
  To: Volodymyr Mytnyk
  Cc: Jakub Kicinski, Andrew Lunn, mickeyr, serhiy.pshyk,
	taras.chornyi, Volodymyr Mytnyk, Taras Chornyi, David S. Miller,
	Vadym Kochan, Yevhen Orlov, netdev, Linux Kernel Mailing List

Hi Volodymyr,

On Wed, Nov 3, 2021 at 10:56 AM Volodymyr Mytnyk
<volodymyr.mytnyk@plvision.eu> wrote:
> From: Volodymyr Mytnyk <vmytnyk@marvell.com>
>
> - fix structure laid out discussed in:
>     [PATCH net-next v4] net: marvell: prestera: add firmware v4.0 support
>     https://www.spinics.net/lists/kernel/msg4127689.html
>
> - fix review comments discussed in:
>     [PATCH] [-next] net: marvell: prestera: Add explicit padding
>     https://www.spinics.net/lists/kernel/msg4130293.html
>
> - fix patchwork issues
> - rebase on net master
>
> Reported-by: kernel test robot <lkp@intel.com>
> Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
> Signed-off-by: Volodymyr Mytnyk <vmytnyk@marvell.com>

Thanks for your patch!

> --- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c

>  struct prestera_msg_port_flood_param {
>         u8 type;
>         u8 enable;
> -};
> +       u8 __pad[2];
> +} __packed;

What's the point of having __packed on a struct of bytes?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH net v2] net: marvell: prestera: fix hw structure laid out
  2021-11-04  9:12 ` Geert Uytterhoeven
@ 2021-11-04  9:45   ` Volodymyr Mytnyk [C]
  0 siblings, 0 replies; 4+ messages in thread
From: Volodymyr Mytnyk [C] @ 2021-11-04  9:45 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jakub Kicinski, Andrew Lunn, Mickey Rachamim, Serhiy Pshyk,
	Taras Chornyi, Taras Chornyi [C],
	David S. Miller, Vadym Kochan [C],
	Yevhen Orlov, netdev, Linux Kernel Mailing List

> On Wed, Nov 3, 2021 at 10:56 AM Volodymyr Mytnyk
> <volodymyr.mytnyk@plvision.eu> wrote:
> > From: Volodymyr Mytnyk <vmytnyk@marvell.com>
> >
> > - fix structure laid out discussed in:
> >     [PATCH net-next v4] net: marvell: prestera: add firmware v4.0 support
> >     https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_kernel_msg4127689.html&d=DwIBaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=Y41pILcavAE6E85lMlyXdQBpY03LUi5-euLmDcLBBRw&m=MggFlhvEsV0dikgTUWWhK5i05HFJvv2BF0EdMIAghqSI92og-BAfZXe2Wm82FjG7&s=KkA8BuuYlG-6UWpaKKvGkvVRbhyoKnSNnftLBCYELDE&e= 
> >
> > - fix review comments discussed in:
> >     [PATCH] [-next] net: marvell: prestera: Add explicit padding
> >     https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_kernel_msg4130293.html&d=DwIBaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=Y41pILcavAE6E85lMlyXdQBpY03LUi5-euLmDcLBBRw&m=MggFlhvEsV0dikgTUWWhK5i05HFJvv2BF0EdMIAghqSI92og-BAfZXe2Wm82FjG7&s=Hs1u5qLhVlePG9KiNdOJiDpLTF200_9hn0gL9WLRJUA&e= 
> >
> > - fix patchwork issues
> > - rebase on net master
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Fixes: bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
> > Signed-off-by: Volodymyr Mytnyk <vmytnyk@marvell.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> > +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> 
> >  struct prestera_msg_port_flood_param {
> >         u8 type;
> >         u8 enable;
> > -};
> > +       u8 __pad[2];
> > +} __packed;
> 
> What's the point of having __packed on a struct of bytes?

This one can be removed probaby. Thanks. Will fix it in follow up patch set.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

              Volodymyr

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-04  9:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03  9:54 [PATCH net v2] net: marvell: prestera: fix hw structure laid out Volodymyr Mytnyk
2021-11-03 10:30 ` Denis Kirjanov
2021-11-04  9:12 ` Geert Uytterhoeven
2021-11-04  9:45   ` Volodymyr Mytnyk [C]

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.