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? Thanks for the review comments and suggestions. I will update and send the patch. Thanks, Kamlakant Patel -- 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