[PATCH v2] firmware: arm_sdei: Make sdei_unregister_ghes() return void

[Uwe Kleine-König]

Hello Rafael,

On Wed, Dec 21, 2022 at 02:53:05PM +0100, Rafael J. Wysocki wrote:
> On Tue, Dec 20, 2022 at 4:45 PM Uwe Kleine-König
> <u.kleine-koenig at pengutronix.de> wrote:
> > [...]
> > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> > index 066dc1f5c235..7d705930e21b 100644
> > --- a/drivers/acpi/apei/ghes.c
> > +++ b/drivers/acpi/apei/ghes.c
> > @@ -1275,12 +1275,20 @@ static int apei_sdei_register_ghes(struct ghes *ghes)
> >                                  ghes_sdei_critical_callback);
> >  }
> >
> > -static int apei_sdei_unregister_ghes(struct ghes *ghes)
> > +static void apei_sdei_unregister_ghes(struct ghes *ghes)
> >  {
> > +       /*
> > +        * If CONFIG_ARM_SDE_INTERFACE isn't enabled apei_sdei_register_ghes()
> > +        * cannot have been called successfully. So ghes_remove() won't be
> > +        * called because either ghes_probe() failed or the notify type isn't
> > +        * ACPI_HEST_NOTIFY_SOFTWARE_DELEGATED.
> > +        * Note the if statement below is necessary to prevent a linker error as
> > +        * the compiler has no chance to understand the above correlation.
> > +        */
> >         if (!IS_ENABLED(CONFIG_ARM_SDE_INTERFACE))
> > -               return -EOPNOTSUPP;
> > +               BUG();
> 
> Well, you could just provide an empty stub for the !CONFIG_ARM_SDE_INTERFACE.
> 
> It would be cleaner and probably fewer lines of code too.

It's you who cares for this code, but I'd prefer my option. If we assume
the describing comment would have a similar length, we're saving 3 or
four lines of code here but need 3 lines for the #if / #else / #endif
plus the stub definition. And compared to my suggested solution we don't
catch someone introducing a (bogus) call to apei_sdei_unregister_ghes()
(or sdei_unregister_ghes()). And (again IMHO) two different
implementations are harder to grasp than a single with an if.

If you don't like the BUG, a plain return is in my eyes the next best
option which is semantically equivalent to an empty stub.

If you still like the stub better (or a return instead of the BUG), I
can send a v3, just tell me your preference.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20221221/930c2008/attachment-0001.sig>