Messages in this thread | | | Date | Tue, 30 Jun 2020 10:10:20 +0100 | From | Lee Jones <> | Subject | Re: [PATCH 17/20] misc: genwqe: card_base: Do not pass unused argument 'fatal_err' |
| |
On Tue, 30 Jun 2020, haver wrote:
> On 2020-06-29 16:04, Lee Jones wrote: > > 'fatal_err' is taken as an argument to a static function which is only > > invoked once. During this invocation 'fatal_err' is not set. There > > doesn't appear to be a good reason to keep it around. > > > > Also fixes the following W=1 kernel build warning: > > > > drivers/misc/genwqe/card_base.c:588: warning: Function parameter or > > member 'fatal_err' not described in 'genwqe_recover_card' > > > > Cc: Michael Jung <mijung@gmx.net> > > Cc: Michael Ruettger <michael@ibmra.de> > > Cc: Frank Haverkamp <haver@linux.ibm.com> > > Cc: Joerg-Stephan Vogt <jsvogt@de.ibm.com> > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > --- > > drivers/misc/genwqe/card_base.c | 18 +++--------------- > > 1 file changed, 3 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/misc/genwqe/card_base.c > > b/drivers/misc/genwqe/card_base.c > > index bceebf49de2d5..809a6f46f6de3 100644 > > --- a/drivers/misc/genwqe/card_base.c > > +++ b/drivers/misc/genwqe/card_base.c > > @@ -569,30 +569,18 @@ static int genwqe_stop(struct genwqe_dev *cd) > > > > /** > > * genwqe_recover_card() - Try to recover the card if it is possible > > - * > > - * If fatal_err is set no register access is possible anymore. It is > > - * likely that genwqe_start fails in that situation. Proper error > > - * handling is required in this case. > > + * @cd: GenWQE device information > > * > > * genwqe_bus_reset() will cause the pci code to call genwqe_remove() > > * and later genwqe_probe() for all virtual functions. > > */ > > -static int genwqe_recover_card(struct genwqe_dev *cd, int fatal_err) > > +static int genwqe_recover_card(struct genwqe_dev *cd) > > { > > int rc; > > struct pci_dev *pci_dev = cd->pci_dev; > > > > genwqe_stop(cd); > > > > - /* > > - * Make sure chip is not reloaded to maintain FFDC. Write SLU > > - * Reset Register, CPLDReset field to 0. > > - */ > > - if (!fatal_err) { > > - cd->softreset = 0x70ull; > > - __genwqe_writeq(cd, IO_SLC_CFGREG_SOFTRESET, cd->softreset); > > - } > > - > > rc = genwqe_bus_reset(cd); > > if (rc != 0) { > > dev_err(&pci_dev->dev, > > @@ -958,7 +946,7 @@ static int genwqe_health_thread(void *data) > > > > cd->card_state = GENWQE_CARD_FATAL_ERROR; > > > > - rc = genwqe_recover_card(cd, 0); > > + rc = genwqe_recover_card(cd); > > if (rc < 0) { > > /* FIXME Card is unusable and needs unbind! */ > > goto fatal_error; > > I think this one I want to keep. Since fatal_err is 0, !fatal_error is 1 and > the register write is actually executed.
Ah yes, good catch.
What if we *always* did instead then?
> I also want to keep the parameter even though it is only used with 0. The > register bit causes a less drastic recovery, but if we would discover that > that is not working ok, we rather discard the debug data we get in this case > instead of letting the recovery fail. I think it does not hurt to keep it.
I'm not 100% against it, but it is dead code.
> Maybe you can add a comment?
If you really want to keep it, I can just update the kerneldoc instead.
-- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
| |