Hi Andy, On Thu, 29 Oct 2020 at 06:22, Andy Shevchenko <andriy.shevche...@linux.intel.com> wrote: > > On Tue, Sep 22, 2020 at 12:45:43PM -0600, Simon Glass wrote: > > Add some documentation provided by Andy Shevchenko to describe how to > > use struct acpi_gpio. > > Thanks! > > I see Bin already applied this, perhaps follow up fix is needed. See below. > > ... > > > + * Note that GpioIo doesn't have any means of Active Low / High setting, > > so a > > GpioIo -> GpioIo() > > > + * _DSD must be provided to mitigate this. > > Plus the following: > > "This parameter does not make sense for GpioInt() since it has its own means > to define it." > > > + * GpioIo doesn't properly communicate the initial state of the output pin, > > GpioIo -> GpioIo() > > > + * 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 > > Should be read: > > " * Down High as low, assuming non-active" > > > + * Up Low as high, assuming non-active > > + * Up High as high, assuming active > > + * > > + * 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). >
Thanks, I sent a patch for these. Regards, Simon