linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v4] net: marvell: prestera: fix hw structure laid out
@ 2021-11-05 16:49 Volodymyr Mytnyk
  2021-11-05 20:14 ` Guenter Roeck
  2021-11-07 19:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 6+ messages in thread
From: Volodymyr Mytnyk @ 2021-11-05 16:49 UTC (permalink / raw)
  To: netdev
  Cc: Taras Chornyi, Mickey Rachamim, Serhiy Pshyk, Andrew Lunn,
	Arnd Bergmann, Geert Uytterhoeven, Denis Kirjanov, Guenter Roeck,
	Volodymyr Mytnyk, Taras Chornyi, David S. Miller, Jakub Kicinski,
	Vadym Kochan, Yevhen Orlov, linux-kernel

From: Volodymyr Mytnyk <vmytnyk@marvell.com>

The prestera FW v4.0 support commit has been merged
accidentally w/o review comments addressed and waiting
for the final patch set to be uploaded. So, fix the remaining
comments related to structure laid out and build issues.

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>
---

Changes in V2:
  - 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
    + [PATCH] [-next] net: marvell: prestera: Add explicit padding
        https://www.spinics.net/lists/kernel/msg4130293.html

Changes in V3:
  - update commit message
  - fix more laid out comments
  - split into two patches suggested in:
      https://www.spinics.net/lists/netdev/msg778322.html

Changes in V4:
  - removed unnecessary __packed attr
  - get rid of __packed by adding alignment field
    to prestera_msg_port_param.
  - fix prestera_msg_acl_match alignment

 .../net/ethernet/marvell/prestera/prestera_hw.c    | 131 +++++++++++----------
 1 file changed, 68 insertions(+), 63 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
index 4f5f52dcdd9d..5550c0a8bec3 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;
-		} __packed phy;
-	} __packed;
-} __packed __aligned(4);
+			u8 __pad[2];
+		} __packed phy; /* make sure always 12 bytes size */
+	};
+};
 
 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;
 };
 
 struct prestera_msg_port_flood_param {
 	u8 type;
 	u8 enable;
+	u8 __pad[2];
 };
 
 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[5];
 			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;
-	} __packed link;
+			u8 __pad;
+		} phy;
+	} 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;
@@ -363,11 +366,12 @@ struct prestera_msg_acl_action {
 
 struct prestera_msg_acl_match {
 	__le32 type;
+	__le32 __reserved;
 	union {
 		struct {
 			u8 key;
 			u8 mask;
-		} __packed u8;
+		} u8;
 		struct {
 			__le16 key;
 			__le16 mask;
@@ -383,7 +387,7 @@ struct prestera_msg_acl_match {
 		struct {
 			u8 key[ETH_ALEN];
 			u8 mask[ETH_ALEN];
-		} __packed mac;
+		} mac;
 	} keymask;
 };
 
@@ -446,7 +450,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 +502,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) != 144);
 	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 +533,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) != 136);
 	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 +566,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;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net v4] net: marvell: prestera: fix hw structure laid out
  2021-11-05 16:49 [PATCH net v4] net: marvell: prestera: fix hw structure laid out Volodymyr Mytnyk
@ 2021-11-05 20:14 ` Guenter Roeck
  2021-11-05 22:42   ` Volodymyr Mytnyk
  2021-11-07 19:40 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2021-11-05 20:14 UTC (permalink / raw)
  To: Volodymyr Mytnyk, netdev
  Cc: Taras Chornyi, Mickey Rachamim, Serhiy Pshyk, Andrew Lunn,
	Arnd Bergmann, Geert Uytterhoeven, Denis Kirjanov,
	Volodymyr Mytnyk, Taras Chornyi, David S. Miller, Jakub Kicinski,
	Vadym Kochan, Yevhen Orlov, linux-kernel

On 11/5/21 9:49 AM, Volodymyr Mytnyk wrote:
> From: Volodymyr Mytnyk <vmytnyk@marvell.com>
> 
> The prestera FW v4.0 support commit has been merged
> accidentally w/o review comments addressed and waiting
> for the final patch set to be uploaded. So, fix the remaining
> comments related to structure laid out and build issues.
> 
> 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>

The patch does not apply to the mainline kernel, so I can not test it there.
It does apply to linux-next, and m68k:allmodconfig builds there with the patch
applied. However, m68k:allmodconfig also builds in -next with this patch _not_
applied, so I can not really say if it does any good or bad.
In the meantime, the mainline kernel (as of v5.15-10643-gfe91c4725aee)
still fails to build.

Guenter

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v4] net: marvell: prestera: fix hw structure laid out
  2021-11-05 20:14 ` Guenter Roeck
@ 2021-11-05 22:42   ` Volodymyr Mytnyk
  2021-11-06  8:42     ` Geert Uytterhoeven
  0 siblings, 1 reply; 6+ messages in thread
From: Volodymyr Mytnyk @ 2021-11-05 22:42 UTC (permalink / raw)
  To: Guenter Roeck, netdev
  Cc: Taras Chornyi, Mickey Rachamim, Serhiy Pshyk, Andrew Lunn,
	Arnd Bergmann, Geert Uytterhoeven, Denis Kirjanov,
	Volodymyr Mytnyk, Taras Chornyi, David S. Miller, Jakub Kicinski,
	Vadym Kochan, Yevhen Orlov, linux-kernel

>
> > From: Volodymyr Mytnyk <vmytnyk@marvell.com>
> > 
> > The prestera FW v4.0 support commit has been merged
> > accidentally w/o review comments addressed and waiting
> > for the final patch set to be uploaded. So, fix the remaining
> > comments related to structure laid out and build issues.
> > 
> > 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>
> 
> The patch does not apply to the mainline kernel, so I can not test it there.
> It does apply to linux-next, and m68k:allmodconfig builds there with the patch
> applied. However, m68k:allmodconfig also builds in -next with this patch _not_
> applied, so I can not really say if it does any good or bad.
> In the meantime, the mainline kernel (as of v5.15-10643-gfe91c4725aee)
> still fails to build.
> 
> Guenter

Hi Guenter,

	The mainline kernel doesn't have the base ("net: marvell: prestera: add firmware v4.0 support") commit yet, so the patch will not be applied.

This patch is based on net/master, so you can try the patch there.

To apply this patch to mainline, the following list of patches should be ported from net/master first:
 - bb5dbf2cc64d ("net: marvell: prestera: add firmware v4.0 support")
 - 236f57fe1b88 ("net: marvell: prestera: Add explicit padding")
 - a46a5036e7d2 ("net: marvell: prestera: fix patchwork build problems")

    Volodymyr

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v4] net: marvell: prestera: fix hw structure laid out
  2021-11-05 22:42   ` Volodymyr Mytnyk
@ 2021-11-06  8:42     ` Geert Uytterhoeven
  2021-11-06 10:54       ` Volodymyr Mytnyk
  0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2021-11-06  8:42 UTC (permalink / raw)
  To: Volodymyr Mytnyk
  Cc: Guenter Roeck, netdev, Taras Chornyi, Mickey Rachamim,
	Serhiy Pshyk, Andrew Lunn, Arnd Bergmann, Denis Kirjanov,
	Volodymyr Mytnyk, Taras Chornyi, David S. Miller, Jakub Kicinski,
	Vadym Kochan, Yevhen Orlov, linux-kernel

Hi Volodymyr,

On Fri, Nov 5, 2021 at 11:42 PM Volodymyr Mytnyk
<volodymyr.mytnyk@plvision.eu> wrote:
> > > From: Volodymyr Mytnyk <vmytnyk@marvell.com>
> > >
> > > The prestera FW v4.0 support commit has been merged
> > > accidentally w/o review comments addressed and waiting
> > > for the final patch set to be uploaded. So, fix the remaining
> > > comments related to structure laid out and build issues.
> > >
> > > 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>
> >
> > The patch does not apply to the mainline kernel, so I can not test it there.
> > It does apply to linux-next, and m68k:allmodconfig builds there with the patch
> > applied. However, m68k:allmodconfig also builds in -next with this patch _not_
> > applied, so I can not really say if it does any good or bad.
> > In the meantime, the mainline kernel (as of v5.15-10643-gfe91c4725aee)
> > still fails to build.

>         The mainline kernel doesn't have the base ("net: marvell: prestera: add firmware v4.0 support") commit yet, so the patch will not be applied.

Mainline has this broken commit as of Nov 2.

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] 6+ messages in thread

* Re: [PATCH net v4] net: marvell: prestera: fix hw structure laid out
  2021-11-06  8:42     ` Geert Uytterhoeven
@ 2021-11-06 10:54       ` Volodymyr Mytnyk
  0 siblings, 0 replies; 6+ messages in thread
From: Volodymyr Mytnyk @ 2021-11-06 10:54 UTC (permalink / raw)
  To: Geert Uytterhoeven, Guenter Roeck
  Cc: netdev, Taras Chornyi, Mickey Rachamim, Serhiy Pshyk,
	Andrew Lunn, Arnd Bergmann, Denis Kirjanov, Volodymyr Mytnyk,
	Taras Chornyi, David S. Miller, Jakub Kicinski, Vadym Kochan,
	Yevhen Orlov, linux-kernel

> On Fri, Nov 5, 2021 at 11:42 PM Volodymyr Mytnyk
> <volodymyr.mytnyk@plvision.eu> wrote:
> > > > From: Volodymyr Mytnyk <vmytnyk@marvell.com>
> > > >
> > > > The prestera FW v4.0 support commit has been merged
> > > > accidentally w/o review comments addressed and waiting
> > > > for the final patch set to be uploaded. So, fix the remaining
> > > > comments related to structure laid out and build issues.
> > > >
> > > > 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>
> > >
> > > The patch does not apply to the mainline kernel, so I can not test it there.
> > > It does apply to linux-next, and m68k:allmodconfig builds there with the patch
> > > applied. However, m68k:allmodconfig also builds in -next with this patch _not_
> > > applied, so I can not really say if it does any good or bad.
> > > In the meantime, the mainline kernel (as of v5.15-10643-gfe91c4725aee)
> > > still fails to build.
> 
> >         The mainline kernel doesn't have the base ("net: marvell: prestera: add firmware v4.0 support") commit yet, so the patch will not be applied.
> 
> Mainline has this broken commit as of Nov 2.

Hi Geert,

	Right, this one is it there. My fault, thx.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/prestera/prestera_hw.c?id=bb5dbf2cc64d5cfa696765944c784c0010c48ae8
	
So, only 236f57fe1b88 ("net: marvell: prestera: Add explicit padding") from net/master is needed to fix the build and to be able to apply this patch.

Sorry for confusion.

    Volodymyr

> 
> 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] 6+ messages in thread

* Re: [PATCH net v4] net: marvell: prestera: fix hw structure laid out
  2021-11-05 16:49 [PATCH net v4] net: marvell: prestera: fix hw structure laid out Volodymyr Mytnyk
  2021-11-05 20:14 ` Guenter Roeck
@ 2021-11-07 19:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-11-07 19:40 UTC (permalink / raw)
  To: Volodymyr Mytnyk
  Cc: netdev, taras.chornyi, mickeyr, serhiy.pshyk, andrew, arnd,
	geert, dkirjanov, linux, vmytnyk, tchornyi, davem, kuba, vkochan,
	yevhen.orlov, linux-kernel

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri,  5 Nov 2021 18:49:24 +0200 you wrote:
> From: Volodymyr Mytnyk <vmytnyk@marvell.com>
> 
> The prestera FW v4.0 support commit has been merged
> accidentally w/o review comments addressed and waiting
> for the final patch set to be uploaded. So, fix the remaining
> comments related to structure laid out and build issues.
> 
> [...]

Here is the summary with links:
  - [net,v4] net: marvell: prestera: fix hw structure laid out
    https://git.kernel.org/netdev/net/c/e1464db5c57e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-07 19:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-05 16:49 [PATCH net v4] net: marvell: prestera: fix hw structure laid out Volodymyr Mytnyk
2021-11-05 20:14 ` Guenter Roeck
2021-11-05 22:42   ` Volodymyr Mytnyk
2021-11-06  8:42     ` Geert Uytterhoeven
2021-11-06 10:54       ` Volodymyr Mytnyk
2021-11-07 19:40 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).