Re: [Patch v2 net-next 4/7] octeontx2-af: Physical link configuration support

Hi Andrew Lunn,

-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch>
Sent: Wednesday, January 27, 2021 6:56 PM
To: Hariprasad Kelam <redacted>
Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
davem@davemloft.net; kuba@kernel.org; Sunil Kovvuri Goutham
[off-list ref]; Linu Cherian [off-list ref];
Geethasowjanya Akula [off-list ref]; Jerin Jacob Kollanukkaran
[off-list ref]; Subbaraya Sundeep Bhatta [off-list ref];
Christina Jacob [off-list ref]
Subject: [EXT] Re: [Patch v2 net-next 4/7] octeontx2-af: Physical link
configuration support

On Wed, Jan 27, 2021 at 01:15:49PM +0530, Hariprasad Kelam wrote:

quoted

From: Christina Jacob <redacted>

CGX LMAC, the physical interface support link configuration parameters
like speed, auto negotiation, duplex  etc. Firmware saves these into
memory region shared between firmware and this driver.

This patch adds mailbox handler set_link_mode, fw_data_get to
configure and read these parameters.

Signed-off-by: Christina Jacob <redacted>
Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
Signed-off-by: Hariprasad Kelam <redacted>
---
 drivers/net/ethernet/marvell/octeontx2/af/cgx.c    | 60

+++++++++++++++++++++-

quoted

 drivers/net/ethernet/marvell/octeontx2/af/cgx.h    |  2 +
 .../net/ethernet/marvell/octeontx2/af/cgx_fw_if.h  | 18 ++++++-
 drivers/net/ethernet/marvell/octeontx2/af/mbox.h   | 21 ++++++++
 .../net/ethernet/marvell/octeontx2/af/rvu_cgx.c    | 17 ++++++
 5 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c

b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index b3ae84c..42ee67e 100644

--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c

@@ -658,6 +658,39 @@ static inline void cgx_link_usertable_init(void)
 	cgx_lmactype_string[LMAC_MODE_USXGMII] = "USXGMII";  }

+static inline int cgx_link_usertable_index_map(int speed) {

Hi Christina, Hariprasad

No inline functions in .c files please. Let the compiler decide.

  will fix this in next version
 

quoted

+	switch (speed) {
+	case SPEED_10:
+		return CGX_LINK_10M;
+	case SPEED_100:
+		return CGX_LINK_100M;
+	case SPEED_1000:
+		return CGX_LINK_1G;
+	case SPEED_2500:
+		return CGX_LINK_2HG;
+	case SPEED_5000:
+		return CGX_LINK_5G;
+	case SPEED_10000:
+		return CGX_LINK_10G;
+	case SPEED_20000:
+		return CGX_LINK_20G;
+	case SPEED_25000:
+		return CGX_LINK_25G;
+	case SPEED_40000:
+		return CGX_LINK_40G;
+	case SPEED_50000:
+		return CGX_LINK_50G;
+	case 80000:
+		return CGX_LINK_80G;
+	case SPEED_100000:
+		return CGX_LINK_100G;
+	case SPEED_UNKNOWN:
+		return CGX_LINK_NONE;
+	}
+	return CGX_LINK_NONE;
+}
+
 static inline void link_status_user_format(u64 lstat,
 					   struct cgx_link_user_info *linfo,
 					   struct cgx *cgx, u8 lmac_id)

So it looks like previous reviews did not catch inline functions. So lets say, no
new inline functions.

     Andrew

Thanks for the review. I will fix this in next version.

Thanks,
Hariprasad k