On Mon, Jan 23, 2023 at 03:13:28PM +0100, Hans de Goede wrote:
> On 1/23/23 14:49, Lukas Wunner wrote:
> > On Mon, Jan 23, 2023 at 12:37:49PM +0100, Hans de Goede wrote:
> > > --- a/include/linux/apple-gmux.h
> > > +++ b/include/linux/apple-gmux.h
> > [...]
> > > +static inline bool apple_gmux_is_indexed(unsigned long iostart)
> > > +{
> > > + u16 val;
> > > +
> > > + outb(0xaa, iostart + 0xcc);
> > > + outb(0x55, iostart + 0xcd);
> > > + outb(0x00, iostart + 0xce);
> > > +
> > > + val = inb(iostart + 0xcc) | (inb(iostart + 0xcd) << 8);
> > > + if (val == 0x55aa)
> > > + return true;
> > > +
> > > + return false;
> > > +}
> >
> > Something like this, and especially the large apple_gmux_detect() below,
> > should not live in a header file.
>
> I understand where you are coming from, but these functions really
> are not that large.
>
> > Why can't apple_gmux.ko just export a detection function which is used
> > both internally and as a helper by the backlight detection?
>
> Both the acpi-video code and the apple-gmux code can be built as
> modules. So this will break if the acpi-video code get builtin
> and the apple-gmux code does not.
>
> This can be worked around in Kconfig by adding something like:
>
> depends on APPLE_GMUX || APPLE_GMUX=n
>
> to the ACPI_VIDEO Kconfig bits and then cross our fingers
> we don't get some Kconfig circular dep thing causing things
> to error out.
Can we force APPLE_GMUX to be built-in if ACPI_VIDEO is?
Would making APPLE_GMUX depend on ACPI_VIDEO (instead of
"ACPI_VIDEO=n || ACPI_VIDEO") achieve that? I believe
APPLE_GMUX would then inherit the setting of ACPI_VIDEO?
Thanks,
Lukas
