Re: [PATCH v2 5/5] misc/pvpanic: Make 'pvpanic_probe()' resource managed

[Christophe JAILLET]

Le 22/05/2021 à 12:09, Andy Shevchenko a écrit :
> On Sat, May 22, 2021 at 1:06 PM Andy Shevchenko
> <andy.shevchenko@xxxxxxxxx> wrote:
> > On Sat, May 22, 2021 at 9:56 AM Christophe JAILLET
> > <christophe.jaillet@xxxxxxxxxx> wrote:
>
> Hmm... Couple of (minor) comments though.
>
> > > Simplify code and turn 'pvpanic_probe()' into a managed resource version.
> > > This simplify callers that don't need to do some clean-up on error in the
>
> simplifies
> errors

Ok

> > > probe and on remove.
> > >
> > > Update pvpanic-mmio.c and pvpanic-pci.c accordingly.
> > >
> > > 'pvpanic_remove()' don't need to be exported anymore.
> >
> > LGTM, thanks!
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> >
> > > Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> > > Suggested-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>
> ...
>
> > > +static void pvpanic_remove(void *param)
> > >   {
> > >          struct pvpanic_instance *pi_cur, *pi_next;
> > > +       struct pvpanic_instance *pi = param;
>
> > >          if (!pi)
> > >                  return;
>
> Looking at this I'm wondering why it's not a dead code.
Agreed.

I'll send a v3, but my turn to nitpick now:

   Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
   Suggested-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

Which one, should I use?
I guess the later.

CJ