On Fri, 10 Feb 2023 20:43:58 +0100
Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> Hi,
>
> On 2/10/23 05:48, Orlando Chamberlain wrote:
> > This is needed for interrupts to be cleared correctly on MMIO based
> > gmux's. It is untested if this helps/hinders other gmux types, but I
> > have seen the GMSP method in the acpi tables of a MacBook with an
> > indexed gmux.
> >
> > If this turns out to break support for older gmux's, this can
> > instead be only done on MMIO gmux's.
> >
> > There is also a "GMLV" acpi method, and the "GMSP" method can be
> > called with 1 as its argument, but the purposes of these aren't
> > known and they don't seem to be needed.
> >
> > Signed-off-by: Orlando Chamberlain <orlandoch.dev@xxxxxxxxx>
> > ---
> > drivers/platform/x86/apple-gmux.c | 26 +++++++++++++++++++++++++-
> > 1 file changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/apple-gmux.c
> > b/drivers/platform/x86/apple-gmux.c index
> > 760434a527c1..c605f036ea0b 100644 ---
> > a/drivers/platform/x86/apple-gmux.c +++
> > b/drivers/platform/x86/apple-gmux.c @@ -494,8 +494,29 @@ static
> > const struct apple_gmux_config apple_gmux_index = {
> > * MCP79, on all following generations it's GPIO pin 6 of the
> > Intel PCH.
> > * The GPE merely signals that an interrupt occurred, the actual
> > type of event
> > * is identified by reading a gmux register.
> > + *
> > + * On MMIO gmux's, we also need to call the acpi method GMSP to
> > properly clear
> > + * interrupts. TODO: Do other types need this? Does this break
> > other types? */
> >
> > +static int gmux_call_acpi_gmsp(struct apple_gmux_data *gmux_data,
> > int arg) +{
> > + acpi_status status = AE_OK;
> > + union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> > + struct acpi_object_list arg_list = { 1, &arg0 };
> > +
> > + arg0.integer.value = arg;
> > +
> > + status = acpi_evaluate_object(gmux_data->dhandle, "GMSP",
> > &arg_list, NULL);
> > + if (ACPI_FAILURE(status)) {
> > + pr_err("GMSP call failed: %s\n",
> > + acpi_format_exception(status));
> > + return -ENODEV;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static inline void gmux_disable_interrupts(struct apple_gmux_data
> > *gmux_data) {
> > gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_ENABLE,
> > @@ -519,7 +540,10 @@ static void gmux_clear_interrupts(struct
> > apple_gmux_data *gmux_data)
> > /* to clear interrupts write back current status */
> > status = gmux_interrupt_get_status(gmux_data);
> > - gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_STATUS, status);
> > + if (status) {
> > + gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_STATUS,
> > status);
> > + gmux_call_acpi_gmsp(gmux_data, 0);
>
> Ugh no, please don't go around calling random ACPI methods from
> untested firmware revisions / device models.
>
> ACPI code (even Apple's I have learned) tends to be full of bugs. If
> we did not need to call GMSP before then please lets keep not calling
> it on the older models. Just because it is there does not mean that
> calling it is useful, it might even be harmful.
I'll make it only use this ACPI method on MMIO gmux's in v2 then.
>
> Regards,
>
> Hans
>
>
>
>
>
>
> > + }
> > }
> >
> > static void gmux_notify_handler(acpi_handle device, u32 value,
> > void *context)
>
