Re: [PATCH v4 08/63] Documentation: ACPI: move gpio-properties.txt to firmware-guide/acpi and convert to reST

[Changbin Du <changbin.du@xxxxxxxxx>]

On Tue, Apr 23, 2019 at 05:55:15PM -0300, Mauro Carvalho Chehab wrote:
> Em Wed, 24 Apr 2019 00:28:37 +0800
> Changbin Du <changbin.du@xxxxxxxxx> escreveu:
> 
> > This converts the plain text documentation to reStructuredText format and
> > add it to Sphinx TOC tree. No essential content change.
> > 
> > Signed-off-by: Changbin Du <changbin.du@xxxxxxxxx>
> > ---
> >  .../acpi/gpio-properties.rst}                 | 78 +++++++++++--------
> >  Documentation/firmware-guide/acpi/index.rst   |  1 +
> >  MAINTAINERS                                   |  2 +-
> >  3 files changed, 46 insertions(+), 35 deletions(-)
> >  rename Documentation/{acpi/gpio-properties.txt => firmware-guide/acpi/gpio-properties.rst} (81%)
> > 
> > diff --git a/Documentation/acpi/gpio-properties.txt b/Documentation/firmware-guide/acpi/gpio-properties.rst
> > similarity index 81%
> > rename from Documentation/acpi/gpio-properties.txt
> > rename to Documentation/firmware-guide/acpi/gpio-properties.rst
> > index 88c65cb5bf0a..89c636963544 100644
> > --- a/Documentation/acpi/gpio-properties.txt
> > +++ b/Documentation/firmware-guide/acpi/gpio-properties.rst
> > @@ -1,5 +1,8 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +======================================
> >  _DSD Device Properties Related to GPIO
> > ---------------------------------------
> > +======================================
> >  
> >  With the release of ACPI 5.1, the _DSD configuration object finally
> >  allows names to be given to GPIOs (and other things as well) returned
> > @@ -8,7 +11,7 @@ the corresponding GPIO, which is pretty error prone (it depends on
> >  the _CRS output ordering, for example).
> >  
> >  With _DSD we can now query GPIOs using a name instead of an integer
> > -index, like the ASL example below shows:
> > +index, like the ASL example below shows::
> >  
> >    // Bluetooth device with reset and shutdown GPIOs
> >    Device (BTH)
> > @@ -34,15 +37,19 @@ index, like the ASL example below shows:
> >        })
> >    }
> >  
> > -The format of the supported GPIO property is:
> > +The format of the supported GPIO property is::
> >  
> >    Package () { "name", Package () { ref, index, pin, active_low }}
> >  
> > -  ref - The device that has _CRS containing GpioIo()/GpioInt() resources,
> > -        typically this is the device itself (BTH in our case).
> > -  index - Index of the GpioIo()/GpioInt() resource in _CRS starting from zero.
> > -  pin - Pin in the GpioIo()/GpioInt() resource. Typically this is zero.
> > -  active_low - If 1 the GPIO is marked as active_low.
> > +ref
> > +  The device that has _CRS containing GpioIo()/GpioInt() resources,
> > +  typically this is the device itself (BTH in our case).
> > +index
> > +  Index of the GpioIo()/GpioInt() resource in _CRS starting from zero.
> > +pin
> > +  Pin in the GpioIo()/GpioInt() resource. Typically this is zero.
> > +active_low
> > +  If 1 the GPIO is marked as active_low.
> >  
> >  Since ACPI GpioIo() resource does not have a field saying whether it is
> >  active low or high, the "active_low" argument can be used here.  Setting
> > @@ -55,7 +62,7 @@ It is possible to leave holes in the array of GPIOs. This is useful in
> >  cases like with SPI host controllers where some chip selects may be
> >  implemented as GPIOs and some as native signals. For example a SPI host
> >  controller can have chip selects 0 and 2 implemented as GPIOs and 1 as
> > -native:
> > +native::
> >  
> >    Package () {
> >        "cs-gpios",
> > @@ -67,7 +74,7 @@ native:
> >    }
> >  
> >  Other supported properties
> > ---------------------------
> > +==========================
> >  
> >  Following Device Tree compatible device properties are also supported by
> >  _DSD device properties for GPIO controllers:
> > @@ -78,7 +85,7 @@ _DSD device properties for GPIO controllers:
> >  - input
> >  - line-name
> >  
> > -Example:
> > +Example::
> >  
> >    Name (_DSD, Package () {
> >        // _DSD Hierarchical Properties Extension UUID
> > @@ -100,7 +107,7 @@ Example:
> >  
> >  - gpio-line-names
> >  
> > -Example:
> > +Example::
> >  
> >    Package () {
> >        "gpio-line-names",
> > @@ -114,7 +121,7 @@ See Documentation/devicetree/bindings/gpio/gpio.txt for more information
> >  about these properties.
> >  
> >  ACPI GPIO Mappings Provided by Drivers
> > ---------------------------------------
> > +======================================
> >  
> >  There are systems in which the ACPI tables do not contain _DSD but provide _CRS
> >  with GpioIo()/GpioInt() resources and device drivers still need to work with
> > @@ -139,16 +146,16 @@ line in that resource starting from zero, and the active-low flag for that line,
> >  respectively, in analogy with the _DSD GPIO property format specified above.
> >  
> >  For the example Bluetooth device discussed previously the data structures in
> > -question would look like this:
> > +question would look like this::
> >  
> > -static const struct acpi_gpio_params reset_gpio = { 1, 1, false };
> > -static const struct acpi_gpio_params shutdown_gpio = { 0, 0, false };
> > +  static const struct acpi_gpio_params reset_gpio = { 1, 1, false };
> > +  static const struct acpi_gpio_params shutdown_gpio = { 0, 0, false };
> >  
> > -static const struct acpi_gpio_mapping bluetooth_acpi_gpios[] = {
> > -  { "reset-gpios", &reset_gpio, 1 },
> > -  { "shutdown-gpios", &shutdown_gpio, 1 },
> > -  { },
> > -};
> > +  static const struct acpi_gpio_mapping bluetooth_acpi_gpios[] = {
> > +    { "reset-gpios", &reset_gpio, 1 },
> > +    { "shutdown-gpios", &shutdown_gpio, 1 },
> > +    { },
> > +  };
> >  
> >  Next, the mapping table needs to be passed as the second argument to
> >  acpi_dev_add_driver_gpios() that will register it with the ACPI device object
> > @@ -158,12 +165,12 @@ calling acpi_dev_remove_driver_gpios() on the ACPI device object where that
> >  table was previously registered.
> >  
> >  Using the _CRS fallback
> > ------------------------
> > +=======================
> >  
> >  If a device does not have _DSD or the driver does not create ACPI GPIO
> >  mapping, the Linux GPIO framework refuses to return any GPIOs. This is
> >  because the driver does not know what it actually gets. For example if we
> > -have a device like below:
> > +have a device like below::
> >  
> >    Device (BTH)
> >    {
> > @@ -177,7 +184,7 @@ have a device like below:
> >        })
> >    }
> >  
> > -The driver might expect to get the right GPIO when it does:
> > +The driver might expect to get the right GPIO when it does:：
> 
> Hmm... there is a small typo here:
> 
> 	": :" -> "::"
>
Good catch! Thanks.

> For the conversion itself, after correcting the above typo:
> 
> Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@xxxxxxxxxx>
> 
> 
> 
> >  
> >    desc = gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> >  
> > @@ -193,22 +200,25 @@ the ACPI GPIO mapping tables are hardly linked to ACPI ID and certain
> >  objects, as listed in the above chapter, of the device in question.
> >  
> >  Getting GPIO descriptor
> > ------------------------
> > +=======================
> > +
> > +There are two main approaches to get GPIO resource from ACPI::
> >  
> > -There are two main approaches to get GPIO resource from ACPI:
> > -	desc = gpiod_get(dev, connection_id, flags);
> > -	desc = gpiod_get_index(dev, connection_id, index, flags);
> > +  desc = gpiod_get(dev, connection_id, flags);
> > +  desc = gpiod_get_index(dev, connection_id, index, flags);
> >  
> >  We may consider two different cases here, i.e. when connection ID is
> >  provided and otherwise.
> >  
> > -Case 1:
> > -	desc = gpiod_get(dev, "non-null-connection-id", flags);
> > -	desc = gpiod_get_index(dev, "non-null-connection-id", index, flags);
> > +Case 1::
> > +
> > +  desc = gpiod_get(dev, "non-null-connection-id", flags);
> > +  desc = gpiod_get_index(dev, "non-null-connection-id", index, flags);
> > +
> > +Case 2::
> >  
> > -Case 2:
> > -	desc = gpiod_get(dev, NULL, flags);
> > -	desc = gpiod_get_index(dev, NULL, index, flags);
> > +  desc = gpiod_get(dev, NULL, flags);
> > +  desc = gpiod_get_index(dev, NULL, index, flags);
> >  
> >  Case 1 assumes that corresponding ACPI device description must have
> >  defined device properties and will prevent to getting any GPIO resources
> > diff --git a/Documentation/firmware-guide/acpi/index.rst b/Documentation/firmware-guide/acpi/index.rst
> > index 0e05b843521c..61d67763851b 100644
> > --- a/Documentation/firmware-guide/acpi/index.rst
> > +++ b/Documentation/firmware-guide/acpi/index.rst
> > @@ -11,3 +11,4 @@ ACPI Support
> >     enumeration
> >     osi
> >     DSD-properties-rules
> > +   gpio-properties
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 09f43f1bdd15..87f930bf32ad 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6593,7 +6593,7 @@ M:	Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> >  L:	linux-gpio@xxxxxxxxxxxxxxx
> >  L:	linux-acpi@xxxxxxxxxxxxxxx
> >  S:	Maintained
> > -F:	Documentation/acpi/gpio-properties.txt
> > +F:	Documentation/firmware-guide/acpi/gpio-properties.rst
> >  F:	drivers/gpio/gpiolib-acpi.c
> >  
> >  GPIO IR Transmitter
> 
> 
> 
> Thanks,
> Mauro

-- 
Cheers,
Changbin Du