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@xxxxxxxxxxxxxx> 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/ |
Attachment:
signature.asc
Description: PGP signature
