Re: [PATCH 2/3] platform/x86: apple-gmux: Add apple_gmux_detect() helper

[Lukas Wunner <lukas@wunner.de>]

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