Re: [PATCH v1 5/5] gpio: document basic mmio gpio library

[Kamlakant Patel <kamlakant.patel@xxxxxxxxxx>]

On Thu, Dec 04, 2014 at 05:52:24PM +0530, Kamlakant Patel wrote:
> On 4 December 2014 at 14:42, Alexandre Courbot <gnurou@xxxxxxxxx> wrote:
> > On Mon, Dec 1, 2014 at 9:09 PM,  <kamlakant.patel@xxxxxxxxxx> wrote:
> >> From: Kamlakant Patel <kamlakant.patel@xxxxxxxxxx>
> >>
> >> This is a brief documentation on how to use GPIO Generic
> >> library for memory-mapped GPIO controllers.
> >>
> >> Signed-off-by: Kamlakant Patel <kamlakant.patel@xxxxxxxxxx>
> >> ---
> >>  Documentation/gpio/driver.txt | 50 +++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 50 insertions(+)
> >
> > Yum, more doc!
> >
> >>
> >> diff --git a/Documentation/gpio/driver.txt b/Documentation/gpio/driver.txt
> >> index 31e0b5d..563abea 100644
> >> --- a/Documentation/gpio/driver.txt
> >> +++ b/Documentation/gpio/driver.txt
> >> @@ -190,3 +190,53 @@ gpiochip_free_own_desc().
> >>  These functions must be used with care since they do not affect module use
> >>  count. Do not use the functions to request gpio descriptors not owned by the
> >>  calling driver.
> >> +
> >> +
> >> +Generic driver for memory-mapped GPIO controllers
> >> +-------------------------------------------------
> >> +The GPIO generic library provides support for basic platform_device
> >> +memory-mapped GPIO controllers, which can be accessed by selecting Kconfig
> >> +symbol GPIO_GENERIC and using library functions provided by GPIO generic
> >> +driver (see drivers/gpio/gpio-generic.c).
> >> +The simplest form of a GPIO controller that the driver support is just a
> >
> > s/support/supports
> >
> >> +single "data" register, where GPIO state can be read and/or written.
> >> +
> >> +The driver can be registered using "basic-mmio-gpio" or for big-endian
> >> +notation support use "basic-mmio-gpio-be". The code will configure gpio_chip
> >
> > Using where? You should say that this is for the platform device name.
> >
> >> +and issue gpiochip_add().
> >
> >> +
> >> +The driver supports:
> >> +- 8/16/32/64 bits registers. The number of GPIOs is determined by the width of
> >> +  the registers.
> >> +- GPIO controllers with clear/set registers.
> >> +- GPIO controllers with a single "data" register.
> >> +- Big endian bits/GPIOs ordering.
> >
> > Maybe add a sentence indicating that these settings are defined in the
> > drivers using named memory resources.
> >
> >> +
> >> +For setting GPIO's there are three supported configurations:
> >> +- single input/output register resource (named "dat").
> >
> > This resource seems to be mandatory - please make sure you mention this fact.
> >
> >> +- set/clear pair (named "set" and "clr").
> >> +- single output register resource and single input resource ("set" and dat").
> >> +
> >> +For setting the GPIO direction, there are three supported configurations:
> >> +- simple bidirection GPIO that requires no configuration.
> >
> > s/bidirection/bidirectional maybe?
> >
> >> +- an output direction register (named "dirout") where a 1 bit indicates the
> >> +  GPIO is an output.
> >> +- an input direction register (named "dirin") where a 1 bit indicates the GPIO
> >> +  is an input.
> >> +
> >> +It is possible to use only parts of GPIO generic library. Each GPIO controller
> >> +using GPIO generic library needs to include the following header.
> >> +
> >> +        #include <linux/basic_mmio_gpio.h>
> >> +
> >> +Use bgpio_init to configure gpio_chip and bgpio_remove to remove the controller.
> >> +int bgpio_init(struct bgpio_chip *bgc, struct device *dev,
> >> +               unsigned long sz, void __iomem *dat, void __iomem *set,
> >> +               void __iomem *clr, void __iomem *dirout, void __iomem *dirin,
> >> +               unsigned long flags);
> >
> > If you put the prototype for bgpio_init(), please also put the one of
> > bgpio_remove()...
> >
> >> +
> >> +The "flag" parameter can be following depending on controller configuration:
> >> +BGPIOF_BIG_ENDIAN               BIT(0)
> >> +BGPIOF_UNREADABLE_REG_SET       BIT(1) /* reg_set is unreadable */
> >> +BGPIOF_UNREADABLE_REG_DIR       BIT(2) /* reg_dir is unreadable */
> >> +BGPIOF_BIG_ENDIAN_BYTE_ORDER    BIT(3)
> >
> > Right now this documentation is a little bit confusing. Basically
> > there are two ways to use this driver:
> >
> > 1) Name your platform device ""basic-mmio-gpio" or
> > "basic-mmio-gpio-be", set the right named memory resources to specify
> > the desired configuration, and let bgpio_pdev_probe() do all the work.
> >
> > 2) Allocate a bgpio_chip yourself, call bgpio_init() on it and its
> > resources, and finally invoke gpiochip_add() yourself.
> >
> > These two different ways of doing kind of seem to be mixed together.
> > Can you try to highlight the fact that these are alternatives?
> 

This is an updated version of previous patch. Please review.

diff --git a/Documentation/gpio/driver.txt b/Documentation/gpio/driver.txt
index 31e0b5d..f6b617a 100644
--- a/Documentation/gpio/driver.txt
+++ b/Documentation/gpio/driver.txt
@@ -190,3 +190,45 @@ gpiochip_free_own_desc().
 These functions must be used with care since they do not affect module use
 count. Do not use the functions to request gpio descriptors not owned by the
 calling driver.
+
+
+Generic driver for memory-mapped GPIO controllers
+-------------------------------------------------
+The GPIO generic library provides support for basic platform_device
+memory-mapped GPIO controllers, which can be accessed by selecting Kconfig
+symbol GPIO_GENERIC and using library functions provided by GPIO generic
+driver (see drivers/gpio/gpio-generic.c).
+
+The driver supports registers of the sizes of 8/16/32/64 bits and the number
+of GPIOs are determined by the width of the registers. A set of named memory
+resources should be defined in the drivers (e.g "dat", "set", "clr", "dirout"
+and "dirin"), where "dat" is a mandatory resource.
+
+Each GPIO controller using GPIO generic library needs to include the following
+header.
+        #include <linux/basic_mmio_gpio.h>
+
+There are two ways to use this driver:
+1. Using basic GPIO MMIO Generic driver directly:
+   Name your platform device "basic-mmio-gpio" or "basic-mmio-gpio-be", set the
+   right named memory resources to specify the desired configuration, and let
+   bgpio_pdev_probe do all the work.
+
+2. Using basic GPIO MMIO Generic library in your driver:
+   Allocate a bgpio_chip yourself in your GPIO driver, call bgpio_init() on it
+   and its resources, and finally invoke gpiochip_add yourself. It is possible
+   to use only parts of the driver, you can overwrite functions and variables
+   in your driver, if necessary.
+   You can call bgpio_remove() to unregister a gpio_chip.
+
+For setting up GPIO's there are three supported configurations:
+- single input/output register resource (named "dat").
+- set/clear pair (named "set" and "clr").
+- single output register resource and single input resource ("set" and "dat").
+
+For setting the GPIO direction, there are three supported configurations:
+- simple bidirectional GPIO that requires no configuration.
+- an output direction register (named "dirout") where a 1 bit indicates the
+  GPIO is an output.
+- an input direction register (named "dirin") where a 1 bit indicates the GPIO
+  is an input.
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html