Re: [Patch v2 net-next 4/7] octeontx2-af: Physical link configuration support
Hi Willem,
> -Original Message-
> From: Willem de Bruijn
> Sent: Thursday, January 28, 2021 2:04 AM
> To: Hariprasad Kelam
> Cc: Network Development ; LKML ker...@vger.kernel.org>; David Miller ; Jakub
> Kicinski ; Sunil Kovvuri Goutham
> ; Linu Cherian ;
> Geethasowjanya Akula ; Jerin Jacob Kollanukkaran
> ; Subbaraya Sundeep Bhatta ;
> Christina Jacob
> Subject: [EXT] Re: [Patch v2 net-next 4/7] octeontx2-af: Physical link
> configuration support
>
> On Wed, Jan 27, 2021 at 4:02 AM Hariprasad Kelam
> wrote:
> >
> > From: Christina Jacob
> >
> > 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
> > Signed-off-by: Sunil Goutham
> > Signed-off-by: Hariprasad Kelam
>
> > +int rvu_mbox_handler_cgx_set_link_mode(struct rvu *rvu,
> > + struct cgx_set_link_mode_req *req,
> > + struct cgx_set_link_mode_rsp
> > +*rsp) {
> > + int pf = rvu_get_pf(req->hdr.pcifunc);
> > + u8 cgx_idx, lmac;
> > + void *cgxd;
> > +
> > + if (!is_cgx_config_permitted(rvu, req->hdr.pcifunc))
> > + return -EPERM;
> > +
> > + rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], _idx, );
> > + cgxd = rvu_cgx_pdata(cgx_idx, rvu);
> > + rsp->status = cgx_set_link_mode(cgxd, req->args, cgx_idx,
> > + lmac);
>
> nit: two spaces after assignment operator.
>
> on the point of no new inline: do also check the status in patchwork.
> that also flags such issues.
Thanks for the review. Will fix in next version.
Thanks,
Hariprasad k
Re: [Patch v2 net-next 4/7] octeontx2-af: Physical link configuration support
Hi Andrew Lunn,
> -Original Message-
> From: Andrew Lunn
> Sent: Wednesday, January 27, 2021 6:56 PM
> To: Hariprasad Kelam
> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> da...@davemloft.net; k...@kernel.org; Sunil Kovvuri Goutham
> ; Linu Cherian ;
> Geethasowjanya Akula ; Jerin Jacob Kollanukkaran
> ; Subbaraya Sundeep Bhatta ;
> Christina Jacob
> 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:
> > From: Christina Jacob
> >
> > 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
> > Signed-off-by: Sunil Goutham
> > Signed-off-by: Hariprasad Kelam
> > ---
> > drivers/net/ethernet/marvell/octeontx2/af/cgx.c| 60
> +-
> > 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
>
> > + 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_1:
> > + return CGX_LINK_10G;
> > + case SPEED_2:
> > + return CGX_LINK_20G;
> > + case SPEED_25000:
> > + return CGX_LINK_25G;
> > + case SPEED_4:
> > + return CGX_LINK_40G;
> > + case SPEED_5:
> > + return CGX_LINK_50G;
> > + case 8:
> > + return CGX_LINK_80G;
> > + case SPEED_10:
> > + 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
Re: [Patch v2 net-next 4/7] octeontx2-af: Physical link configuration support
On Wed, Jan 27, 2021 at 4:02 AM Hariprasad Kelam wrote:
>
> From: Christina Jacob
>
> 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
> Signed-off-by: Sunil Goutham
> Signed-off-by: Hariprasad Kelam
> +int rvu_mbox_handler_cgx_set_link_mode(struct rvu *rvu,
> + struct cgx_set_link_mode_req *req,
> + struct cgx_set_link_mode_rsp *rsp)
> +{
> + int pf = rvu_get_pf(req->hdr.pcifunc);
> + u8 cgx_idx, lmac;
> + void *cgxd;
> +
> + if (!is_cgx_config_permitted(rvu, req->hdr.pcifunc))
> + return -EPERM;
> +
> + rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], _idx, );
> + cgxd = rvu_cgx_pdata(cgx_idx, rvu);
> + rsp->status = cgx_set_link_mode(cgxd, req->args, cgx_idx, lmac);
nit: two spaces after assignment operator.
on the point of no new inline: do also check the status in patchwork.
that also flags such issues.
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:
> From: Christina Jacob
>
> 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
> Signed-off-by: Sunil Goutham
> Signed-off-by: Hariprasad Kelam
> ---
> drivers/net/ethernet/marvell/octeontx2/af/cgx.c| 60
> +-
> 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.
> + 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_1:
> + return CGX_LINK_10G;
> + case SPEED_2:
> + return CGX_LINK_20G;
> + case SPEED_25000:
> + return CGX_LINK_25G;
> + case SPEED_4:
> + return CGX_LINK_40G;
> + case SPEED_5:
> + return CGX_LINK_50G;
> + case 8:
> + return CGX_LINK_80G;
> + case SPEED_10:
> + 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
[Patch v2 net-next 4/7] octeontx2-af: Physical link configuration support
From: Christina Jacob
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
Signed-off-by: Sunil Goutham
Signed-off-by: Hariprasad Kelam
---
drivers/net/ethernet/marvell/octeontx2/af/cgx.c| 60 +-
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)
+{
+ 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_1:
+ return CGX_LINK_10G;
+ case SPEED_2:
+ return CGX_LINK_20G;
+ case SPEED_25000:
+ return CGX_LINK_25G;
+ case SPEED_4:
+ return CGX_LINK_40G;
+ case SPEED_5:
+ return CGX_LINK_50G;
+ case 8:
+ return CGX_LINK_80G;
+ case SPEED_10:
+ 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)
@@ -667,6 +700,7 @@ static inline void link_status_user_format(u64 lstat,
linfo->link_up = FIELD_GET(RESP_LINKSTAT_UP, lstat);
linfo->full_duplex = FIELD_GET(RESP_LINKSTAT_FDUPLEX, lstat);
linfo->speed = cgx_speed_mbps[FIELD_GET(RESP_LINKSTAT_SPEED, lstat)];
+ linfo->an = FIELD_GET(RESP_LINKSTAT_AN, lstat);
linfo->fec = FIELD_GET(RESP_LINKSTAT_FEC, lstat);
linfo->lmac_type_id = cgx_get_lmac_type(cgx, lmac_id);
lmac_string = cgx_lmactype_string[linfo->lmac_type_id];
@@ -695,6 +729,9 @@ static inline void cgx_link_change_handler(u64 lstat,
lmac->link_info = event.link_uinfo;
linfo = >link_info;
+ if (err_type == CGX_ERR_SPEED_CHANGE_INVALID)
+ return;
+
/* Ensure callback doesn't get unregistered until we finish it */
spin_lock(>event_cb_lock);
@@ -723,7 +760,8 @@ static inline bool cgx_cmdresp_is_linkevent(u64 event)
id = FIELD_GET(EVTREG_ID, event);
if (id == CGX_CMD_LINK_BRING_UP ||
- id == CGX_CMD_LINK_BRING_DOWN)
+ id == CGX_CMD_LINK_BRING_DOWN ||
+ id == CGX_CMD_MODE_CHANGE)
return true;
else
return false;
@@ -838,6 +876,26 @@ int cgx_get_fwdata_base(u64 *base)
return err;
}
+int cgx_set_link_mode(void *cgxd, struct cgx_set_link_mode_args args,
+ int cgx_id, int lmac_id)
+{
+ struct cgx *cgx = cgxd;
+ u64 req = 0, resp;
+ int err = 0;
+
+ if (!cgx)
+ return -ENODEV;
+
+ req = FIELD_SET(CMDREG_ID, CGX_CMD_MODE_CHANGE, req);
+ req = FIELD_SET(CMDMODECHANGE_SPEED,
+ cgx_link_usertable_index_map(args.speed), req);
+ req = FIELD_SET(CMDMODECHANGE_DUPLEX, args.duplex, req);
+ req = FIELD_SET(CMDMODECHANGE_AN, args.an, req);
+ req = FIELD_SET(CMDMODECHANGE_PORT, args.ports, req);
+ req = FIELD_SET(CMDMODECHANGE_FLAGS, args.flags, req);
+ err = cgx_fwi_cmd_generic(req, , cgx, lmac_id);
+ return err;
+}
int cgx_set_fec(u64 fec, int cgx_id, int lmac_id)
{
u64 req = 0, resp;
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
index c5294b7..b458ad0 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
@@ -155,5 +155,7 @@ u8 cgx_lmac_get_p2x(int cgx_id, int lmac_id);
int cgx_set_fec(u64 fec, int cgx_id, int lmac_id);
int cgx_get_fec_stats(void *cgxd, int lmac_id, struct cgx_fec_stats_rsp *rsp);
int cgx_get_phy_fec_stats(void *cgxd, int lmac_id);
+int
