* [PATCH v3 0/2] gpio: Add gpio-latch driver @ 2022-09-14 7:13 Sascha Hauer 2022-09-14 7:13 ` [PATCH v3 1/2] gpio: Add gpio latch driver Sascha Hauer 2022-09-14 7:13 ` [PATCH v3 2/2] dt-bindings: gpio: Add gpio-latch binding document Sascha Hauer 0 siblings, 2 replies; 10+ messages in thread From: Sascha Hauer @ 2022-09-14 7:13 UTC (permalink / raw) To: linux-gpio Cc: devicetree, linux-kernel, Linus Walleij, Bartosz Golaszewski, kernel, Sascha Hauer Third round of the gpio-latch driver. The review comments I received from v2 are integrated, for a changelog see the individual patches. This time there's also devicetree@vger.kernel.org on Cc which I forgot to add the last two rounds. Sascha Sascha Hauer (2): gpio: Add gpio latch driver dt-bindings: gpio: Add gpio-latch binding document .../devicetree/bindings/gpio/gpio-latch.yaml | 85 ++++++++ drivers/gpio/Kconfig | 6 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-latch.c | 192 ++++++++++++++++++ 4 files changed, 284 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-latch.yaml create mode 100644 drivers/gpio/gpio-latch.c -- 2.30.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/2] gpio: Add gpio latch driver 2022-09-14 7:13 [PATCH v3 0/2] gpio: Add gpio-latch driver Sascha Hauer @ 2022-09-14 7:13 ` Sascha Hauer 2022-09-14 14:03 ` Serge Semin 2022-09-18 14:56 ` Linus Walleij 2022-09-14 7:13 ` [PATCH v3 2/2] dt-bindings: gpio: Add gpio-latch binding document Sascha Hauer 1 sibling, 2 replies; 10+ messages in thread From: Sascha Hauer @ 2022-09-14 7:13 UTC (permalink / raw) To: linux-gpio Cc: devicetree, linux-kernel, Linus Walleij, Bartosz Golaszewski, kernel, Sascha Hauer This driver implements a GPIO multiplexer based on latches connected to other GPIOs. A set of data GPIOs is connected to the data input of multiple latches. The clock input of each latch is driven by another set of GPIOs. With two 8-bit latches 10 GPIOs can be multiplexed into 16 GPIOs. GPOs might be a better term as in fact the multiplexed pins are output only. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- Notes: Changes since v2: - Fix inconsistent licensing - document locks - use regular bit operations - include linux/mod_devicetable.h rather than linux/of_device.h - Put spinlock and mutex into a union to make clear that only one of them is used - rename __gpio_latch_set to gpio_latch_set_unlocked Changes since v1: - Use gpiod_set_value_cansleep when the underlying GPIOs might sleep - Move MODULE_DEVICE_TABLE near to the end drivers/gpio/Kconfig | 6 ++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-latch.c | 192 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 199 insertions(+) create mode 100644 drivers/gpio/gpio-latch.c diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 0642f579196f2..e4603810ec910 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1690,6 +1690,12 @@ config GPIO_AGGREGATOR industrial control context, to be operated from userspace using the GPIO chardev interface. +config GPIO_LATCH + tristate "GPIO latch driver" + help + Say yes here to enable a driver for GPIO multiplexers based on latches + connected to other GPIOs. + config GPIO_MOCKUP tristate "GPIO Testing Driver" select IRQ_SIM diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index a0985d30f51bb..310fa08decc69 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -75,6 +75,7 @@ obj-$(CONFIG_GPIO_IT87) += gpio-it87.o obj-$(CONFIG_GPIO_IXP4XX) += gpio-ixp4xx.o obj-$(CONFIG_GPIO_JANZ_TTL) += gpio-janz-ttl.o obj-$(CONFIG_GPIO_KEMPLD) += gpio-kempld.o +obj-$(CONFIG_GPIO_LATCH) += gpio-latch.o obj-$(CONFIG_GPIO_LOGICVC) += gpio-logicvc.o obj-$(CONFIG_GPIO_LOONGSON1) += gpio-loongson1.o obj-$(CONFIG_GPIO_LOONGSON) += gpio-loongson.o diff --git a/drivers/gpio/gpio-latch.c b/drivers/gpio/gpio-latch.c new file mode 100644 index 0000000000000..4134cba1c88a8 --- /dev/null +++ b/drivers/gpio/gpio-latch.c @@ -0,0 +1,192 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * GPIO latch driver + * + * Copyright (C) 2022 Sascha Hauer <s.hauer@pengutronix.de> + * + * This driver implements a GPIO (or better GPO as there is no input) + * multiplexer based on latches like this: + * + * CLK0 ----------------------. ,--------. + * CLK1 -------------------. `--------|> #0 | + * | | | + * OUT0 ----------------+--|-----------|D0 Q0|-----|< + * OUT1 --------------+-|--|-----------|D1 Q1|-----|< + * OUT2 ------------+-|-|--|-----------|D2 Q2|-----|< + * OUT3 ----------+-|-|-|--|-----------|D3 Q3|-----|< + * OUT4 --------+-|-|-|-|--|-----------|D4 Q4|-----|< + * OUT5 ------+-|-|-|-|-|--|-----------|D5 Q5|-----|< + * OUT6 ----+-|-|-|-|-|-|--|-----------|D6 Q6|-----|< + * OUT7 --+-|-|-|-|-|-|-|--|-----------|D7 Q7|-----|< + * | | | | | | | | | `--------' + * | | | | | | | | | + * | | | | | | | | | ,--------. + * | | | | | | | | `-----------|> #1 | + * | | | | | | | | | | + * | | | | | | | `--------------|D0 Q0|-----|< + * | | | | | | `----------------|D1 Q1|-----|< + * | | | | | `------------------|D2 Q2|-----|< + * | | | | `--------------------|D3 Q3|-----|< + * | | | `----------------------|D4 Q4|-----|< + * | | `------------------------|D5 Q5|-----|< + * | `--------------------------|D6 Q6|-----|< + * `----------------------------|D7 Q7|-----|< + * `--------' + * + * The above is just an example. The actual number of number of latches and + * the number of inputs per latch is derived from the number of GPIOs given + * in the corresponding device tree properties. + */ + +#include <linux/err.h> +#include <linux/gpio/consumer.h> +#include <linux/gpio/driver.h> +#include <linux/module.h> +#include <linux/mod_devicetable.h> +#include <linux/platform_device.h> + +#include "gpiolib.h" + +struct gpio_latch_priv { + struct gpio_chip gc; + struct gpio_descs *clk_gpios; + struct gpio_descs *latched_gpios; + int n_latched_gpios; + unsigned long *shadow; + /* + * Depending on whether any of the underlying GPIOs may sleep we either + * use a mutex or a spinlock to protect our shadow map. + */ + union { + struct mutex mutex; /* protects @shadow */ + spinlock_t spinlock; /* protects @shadow */ + }; +}; + +static int gpio_latch_get_direction(struct gpio_chip *gc, unsigned int offset) +{ + return GPIO_LINE_DIRECTION_OUT; +} + +static void gpio_latch_set_unlocked(struct gpio_latch_priv *priv, + void (*set)(struct gpio_desc *desc, int value), + unsigned int offset, bool val) +{ + int latch = offset / priv->n_latched_gpios; + int i; + + assign_bit(offset, priv->shadow, val); + + for (i = 0; i < priv->n_latched_gpios; i++) + set(priv->latched_gpios->desc[i], + test_bit(latch * priv->n_latched_gpios + i, priv->shadow)); + + set(priv->clk_gpios->desc[latch], 1); + set(priv->clk_gpios->desc[latch], 0); +} + +static void gpio_latch_set(struct gpio_chip *gc, unsigned int offset, int val) +{ + struct gpio_latch_priv *priv = gpiochip_get_data(gc); + unsigned long flags; + + spin_lock_irqsave(&priv->spinlock, flags); + + gpio_latch_set_unlocked(priv, gpiod_set_value, offset, val); + + spin_unlock_irqrestore(&priv->spinlock, flags); +} + +static void gpio_latch_set_can_sleep(struct gpio_chip *gc, unsigned int offset, int val) +{ + struct gpio_latch_priv *priv = gpiochip_get_data(gc); + + mutex_lock(&priv->mutex); + + gpio_latch_set_unlocked(priv, gpiod_set_value_cansleep, offset, val); + + mutex_unlock(&priv->mutex); +} + +static bool gpio_latch_can_sleep(struct gpio_latch_priv *priv, unsigned int n_latches) +{ + int i; + + for (i = 0; i < n_latches; i++) + if (gpiod_cansleep(priv->clk_gpios->desc[i])) + return true; + + for (i = 0; i < priv->n_latched_gpios; i++) + if (gpiod_cansleep(priv->latched_gpios->desc[i])) + return true; + + return false; +} + +static int gpio_latch_probe(struct platform_device *pdev) +{ + struct gpio_latch_priv *priv; + unsigned int n_latches; + + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->clk_gpios = devm_gpiod_get_array(&pdev->dev, "clk", GPIOD_OUT_LOW); + if (IS_ERR(priv->clk_gpios)) + return PTR_ERR(priv->clk_gpios); + + priv->latched_gpios = devm_gpiod_get_array(&pdev->dev, "latched", GPIOD_OUT_LOW); + if (IS_ERR(priv->latched_gpios)) + return PTR_ERR(priv->latched_gpios); + + n_latches = priv->clk_gpios->ndescs; + priv->n_latched_gpios = priv->latched_gpios->ndescs; + + priv->shadow = devm_bitmap_zalloc(&pdev->dev, n_latches * priv->n_latched_gpios, + GFP_KERNEL); + if (!priv->shadow) + return -ENOMEM; + + if (gpio_latch_can_sleep(priv, n_latches)) { + priv->gc.can_sleep = true; + priv->gc.set = gpio_latch_set_can_sleep; + mutex_init(&priv->mutex); + } else { + priv->gc.can_sleep = false; + priv->gc.set = gpio_latch_set; + spin_lock_init(&priv->spinlock); + } + + priv->gc.get_direction = gpio_latch_get_direction; + priv->gc.ngpio = n_latches * priv->n_latched_gpios; + priv->gc.owner = THIS_MODULE; + priv->gc.base = -1; + priv->gc.parent = &pdev->dev; + + platform_set_drvdata(pdev, priv); + + return devm_gpiochip_add_data(&pdev->dev, &priv->gc, priv); +} + +static const struct of_device_id gpio_latch_ids[] = { + { + .compatible = "gpio-latch", + }, { + /* sentinel */ + } +}; +MODULE_DEVICE_TABLE(of, gpio_latch_ids); + +static struct platform_driver gpio_latch_driver = { + .driver = { + .name = "gpio-latch", + .of_match_table = gpio_latch_ids, + }, + .probe = gpio_latch_probe, +}; +module_platform_driver(gpio_latch_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>"); +MODULE_DESCRIPTION("GPIO latch driver"); -- 2.30.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] gpio: Add gpio latch driver 2022-09-14 7:13 ` [PATCH v3 1/2] gpio: Add gpio latch driver Sascha Hauer @ 2022-09-14 14:03 ` Serge Semin 2022-09-22 13:31 ` Sascha Hauer 2022-09-18 14:56 ` Linus Walleij 1 sibling, 1 reply; 10+ messages in thread From: Serge Semin @ 2022-09-14 14:03 UTC (permalink / raw) To: Sascha Hauer Cc: linux-gpio, devicetree, linux-kernel, Linus Walleij, Bartosz Golaszewski, kernel On Wed, Sep 14, 2022 at 09:13:05AM +0200, Sascha Hauer wrote: > This driver implements a GPIO multiplexer based on latches connected to > other GPIOs. A set of data GPIOs is connected to the data input of > multiple latches. The clock input of each latch is driven by another > set of GPIOs. With two 8-bit latches 10 GPIOs can be multiplexed into > 16 GPIOs. GPOs might be a better term as in fact the multiplexed pins > are output only. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > > Notes: > Changes since v2: > - Fix inconsistent licensing > - document locks > - use regular bit operations > - include linux/mod_devicetable.h rather than linux/of_device.h > - Put spinlock and mutex into a union to make clear that only one of them is used > - rename __gpio_latch_set to gpio_latch_set_unlocked > > Changes since v1: > - Use gpiod_set_value_cansleep when the underlying GPIOs might sleep > - Move MODULE_DEVICE_TABLE near to the end > > drivers/gpio/Kconfig | 6 ++ > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-latch.c | 192 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 199 insertions(+) > create mode 100644 drivers/gpio/gpio-latch.c > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 0642f579196f2..e4603810ec910 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -1690,6 +1690,12 @@ config GPIO_AGGREGATOR > industrial control context, to be operated from userspace using > the GPIO chardev interface. > > +config GPIO_LATCH > + tristate "GPIO latch driver" > + help > + Say yes here to enable a driver for GPIO multiplexers based on latches > + connected to other GPIOs. > + > config GPIO_MOCKUP > tristate "GPIO Testing Driver" > select IRQ_SIM > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index a0985d30f51bb..310fa08decc69 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -75,6 +75,7 @@ obj-$(CONFIG_GPIO_IT87) += gpio-it87.o > obj-$(CONFIG_GPIO_IXP4XX) += gpio-ixp4xx.o > obj-$(CONFIG_GPIO_JANZ_TTL) += gpio-janz-ttl.o > obj-$(CONFIG_GPIO_KEMPLD) += gpio-kempld.o > +obj-$(CONFIG_GPIO_LATCH) += gpio-latch.o > obj-$(CONFIG_GPIO_LOGICVC) += gpio-logicvc.o > obj-$(CONFIG_GPIO_LOONGSON1) += gpio-loongson1.o > obj-$(CONFIG_GPIO_LOONGSON) += gpio-loongson.o > diff --git a/drivers/gpio/gpio-latch.c b/drivers/gpio/gpio-latch.c > new file mode 100644 > index 0000000000000..4134cba1c88a8 > --- /dev/null > +++ b/drivers/gpio/gpio-latch.c > @@ -0,0 +1,192 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * GPIO latch driver > + * > + * Copyright (C) 2022 Sascha Hauer <s.hauer@pengutronix.de> > + * > + * This driver implements a GPIO (or better GPO as there is no input) > + * multiplexer based on latches like this: > + * > + * CLK0 ----------------------. ,--------. > + * CLK1 -------------------. `--------|> #0 | > + * | | | > + * OUT0 ----------------+--|-----------|D0 Q0|-----|< > + * OUT1 --------------+-|--|-----------|D1 Q1|-----|< > + * OUT2 ------------+-|-|--|-----------|D2 Q2|-----|< > + * OUT3 ----------+-|-|-|--|-----------|D3 Q3|-----|< > + * OUT4 --------+-|-|-|-|--|-----------|D4 Q4|-----|< > + * OUT5 ------+-|-|-|-|-|--|-----------|D5 Q5|-----|< > + * OUT6 ----+-|-|-|-|-|-|--|-----------|D6 Q6|-----|< > + * OUT7 --+-|-|-|-|-|-|-|--|-----------|D7 Q7|-----|< > + * | | | | | | | | | `--------' > + * | | | | | | | | | > + * | | | | | | | | | ,--------. > + * | | | | | | | | `-----------|> #1 | > + * | | | | | | | | | | > + * | | | | | | | `--------------|D0 Q0|-----|< > + * | | | | | | `----------------|D1 Q1|-----|< > + * | | | | | `------------------|D2 Q2|-----|< > + * | | | | `--------------------|D3 Q3|-----|< > + * | | | `----------------------|D4 Q4|-----|< > + * | | `------------------------|D5 Q5|-----|< > + * | `--------------------------|D6 Q6|-----|< > + * `----------------------------|D7 Q7|-----|< > + * `--------' > + * > + * The above is just an example. The actual number of number of latches and > + * the number of inputs per latch is derived from the number of GPIOs given > + * in the corresponding device tree properties. > + */ > + > +#include <linux/err.h> > +#include <linux/gpio/consumer.h> > +#include <linux/gpio/driver.h> > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > +#include <linux/platform_device.h> > + > +#include "gpiolib.h" > + > +struct gpio_latch_priv { > + struct gpio_chip gc; > + struct gpio_descs *clk_gpios; > + struct gpio_descs *latched_gpios; > + int n_latched_gpios; > + unsigned long *shadow; > + /* > + * Depending on whether any of the underlying GPIOs may sleep we either > + * use a mutex or a spinlock to protect our shadow map. > + */ > + union { > + struct mutex mutex; /* protects @shadow */ > + spinlock_t spinlock; /* protects @shadow */ > + }; > +}; > + > +static int gpio_latch_get_direction(struct gpio_chip *gc, unsigned int offset) > +{ > + return GPIO_LINE_DIRECTION_OUT; > +} > + > +static void gpio_latch_set_unlocked(struct gpio_latch_priv *priv, > + void (*set)(struct gpio_desc *desc, int value), > + unsigned int offset, bool val) > +{ > + int latch = offset / priv->n_latched_gpios; > + int i; > + > + assign_bit(offset, priv->shadow, val); > + > + for (i = 0; i < priv->n_latched_gpios; i++) > + set(priv->latched_gpios->desc[i], > + test_bit(latch * priv->n_latched_gpios + i, priv->shadow)); -> duration? > + > + set(priv->clk_gpios->desc[latch], 1); -> duration? > + set(priv->clk_gpios->desc[latch], 0); I am pretty much sure there must be some duration between the actions above *. See for instance the tw and (tsu + th) timing requirements in the next edge-triggered flip-flops: https://www.ti.com/lit/ds/symlink/sn74lv74a.pdf?ts=1663163389954&ref_url=https%253A%252F%252Fwww.google.com%252F The durations are normally small (ns or a bit smaller) but still need to be added anyway. Note since the durations are device-specific an additional DT-property array with durations should be added too. * already faced weird problems in this regard. -Sergey > +} > + > +static void gpio_latch_set(struct gpio_chip *gc, unsigned int offset, int val) > +{ > + struct gpio_latch_priv *priv = gpiochip_get_data(gc); > + unsigned long flags; > + > + spin_lock_irqsave(&priv->spinlock, flags); > + > + gpio_latch_set_unlocked(priv, gpiod_set_value, offset, val); > + > + spin_unlock_irqrestore(&priv->spinlock, flags); > +} > + > +static void gpio_latch_set_can_sleep(struct gpio_chip *gc, unsigned int offset, int val) > +{ > + struct gpio_latch_priv *priv = gpiochip_get_data(gc); > + > + mutex_lock(&priv->mutex); > + > + gpio_latch_set_unlocked(priv, gpiod_set_value_cansleep, offset, val); > + > + mutex_unlock(&priv->mutex); > +} > + > +static bool gpio_latch_can_sleep(struct gpio_latch_priv *priv, unsigned int n_latches) > +{ > + int i; > + > + for (i = 0; i < n_latches; i++) > + if (gpiod_cansleep(priv->clk_gpios->desc[i])) > + return true; > + > + for (i = 0; i < priv->n_latched_gpios; i++) > + if (gpiod_cansleep(priv->latched_gpios->desc[i])) > + return true; > + > + return false; > +} > + > +static int gpio_latch_probe(struct platform_device *pdev) > +{ > + struct gpio_latch_priv *priv; > + unsigned int n_latches; > + > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->clk_gpios = devm_gpiod_get_array(&pdev->dev, "clk", GPIOD_OUT_LOW); > + if (IS_ERR(priv->clk_gpios)) > + return PTR_ERR(priv->clk_gpios); > + > + priv->latched_gpios = devm_gpiod_get_array(&pdev->dev, "latched", GPIOD_OUT_LOW); > + if (IS_ERR(priv->latched_gpios)) > + return PTR_ERR(priv->latched_gpios); > + > + n_latches = priv->clk_gpios->ndescs; > + priv->n_latched_gpios = priv->latched_gpios->ndescs; > + > + priv->shadow = devm_bitmap_zalloc(&pdev->dev, n_latches * priv->n_latched_gpios, > + GFP_KERNEL); > + if (!priv->shadow) > + return -ENOMEM; > + > + if (gpio_latch_can_sleep(priv, n_latches)) { > + priv->gc.can_sleep = true; > + priv->gc.set = gpio_latch_set_can_sleep; > + mutex_init(&priv->mutex); > + } else { > + priv->gc.can_sleep = false; > + priv->gc.set = gpio_latch_set; > + spin_lock_init(&priv->spinlock); > + } > + > + priv->gc.get_direction = gpio_latch_get_direction; > + priv->gc.ngpio = n_latches * priv->n_latched_gpios; > + priv->gc.owner = THIS_MODULE; > + priv->gc.base = -1; > + priv->gc.parent = &pdev->dev; > + > + platform_set_drvdata(pdev, priv); > + > + return devm_gpiochip_add_data(&pdev->dev, &priv->gc, priv); > +} > + > +static const struct of_device_id gpio_latch_ids[] = { > + { > + .compatible = "gpio-latch", > + }, { > + /* sentinel */ > + } > +}; > +MODULE_DEVICE_TABLE(of, gpio_latch_ids); > + > +static struct platform_driver gpio_latch_driver = { > + .driver = { > + .name = "gpio-latch", > + .of_match_table = gpio_latch_ids, > + }, > + .probe = gpio_latch_probe, > +}; > +module_platform_driver(gpio_latch_driver); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>"); > +MODULE_DESCRIPTION("GPIO latch driver"); > -- > 2.30.2 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] gpio: Add gpio latch driver 2022-09-14 14:03 ` Serge Semin @ 2022-09-22 13:31 ` Sascha Hauer 2022-09-25 15:30 ` Serge Semin 0 siblings, 1 reply; 10+ messages in thread From: Sascha Hauer @ 2022-09-22 13:31 UTC (permalink / raw) To: Serge Semin Cc: linux-gpio, devicetree, linux-kernel, Linus Walleij, Bartosz Golaszewski, kernel On Wed, Sep 14, 2022 at 05:03:10PM +0300, Serge Semin wrote: > > + unsigned int offset, bool val) > > +{ > > + int latch = offset / priv->n_latched_gpios; > > + int i; > > + > > + assign_bit(offset, priv->shadow, val); > > + > > > + for (i = 0; i < priv->n_latched_gpios; i++) > > + set(priv->latched_gpios->desc[i], > > + test_bit(latch * priv->n_latched_gpios + i, priv->shadow)); > > -> duration? > > > + > > + set(priv->clk_gpios->desc[latch], 1); > > -> duration? > > > + set(priv->clk_gpios->desc[latch], 0); > > I am pretty much sure there must be some duration between the actions > above *. See for instance the tw and (tsu + th) timing requirements in > the next edge-triggered flip-flops: > https://www.ti.com/lit/ds/symlink/sn74lv74a.pdf?ts=1663163389954&ref_url=https%253A%252F%252Fwww.google.com%252F > > The durations are normally small (ns or a bit smaller) but still need > to be added anyway. > > Note since the durations are device-specific an additional DT-property array > with durations should be added too. Do you think a fixed udelay(1) would be enough for now? Bigger delays shouldn't be needed and smaller delays expand to udelay(1) anyway on architectures not providing an architecture specific ndelay(). Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] gpio: Add gpio latch driver 2022-09-22 13:31 ` Sascha Hauer @ 2022-09-25 15:30 ` Serge Semin 2022-10-04 7:27 ` Linus Walleij 0 siblings, 1 reply; 10+ messages in thread From: Serge Semin @ 2022-09-25 15:30 UTC (permalink / raw) To: Sascha Hauer, Rob Herring, Rob Herring, Linus Walleij Cc: linux-gpio, devicetree, linux-kernel, Bartosz Golaszewski, kernel To += @Rob, @Linus On Thu, Sep 22, 2022 at 03:31:05PM +0200, Sascha Hauer wrote: > On Wed, Sep 14, 2022 at 05:03:10PM +0300, Serge Semin wrote: > > > + unsigned int offset, bool val) > > > +{ > > > + int latch = offset / priv->n_latched_gpios; > > > + int i; > > > + > > > + assign_bit(offset, priv->shadow, val); > > > + > > > > > + for (i = 0; i < priv->n_latched_gpios; i++) > > > + set(priv->latched_gpios->desc[i], > > > + test_bit(latch * priv->n_latched_gpios + i, priv->shadow)); > > > > -> duration? > > > > > + > > > + set(priv->clk_gpios->desc[latch], 1); > > > > -> duration? > > > > > + set(priv->clk_gpios->desc[latch], 0); > > > > I am pretty much sure there must be some duration between the actions > > above *. See for instance the tw and (tsu + th) timing requirements in > > the next edge-triggered flip-flops: > > https://www.ti.com/lit/ds/symlink/sn74lv74a.pdf?ts=1663163389954&ref_url=https%253A%252F%252Fwww.google.com%252F > > > > The durations are normally small (ns or a bit smaller) but still need > > to be added anyway. > > > > Note since the durations are device-specific an additional DT-property array > > with durations should be added too. > > Do you think a fixed udelay(1) would be enough for now? Bigger delays > shouldn't be needed and smaller delays expand to udelay(1) anyway on > architectures not providing an architecture specific ndelay(). I am sure you shouldn't assume what the particular architecture provide and what it doesn't. When it comes to the GPIOs the switching timings can have a critical value in a lot of applications (like i2c bitbang, or real-time systems). There is no point in waiting for micro seconds in the fast-path like this when there is only a few nano seconds delay required. I couldn't find any generic ready-to-use DT-property for this case. So IMO instead the next properties would work: 1. "setup-duration-ns" - data input timing after which the clock input can be asserted (Tsu in the hw-manual above). 2. "clock-duration-ns" - clock input timing for which the CLK signal must be kept asserted so the device would perceive the input states, the outputs would be updated and the clock signal could be driven back to low (Tw including Th in the hw-manual above). @Rob, @Linus, any suggestion regarding the properties and their naming? -Serge > > Sascha > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] gpio: Add gpio latch driver 2022-09-25 15:30 ` Serge Semin @ 2022-10-04 7:27 ` Linus Walleij 0 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2022-10-04 7:27 UTC (permalink / raw) To: Serge Semin Cc: Sascha Hauer, Rob Herring, Rob Herring, linux-gpio, devicetree, linux-kernel, Bartosz Golaszewski, kernel On Sun, Sep 25, 2022 at 5:30 PM Serge Semin <fancer.lancer@gmail.com> wrote: > I couldn't find any generic ready-to-use DT-property for this case. > So IMO instead the next properties would work: > 1. "setup-duration-ns" - data input timing after which the clock input > can be asserted (Tsu in the hw-manual above). > 2. "clock-duration-ns" - clock input timing for which the CLK signal > must be kept asserted so the device would perceive the input > states, the outputs would be updated and the clock signal could be > driven back to low (Tw including Th in the hw-manual above). > > @Rob, @Linus, any suggestion regarding the properties and their naming? I think your suggestions look fine! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] gpio: Add gpio latch driver 2022-09-14 7:13 ` [PATCH v3 1/2] gpio: Add gpio latch driver Sascha Hauer 2022-09-14 14:03 ` Serge Semin @ 2022-09-18 14:56 ` Linus Walleij 1 sibling, 0 replies; 10+ messages in thread From: Linus Walleij @ 2022-09-18 14:56 UTC (permalink / raw) To: Sascha Hauer Cc: linux-gpio, devicetree, linux-kernel, Bartosz Golaszewski, kernel On Wed, Sep 14, 2022 at 9:13 AM Sascha Hauer <s.hauer@pengutronix.de> wrote: > > This driver implements a GPIO multiplexer based on latches connected to > other GPIOs. A set of data GPIOs is connected to the data input of > multiple latches. The clock input of each latch is driven by another > set of GPIOs. With two 8-bit latches 10 GPIOs can be multiplexed into > 16 GPIOs. GPOs might be a better term as in fact the multiplexed pins > are output only. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> I see Serge has additional comments but apart from that this looks acceptable to me: Reviewed-by: Linus Walleij <linus.walleij@linaro.org> You may want to add set_multiple[_cansleep] later, but it will survive without it. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] dt-bindings: gpio: Add gpio-latch binding document 2022-09-14 7:13 [PATCH v3 0/2] gpio: Add gpio-latch driver Sascha Hauer 2022-09-14 7:13 ` [PATCH v3 1/2] gpio: Add gpio latch driver Sascha Hauer @ 2022-09-14 7:13 ` Sascha Hauer 2022-09-14 15:05 ` Rob Herring 2022-09-18 14:57 ` Linus Walleij 1 sibling, 2 replies; 10+ messages in thread From: Sascha Hauer @ 2022-09-14 7:13 UTC (permalink / raw) To: linux-gpio Cc: devicetree, linux-kernel, Linus Walleij, Bartosz Golaszewski, kernel, Sascha Hauer This adds a binding for a GPIO multiplexer driver based on latches connected to other GPIOs. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- Notes: Changes since v1: - Add license to binding file .../devicetree/bindings/gpio/gpio-latch.yaml | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-latch.yaml diff --git a/Documentation/devicetree/bindings/gpio/gpio-latch.yaml b/Documentation/devicetree/bindings/gpio/gpio-latch.yaml new file mode 100644 index 0000000000000..856a9e1e43b45 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-latch.yaml @@ -0,0 +1,85 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/gpio/gpio-latch.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: GPIO latch controller + +maintainers: + - Sascha Hauer <s.hauer@pengutronix.de> + +description: | + This binding describes a GPIO multiplexer based on latches connected to + other GPIOs, like this: + + CLK0 ----------------------. ,--------. + CLK1 -------------------. `--------|> #0 | + | | | + OUT0 ----------------+--|-----------|D0 Q0|-----|< + OUT1 --------------+-|--|-----------|D1 Q1|-----|< + OUT2 ------------+-|-|--|-----------|D2 Q2|-----|< + OUT3 ----------+-|-|-|--|-----------|D3 Q3|-----|< + OUT4 --------+-|-|-|-|--|-----------|D4 Q4|-----|< + OUT5 ------+-|-|-|-|-|--|-----------|D5 Q5|-----|< + OUT6 ----+-|-|-|-|-|-|--|-----------|D6 Q6|-----|< + OUT7 --+-|-|-|-|-|-|-|--|-----------|D7 Q7|-----|< + | | | | | | | | | `--------' + | | | | | | | | | + | | | | | | | | | ,--------. + | | | | | | | | `-----------|> #1 | + | | | | | | | | | | + | | | | | | | `--------------|D0 Q0|-----|< + | | | | | | `----------------|D1 Q1|-----|< + | | | | | `------------------|D2 Q2|-----|< + | | | | `--------------------|D3 Q3|-----|< + | | | `----------------------|D4 Q4|-----|< + | | `------------------------|D5 Q5|-----|< + | `--------------------------|D6 Q6|-----|< + `----------------------------|D7 Q7|-----|< + `--------' + + The number of clk-gpios and latched-gpios is not fixed. The actual number + of number of latches and the number of inputs per latch is derived from + the number of GPIOs given in the corresponding device tree properties. + +properties: + compatible: + const: gpio-latch + "#gpio-cells": + const: 2 + + clk-gpios: + description: Array of GPIOs to be used to clock a latch + + latched-gpios: + description: Array of GPIOs to be used as inputs per latch + + gpio-controller: true + + gpio-line-names: true + +required: + - compatible + - "#gpio-cells" + - gpio-controller + - clk-gpios + - latched-gpios + +additionalProperties: false + +examples: + - | + gpio-latch { + #gpio-cells = <2>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_di_do_leds>; + compatible = "gpio-latch"; + gpio-controller; + + clk-gpios = <&gpio3 7 0>, <&gpio3 8 0>; + latched-gpios = <&gpio3 21 0>, <&gpio3 22 0>, + <&gpio3 23 0>, <&gpio3 24 0>, + <&gpio3 25 0>, <&gpio3 26 0>, + <&gpio3 27 0>, <&gpio3 28 0>; + }; -- 2.30.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: gpio: Add gpio-latch binding document 2022-09-14 7:13 ` [PATCH v3 2/2] dt-bindings: gpio: Add gpio-latch binding document Sascha Hauer @ 2022-09-14 15:05 ` Rob Herring 2022-09-18 14:57 ` Linus Walleij 1 sibling, 0 replies; 10+ messages in thread From: Rob Herring @ 2022-09-14 15:05 UTC (permalink / raw) To: Sascha Hauer Cc: kernel, Bartosz Golaszewski, linux-kernel, linux-gpio, devicetree, Linus Walleij On Wed, 14 Sep 2022 09:13:06 +0200, Sascha Hauer wrote: > This adds a binding for a GPIO multiplexer driver based on latches > connected to other GPIOs. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > > Notes: > Changes since v1: > - Add license to binding file > > .../devicetree/bindings/gpio/gpio-latch.yaml | 85 +++++++++++++++++++ > 1 file changed, 85 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpio/gpio-latch.yaml > Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: gpio: Add gpio-latch binding document 2022-09-14 7:13 ` [PATCH v3 2/2] dt-bindings: gpio: Add gpio-latch binding document Sascha Hauer 2022-09-14 15:05 ` Rob Herring @ 2022-09-18 14:57 ` Linus Walleij 1 sibling, 0 replies; 10+ messages in thread From: Linus Walleij @ 2022-09-18 14:57 UTC (permalink / raw) To: Sascha Hauer Cc: linux-gpio, devicetree, linux-kernel, Bartosz Golaszewski, kernel On Wed, Sep 14, 2022 at 9:13 AM Sascha Hauer <s.hauer@pengutronix.de> wrote: > This adds a binding for a GPIO multiplexer driver based on latches > connected to other GPIOs. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-10-04 7:28 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-09-14 7:13 [PATCH v3 0/2] gpio: Add gpio-latch driver Sascha Hauer 2022-09-14 7:13 ` [PATCH v3 1/2] gpio: Add gpio latch driver Sascha Hauer 2022-09-14 14:03 ` Serge Semin 2022-09-22 13:31 ` Sascha Hauer 2022-09-25 15:30 ` Serge Semin 2022-10-04 7:27 ` Linus Walleij 2022-09-18 14:56 ` Linus Walleij 2022-09-14 7:13 ` [PATCH v3 2/2] dt-bindings: gpio: Add gpio-latch binding document Sascha Hauer 2022-09-14 15:05 ` Rob Herring 2022-09-18 14:57 ` 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).