[RFC v2 1/3] ath79: add gpio-latch driver for Mikrotik RouterBoards
Sergey Ryazanov
ryazanov.s.a at gmail.com
Sun May 23 18:34:32 PDT 2021
On Fri, May 21, 2021 at 2:05 PM Denis Kalashnikov <denis281089 at gmail.com> wrote:
> This is a slighty modified version of ar71xx gpio-latch driver
> written by Gabor Juhos <juhosg at openwrt.org>.
>
> Changes:
> * DTS support,
> * New gpio API (gpiod_*).
>
> Signed-off-by: Denis Kalashnikov <denis281089 at gmail.com>
> ---
>
> Changelog:
>
> v1-->v2:
> - Don't use MFD driver API anymore.
>
> ---
> .../ath79/files/drivers/gpio/gpio-latch.c | 225 ++++++++++++++++++
> .../patches-5.10/939-mikrotik-rb91x.patch | 25 ++
> .../patches-5.4/939-mikrotik-rb91x.patch | 23 ++
> 3 files changed, 273 insertions(+)
> create mode 100644 target/linux/ath79/files/drivers/gpio/gpio-latch.c
> create mode 100644 target/linux/ath79/patches-5.10/939-mikrotik-rb91x.patch
> create mode 100644 target/linux/ath79/patches-5.4/939-mikrotik-rb91x.patch
>
> diff --git a/target/linux/ath79/files/drivers/gpio/gpio-latch.c b/target/linux/ath79/files/drivers/gpio/gpio-latch.c
> new file mode 100644
> index 0000000000..cb80b29052
> --- /dev/null
> +++ b/target/linux/ath79/files/drivers/gpio/gpio-latch.c
> @@ -0,0 +1,225 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * GPIO latch driver
> + *
> + * Copyright (C) 2014 Gabor Juhos <juhosg at openwrt.org>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_gpio.h>
> +
> +#define GPIO_LATCH_DRIVER_NAME "gpio-latch"
> +#define GPIO_LATCH_LINES 9
> +
> +struct gpio_latch_chip {
> + struct gpio_chip gc;
> + struct mutex mutex;
> + struct mutex latch_mutex;
> + bool latch_enabled;
> + int le_gpio;
> + bool le_active_low;
> + struct gpio_desc **gpios;
> +};
> +
> +static inline struct gpio_latch_chip *to_gpio_latch_chip(struct gpio_chip *gc)
> +{
> + return container_of(gc, struct gpio_latch_chip, gc);
> +}
> +
> +static void gpio_latch_lock(struct gpio_latch_chip *glc, bool enable)
> +{
> + mutex_lock(&glc->mutex);
> +
> + if (enable)
> + glc->latch_enabled = true;
> +
> + if (glc->latch_enabled)
> + mutex_lock(&glc->latch_mutex);
> +}
> +
> +static void gpio_latch_unlock(struct gpio_latch_chip *glc, bool disable)
> +{
> + if (glc->latch_enabled)
> + mutex_unlock(&glc->latch_mutex);
> +
> + if (disable)
> + glc->latch_enabled = true;
> +
> + mutex_unlock(&glc->mutex);
> +}
> +
> +static int
> +gpio_latch_get(struct gpio_chip *gc, unsigned offset)
> +{
> + struct gpio_latch_chip *glc = to_gpio_latch_chip(gc);
> + int ret;
> +
> + gpio_latch_lock(glc, false);
> + ret = gpiod_get_value(glc->gpios[offset]);
> + gpio_latch_unlock(glc, false);
> +
> + return ret;
> +}
> +
> +static void
> +gpio_latch_set(struct gpio_chip *gc, unsigned offset, int value)
> +{
> + struct gpio_latch_chip *glc = to_gpio_latch_chip(gc);
> + bool enable_latch = false;
> + bool disable_latch = false;
> +
> + if (offset == glc->le_gpio) {
> + enable_latch = value ^ glc->le_active_low;
> + disable_latch = !enable_latch;
> + }
> +
> + gpio_latch_lock(glc, enable_latch);
> + gpiod_set_value(glc->gpios[offset], value);
> + gpio_latch_unlock(glc, disable_latch);
> +}
> +
> +static int
> +gpio_latch_direction_input(struct gpio_chip *gc, unsigned offset)
Latch is the output-only GPIO controller, so why do you need this callback?
> +{
> + struct gpio_latch_chip *glc = to_gpio_latch_chip(gc);
> + int ret;
> +
> + gpio_latch_lock(glc, false);
> + ret = gpiod_direction_input(glc->gpios[offset]);
> + gpio_latch_unlock(glc, false);
> +
> + return ret;
> +}
> +
> +static int
> +gpio_latch_direction_output(struct gpio_chip *gc, unsigned offset, int value)
> +{
> + struct gpio_latch_chip *glc = to_gpio_latch_chip(gc);
> + bool enable_latch = false;
> + bool disable_latch = false;
> + int ret;
> +
> + if (offset == glc->le_gpio) {
> + enable_latch = value ^ glc->le_active_low;
> + disable_latch = !enable_latch;
> + }
> +
> + gpio_latch_lock(glc, enable_latch);
> + ret = gpiod_direction_output(glc->gpios[offset], value);
> + gpio_latch_unlock(glc, disable_latch);
> +
> + return ret;
> +}
> +
> +static int gpio_latch_probe(struct platform_device *pdev)
> +{
> + struct gpio_latch_chip *glc;
> + struct gpio_chip *gc;
> + struct device *dev = &pdev->dev;
> + struct device_node *of_node = dev->of_node;
> + struct gpio_descs *gpios;
> + int i;
> + enum of_gpio_flags flags;
> +
> + glc = devm_kzalloc(dev, sizeof(*glc), GFP_KERNEL);
> + if (!glc)
> + return -ENOMEM;
> +
> + mutex_init(&glc->mutex);
> + mutex_init(&glc->latch_mutex);
> +
> + gpios = devm_gpiod_get_array(dev, NULL, 0);
> + if (IS_ERR(gpios)) {
> + dev_err(dev, "failed to get gpios: %d\n", (int)gpios);
> + return -EINVAL;
> + }
> + glc->gpios = gpios->desc;
> +
> + if (gpios->ndescs != GPIO_LATCH_LINES) {
> + dev_err(dev, "gpios count must be %d\n", GPIO_LATCH_LINES);
> + return -EINVAL;
> + }
> +
> + glc->le_gpio = of_get_named_gpio_flags(of_node, "latch-enable-gpios", 0,
> + &flags);
> + if (glc->le_gpio < 0) {
> + dev_err(dev,
> + "could not read required 'latch-enable-gpios' property: %d\n",
> + glc->le_gpio);
> + return -EINVAL;
> + }
> + glc->le_active_low = (flags == OF_GPIO_ACTIVE_LOW) ? true : false;
Should this be:
glc->le_active_low = flags & OF_GPIO_ACTIVE_LOW;
> + for (i = 0; i < gpios->ndescs; i++) {
> + if (glc->le_gpio == desc_to_gpio(glc->gpios[i])) {
> + glc->le_gpio = i;
> + break;
> + }
> + }
> +
> + if (i == gpios->ndescs) {
> + dev_err(dev, "latch-enable-gpio must be in gpios list\n");
> + return -EINVAL;
> + }
Why do you need these dances with the latch enable line that must be
specified both in 'gpios' and in 'latch-enable-gpios'? Is it possible
to make DTS and code a bit more simple by:
(1) taking the LE line off the 'gpios' array and specify it only in
the 'latch-enable-gpios';
(2) or turn the 'latch-enable-gpios' property to 'latch-enable-idx'
that will specify the LE line _index_ within the 'gpios' array;
(3) or entirely remove the 'latch-enable-gpios' property and say that
the entry #8 within the 'gpios' array has a magic meaning of the LE
line.
> + gc = &glc->gc;
> + gc->label = GPIO_LATCH_DRIVER_NAME;
> + gc->can_sleep = true;
> + gc->base = -1;
> + gc->ngpio = gpios->ndescs;
> + gc->get = gpio_latch_get;
> + gc->set = gpio_latch_set;
> + gc->direction_input = gpio_latch_direction_input,
> + gc->direction_output = gpio_latch_direction_output;
> + gc->of_node = of_node;
> +
> + platform_set_drvdata(pdev, glc);
> +
> + i = gpiochip_add(&glc->gc);
> + if (i) {
> + dev_err(dev, "gpiochip_add() failed: %d\n", i);
> + return i;
> + }
> +
> + return 0;
> +}
> +
> +static int gpio_latch_remove(struct platform_device *pdev)
> +{
> + struct gpio_latch_chip *glc = platform_get_drvdata(pdev);
> +
> + gpiochip_remove(&glc->gc);
> + return 0;
> +}
> +
> +static const struct of_device_id gpio_latch_match[] = {
> + { .compatible = GPIO_LATCH_DRIVER_NAME },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, gpio_latch_match);
> +
> +static struct platform_driver gpio_latch_driver = {
> + .probe = gpio_latch_probe,
> + .remove = gpio_latch_remove,
> + .driver = {
> + .name = GPIO_LATCH_DRIVER_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = gpio_latch_match,
> + },
> +};
> +
> +module_platform_driver(gpio_latch_driver);
> +
> +MODULE_DESCRIPTION("GPIO latch driver");
> +MODULE_AUTHOR("Gabor Juhos <juhosg at openwrt.org>");
> +MODULE_AUTHOR("Denis Kalashnikov <denis281089 at gmail.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" GPIO_LATCH_DRIVER_NAME);
--
Sergey
More information about the openwrt-devel
mailing list