Re: [PATCH v1 4/7] ACPI: scan: Combine two conditionals in acpi_bus_attach()

On Tue, Sep 1, 2026 at 10:40 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Mon, Aug 31, 2026 at 07:59:32PM +0200, Rafael J. Wysocki wrote:
>
> > There are two conditionals in acpi_bus_attach() that can be combined,
> > which slightly reduces the overhead and makes the code a bit easier
> > to follow, so do that.
> >
> > No intentional functional impact.
>
> ...
>
> > -     if (ret > 0 && !device->flags.enumeration_by_parent) {
> > +     if (!device->flags.enumeration_by_parent && (ret > 0 ||
> > +         (!device->pnp.type.platform_id && !device->pnp.type.backlight)))
> >               acpi_device_set_enumerated(device);
> > -             goto ok;
> > -     }
> > -
> > -     if (device->pnp.type.platform_id || device->pnp.type.backlight ||
> > -         device->flags.enumeration_by_parent)
> > -             acpi_default_enumeration(device);
> >       else
> > -             acpi_device_set_enumerated(device);
> > +             acpi_default_enumeration(device);
>
> I would leave a longer line (having logical split)
>
>         if (!device->flags.enumeration_by_parent &&
>             (ret > 0 || (!device->pnp.type.platform_id && !device->pnp.type.backlight)))
>                 acpi_device_set_enumerated(device);
>         else
>                 acpi_default_enumeration(device);
>
> Or even going further and cleaning too many negations (if I'm not mistaken in
> the logic)
>
>         if (device->flags.enumeration_by_parent ||
> // not sure what the expected ret values here, maybe < 0 or == 0 part is not needed

!ret should be fine.

>             (ret <= 0 && (device->pnp.type.platform_id || device->pnp.type.backlight)))
>                 acpi_default_enumeration(device);
>         else
>                 acpi_device_set_enumerated(device);

Yes, it looks better this way.