Currently for VFs, mailbox returns error when hardware timestamping enable
is requested. This patch fixes this issue.
Fixes: 421572175ba5 ("octeontx2-af: Support to enable/disable HW timestamping")
Signed-off-by: Subbaraya Sundeep <[email protected]>
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Sai Krishna <[email protected]>
---
drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index f2b1edf1bb43..aba0c530160c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -756,12 +756,11 @@ static int rvu_cgx_ptp_rx_cfg(struct rvu *rvu, u16 pcifunc, bool enable)
if (!is_mac_feature_supported(rvu, pf, RVU_LMAC_FEAT_PTP))
return 0;
- /* This msg is expected only from PFs that are mapped to CGX LMACs,
+ /* This msg is expected only from PFs that are mapped to CGX/RPM LMACs,
* if received from other PF/VF simply ACK, nothing to do.
*/
- if ((pcifunc & RVU_PFVF_FUNC_MASK) ||
- !is_pf_cgxmapped(rvu, pf))
- return -ENODEV;
+ if (!is_pf_cgxmapped(rvu, rvu_get_pf(pcifunc)))
+ return -EPERM;
rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
cgxd = rvu_cgx_pdata(cgx_id, rvu);
--
2.25.1
On 9/29/2023 12:16 AM, Sai Krishna wrote:
> Currently for VFs, mailbox returns error when hardware timestamping enable
> is requested. This patch fixes this issue.
>
The subject title implies that this is implementing a new feature (and
thus not really a good candidate for net), but the content implies this
is a fix for an existing feature thats not working properly.
It could use a slightly improved commit message.
> Fixes: 421572175ba5 ("octeontx2-af: Support to enable/disable HW timestamping")
> Signed-off-by: Subbaraya Sundeep <[email protected]>
> Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> Signed-off-by: Sai Krishna <[email protected]>
Typically the author signed-off-by should go first, and then other
signed-off-by are for people in the chain of patch delivery. If the
other two co-authored the patch they should have Co-developed-by tag or
something. Otherwise I would expect that Subbaraya Sundeepd would be the
patch author since that signed-off-by is first.
> ---
> drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
The fix itself seems straight forward, though the comment is confusing
to me. You also changed the return from -ENODEV to -EPERM which makes
sense enough I suppose, but thats not called out in the change.
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> index f2b1edf1bb43..aba0c530160c 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> @@ -756,12 +756,11 @@ static int rvu_cgx_ptp_rx_cfg(struct rvu *rvu, u16 pcifunc, bool enable)
> if (!is_mac_feature_supported(rvu, pf, RVU_LMAC_FEAT_PTP))
> return 0;
>
> - /* This msg is expected only from PFs that are mapped to CGX LMACs,
> + /* This msg is expected only from PFs that are mapped to CGX/RPM LMACs,
> * if received from other PF/VF simply ACK, nothing to do.
> */
This comment implies to me that we wouldn't expect this message from a VF?
> - if ((pcifunc & RVU_PFVF_FUNC_MASK) ||
> - !is_pf_cgxmapped(rvu, pf))
> - return -ENODEV;
> + if (!is_pf_cgxmapped(rvu, rvu_get_pf(pcifunc)))
> + return -EPERM;
>
> rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
> cgxd = rvu_cgx_pdata(cgx_id, rvu);
> -----Original Message-----
> From: Jacob Keller <[email protected]>
> Sent: Friday, September 29, 2023 11:24 PM
> To: Sai Krishna Gajula <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Sunil Kovvuri
> Goutham <[email protected]>; Geethasowjanya Akula
> <[email protected]>; [email protected]; Linu Cherian
> <[email protected]>; Jerin Jacob Kollanukkaran <[email protected]>;
> Hariprasad Kelam <[email protected]>; Subbaraya Sundeep Bhatta
> <[email protected]>
> Subject: Re: [net PATCH] octeontx2-af: Enable hardware timestamping
> for VFs
>
>
> On 9/29/2023 12:16 AM, Sai Krishna wrote:
> > Currently for VFs, mailbox returns error when hardware timestamping
> > enable is requested. This patch fixes this issue.
> >
>
> The subject title implies that this is implementing a new feature (and thus
> not really a good candidate for net), but the content implies this is a fix for an
> existing feature thats not working properly.
>
> It could use a slightly improved commit message.
Ack, I will modify the commit message in V2
>
> > Fixes: 421572175ba5 ("octeontx2-af: Support to enable/disable HW
> > timestamping")
> > Signed-off-by: Subbaraya Sundeep <[email protected]>
> > Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> > Signed-off-by: Sai Krishna <[email protected]>
>
> Typically the author signed-off-by should go first, and then other signed-off-
> by are for people in the chain of patch delivery. If the other two co-authored
> the patch they should have Co-developed-by tag or something. Otherwise I
> would expect that Subbaraya Sundeepd would be the patch author since
> that signed-off-by is first.
Ack, I will change the author name in V2
>
> > ---
> > drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
>
> The fix itself seems straight forward, though the comment is confusing to
> me. You also changed the return from -ENODEV to -EPERM which makes
> sense enough I suppose, but thats not called out in the change.
>
Ack, I will modify the commit message in V2
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> > b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> > index f2b1edf1bb43..aba0c530160c 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> > @@ -756,12 +756,11 @@ static int rvu_cgx_ptp_rx_cfg(struct rvu *rvu, u16
> pcifunc, bool enable)
> > if (!is_mac_feature_supported(rvu, pf, RVU_LMAC_FEAT_PTP))
> > return 0;
> >
> > - /* This msg is expected only from PFs that are mapped to CGX
> LMACs,
> > + /* This msg is expected only from PFs that are mapped to CGX/RPM
> > +LMACs,
> > * if received from other PF/VF simply ACK, nothing to do.
> > */
>
> This comment implies to me that we wouldn't expect this message from a
> VF?
Ack, I will modify the comment in V2
>
> > - if ((pcifunc & RVU_PFVF_FUNC_MASK) ||
> > - !is_pf_cgxmapped(rvu, pf))
> > - return -ENODEV;
> > + if (!is_pf_cgxmapped(rvu, rvu_get_pf(pcifunc)))
> > + return -EPERM;
> >
> > rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id,
> &lmac_id);
> > cgxd = rvu_cgx_pdata(cgx_id, rvu);
On Fri, 29 Sep 2023 12:46:21 +0530 Sai Krishna wrote:
> Subject: [net PATCH] octeontx2-af: Enable hardware timestamping for VFs
> Date: Fri, 29 Sep 2023 12:46:21 +0530
> X-Mailer: git-send-email 2.25.1
>
> Currently for VFs, mailbox returns error when hardware timestamping enable
> is requested. This patch fixes this issue.
>
> Fixes: 421572175ba5 ("octeontx2-af: Support to enable/disable HW timestamping")
If it never worked for VFs but worked for PFs - it's not really a fix.
New functionality enablement is -next material, even if the fact that
it was previously disable was unintentional.
--
pw-bot: cr
> -----Original Message-----
> From: Jakub Kicinski <[email protected]>
> Sent: Thursday, October 5, 2023 2:43 AM
> To: Sai Krishna Gajula <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Sunil Kovvuri
> Goutham <[email protected]>; Geethasowjanya Akula
> <[email protected]>; [email protected]; Linu Cherian
> <[email protected]>; Jerin Jacob Kollanukkaran <[email protected]>;
> Hariprasad Kelam <[email protected]>; Subbaraya Sundeep Bhatta
> <[email protected]>
> Subject: Re: [net PATCH] octeontx2-af: Enable hardware timestamping
> for VFs
>
> On Fri, 29 Sep 2023 12:46:21 +0530 Sai Krishna wrote:
> > Subject: [net PATCH] octeontx2-af: Enable hardware timestamping for
> > VFs
> > Date: Fri, 29 Sep 2023 12:46:21 +0530
> > X-Mailer: git-send-email 2.25.1
> >
> > Currently for VFs, mailbox returns error when hardware timestamping
> > enable is requested. This patch fixes this issue.
> >
> > Fixes: 421572175ba5 ("octeontx2-af: Support to enable/disable HW
> > timestamping")
>
> If it never worked for VFs but worked for PFs - it's not really a fix.
> New functionality enablement is -next material, even if the fact that it was
> previously disable was unintentional.
Ack, will submit the patch to net-next.
> --
> pw-bot: cr