From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Mon, 21 Sep 2020 14:53:34 +0300
Subject: [PATCH v3 39/57] tpm: cr50: Add ACPI support
In-Reply-To: <20200921115055.GQ3956970@smile.fi.intel.com>
References: <20200906214405.71632-1-sjg@chromium.org>
 <20200906154340.v3.39.I1a9174fd9af26d0891e4ccfac247c7359b28b23e@changeid>
 <20200921115055.GQ3956970@smile.fi.intel.com>
Message-ID: <20200921115334.GR3956970@smile.fi.intel.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Mon, Sep 21, 2020 at 02:50:56PM +0300, Andy Shevchenko wrote:
> On Sun, Sep 06, 2020 at 03:43:47PM -0600, Simon Glass wrote:
> > Generate ACPI information for this device so that Linux can use it
> > correctly.
> 
> > +	ret = acpi_device_write_interrupt_or_gpio(ctx, (struct udevice *)dev,
> > +						  "ready-gpios");
> > +	if (ret < 0)
> > +		return log_msg_ret("irq_gpio", ret);
> 
> I looked a bit closer at the acpi_table.c and would like to emphasize on
> lessons learn from BIOS mistakes found in the wild with GPIOs.
> 
> Because GPIO resources are quite badly described in ACPI (it seems MS failed to
> deliver GPIO abstraction to ACPI and to Windows API), there are some corner
> cases, in order to mitigate which we need to consider the following to avoid
> potential glitches and misconfiguration:
> 
> - GpioIo() doesn't have any means of Active Low / High setting, the _DSD must
>   be provided to mitigate this.
> 
> - GpioIo() doesn't properly communicate the initial state of the output pin,
>   thus Linux assumes the simple rule:
> 
>   Pull Bias	  Polarity	Requested...
> 
>   Implicit	  x		AS IS (assumed firmware configured for us)
>   Explicit	  x (no _DSD)	as Pull Bias (Up == High, Down == Low),
>   		  		assuming non-active (Polarity = !Pull Bias)
> 
>   Down		  Low		as low, assuming active
>   Down		  High		as high, assuming non-active
>   Up		  Low		as low, assuming non-active
>   Up		  High		as high, assuming active
> 
> Hopefully this helps (and maybe can be added to some documentation).
> 
> P.S. Why I2cSerialBus() and not I2cSerialBusV2() ?

Forgot one important aspect.

GpioIo() can be used as interrupt and in this case the IoRestriction mustn't be
OutputOnly. It also requires active_low flag from _DSD in cases where it's
needed (better to always provide than rely on above assumption made on OS level).

-- 
With Best Regards,
Andy Shevchenko