* [PATCH v2 0/2] GPIO: add single-register GPIO via CREG driver @ 2018-08-28 11:27 Eugeniy Paltsev 2018-08-28 11:27 ` [PATCH v2 1/2] GPIO: add single-register gpio via creg driver Eugeniy Paltsev 2018-08-28 11:27 ` [PATCH v2 2/2] dt-bindings: Document the Synopsys GPIO via CREG bindings Eugeniy Paltsev 0 siblings, 2 replies; 10+ messages in thread From: Eugeniy Paltsev @ 2018-08-28 11:27 UTC (permalink / raw) To: linux-snps-arc, linux-gpio Cc: linux-kernel, Vineet Gupta, Alexey Brodkin, Linus Walleij, Rob Herring, devicetree, Mark Rutland, Eugeniy Paltsev Add single-register MMIO GPIO driver for complex cases where only several fields in register belong to GPIO lines and each GPIO line owns a field with different length and on/off value. Here is the example: 31 11 8 7 5 0 < bit number | | | | | | [ not used | gpio-1 | shift-1 | gpio-0 | shift-0 ] < 32 bit MMIO ^ ^ register (3 bit) (2 bit) | | | | | write 0x2 == set output to "1" (on) | write 0x3 == set output to "0" (off) | write 0x1 == set output to "1" (on) write 0x4 == set output to "0" (off) This is different from gpio-reg, gpio-mmio and gpio-74xx-mmio: * They all don't support cases when GPIO output register have more than one bit per GPIO line. * They don't support holes in MMIO register. * They don't support cases when GPIO lines have different on/off values. This driver supports GPIOs via CREG on various Synopsys SoCs/boards. Eugeniy Paltsev (2): GPIO: add single-register GPIO via CREG driver dt-bindings: Document the Synopsys GPIO via CREG bindings .../devicetree/bindings/gpio/snps,creg-gpio.txt | 49 +++++ MAINTAINERS | 6 + drivers/gpio/Kconfig | 9 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-creg-snps.c | 242 +++++++++++++++++++++ 5 files changed, 307 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt create mode 100644 drivers/gpio/gpio-creg-snps.c -- 2.14.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] GPIO: add single-register gpio via creg driver 2018-08-28 11:27 [PATCH v2 0/2] GPIO: add single-register GPIO via CREG driver Eugeniy Paltsev @ 2018-08-28 11:27 ` Eugeniy Paltsev 2018-08-28 18:15 ` Randy Dunlap 2018-08-28 11:27 ` [PATCH v2 2/2] dt-bindings: Document the Synopsys GPIO via CREG bindings Eugeniy Paltsev 1 sibling, 1 reply; 10+ messages in thread From: Eugeniy Paltsev @ 2018-08-28 11:27 UTC (permalink / raw) To: linux-snps-arc, linux-gpio Cc: linux-kernel, Vineet Gupta, Alexey Brodkin, Linus Walleij, Rob Herring, devicetree, Mark Rutland, Eugeniy Paltsev Add single-register MMIO gpio driver for complex cases where only several fields in register belong to GPIO and each GPIO owns field with different length and on/off values. Here is the example: 31 11 8 7 5 0 < bit number | | | | | | [ not used | gpio-1 | shift-1 | gpio-0 | shift-0 ] < 32 bit MMIO ^ ^ (3 bit) (2 bit) | | | | | write 0x2 == set output to "1" (on) | write 0x3 == set output to "0" (off) | write 0x1 == set output to "1" (on) write 0x4 == set output to "0" (off) This is different from gpio-reg, gpio-mmio and gpio-74xx-mmio: * They all don't support cases when GPIO output register have more than one bit per GPIO line. * They don't support holes in MMIO register. * They don't support cases when GPIO lines have different on/off values. Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> --- MAINTAINERS | 6 ++ drivers/gpio/Kconfig | 9 ++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-creg-snps.c | 242 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 258 insertions(+) create mode 100644 drivers/gpio/gpio-creg-snps.c diff --git a/MAINTAINERS b/MAINTAINERS index 544cac829cf4..e731f2f9648a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13734,6 +13734,12 @@ S: Supported F: drivers/reset/reset-axs10x.c F: Documentation/devicetree/bindings/reset/snps,axs10x-reset.txt +SYNOPSYS CREG GPIO DRIVER +M: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> +S: Maintained +F: drivers/gpio/gpio-creg-snps.c +F: Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt + SYNOPSYS DESIGNWARE 8250 UART DRIVER R: Andy Shevchenko <andriy.shevchenko@linux.intel.com> S: Maintained diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 71c0ab46f216..0f9cc1582cab 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -430,6 +430,15 @@ config GPIO_REG A 32-bit single register GPIO fixed in/out implementation. This can be used to represent any register as a set of GPIO signals. +config GPIO_SNPS_CREG + bool "GPIO via CREG (Control REGisers) driver" + select OF_GPIO + help + This driver supports GPIOs via CREG on various Synopsys SoCs. + This is is single-register MMIO gpio driver for complex cases + where only several fields in register belong to GPIO and + each GPIO owns field with different length and on/off values. + config GPIO_SPEAR_SPICS bool "ST SPEAr13xx SPI Chip Select as GPIO support" depends on PLAT_SPEAR diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 1324c8f966a7..993f8ad54a19 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -109,6 +109,7 @@ obj-$(CONFIG_GPIO_REG) += gpio-reg.o obj-$(CONFIG_ARCH_SA1100) += gpio-sa1100.o obj-$(CONFIG_GPIO_SCH) += gpio-sch.o obj-$(CONFIG_GPIO_SCH311X) += gpio-sch311x.o +obj-$(CONFIG_GPIO_SNPS_CREG) += gpio-creg-snps.o obj-$(CONFIG_GPIO_SODAVILLE) += gpio-sodaville.o obj-$(CONFIG_GPIO_SPEAR_SPICS) += gpio-spear-spics.o obj-$(CONFIG_GPIO_SPRD) += gpio-sprd.o diff --git a/drivers/gpio/gpio-creg-snps.c b/drivers/gpio/gpio-creg-snps.c new file mode 100644 index 000000000000..b684b7257aae --- /dev/null +++ b/drivers/gpio/gpio-creg-snps.c @@ -0,0 +1,242 @@ +// SPDX-License-Identifier: GPL-2.0+ +// +// Synopsys CREG (Control REGisers) GPIO driver +// +// Copyright (C) 2018 Synopsys +// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> + +#include <linux/of.h> +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/of_gpio.h> +#include <linux/io.h> +#include <linux/of_platform.h> +#include <linux/module.h> + +#include "gpiolib.h" + +/* + * GPIO via CREG (Control REGisers) driver + * + * 31 11 8 7 5 0 < bit number + * | | | | | | + * [ not used | gpio-1 | shift-1 | gpio-0 | shift-0 ] < 32 bit register + * ^ ^ + * | | + * | write 0x2 == set output to "1" (on) + * | write 0x3 == set output to "0" (off) + * | + * write 0x1 == set output to "1" (on) + * write 0x4 == set output to "0" (off) + */ + +#define MAX_GPIO 32 + +struct creg_gpio { + struct of_mm_gpio_chip mmchip; + spinlock_t lock; + + u32 shift[MAX_GPIO]; + u32 on[MAX_GPIO]; + u32 off[MAX_GPIO]; + u32 bit_per_gpio[MAX_GPIO]; +}; + +static void _creg_gpio_set(struct creg_gpio *hcg, unsigned int gpio, u32 val) +{ + u32 reg, reg_shift; + unsigned long flags; + int i; + + reg_shift = hcg->shift[gpio]; + for (i = 0; i < gpio; i++) + reg_shift += hcg->bit_per_gpio[i] + hcg->shift[i]; + + spin_lock_irqsave(&hcg->lock, flags); + reg = readl(hcg->mmchip.regs); + reg &= ~(GENMASK(hcg->bit_per_gpio[i] - 1, 0) << reg_shift); + reg |= (val << reg_shift); + writel(reg, hcg->mmchip.regs); + spin_unlock_irqrestore(&hcg->lock, flags); +} + +static void creg_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val) +{ + struct creg_gpio *hcg = gpiochip_get_data(gc); + u32 value = val ? hcg->on[gpio] : hcg->off[gpio]; + + _creg_gpio_set(hcg, gpio, value); +} + +static int creg_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) +{ + creg_gpio_set(gc, gpio, val); + + return 0; +} + +static int creg_gpio_get_direction(struct gpio_chip *gc, unsigned int offset) +{ + return 0; /* output */ +} + +static int creg_gpio_xlate(struct gpio_chip *gc, + const struct of_phandle_args *gpiospec, u32 *flags) +{ + if (gpiospec->args_count != 1) { + dev_err(&gc->gpiodev->dev, "invalid args_count: %d\n", + gpiospec->args_count); + return -EINVAL; + } + + if (gpiospec->args[0] >= gc->ngpio) { + dev_err(&gc->gpiodev->dev, "gpio number is too big: %d\n", + gpiospec->args[0]); + return -EINVAL; + } + + return gpiospec->args[0]; +} + +static int creg_gpio_validate_pgv(struct device *dev, struct creg_gpio *hcg, + int i, u32 *defaults, bool use_defaults) +{ + if (hcg->bit_per_gpio[i] < 1 || hcg->bit_per_gpio[i] > 8) { + dev_err(dev, "'bit-per-line[%d]' is out of bounds\n", i); + return -EINVAL; + } + + /* Check that on valiue suits it's placeholder */ + if (GENMASK(31, hcg->bit_per_gpio[i]) & hcg->on[i]) { + dev_err(dev, "'on-val[%d]' can't be more than %lu\n", + i, GENMASK(hcg->bit_per_gpio[i] - 1, 0)); + return -EINVAL; + } + + /* Check that off valiue suits it's placeholder */ + if (GENMASK(31, hcg->bit_per_gpio[i]) & hcg->off[i]) { + dev_err(dev, "'off-val[%d]' can't be more than %lu\n", + i, GENMASK(hcg->bit_per_gpio[i] - 1, 0)); + return -EINVAL; + } + + /* Check that default valiue suits it's placeholder */ + if (use_defaults && (GENMASK(31, hcg->bit_per_gpio[i]) & defaults[i])) { + dev_err(dev, "'default-val[%d]' can't be more than %lu\n", + i, GENMASK(hcg->bit_per_gpio[i] - 1, 0)); + return -EINVAL; + } + + if (hcg->on[i] == hcg->off[i]) { + dev_err(dev, "'off-val[%d]' and 'on-val[%d]' can't be equal\n", + i, i); + return -EINVAL; + } + + return 0; +} + +static int creg_gpio_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + struct device *dev = &pdev->dev; + struct creg_gpio *hcg; + + u32 defaults[MAX_GPIO]; + bool use_defaults; + u32 ngpio, reg_len = 0; + int ret, i; + + hcg = devm_kzalloc(dev, sizeof(struct creg_gpio), GFP_KERNEL); + if (!hcg) + return -ENOMEM; + + if (of_property_read_u32(np, "snps,ngpios", &ngpio)) { + dev_err(dev, "'ngpios' isn't set\n"); + return -EINVAL; + } + + if (ngpio < 1 || ngpio > MAX_GPIO) { + dev_err(dev, "'ngpios' is out of bounds\n"); + return -EINVAL; + } + + if (of_property_read_u32_array(np, "snps,shift", hcg->shift, ngpio)) { + dev_err(dev, "'shift' is set incorrectly\n"); + return -EINVAL; + } + + if (of_property_read_u32_array(np, "snps,bit-per-line", + hcg->bit_per_gpio, ngpio)) { + dev_err(dev, "'bit-per-line' is set incorrectly\n"); + return -EINVAL; + } + + if (of_property_read_u32_array(np, "snps,on-val", hcg->on, ngpio)) { + dev_err(dev, "'on-val' is set incorrectly\n"); + return -EINVAL; + } + + if (of_property_read_u32_array(np, "snps,off-val", hcg->off, ngpio)) { + dev_err(dev, "'off-val' is set incorrectly\n"); + return -EINVAL; + } + + ret = of_property_read_u32_array(np, "snps,default-val", defaults, ngpio); + if (ret && ret != -EINVAL) { + dev_err(dev, "'default-val' is set incorrectly\n"); + return ret; + } + use_defaults = !ret; + + for (i = 0; i < ngpio; i++) { + if (creg_gpio_validate_pgv(dev, hcg, i, defaults, use_defaults)) + return -EINVAL; + + reg_len += hcg->shift[i] + hcg->bit_per_gpio[i]; + } + + /* Check that we suit in 32 bit register */ + if (reg_len > 32) { + dev_err(dev, + "32-bit io register overflow: attempt to use %u bits\n", + reg_len); + return -EINVAL; + } + + spin_lock_init(&hcg->lock); + + hcg->mmchip.gc.ngpio = ngpio; + hcg->mmchip.gc.set = creg_gpio_set; + hcg->mmchip.gc.get_direction = creg_gpio_get_direction; + hcg->mmchip.gc.direction_output = creg_gpio_dir_out; + hcg->mmchip.gc.of_xlate = creg_gpio_xlate; + hcg->mmchip.gc.of_gpio_n_cells = 1; + + ret = of_mm_gpiochip_add_data(pdev->dev.of_node, &hcg->mmchip, hcg); + if (ret) + return ret; + + /* Setup default GPIO value if we have "snps,default-val" array */ + if (use_defaults) + for (i = 0; i < ngpio; i++) + _creg_gpio_set(hcg, i, defaults[i]); + + dev_info(dev, "GPIO controller with %d gpios probed\n", ngpio); + + return 0; +} + +static const struct of_device_id creg_gpio_ids[] = { + { .compatible = "snps,creg-gpio" }, + { } +}; + +static struct platform_driver creg_gpio_snps_driver = { + .driver = { + .name = "snps-creg-gpio", + .of_match_table = creg_gpio_ids, + }, + .probe = creg_gpio_probe, +}; +builtin_platform_driver(creg_gpio_snps_driver); -- 2.14.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] GPIO: add single-register gpio via creg driver 2018-08-28 11:27 ` [PATCH v2 1/2] GPIO: add single-register gpio via creg driver Eugeniy Paltsev @ 2018-08-28 18:15 ` Randy Dunlap 2018-08-30 12:14 ` Eugeniy Paltsev 0 siblings, 1 reply; 10+ messages in thread From: Randy Dunlap @ 2018-08-28 18:15 UTC (permalink / raw) To: Eugeniy Paltsev, linux-snps-arc, linux-gpio Cc: linux-kernel, Vineet Gupta, Alexey Brodkin, Linus Walleij, Rob Herring, devicetree, Mark Rutland Hi, I don't see any updates/corrections here. :( On 08/28/2018 04:27 AM, Eugeniy Paltsev wrote: > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 71c0ab46f216..0f9cc1582cab 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -430,6 +430,15 @@ config GPIO_REG > A 32-bit single register GPIO fixed in/out implementation. This > can be used to represent any register as a set of GPIO signals. > > +config GPIO_SNPS_CREG > + bool "GPIO via CREG (Control REGisers) driver" > + select OF_GPIO > + help > + This driver supports GPIOs via CREG on various Synopsys SoCs. > + This is is single-register MMIO gpio driver for complex cases > + where only several fields in register belong to GPIO and > + each GPIO owns field with different length and on/off values. > + > config GPIO_SPEAR_SPICS > bool "ST SPEAr13xx SPI Chip Select as GPIO support" > depends on PLAT_SPEAR -- ~Randy ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] GPIO: add single-register gpio via creg driver 2018-08-28 18:15 ` Randy Dunlap @ 2018-08-30 12:14 ` Eugeniy Paltsev 0 siblings, 0 replies; 10+ messages in thread From: Eugeniy Paltsev @ 2018-08-30 12:14 UTC (permalink / raw) To: rdunlap, Eugeniy.Paltsev, linux-gpio, linux-snps-arc Cc: linux-kernel, Vineet.Gupta1, Alexey.Brodkin, mark.rutland, linus.walleij, robh+dt, devicetree On Tue, 2018-08-28 at 11:15 -0700, Randy Dunlap wrote: > Hi, > > I don't see any updates/corrections here. :( My fault - I've forgotten to re-generate new patch via git format-patch and send you previous version. > > On 08/28/2018 04:27 AM, Eugeniy Paltsev wrote: > > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > > index 71c0ab46f216..0f9cc1582cab 100644 > > --- a/drivers/gpio/Kconfig > > +++ b/drivers/gpio/Kconfig > > @@ -430,6 +430,15 @@ config GPIO_REG > > A 32-bit single register GPIO fixed in/out implementation. This > > can be used to represent any register as a set of GPIO signals. > > > > +config GPIO_SNPS_CREG > > + bool "GPIO via CREG (Control REGisers) driver" > > + select OF_GPIO > > + help > > + This driver supports GPIOs via CREG on various Synopsys SoCs. > > + This is is single-register MMIO gpio driver for complex cases > > + where only several fields in register belong to GPIO and > > + each GPIO owns field with different length and on/off values. > > + > > config GPIO_SPEAR_SPICS > > bool "ST SPEAr13xx SPI Chip Select as GPIO support" > > depends on PLAT_SPEAR > > -- Eugeniy Paltsev ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] dt-bindings: Document the Synopsys GPIO via CREG bindings 2018-08-28 11:27 [PATCH v2 0/2] GPIO: add single-register GPIO via CREG driver Eugeniy Paltsev 2018-08-28 11:27 ` [PATCH v2 1/2] GPIO: add single-register gpio via creg driver Eugeniy Paltsev @ 2018-08-28 11:27 ` Eugeniy Paltsev 2018-08-29 1:02 ` Rob Herring 2018-08-30 8:43 ` Linus Walleij 1 sibling, 2 replies; 10+ messages in thread From: Eugeniy Paltsev @ 2018-08-28 11:27 UTC (permalink / raw) To: linux-snps-arc, linux-gpio Cc: linux-kernel, Vineet Gupta, Alexey Brodkin, Linus Walleij, Rob Herring, devicetree, Mark Rutland, Eugeniy Paltsev This patch adds documentation of device tree bindings for the Synopsys GPIO via CREG driver. Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> --- .../devicetree/bindings/gpio/snps,creg-gpio.txt | 49 ++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt diff --git a/Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt b/Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt new file mode 100644 index 000000000000..eb022d44ccda --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt @@ -0,0 +1,49 @@ +GPIO via CREG (Control REGisers) driver + +This is is single-register MMIO GPIO driver to control such strangely mapped +outputs: + +31 11 8 7 5 0 < bit number +| | | | | | +[ not used | gpio-1 | shift-1 | gpio-0 | shift-0 ] < 32 bit MMIO register + ^ ^ + | | + | write 0x2 == set output to "1" (on) + | write 0x3 == set output to "0" (off) + | + write 0x1 == set output to "1" (on) + write 0x4 == set output to "0" (off) + + +Required properties: +- compatible : "snps,creg-gpio" +- reg : Exactly one register range with length 0x4. +- #gpio-cells : Should be one - the pin number. +- gpio-controller : Marks the device node as a GPIO controller. +- snps,ngpios: Number of GPIO pins. +- snps,bit-per-line: Number of bits per each gpio line (see picture). + Array the size of "snps,ngpios" +- snps,shift: Shift (in bits) of the each GPIO field from the previous one in + register (see picture). Array the size of "snps,ngpios" +- snps,on-val: Value should be set in corresponding field to set + output to "1" (see picture). Array the size of "snps,ngpios" +- snps,off-val: Value should be set in corresponding field to set + output to "0" (see picture). Array the size of "snps,ngpios" + +Optional properties: +- snps,default-val: default output field values. Array the size of "snps,ngpios" + +Example (see picture): + +gpio: gpio@f00014b0 { + compatible = "snps,creg-gpio"; + reg = <0xf00014b0 0x4>; + gpio-controller; + #gpio-cells = <1>; + snps,ngpios = <2>; + snps,shift = <5 1>; + snps,bit-per-line = <2 3>; + snps,on-val = <2 1>; + snps,off-val = <3 4>; + snps,default-val = <2 1>; +}; -- 2.14.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: Document the Synopsys GPIO via CREG bindings 2018-08-28 11:27 ` [PATCH v2 2/2] dt-bindings: Document the Synopsys GPIO via CREG bindings Eugeniy Paltsev @ 2018-08-29 1:02 ` Rob Herring 2018-08-30 13:12 ` Eugeniy Paltsev 2018-08-30 8:43 ` Linus Walleij 1 sibling, 1 reply; 10+ messages in thread From: Rob Herring @ 2018-08-29 1:02 UTC (permalink / raw) To: Eugeniy Paltsev Cc: linux-snps-arc, linux-gpio, linux-kernel, Vineet Gupta, Alexey Brodkin, Linus Walleij, devicetree, Mark Rutland On Tue, Aug 28, 2018 at 02:27:21PM +0300, Eugeniy Paltsev wrote: > This patch adds documentation of device tree bindings for the Synopsys > GPIO via CREG driver. > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > --- > .../devicetree/bindings/gpio/snps,creg-gpio.txt | 49 ++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt > > diff --git a/Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt b/Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt > new file mode 100644 > index 000000000000..eb022d44ccda > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt > @@ -0,0 +1,49 @@ > +GPIO via CREG (Control REGisers) driver REGisters? Bindings don't describe drivers. > + > +This is is single-register MMIO GPIO driver to control such strangely mapped > +outputs: > + > +31 11 8 7 5 0 < bit number > +| | | | | | > +[ not used | gpio-1 | shift-1 | gpio-0 | shift-0 ] < 32 bit MMIO register > + ^ ^ > + | | > + | write 0x2 == set output to "1" (on) > + | write 0x3 == set output to "0" (off) > + | > + write 0x1 == set output to "1" (on) > + write 0x4 == set output to "0" (off) What kind of crazy h/w designer designed this? > +Required properties: > +- compatible : "snps,creg-gpio" > +- reg : Exactly one register range with length 0x4. > +- #gpio-cells : Should be one - the pin number. > +- gpio-controller : Marks the device node as a GPIO controller. > +- snps,ngpios: Number of GPIO pins. > +- snps,bit-per-line: Number of bits per each gpio line (see picture). > + Array the size of "snps,ngpios" > +- snps,shift: Shift (in bits) of the each GPIO field from the previous one in > + register (see picture). Array the size of "snps,ngpios" > +- snps,on-val: Value should be set in corresponding field to set > + output to "1" (see picture). Array the size of "snps,ngpios" > +- snps,off-val: Value should be set in corresponding field to set > + output to "0" (see picture). Array the size of "snps,ngpios" Convince me we need to parameterize all this. We try to avoid describing h/w like this. > + > +Optional properties: > +- snps,default-val: default output field values. Array the size of "snps,ngpios" > + > +Example (see picture): > + > +gpio: gpio@f00014b0 { > + compatible = "snps,creg-gpio"; > + reg = <0xf00014b0 0x4>; > + gpio-controller; > + #gpio-cells = <1>; > + snps,ngpios = <2>; > + snps,shift = <5 1>; > + snps,bit-per-line = <2 3>; > + snps,on-val = <2 1>; > + snps,off-val = <3 4>; > + snps,default-val = <2 1>; > +}; > -- > 2.14.4 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: Document the Synopsys GPIO via CREG bindings 2018-08-29 1:02 ` Rob Herring @ 2018-08-30 13:12 ` Eugeniy Paltsev 0 siblings, 0 replies; 10+ messages in thread From: Eugeniy Paltsev @ 2018-08-30 13:12 UTC (permalink / raw) To: robh, Eugeniy.Paltsev Cc: linux-kernel, Alexey.Brodkin, Vineet.Gupta1, devicetree, linus.walleij, linux-snps-arc, linux-gpio, mark.rutland On Tue, 2018-08-28 at 20:02 -0500, Rob Herring wrote: > On Tue, Aug 28, 2018 at 02:27:21PM +0300, Eugeniy Paltsev wrote: > > This patch adds documentation of device tree bindings for the Synopsys > > GPIO via CREG driver. > > > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > > --- > > .../devicetree/bindings/gpio/snps,creg-gpio.txt | 49 ++++++++++++++++++++++ > > 1 file changed, 49 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt > > > > diff --git a/Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt b/Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt > > new file mode 100644 > > index 000000000000..eb022d44ccda > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt > > @@ -0,0 +1,49 @@ > > +GPIO via CREG (Control REGisers) driver > > Bindings don't describe drivers. > > > + > > +This is is single-register MMIO GPIO driver to control such strangely mapped > > +outputs: > > + > > +31 11 8 7 5 0 < bit number > > +| | | | | | > > +[ not used | gpio-1 | shift-1 | gpio-0 | shift-0 ] < 32 bit MMIO register > > + ^ ^ > > + | | > > + | write 0x2 == set output to "1" (on) > > + | write 0x3 == set output to "0" (off) > > + | > > + write 0x1 == set output to "1" (on) > > + write 0x4 == set output to "0" (off) > > What kind of crazy h/w designer designed this? Actually this fields in register controls some multiplexers, which we want to use as IO port, see the example: /| / | | |------ some internal line IO PIN -------| |------ logic 0 | |------ logic 1 | |------ not used \ | |\| | CREG field > > +Required properties: > > +- compatible : "snps,creg-gpio" > > +- reg : Exactly one register range with length 0x4. > > +- #gpio-cells : Should be one - the pin number. > > +- gpio-controller : Marks the device node as a GPIO controller. > > +- snps,ngpios: Number of GPIO pins. > > +- snps,bit-per-line: Number of bits per each gpio line (see picture). > > + Array the size of "snps,ngpios" > > +- snps,shift: Shift (in bits) of the each GPIO field from the previous one in > > + register (see picture). Array the size of "snps,ngpios" > > +- snps,on-val: Value should be set in corresponding field to set > > + output to "1" (see picture). Array the size of "snps,ngpios" > > +- snps,off-val: Value should be set in corresponding field to set > > + output to "0" (see picture). Array the size of "snps,ngpios" > > Convince me we need to parameterize all this. We try to avoid describing > h/w like this. Well, I going to use this driver on 3 already upstreamed platforms and one upcoming. They all have such CREG 'GPIOs' differently mapped with different IO lines number, different enable/disable value, etc... So I really want to create some generic and configurable driver to handle both existing and upcoming platforms. > > + > > +Optional properties: > > +- snps,default-val: default output field values. Array the size of "snps,ngpios" > > + > > +Example (see picture): > > + > > +gpio: gpio@f00014b0 { > > + compatible = "snps,creg-gpio"; > > + reg = <0xf00014b0 0x4>; > > + gpio-controller; > > + #gpio-cells = <1>; > > + snps,ngpios = <2>; > > + snps,shift = <5 1>; > > + snps,bit-per-line = <2 3>; > > + snps,on-val = <2 1>; > > + snps,off-val = <3 4>; > > + snps,default-val = <2 1>; > > +}; > > -- > > 2.14.4 > > -- Eugeniy Paltsev ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: Document the Synopsys GPIO via CREG bindings 2018-08-28 11:27 ` [PATCH v2 2/2] dt-bindings: Document the Synopsys GPIO via CREG bindings Eugeniy Paltsev 2018-08-29 1:02 ` Rob Herring @ 2018-08-30 8:43 ` Linus Walleij 2018-08-30 18:16 ` Eugeniy Paltsev 1 sibling, 1 reply; 10+ messages in thread From: Linus Walleij @ 2018-08-30 8:43 UTC (permalink / raw) To: Eugeniy.Paltsev Cc: open list:SYNOPSYS ARC ARCHITECTURE, open list:GPIO SUBSYSTEM, linux-kernel, Vineet Gupta, Alexey Brodkin, Rob Herring, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Mark Rutland On Tue, Aug 28, 2018 at 1:27 PM Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> wrote: > +++ b/Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt > @@ -0,0 +1,49 @@ > +GPIO via CREG (Control REGisers) driver Speling Also should be "Synopsys GPIO via CREG" as this is likely just for Synopsys and not general purpose. > +This is is single-register MMIO GPIO driver to control such strangely mapped > +outputs: > + > +31 11 8 7 5 0 < bit number > +| | | | | | > +[ not used | gpio-1 | shift-1 | gpio-0 | shift-0 ] < 32 bit MMIO register > + ^ ^ > + | | > + | write 0x2 == set output to "1" (on) > + | write 0x3 == set output to "0" (off) > + | > + write 0x1 == set output to "1" (on) > + write 0x4 == set output to "0" (off) Move this documentation into the driver instead. > +Required properties: > +- compatible : "snps,creg-gpio" > +- reg : Exactly one register range with length 0x4. > +- #gpio-cells : Should be one - the pin number. > +- gpio-controller : Marks the device node as a GPIO controller. OK > +- snps,ngpios: Number of GPIO pins. Use the existing ngpios attribute for this, see gpio.txt > +- snps,bit-per-line: Number of bits per each gpio line (see picture). > + Array the size of "snps,ngpios" > +- snps,shift: Shift (in bits) of the each GPIO field from the previous one in > + register (see picture). Array the size of "snps,ngpios" > +- snps,on-val: Value should be set in corresponding field to set > + output to "1" (see picture). Array the size of "snps,ngpios" > +- snps,off-val: Value should be set in corresponding field to set > + output to "0" (see picture). Array the size of "snps,ngpios" Move this into a lookup table in the driver instead, and match the lookup table to the compatible string. The format of the register is known for a certain compatible, right? > +Optional properties: > +- snps,default-val: default output field values. Array the size of "snps,ngpios" Default values for different lines can be achieved by hogs if it's OK to tie them up perpetually, else work on creating generic inialization values in gpio.txt and implement that in gpiolib-of.c for everyone. This discussion comes up from time to time. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: Document the Synopsys GPIO via CREG bindings 2018-08-30 8:43 ` Linus Walleij @ 2018-08-30 18:16 ` Eugeniy Paltsev 2018-09-05 9:34 ` Linus Walleij 0 siblings, 1 reply; 10+ messages in thread From: Eugeniy Paltsev @ 2018-08-30 18:16 UTC (permalink / raw) To: Eugeniy.Paltsev, linus.walleij Cc: linux-kernel, robh+dt, Alexey.Brodkin, Vineet.Gupta1, devicetree, linux-snps-arc, linux-gpio, mark.rutland On Thu, 2018-08-30 at 10:43 +0200, Linus Walleij wrote: > On Tue, Aug 28, 2018 at 1:27 PM Eugeniy Paltsev > <Eugeniy.Paltsev@synopsys.com> wrote: > > > +++ b/Documentation/devicetree/bindings/gpio/snps,creg-gpio.txt > > @@ -0,0 +1,49 @@ > > +GPIO via CREG (Control REGisers) driver [snip] > > +- snps,ngpios: Number of GPIO pins. > > Use the existing ngpios attribute for this, see gpio.txt Ok > > +- snps,bit-per-line: Number of bits per each gpio line (see picture). > > + Array the size of "snps,ngpios" > > +- snps,shift: Shift (in bits) of the each GPIO field from the previous one in > > + register (see picture). Array the size of "snps,ngpios" > > +- snps,on-val: Value should be set in corresponding field to set > > + output to "1" (see picture). Array the size of "snps,ngpios" > > +- snps,off-val: Value should be set in corresponding field to set > > + output to "0" (see picture). Array the size of "snps,ngpios" > > Move this into a lookup table in the driver instead, and match > the lookup table to the compatible string. The format of the > register is known for a certain compatible, right? Actually I really don't want to hardcode this values into lookup table as I going to use this driver on 3 already upstreamed platforms and at least one upcoming. They all have such CREG pseudo-'GPIOs' differently mapped with different IO lines number, different enable/disable value, etc... Is it really a problem to have this values configured via device tree? If we read them from DT we are able to use this generic and configurable driver to handle both existing and upcoming platforms without the need of patching the driver on every new platform upstreaming. > > Yours, > Linus Walleij -- Eugeniy Paltsev ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: Document the Synopsys GPIO via CREG bindings 2018-08-30 18:16 ` Eugeniy Paltsev @ 2018-09-05 9:34 ` Linus Walleij 0 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2018-09-05 9:34 UTC (permalink / raw) To: Eugeniy.Paltsev Cc: linux-kernel, Rob Herring, Alexey Brodkin, Vineet Gupta, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list:SYNOPSYS ARC ARCHITECTURE, open list:GPIO SUBSYSTEM, Mark Rutland On Thu, Aug 30, 2018 at 8:16 PM Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> wrote: > On Thu, 2018-08-30 at 10:43 +0200, Linus Walleij wrote: > > > +- snps,bit-per-line: Number of bits per each gpio line (see picture). > > > + Array the size of "snps,ngpios" > > > +- snps,shift: Shift (in bits) of the each GPIO field from the previous one in > > > + register (see picture). Array the size of "snps,ngpios" > > > +- snps,on-val: Value should be set in corresponding field to set > > > + output to "1" (see picture). Array the size of "snps,ngpios" > > > +- snps,off-val: Value should be set in corresponding field to set > > > + output to "0" (see picture). Array the size of "snps,ngpios" > > > > Move this into a lookup table in the driver instead, and match > > the lookup table to the compatible string. The format of the > > register is known for a certain compatible, right? > > Actually I really don't want to hardcode this values into lookup table as I going to use > this driver on 3 already upstreamed platforms and at least one upcoming. > > They all have such CREG pseudo-'GPIOs' differently mapped with different IO lines number, > different enable/disable value, etc... So each of them will have their own compatible, and table entry, so what's the problem? If they don't have their own compatible, they should be added, because they are per definition not compatible if they need different values into different parts of the register. > Is it really a problem to have this values configured via device tree? Yes because the DT maintainers do not like that we use the device tree as a data dumping ground. pinctrl-single.c and some other real big pin controllers are dumping data into the device tree, but it is a dubious practice. Two wrongs doesn't make one right. > If we read them from DT we are able to use this generic and configurable driver to handle > both existing and upcoming platforms without the need of patching the driver on every new > platform upstreaming. But you will have to patch the driver to add a new compatible for each platform you're upstreaming anyway, so this isn't going to make things easier. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-09-05 9:35 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-28 11:27 [PATCH v2 0/2] GPIO: add single-register GPIO via CREG driver Eugeniy Paltsev 2018-08-28 11:27 ` [PATCH v2 1/2] GPIO: add single-register gpio via creg driver Eugeniy Paltsev 2018-08-28 18:15 ` Randy Dunlap 2018-08-30 12:14 ` Eugeniy Paltsev 2018-08-28 11:27 ` [PATCH v2 2/2] dt-bindings: Document the Synopsys GPIO via CREG bindings Eugeniy Paltsev 2018-08-29 1:02 ` Rob Herring 2018-08-30 13:12 ` Eugeniy Paltsev 2018-08-30 8:43 ` Linus Walleij 2018-08-30 18:16 ` Eugeniy Paltsev 2018-09-05 9:34 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).