Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-acpi@vger.kernel.org, linux-gpio@vger.kernel.org,
	stable@vger.kernel.org,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Bob Moore <robert.moore@intel.com>,
	Erik Kaneda <erik.kaneda@intel.com>
Subject: Re: [PATCH v2] pinctrl: cherryview: Ensure _REG(ACPI_ADR_SPACE_GPIO, 1) gets called
Date: Wed, 6 May 2020 09:40:57 +0300
Message-ID: <20200506064057.GU487496@lahna.fi.intel.com> (raw)
In-Reply-To: <20200504145957.480418-1-hdegoede@redhat.com>

+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@vger.kernel.org
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> 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

  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-04 14:59 Hans de Goede
2020-05-06  6:40 ` Mika Westerberg [this message]
2020-05-07 10:15   ` Hans de Goede
2020-05-07 12:30     ` Mika Westerberg
2020-05-07 12:39       ` Hans de Goede
2020-05-07 13:39         ` Andy Shevchenko
2020-10-08  9:31       ` Hans de Goede
2020-10-08 14:44         ` Mika Westerberg
2020-10-08 15:37           ` Hans de Goede
2020-10-08 15:52             ` Mika Westerberg
2020-10-08 17:39               ` Hans de Goede
2020-05-06 23:42 ` Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200506064057.GU487496@lahna.fi.intel.com \
    --to=mika.westerberg@linux.intel.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=erik.kaneda@intel.com \
    --cc=hdegoede@redhat.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=robert.moore@intel.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git