Messages in this thread | | | Date | Tue, 31 Mar 2020 00:42:13 +0200 | From | Stefano Brivio <> | Subject | Re: [PATCH v7] staging: vt6656: add error code handling to unused variable |
| |
On Mon, 30 Mar 2020 15:37:18 -0700 "John B. Wyatt IV" <jbwyatt4@gmail.com> wrote:
> Add error code handling to unused 'ret' variable that was never used. > Return an error code from functions called within vnt_radio_power_on. > > Issue reported by coccinelle (coccicheck). > > Suggested-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk> > Suggested-by: Stefano Brivio <sbrivio@redhat.com> > Reviewed-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk> > Signed-off-by: John B. Wyatt IV <jbwyatt4@gmail.com> > --- > v7: Move an if check. > Suggested by Stefano Brivio <sbrivio@redhat.com> > > v6: Forgot to add all the v5 code to commit. > > v5: Remove Suggested-by: Julia Lawall above seperator line. > Remove break; statement in switch block. > break; removal checked by both gcc compile and checkpatch. > Suggested by Stefano Brivio <sbrivio@redhat.com> > > v4: Move Suggested-by: Julia Lawall above seperator line. > Add Reviewed-by tag as requested by Quentin Deslandes. > > v3: Forgot to add v2 code changes to commit. > > v2: Replace goto statements with return. > Remove last if check because it was unneeded. > Suggested-by: Julia Lawall <julia.lawall@inria.fr> > > drivers/staging/vt6656/card.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c > index dc3ab10eb630..df12c73097e0 100644 > --- a/drivers/staging/vt6656/card.c > +++ b/drivers/staging/vt6656/card.c > @@ -723,9 +723,13 @@ int vnt_radio_power_on(struct vnt_private *priv) > { > int ret = 0; > > - vnt_exit_deep_sleep(priv); > + ret = vnt_exit_deep_sleep(priv); > + if (ret) > + return ret; > > - vnt_mac_reg_bits_on(priv, MAC_REG_HOSTCR, HOSTCR_RXON); > + ret = vnt_mac_reg_bits_on(priv, MAC_REG_HOSTCR, HOSTCR_RXON); > + if (ret) > + return ret; > > switch (priv->rf_type) { > case RF_AL2230: > @@ -734,14 +738,14 @@ int vnt_radio_power_on(struct vnt_private *priv) > case RF_VT3226: > case RF_VT3226D0: > case RF_VT3342A0: > - vnt_mac_reg_bits_on(priv, MAC_REG_SOFTPWRCTL, > - (SOFTPWRCTL_SWPE2 | SOFTPWRCTL_SWPE3)); > - break; > + ret = vnt_mac_reg_bits_on(priv, MAC_REG_SOFTPWRCTL, > + (SOFTPWRCTL_SWPE2 | > + SOFTPWRCTL_SWPE3));
Grrr, sorry, almost there, I didn't notice this: SOFTPWRCTL_SWPE3 needs to be aligned *after* the open (useless) parenthesis:
(SOFTPWRCTL_SWPE2 | SOFTPWRCTL_SWPE3));
because it's another operand of the | operation surrounded by (). Doesn't checkpatch warn?
The rest looks good to me.
-- Stefano
| |