[PATCH v3 39/57] tpm: cr50: Add ACPI support

[Bin Meng]

Hi Simon,

On Mon, Sep 21, 2020 at 7:51 PM Andy Shevchenko
<andriy.shevchenko at linux.intel.com> 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() ?
>

Will you address Andy's comments by adding documentation in the next version?

Regards,
Bin