* [PATCH net-next 0/2] Small fixes for redundant checks @ 2021-10-19 6:26 Jεan Sacren 2021-10-19 6:26 ` [PATCH net-next 1/2] net: qed_ptp: fix redundant check of rc and against -EINVAL Jεan Sacren 2021-10-19 6:26 ` [PATCH net-next 2/2] net: qed_dev: " Jεan Sacren 0 siblings, 2 replies; 8+ messages in thread From: Jεan Sacren @ 2021-10-19 6:26 UTC (permalink / raw) To: Ariel Elior; +Cc: GR-everest-linux-l2, davem, kuba, netdev From: Jean Sacren <sakiwit@gmail.com> This series fixes some redundant checks of certain variables and expressions. Jean Sacren (2): net: qed_ptp: fix redundant check of rc and against -EINVAL net: qed_dev: fix redundant check of rc and against -EINVAL drivers/net/ethernet/qlogic/qed/qed_dev.c | 35 +++++++++++++---------- drivers/net/ethernet/qlogic/qed/qed_ptp.c | 12 ++++---- 2 files changed, 27 insertions(+), 20 deletions(-) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next 1/2] net: qed_ptp: fix redundant check of rc and against -EINVAL 2021-10-19 6:26 [PATCH net-next 0/2] Small fixes for redundant checks Jεan Sacren @ 2021-10-19 6:26 ` Jεan Sacren 2021-10-20 8:48 ` Simon Horman 2021-10-19 6:26 ` [PATCH net-next 2/2] net: qed_dev: " Jεan Sacren 1 sibling, 1 reply; 8+ messages in thread From: Jεan Sacren @ 2021-10-19 6:26 UTC (permalink / raw) To: Ariel Elior; +Cc: GR-everest-linux-l2, davem, kuba, netdev From: Jean Sacren <sakiwit@gmail.com> We should first check rc alone and then check it against -EINVAL to avoid repeating the same operation. With this change, we could also use constant 0 for return. Signed-off-by: Jean Sacren <sakiwit@gmail.com> --- drivers/net/ethernet/qlogic/qed/qed_ptp.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/qlogic/qed/qed_ptp.c b/drivers/net/ethernet/qlogic/qed/qed_ptp.c index 2c62d732e5c2..c927ff409109 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_ptp.c +++ b/drivers/net/ethernet/qlogic/qed/qed_ptp.c @@ -52,9 +52,9 @@ static int qed_ptp_res_lock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) qed_mcp_resc_lock_default_init(¶ms, NULL, resource, true); rc = qed_mcp_resc_lock(p_hwfn, p_ptt, ¶ms); - if (rc && rc != -EINVAL) { - return rc; - } else if (rc == -EINVAL) { + if (rc) { + if (rc != -EINVAL) + return rc; /* MFW doesn't support resource locking, first PF on the port * has lock ownership. */ @@ -63,12 +63,14 @@ static int qed_ptp_res_lock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) DP_INFO(p_hwfn, "PF doesn't have lock ownership\n"); return -EBUSY; - } else if (!rc && !params.b_granted) { + } + + if (!params.b_granted) { DP_INFO(p_hwfn, "Failed to acquire ptp resource lock\n"); return -EBUSY; } - return rc; + return 0; } static int qed_ptp_res_unlock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] net: qed_ptp: fix redundant check of rc and against -EINVAL 2021-10-19 6:26 ` [PATCH net-next 1/2] net: qed_ptp: fix redundant check of rc and against -EINVAL Jεan Sacren @ 2021-10-20 8:48 ` Simon Horman 2021-10-21 7:35 ` Jεan Sacren 0 siblings, 1 reply; 8+ messages in thread From: Simon Horman @ 2021-10-20 8:48 UTC (permalink / raw) To: Jεan Sacren; +Cc: Ariel Elior, GR-everest-linux-l2, davem, kuba, netdev On Tue, Oct 19, 2021 at 12:26:41AM -0600, Jεan Sacren wrote: > From: Jean Sacren <sakiwit@gmail.com> > > We should first check rc alone and then check it against -EINVAL to > avoid repeating the same operation. > > With this change, we could also use constant 0 for return. > > Signed-off-by: Jean Sacren <sakiwit@gmail.com> > --- > drivers/net/ethernet/qlogic/qed/qed_ptp.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_ptp.c b/drivers/net/ethernet/qlogic/qed/qed_ptp.c > index 2c62d732e5c2..c927ff409109 100644 > --- a/drivers/net/ethernet/qlogic/qed/qed_ptp.c > +++ b/drivers/net/ethernet/qlogic/qed/qed_ptp.c > @@ -52,9 +52,9 @@ static int qed_ptp_res_lock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) > qed_mcp_resc_lock_default_init(¶ms, NULL, resource, true); > > rc = qed_mcp_resc_lock(p_hwfn, p_ptt, ¶ms); > - if (rc && rc != -EINVAL) { > - return rc; > - } else if (rc == -EINVAL) { > + if (rc) { > + if (rc != -EINVAL) > + return rc; > /* MFW doesn't support resource locking, first PF on the port > * has lock ownership. > */ > @@ -63,12 +63,14 @@ static int qed_ptp_res_lock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) > > DP_INFO(p_hwfn, "PF doesn't have lock ownership\n"); > return -EBUSY; > - } else if (!rc && !params.b_granted) { > + } > + > + if (!params.b_granted) { Can it be the case where the condition above is met and !rc is false? If so your patch seems to have changed the logic of this function. > DP_INFO(p_hwfn, "Failed to acquire ptp resource lock\n"); > return -EBUSY; > } > > - return rc; > + return 0; > } > > static int qed_ptp_res_unlock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] net: qed_ptp: fix redundant check of rc and against -EINVAL 2021-10-20 8:48 ` Simon Horman @ 2021-10-21 7:35 ` Jεan Sacren 2021-10-21 10:46 ` Simon Horman 0 siblings, 1 reply; 8+ messages in thread From: Jεan Sacren @ 2021-10-21 7:35 UTC (permalink / raw) To: Simon Horman; +Cc: Ariel Elior, GR-everest-linux-l2, davem, kuba, netdev From: Simon Horman <horms@kernel.org> Date: Wed, 20 Oct 2021 10:48:35 +0200 > > On Tue, Oct 19, 2021 at 12:26:41AM -0600, Jεan Sacren wrote: > > From: Jean Sacren <sakiwit@gmail.com> > > > > We should first check rc alone and then check it against -EINVAL to > > avoid repeating the same operation. > > > > With this change, we could also use constant 0 for return. > > > > Signed-off-by: Jean Sacren <sakiwit@gmail.com> > > --- > > drivers/net/ethernet/qlogic/qed/qed_ptp.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_ptp.c b/drivers/net/ethernet/qlogic/qed/qed_ptp.c > > index 2c62d732e5c2..c927ff409109 100644 > > --- a/drivers/net/ethernet/qlogic/qed/qed_ptp.c > > +++ b/drivers/net/ethernet/qlogic/qed/qed_ptp.c > > @@ -52,9 +52,9 @@ static int qed_ptp_res_lock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) > > qed_mcp_resc_lock_default_init(¶ms, NULL, resource, true); > > > > rc = qed_mcp_resc_lock(p_hwfn, p_ptt, ¶ms); > > - if (rc && rc != -EINVAL) { > > - return rc; > > - } else if (rc == -EINVAL) { > > + if (rc) { > > + if (rc != -EINVAL) > > + return rc; > > /* MFW doesn't support resource locking, first PF on the port > > * has lock ownership. > > */ > > @@ -63,12 +63,14 @@ static int qed_ptp_res_lock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) > > > > DP_INFO(p_hwfn, "PF doesn't have lock ownership\n"); > > return -EBUSY; > > - } else if (!rc && !params.b_granted) { > > + } > > + > > + if (!params.b_granted) { > > Can it be the case where the condition above is met and !rc is false? > If so your patch seems to have changed the logic of this function. Mr. Horman, I'm so much appreciative to you for the review. I'm so sorry this patch is wrong. I redid the patch. Could you please help me review it? I did verify at the point where we check (!params.b_granted), !rc is always true. Earlier when we check rc alone, it has to be 0 to let it reach the point where we check (!params.b_granted). If it is not 0, it will hit one of the returns in the branch. I'll add the following text in the changelog to curb the confusion I incur. What do you think? We should also remove the check of !rc in (!rc && !params.b_granted) since it is always true. // diff --git a/drivers/net/ethernet/qlogic/qed/qed_ptp.c b/drivers/net/ethernet/qlogic/qed/qed_ptp.c // index 2c62d732e5c2..4e1b741ebb46 100644 // --- a/drivers/net/ethernet/qlogic/qed/qed_ptp.c // +++ b/drivers/net/ethernet/qlogic/qed/qed_ptp.c // @@ -52,23 +52,27 @@ static int qed_ptp_res_lock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) // qed_mcp_resc_lock_default_init(¶ms, NULL, resource, true); // // rc = qed_mcp_resc_lock(p_hwfn, p_ptt, ¶ms); // - if (rc && rc != -EINVAL) { // + if (rc) { // + if (rc == -EINVAL) { // + /* MFW doesn't support resource locking, first PF on the port // + * has lock ownership. // + */ // + if (p_hwfn->abs_pf_id < p_hwfn->cdev->num_ports_in_engine) // + return 0; // + // + DP_INFO(p_hwfn, "PF doesn't have lock ownership\n"); // + return -EBUSY; // + } // + // return rc; // - } else if (rc == -EINVAL) { // - /* MFW doesn't support resource locking, first PF on the port // - * has lock ownership. // - */ // - if (p_hwfn->abs_pf_id < p_hwfn->cdev->num_ports_in_engine) // - return 0; // + } // // - DP_INFO(p_hwfn, "PF doesn't have lock ownership\n"); // - return -EBUSY; // - } else if (!rc && !params.b_granted) { // + if (!params.b_granted) { // DP_INFO(p_hwfn, "Failed to acquire ptp resource lock\n"); // return -EBUSY; // } // // - return rc; // + return 0; // } // // static int qed_ptp_res_unlock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] net: qed_ptp: fix redundant check of rc and against -EINVAL 2021-10-21 7:35 ` Jεan Sacren @ 2021-10-21 10:46 ` Simon Horman 0 siblings, 0 replies; 8+ messages in thread From: Simon Horman @ 2021-10-21 10:46 UTC (permalink / raw) To: Jεan Sacren; +Cc: Ariel Elior, GR-everest-linux-l2, davem, kuba, netdev On Thu, Oct 21, 2021 at 01:35:48AM -0600, Jεan Sacren wrote: > From: Simon Horman <horms@kernel.org> > Date: Wed, 20 Oct 2021 10:48:35 +0200 > > > > On Tue, Oct 19, 2021 at 12:26:41AM -0600, Jεan Sacren wrote: > > > From: Jean Sacren <sakiwit@gmail.com> > > > > > > We should first check rc alone and then check it against -EINVAL to > > > avoid repeating the same operation. > > > > > > With this change, we could also use constant 0 for return. > > > > > > Signed-off-by: Jean Sacren <sakiwit@gmail.com> > > > --- > > > drivers/net/ethernet/qlogic/qed/qed_ptp.c | 12 +++++++----- > > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_ptp.c b/drivers/net/ethernet/qlogic/qed/qed_ptp.c > > > index 2c62d732e5c2..c927ff409109 100644 > > > --- a/drivers/net/ethernet/qlogic/qed/qed_ptp.c > > > +++ b/drivers/net/ethernet/qlogic/qed/qed_ptp.c > > > @@ -52,9 +52,9 @@ static int qed_ptp_res_lock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) > > > qed_mcp_resc_lock_default_init(¶ms, NULL, resource, true); > > > > > > rc = qed_mcp_resc_lock(p_hwfn, p_ptt, ¶ms); > > > - if (rc && rc != -EINVAL) { > > > - return rc; > > > - } else if (rc == -EINVAL) { > > > + if (rc) { > > > + if (rc != -EINVAL) > > > + return rc; > > > /* MFW doesn't support resource locking, first PF on the port > > > * has lock ownership. > > > */ > > > @@ -63,12 +63,14 @@ static int qed_ptp_res_lock(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) > > > > > > DP_INFO(p_hwfn, "PF doesn't have lock ownership\n"); > > > return -EBUSY; > > > - } else if (!rc && !params.b_granted) { > > > + } > > > + > > > + if (!params.b_granted) { > > > > Can it be the case where the condition above is met and !rc is false? > > If so your patch seems to have changed the logic of this function. > > Mr. Horman, > > I'm so much appreciative to you for the review. I'm so sorry this patch > is wrong. I redid the patch. Could you please help me review it? > > I did verify at the point where we check (!params.b_granted), !rc is > always true. Earlier when we check rc alone, it has to be 0 to let it > reach the point where we check (!params.b_granted). If it is not 0, it > will hit one of the returns in the branch. > > I'll add the following text in the changelog to curb the confusion I > incur. What do you think? > > We should also remove the check of !rc in (!rc && !params.b_granted) > since it is always true. Thanks I see that now, and I agree that your patch doesn't change the logic of the code (as far as I can tell). Reviewed-by: Simon Horman <horms@kernel.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next 2/2] net: qed_dev: fix redundant check of rc and against -EINVAL 2021-10-19 6:26 [PATCH net-next 0/2] Small fixes for redundant checks Jεan Sacren 2021-10-19 6:26 ` [PATCH net-next 1/2] net: qed_ptp: fix redundant check of rc and against -EINVAL Jεan Sacren @ 2021-10-19 6:26 ` Jεan Sacren 2021-10-20 8:47 ` Simon Horman 1 sibling, 1 reply; 8+ messages in thread From: Jεan Sacren @ 2021-10-19 6:26 UTC (permalink / raw) To: Ariel Elior; +Cc: GR-everest-linux-l2, davem, kuba, netdev From: Jean Sacren <sakiwit@gmail.com> We should first check rc alone and then check it against -EINVAL to avoid repeating the same operation multiple times. Signed-off-by: Jean Sacren <sakiwit@gmail.com> --- drivers/net/ethernet/qlogic/qed/qed_dev.c | 35 +++++++++++++---------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c index 18f3bf7c4dfe..fe8bdb4523b5 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c @@ -3987,30 +3987,35 @@ static int qed_hw_get_resc(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) QED_RESC_LOCK_RESC_ALLOC, false); rc = qed_mcp_resc_lock(p_hwfn, p_ptt, &resc_lock_params); - if (rc && rc != -EINVAL) { - return rc; - } else if (rc == -EINVAL) { + if (rc) { + if (rc != -EINVAL) + return rc; + DP_INFO(p_hwfn, "Skip the max values setting of the soft resources since the resource lock is not supported by the MFW\n"); - } else if (!rc && !resc_lock_params.b_granted) { + } + + if (!resc_lock_params.b_granted) { DP_NOTICE(p_hwfn, "Failed to acquire the resource lock for the resource allocation commands\n"); return -EBUSY; - } else { - rc = qed_hw_set_soft_resc_size(p_hwfn, p_ptt); - if (rc && rc != -EINVAL) { + } + + rc = qed_hw_set_soft_resc_size(p_hwfn, p_ptt); + if (rc) { + if (rc != -EINVAL) { DP_NOTICE(p_hwfn, "Failed to set the max values of the soft resources\n"); goto unlock_and_exit; - } else if (rc == -EINVAL) { - DP_INFO(p_hwfn, - "Skip the max values setting of the soft resources since it is not supported by the MFW\n"); - rc = qed_mcp_resc_unlock(p_hwfn, p_ptt, - &resc_unlock_params); - if (rc) - DP_INFO(p_hwfn, - "Failed to release the resource lock for the resource allocation commands\n"); } + + DP_INFO(p_hwfn, + "Skip the max values setting of the soft resources since it is not supported by the MFW\n"); + rc = qed_mcp_resc_unlock(p_hwfn, p_ptt, + &resc_unlock_params); + if (rc) + DP_INFO(p_hwfn, + "Failed to release the resource lock for the resource allocation commands\n"); } rc = qed_hw_set_resc_info(p_hwfn); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: qed_dev: fix redundant check of rc and against -EINVAL 2021-10-19 6:26 ` [PATCH net-next 2/2] net: qed_dev: " Jεan Sacren @ 2021-10-20 8:47 ` Simon Horman 2021-10-21 7:36 ` Jεan Sacren 0 siblings, 1 reply; 8+ messages in thread From: Simon Horman @ 2021-10-20 8:47 UTC (permalink / raw) To: Jεan Sacren; +Cc: Ariel Elior, GR-everest-linux-l2, davem, kuba, netdev On Tue, Oct 19, 2021 at 12:26:42AM -0600, Jεan Sacren wrote: > From: Jean Sacren <sakiwit@gmail.com> > > We should first check rc alone and then check it against -EINVAL to > avoid repeating the same operation multiple times. > > Signed-off-by: Jean Sacren <sakiwit@gmail.com> > --- > drivers/net/ethernet/qlogic/qed/qed_dev.c | 35 +++++++++++++---------- > 1 file changed, 20 insertions(+), 15 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c > index 18f3bf7c4dfe..fe8bdb4523b5 100644 > --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c > +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c > @@ -3987,30 +3987,35 @@ static int qed_hw_get_resc(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) > QED_RESC_LOCK_RESC_ALLOC, false); > > rc = qed_mcp_resc_lock(p_hwfn, p_ptt, &resc_lock_params); > - if (rc && rc != -EINVAL) { > - return rc; > - } else if (rc == -EINVAL) { > + if (rc) { > + if (rc != -EINVAL) > + return rc; > + > DP_INFO(p_hwfn, > "Skip the max values setting of the soft resources since the resource lock is not supported by the MFW\n"); > - } else if (!rc && !resc_lock_params.b_granted) { > + } > + > + if (!resc_lock_params.b_granted) { Can it be the case where the condition above is met and !rc is false? If so your patch seems to have changed the logic of this function. > DP_NOTICE(p_hwfn, > "Failed to acquire the resource lock for the resource allocation commands\n"); > return -EBUSY; > - } else { > - rc = qed_hw_set_soft_resc_size(p_hwfn, p_ptt); > - if (rc && rc != -EINVAL) { > + } > + > + rc = qed_hw_set_soft_resc_size(p_hwfn, p_ptt); > + if (rc) { > + if (rc != -EINVAL) { > DP_NOTICE(p_hwfn, > "Failed to set the max values of the soft resources\n"); > goto unlock_and_exit; > - } else if (rc == -EINVAL) { > - DP_INFO(p_hwfn, > - "Skip the max values setting of the soft resources since it is not supported by the MFW\n"); > - rc = qed_mcp_resc_unlock(p_hwfn, p_ptt, > - &resc_unlock_params); nit: it looks like the two lines above would now fit on one. > - if (rc) > - DP_INFO(p_hwfn, > - "Failed to release the resource lock for the resource allocation commands\n"); > } > + > + DP_INFO(p_hwfn, > + "Skip the max values setting of the soft resources since it is not supported by the MFW\n"); > + rc = qed_mcp_resc_unlock(p_hwfn, p_ptt, > + &resc_unlock_params); > + if (rc) > + DP_INFO(p_hwfn, > + "Failed to release the resource lock for the resource allocation commands\n"); > } > > rc = qed_hw_set_resc_info(p_hwfn); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: qed_dev: fix redundant check of rc and against -EINVAL 2021-10-20 8:47 ` Simon Horman @ 2021-10-21 7:36 ` Jεan Sacren 0 siblings, 0 replies; 8+ messages in thread From: Jεan Sacren @ 2021-10-21 7:36 UTC (permalink / raw) To: Simon Horman; +Cc: Ariel Elior, GR-everest-linux-l2, davem, kuba, netdev From: Simon Horman <horms@kernel.org> Date: Wed, 20 Oct 2021 10:47:17 +0200 > > On Tue, Oct 19, 2021 at 12:26:42AM -0600, Jεan Sacren wrote: > > From: Jean Sacren <sakiwit@gmail.com> > > > > We should first check rc alone and then check it against -EINVAL to > > avoid repeating the same operation multiple times. > > > > Signed-off-by: Jean Sacren <sakiwit@gmail.com> > > --- > > drivers/net/ethernet/qlogic/qed/qed_dev.c | 35 +++++++++++++---------- > > 1 file changed, 20 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c > > index 18f3bf7c4dfe..fe8bdb4523b5 100644 > > --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c > > +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c > > @@ -3987,30 +3987,35 @@ static int qed_hw_get_resc(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) > > QED_RESC_LOCK_RESC_ALLOC, false); > > > > rc = qed_mcp_resc_lock(p_hwfn, p_ptt, &resc_lock_params); > > - if (rc && rc != -EINVAL) { > > - return rc; > > - } else if (rc == -EINVAL) { > > + if (rc) { > > + if (rc != -EINVAL) > > + return rc; > > + > > DP_INFO(p_hwfn, > > "Skip the max values setting of the soft resources since the resource lock is not supported by the MFW\n"); > > - } else if (!rc && !resc_lock_params.b_granted) { > > + } > > + > > + if (!resc_lock_params.b_granted) { > > Can it be the case where the condition above is met and !rc is false? > If so your patch seems to have changed the logic of this function. Mr. Horman, I'm so much appreciative to you for the review. I'm so sorry this patch is wrong. I redid the patch. Could you please help me review it? I'll add the following text in the changelog to curb the confusion I incur. What do you think? We should also remove the check of !rc in this expression since it is always true: (!rc && !resc_lock_params.b_granted) [...] > > - rc = qed_mcp_resc_unlock(p_hwfn, p_ptt, > > - &resc_unlock_params); > > nit: it looks like the two lines above would now fit on one. Absolutely. I'll put these two lines together as one. I'd be very much grateful if you could help me review both patches. I'll respin the whole series and resubmit as v2 upon your review. Thank you! // diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c // index 18f3bf7c4dfe..44b0d4b42bc3 100644 // --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c // +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c // @@ -3987,29 +3987,33 @@ static int qed_hw_get_resc(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) // QED_RESC_LOCK_RESC_ALLOC, false); // // rc = qed_mcp_resc_lock(p_hwfn, p_ptt, &resc_lock_params); // - if (rc && rc != -EINVAL) { // - return rc; // - } else if (rc == -EINVAL) { // - DP_INFO(p_hwfn, // - "Skip the max values setting of the soft resources since the resource lock is not supported by the MFW\n"); // - } else if (!rc && !resc_lock_params.b_granted) { // - DP_NOTICE(p_hwfn, // - "Failed to acquire the resource lock for the resource allocation commands\n"); // - return -EBUSY; // + if (rc) { // + if (rc == -EINVAL) // + DP_INFO(p_hwfn, // + "Skip the max values setting of the soft resources since the resource lock is not supported by the MFW\n"); // + else // + return rc; // } else { // - rc = qed_hw_set_soft_resc_size(p_hwfn, p_ptt); // - if (rc && rc != -EINVAL) { // + if (!resc_lock_params.b_granted) { // DP_NOTICE(p_hwfn, // - "Failed to set the max values of the soft resources\n"); // - goto unlock_and_exit; // - } else if (rc == -EINVAL) { // - DP_INFO(p_hwfn, // - "Skip the max values setting of the soft resources since it is not supported by the MFW\n"); // - rc = qed_mcp_resc_unlock(p_hwfn, p_ptt, // - &resc_unlock_params); // - if (rc) // + "Failed to acquire the resource lock for the resource allocation commands\n"); // + return -EBUSY; // + } // + // + rc = qed_hw_set_soft_resc_size(p_hwfn, p_ptt); // + if (rc) { // + if (rc == -EINVAL) { // DP_INFO(p_hwfn, // - "Failed to release the resource lock for the resource allocation commands\n"); // + "Skip the max values setting of the soft resources since it is not supported by the MFW\n"); // + rc = qed_mcp_resc_unlock(p_hwfn, p_ptt, &resc_unlock_params); // + if (rc) // + DP_INFO(p_hwfn, // + "Failed to release the resource lock for the resource allocation commands\n"); // + } else { // + DP_NOTICE(p_hwfn, // + "Failed to set the max values of the soft resources\n"); // + goto unlock_and_exit; // + } // } // } // ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-10-21 10:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-19 6:26 [PATCH net-next 0/2] Small fixes for redundant checks Jεan Sacren 2021-10-19 6:26 ` [PATCH net-next 1/2] net: qed_ptp: fix redundant check of rc and against -EINVAL Jεan Sacren 2021-10-20 8:48 ` Simon Horman 2021-10-21 7:35 ` Jεan Sacren 2021-10-21 10:46 ` Simon Horman 2021-10-19 6:26 ` [PATCH net-next 2/2] net: qed_dev: " Jεan Sacren 2021-10-20 8:47 ` Simon Horman 2021-10-21 7:36 ` Jεan Sacren
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.