+Rafael and ACPICA folks.
On Mon, May 04, 2020 at 04:59:57PM +0200, Hans de Goede wrote:
> On Cherry Trail devices there are 2 possible ACPI OpRegions for
> accessing GPIOs. The standard GeneralPurposeIo OpRegion and the Cherry
> Trail specific UserDefined 0x9X OpRegions.
>
> Having 2 different types of OpRegions leads to potential issues with
> checks for OpRegion availability, or in other words checks if _REG has
> been called for the OpRegion which the ACPI code wants to use.
>
> The ACPICA core does not call _REG on an ACPI node which does not
> define an OpRegion matching the type being registered; and the reference
> design DSDT, from which most Cherry Trail DSDTs are derived, does not
> define GeneralPurposeIo, nor UserDefined(0x93) OpRegions for the GPO2
> (UID 3) device, because no pins were assigned ACPI controlled functions
> in the reference design.
>
> Together this leads to the perfect storm, at least on the Cherry Trail
> based Medion Akayo E1239T. This design does use a GPO2 pin from its ACPI
> code and has added the Cherry Trail specific UserDefined(0x93) opregion
> to its GPO2 ACPI node to access this pin.
>
> But it uses a has _REG been called availability check for the standard
> GeneralPurposeIo OpRegion. This clearly is a bug in the DSDT, but this
> does work under Windows.
Do we know why this works under Windows? I mean if possible we should do
the same and I kind of suspect that they forcibly call _REG in their
GPIO driver.
Are the ACPI tables from this system available somewhere?
> This issue leads to the intel_vbtn driver
> reporting the device always being in tablet-mode at boot, even if it
> is in laptop mode. Which in turn causes userspace to ignore touchpad
> events. So iow this issues causes the touchpad to not work at boot.
>
> Since the bug in the DSDT stems from the confusion of having 2 different
> OpRegion types for accessing GPIOs on Cherry Trail devices, I believe
> that this is best fixed inside the Cherryview pinctrl driver.
>
> This commit adds a workaround to the Cherryview pinctrl driver so
> that the DSDT's expectations of _REG always getting called for the
> GeneralPurposeIo OpRegion are met.
I would like to understand the issue bit better before we do this.
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> Changes in v2:
> - Drop unnecessary if (acpi_has_method(adev->handle, "_REG")) check
> - Fix Cherryview spelling in the commit message
> ---
> drivers/pinctrl/intel/pinctrl-cherryview.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
> index 4c74fdde576d..4817aec114d6 100644
> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
> @@ -1693,6 +1693,8 @@ static acpi_status chv_pinctrl_mmio_access_handler(u32 function,
>
> static int chv_pinctrl_probe(struct platform_device *pdev)
> {
> + struct acpi_object_list input;
> + union acpi_object params[2];
> struct chv_pinctrl *pctrl;
> struct acpi_device *adev;
> acpi_status status;
> @@ -1755,6 +1757,22 @@ static int chv_pinctrl_probe(struct platform_device *pdev)
> if (ACPI_FAILURE(status))
> dev_err(&pdev->dev, "failed to install ACPI addr space handler\n");
>
> + /*
> + * Some DSDT-s use the chv_pinctrl_mmio_access_handler while checking
> + * for the regular GeneralPurposeIo OpRegion availability, mixed with
> + * the DSDT not defining a GeneralPurposeIo OpRegion at all. In this
> + * case the ACPICA code will not call _REG to signal availability of
> + * the GeneralPurposeIo OpRegion. Manually call _REG here so that
> + * the DSDT-s GeneralPurposeIo availability checks will succeed.
> + */
> + params[0].type = ACPI_TYPE_INTEGER;
> + params[0].integer.value = ACPI_ADR_SPACE_GPIO;
> + params[1].type = ACPI_TYPE_INTEGER;
> + params[1].integer.value = 1;
> + input.count = 2;
> + input.pointer = params;
> + acpi_evaluate_object(adev->handle, "_REG", &input, NULL);
> +
> platform_set_drvdata(pdev, pctrl);
>
> return 0;
> --
> 2.26.0
