[OpenWrt-Devel] [PATCH V3 1/5] brcm63xx: add basic-mmio-gpio DT support
Jonas Gorski
jogo at openwrt.org
Fri Feb 27 13:13:48 EST 2015
Hi,
On Sun, Jan 25, 2015 at 6:33 PM, Álvaro Fernández Rojas
<noltari at gmail.com> wrote:
> Signed-off-by: Álvaro Fernández Rojas <noltari at gmail.com>
> ---
I decided to add a basic mmio gpio based driver with its own match; to
reduce the amount ot backports/patches, I'll push it later.
Still I don't want to leave the issues in this patch(-set) uncommented:
> diff --git a/target/linux/brcm63xx/patches-3.14/374-GPIO-generic-pdata-add-label-backport.patch b/target/linux/brcm63xx/patches-3.14/374-GPIO-generic-pdata-add-label-backport.patch
> new file mode 100644
> index 0000000..cba5a0a
> --- /dev/null
> +++ b/target/linux/brcm63xx/patches-3.14/374-GPIO-generic-pdata-add-label-backport.patch
> @@ -0,0 +1,21 @@
Please do not remove the git headers when backporting commits from
upstream. Also please use 0xx- for upstream commits.
> +--- a/drivers/gpio/gpio-generic.c
> ++++ b/drivers/gpio/gpio-generic.c
> +@@ -531,6 +531,8 @@ static int bgpio_pdev_probe(struct platf
> + return err;
> +
> + if (pdata) {
> ++ if (pdata->label)
> ++ bgc->gc.label = pdata->label;
> + bgc->gc.base = pdata->base;
> + if (pdata->ngpio > 0)
> + bgc->gc.ngpio = pdata->ngpio;
> +--- a/include/linux/basic_mmio_gpio.h
> ++++ b/include/linux/basic_mmio_gpio.h
> +@@ -19,6 +19,7 @@
> + #include <linux/spinlock_types.h>
> +
> + struct bgpio_pdata {
> ++ const char *label;
> + int base;
> + int ngpio;
> + };
> diff --git a/target/linux/brcm63xx/patches-3.14/375-GPIO-generic-driver-flags-backport.patch b/target/linux/brcm63xx/patches-3.14/375-GPIO-generic-driver-flags-backport.patch
> new file mode 100644
> index 0000000..3915d2c
> --- /dev/null
> +++ b/target/linux/brcm63xx/patches-3.14/375-GPIO-generic-driver-flags-backport.patch
> @@ -0,0 +1,39 @@
> +--- a/drivers/gpio/gpio-generic.c
> ++++ b/drivers/gpio/gpio-generic.c
Same as above.
> +@@ -488,7 +488,7 @@ static int bgpio_pdev_probe(struct platf
> + void __iomem *dirout;
> + void __iomem *dirin;
> + unsigned long sz;
> +- unsigned long flags = 0;
> ++ unsigned long flags = pdev->id_entry->driver_data;
> + int err;
> + struct bgpio_chip *bgc;
> + struct bgpio_pdata *pdata = dev_get_platdata(dev);
> +@@ -519,9 +519,6 @@ static int bgpio_pdev_probe(struct platf
> + if (err)
> + return err;
> +
> +- if (!strcmp(platform_get_device_id(pdev)->name, "basic-mmio-gpio-be"))
> +- flags |= BGPIOF_BIG_ENDIAN;
> +-
> + bgc = devm_kzalloc(&pdev->dev, sizeof(*bgc), GFP_KERNEL);
> + if (!bgc)
> + return -ENOMEM;
> +@@ -551,9 +548,14 @@ static int bgpio_pdev_remove(struct plat
> + }
> +
> + static const struct platform_device_id bgpio_id_table[] = {
> +- { "basic-mmio-gpio", },
> +- { "basic-mmio-gpio-be", },
> +- {},
> ++ {
> ++ .name = "basic-mmio-gpio",
> ++ .driver_data = 0,
> ++ }, {
> ++ .name = "basic-mmio-gpio-be",
> ++ .driver_data = BGPIOF_BIG_ENDIAN,
> ++ },
> ++ { }
> + };
> + MODULE_DEVICE_TABLE(platform, bgpio_id_table);
> +
> diff --git a/target/linux/brcm63xx/patches-3.14/376-DT-basic-mmio-gpio.patch b/target/linux/brcm63xx/patches-3.14/376-DT-basic-mmio-gpio.patch
> new file mode 100644
> index 0000000..cda38b2
> --- /dev/null
> +++ b/target/linux/brcm63xx/patches-3.14/376-DT-basic-mmio-gpio.patch
> @@ -0,0 +1,204 @@
Please add proper git patch headers here, to make tracking code
ownership easier.
> +--- a/Documentation/devicetree/bindings/gpio/gpio.txt
> ++++ b/Documentation/devicetree/bindings/gpio/gpio.txt
> +@@ -153,3 +153,75 @@ Example 2:
> + Here, three GPIO ranges are defined wrt. two pin controllers. pinctrl1 GPIO
> + ranges are defined using pin numbers whereas the GPIO ranges wrt. pinctrl2
> + are named "foo" and "bar".
> ++
> ++3) 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
> ++single "data" register, where GPIO state can be read and/or written.
> ++
> ++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.
> ++
> ++For setting 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 bidirection 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.
> ++
> ++Required properties:
> ++- compatible : Should be "basic-mmio-gpio".
> ++- reg : Address and length of the registers needed for the device.
> ++- reg-names : Names of the needed registers.
> ++- #gpio-cells : Should be two. The first cell is the pin number and
> ++ the second cell is used to specify optional parameters (currently
> ++ unused).
Despite claimed for many drivers, this is not true for any controller
driver using 2+ gpio-cells and not providing a custom xlate function;
the second cell will be used as gpio flags (like active high/low).
> ++- gpio-controller : Marks the device node as a gpio controller.
> ++
> ++Optional properties:
> ++- num-gpios : Specify the number of configurable gpios.
> ++- bit-be : Use big endian order for pins.
No need for brevity, call it "big-endian-bit-order".
> ++- byte-be : Use big endian byte order for registers.
Kevin proposed generic properties for this, see
http://patchwork.linux-mips.org/patch/8566/ so I think it would be
good to use at least the same property name. For some unknown reason
it does not seem to have made it into 3.20.
> ++- regset-wo : set register is unreadable.
> ++- regdir-wo : dir register is unreadable.
Same comment regarding bit-be.
> ++
> ++Examples:
> ++
> ++gpio0: gpio-controller at 10000084 {
> ++ compatible = "brcm,brcm6368", "basic-mmio-gpio";
This should probably be "brcm,brcm6368-gpio" ;p
> ++ reg = <0x10000084 0x4>, <0x1000008c 0x4>;
> ++ reg-names = "dirout", "dat";
> ++
> ++ gpio-controller;
> ++ #gpio-cells = <2>;
> ++
> ++ num-gpios = <32>;
Setting the gpios to the same as the register width seems pointless
(but probably also harmless).
> ++ byte-be;
> ++};
> ++
> ++gpio1: gpio-controller at 10000180 {
> ++ compatible = "foo,bar", "basic-mmio-gpio";
> ++ reg = <0x10000180 0x4>, <0x10000184 0x4>, <0x10000188 0x4>;
> ++ reg-names = "dirin", "dirout", "dat";
> ++
> ++ gpio-controller;
> ++ #gpio-cells = <2>;
> ++
> ++ num-gpios = <20>;
> ++ bit-be;
> ++ byte-be;
> ++ regdir-wo;
> ++};
> +--- a/drivers/gpio/gpio-generic.c
> ++++ b/drivers/gpio/gpio-generic.c
> +@@ -61,6 +61,9 @@ o ` ~~~~\___/
> + #include <linux/platform_device.h>
> + #include <linux/mod_devicetable.h>
> + #include <linux/basic_mmio_gpio.h>
> ++#include <linux/of.h>
> ++#include <linux/of_device.h>
> ++#include <linux/of_gpio.h>
> +
> + static void bgpio_write8(void __iomem *reg, unsigned long data)
> + {
> +@@ -478,8 +481,58 @@ static void __iomem *bgpio_map(struct pl
> + return ret;
> + }
> +
> ++#ifdef CONFIG_OF
> ++static const struct of_device_id bgpio_dt_ids[] = {
> ++ { .compatible = "basic-mmio-gpio" },
> ++};
> ++MODULE_DEVICE_TABLE(of, bgpio_dt_ids);
> ++
> ++static int bgpio_probe_dt(struct platform_device *pdev)
> ++{
> ++ u32 tmp;
> ++ struct bgpio_pdata *pdata;
> ++ struct device_node *np;
> ++
> ++ np = pdev->dev.of_node;
> ++ if (!np)
> ++ return 0;
> ++
> ++ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> ++ if (!pdata)
> ++ return -ENOMEM;
> ++
> ++ pdata->label = dev_name(&pdev->dev);
> ++ pdata->base = -1;
> ++ if (of_find_property(np, "byte-be", NULL)) {
> ++ pdata->flags |= BGPIOF_BIG_ENDIAN_BYTE_ORDER;
> ++ }
Superfluous braces.
> ++ if (of_find_property(np, "bit-be", NULL)) {
> ++ pdata->flags |= BGPIOF_BIG_ENDIAN;
> ++ }
Superfluous braces.
> ++ if (of_find_property(np, "regset-wo", NULL)) {
> ++ pdata->flags |= BGPIOF_UNREADABLE_REG_SET;
> ++ }
Superfluous braces.
> ++ if (of_find_property(np, "regdir-wo", NULL)) {
> ++ pdata->flags |= BGPIOF_UNREADABLE_REG_DIR;
> ++ }
Superfluous braces.
> ++ if (!of_property_read_u32(np, "num-gpios", &tmp)) {
> ++ pdata->ngpio = tmp;
> ++ }
Superfluous braces.
> ++
> ++ pdev->dev.platform_data = pdata;
> ++
> ++ return 1;
> ++}
> ++#else
> ++static inline int bgpio_probe_dt(struct platform_device *pdev)
> ++{
> ++ return 0;
> ++}
> ++#endif
> ++
> + static int bgpio_pdev_probe(struct platform_device *pdev)
> + {
> ++ int status;
> + struct device *dev = &pdev->dev;
> + struct resource *r;
> + void __iomem *dat;
> +@@ -488,10 +541,24 @@ static int bgpio_pdev_probe(struct platf
> + void __iomem *dirout;
> + void __iomem *dirin;
> + unsigned long sz;
> +- unsigned long flags = pdev->id_entry->driver_data;
> ++ unsigned long flags;
> + int err;
> + struct bgpio_chip *bgc;
> +- struct bgpio_pdata *pdata = dev_get_platdata(dev);
> ++ struct bgpio_pdata *pdata;
> ++ bool use_of = 0;
> ++
> ++ status = bgpio_probe_dt(pdev);
> ++ if (status < 0)
> ++ return status;
> ++ if (status > 0)
> ++ use_of = 1;
> ++
> ++ pdata = dev_get_platdata(dev);
> ++
> ++ if (!use_of)
> ++ flags = pdev->id_entry->driver_data;
> ++ else if (pdata)
> ++ flags = pdata->flags;
How about instead:
if (dev->of_node)
flags = bgpio_of_get_flags(pdev);
else
flags = pdev->id_entry->driver_data;
and then later
if (dev->of_node) {
of_read_property_u16(dev->of_node, "ngpios", &bgc->gc.ngpios);
else if (pdata) {
bgc->gc.ngpios = pdata->ngpios;
...
}
Then you don't need to allocate any extra pdata.
> +
> + r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
> + if (!r)
> +@@ -537,6 +604,9 @@ static int bgpio_pdev_probe(struct platf
> +
> + platform_set_drvdata(pdev, bgc);
> +
> ++ if (use_of)
> ++ of_gpiochip_add(&bgc->gc);
> ++
gpiochip_add will already call of_gpiochip_add, no need for this one.
> + return gpiochip_add(&bgc->gc);
> + }
> +
> +@@ -562,6 +632,7 @@ MODULE_DEVICE_TABLE(platform, bgpio_id_t
> + static struct platform_driver bgpio_driver = {
> + .driver = {
> + .name = "basic-mmio-gpio",
> ++ .of_match_table = bgpio_dt_ids,
this will break compilation with !OF, you need to use
of_match_ptr(bgpio_dt_ids).
> + },
> + .id_table = bgpio_id_table,
> + .probe = bgpio_pdev_probe,
> +--- a/include/linux/basic_mmio_gpio.h
> ++++ b/include/linux/basic_mmio_gpio.h
> +@@ -22,6 +22,7 @@ struct bgpio_pdata {
> + const char *label;
> + int base;
> + int ngpio;
> ++ unsigned long flags;
> + };
> +
> + struct device;
Jonas
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list