Re: [PATCH v3 2/6] ACPI / x86: Consolidate Apple DMI checks

["Rafael J. Wysocki" <rafael@xxxxxxxxxx>]

On Thu, Jul 20, 2017 at 4:03 PM, Lukas Wunner <lukas@xxxxxxxxx> wrote:
> On Sat, Jul 15, 2017 at 12:03:31AM +0200, Rafael J. Wysocki wrote:
>> On Friday, July 14, 2017 12:36:19 AM Lukas Wunner wrote:
>> > --- a/drivers/acpi/osi.c
>> > +++ b/drivers/acpi/osi.c
>> > @@ -258,12 +258,16 @@ bool acpi_osi_is_win8(void)
>> >  EXPORT_SYMBOL(acpi_osi_is_win8);
>> >
>> >  #ifdef CONFIG_X86
>> > +bool is_apple_system;
>> > +EXPORT_SYMBOL(is_apple_system);
>>
>> Maybe prepend the name of this variable with acpi_ to indicate that this is
>> ACPI-specific.
>
> It's not really ACPI-specific, osi.c just happens to be the best place
> to set the variable because the acpi_osi_dmi_table[] is checked very
> early during boot.  So early in fact, that I could even replace the
> existing Apple DMI check in arch/x86/kernel/early-quirks.c with
> "is_apple_system".

OK

> These non-ACPI files currently contain an Apple DMI check:
> arch/x86/kernel/early-quirks.c
> drivers/pci/quirks.c (2x)
> drivers/firmware/efi/apple-properties.c
> drivers/thunderbolt/tb.c
> drivers/thunderbolt/icm.c
>
> The latter three do not #include <linux/acpi.h> yet.  Somehow it feels
> odd to include that header to check for presence of an Apple system
> because that's not really ACPI-related.
>
> I guess I could introduce a new <linux/apple.h> but I hate the insane
> proliferation of additional files in include/linux/.

There is include/linux/platform_data/x86/ so maybe put it in there?

>  I could merge the contents of apple_bl.h and apple-gmux.h into that new header to
> reduce the number of files a bit.
>
> Struggling to find a solution that's nice and clean.  Any ideas?

I guess you still want it to work if someone configures the kernel
without CONFIG_ACPI, although that's slightly debatable, so the
variable should be defined somewhere in the arch code I suppose.

I also guess you could add something like arch/x86/platform/apple/ and
put the checks and the variable in there (in which case I'd call it
x86_apple_machine or similar).

Then invoke the check from acpi_osi_dmi_table[] code and the
early-quirks.c one is only needed for !CONFIG_ACPI.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html