On 13/07/2020 23:26, Linus Walleij wrote:
> On Mon, Jul 13, 2020 at 4:20 PM Rodolfo Giometti <giometti@xxxxxxxxxxxx> wrote:
>> Maybe I can do something similar to hog-gpio as below, if you prefer...
>>
>> bypass0 {
>> gpios = <&gpionb 10 GPIO_ACTIVE_LOW>;
>> output-low;
>
> Yes this is better, just boolean flags is not natural than strings
> for this.
OK, changed.
> However it addresses in a way an issue we have had popping
> up from time to time which is assignment of default values to
> lines before they are used overall.
>
> I think that would be a bit of thing that should be proper to
> solve as part of this.
>
> The discussion has often stopped short due to different
> opinions on the device tree bindings for that.
I see... however attached is a new version of my proposal patch with the
following changelog:
- GPIO line modes are now decoded as boolean properties (as for gpio-hogs).
Allowed values are" "input", "output-low" and "output-high". If nothing is
specified defaults to "as-is";
- At boot a more descriptive message for each line is displayed as below:
[ 1.834901] line bypass0: GPIO486 added as output-low
Ciao,
Rodolfo
--
GNU/Linux Solutions e-mail: giometti@xxxxxxxxxxxx
Linux Device Driver giometti@xxxxxxxx
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 4f52c3a8ec99..f117b0b9d33e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -73,6 +73,16 @@ config GPIO_SYSFS
Kernel drivers may also request that a particular GPIO be
exported to userspace; this can be useful when debugging.
+config GPIO_LINE
+ bool "/sys/class/line/... (GPIO lines interface)"
+ depends on SYSFS
+ help
+ Say Y here to add a sysfs interface to manage system's GPIO lines.
+
+ Instead of the GPIO_SYSFS support, by using this support, you'll be
+ able to use GPIOs from userspace as stated in the device-tree
+ for well defined pourposes and by using proper names.
+
config GPIO_GENERIC
depends on HAS_IOMEM # Only for IOMEM drivers
tristate
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index c256aff66a65..033a6b836dec 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_GPIOLIB) += gpiolib-legacy.o
obj-$(CONFIG_GPIOLIB) += gpiolib-devprop.o
obj-$(CONFIG_OF_GPIO) += gpiolib-of.o
obj-$(CONFIG_GPIO_SYSFS) += gpiolib-sysfs.o
+obj-$(CONFIG_GPIO_LINE) += gpiolib-line.o
obj-$(CONFIG_GPIO_ACPI) += gpiolib-acpi.o
# Device drivers. Generally keep list sorted alphabetically
diff --git a/drivers/gpio/gpiolib-line.c b/drivers/gpio/gpiolib-line.c
new file mode 100644
index 000000000000..640c9971d97e
--- /dev/null
+++ b/drivers/gpio/gpiolib-line.c
@@ -0,0 +1,251 @@
+/*
+ * GPIOlib - userspace I/O line interface
+ *
+ *
+ * Copyright (C) 2020 Rodolfo Giometti <giometti@xxxxxxxxxxxx>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/idr.h>
+#include <linux/kdev_t.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+
+#define GPIO_LINE_MAX_SOURCES 128 /* should be enough... */
+
+/*
+ * Local variables
+ */
+
+static dev_t gpio_line_devt;
+static struct class *gpio_line_class;
+
+static DEFINE_MUTEX(gpio_line_idr_lock);
+static DEFINE_IDR(gpio_line_idr);
+
+struct gpio_line_device {
+ struct gpio_desc *gpiod;
+ const char *name;
+ unsigned int id;
+ struct device *dev;
+};
+
+/*
+ * sysfs methods
+ */
+
+static ssize_t state_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct gpio_line_device *gpio_line = dev_get_drvdata(dev);
+ int status, ret;
+
+ ret = sscanf(buf, "%d", &status);
+ if (ret != 1 && status != 0 && status != 1)
+ return -EINVAL;
+
+ gpiod_set_value_cansleep(gpio_line->gpiod, status);
+
+ return count;
+}
+
+static ssize_t state_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct gpio_line_device *gpio_line = dev_get_drvdata(dev);
+ int status = gpiod_get_value_cansleep(gpio_line->gpiod);
+
+ return sprintf(buf, "%d\n", status);
+}
+static DEVICE_ATTR_RW(state);
+
+/*
+ * Class attributes
+ */
+
+static struct attribute *gpio_line_attrs[] = {
+ &dev_attr_state.attr,
+ NULL,
+};
+
+static const struct attribute_group gpio_line_group = {
+ .attrs = gpio_line_attrs,
+};
+
+static const struct attribute_group *gpio_line_groups[] = {
+ &gpio_line_group,
+ NULL,
+};
+
+/*
+ * Driver stuff
+ */
+
+static struct gpio_line_device *gpio_line_create_entry(const char *name,
+ struct gpio_desc *gpiod,
+ struct device *parent)
+{
+ struct gpio_line_device *gpio_line;
+ dev_t devt;
+ int ret;
+
+ /* First allocate a new gpio_line device */
+ gpio_line = kmalloc(sizeof(struct gpio_line_device), GFP_KERNEL);
+ if (!gpio_line)
+ return ERR_PTR(-ENOMEM);
+
+ mutex_lock(&gpio_line_idr_lock);
+ /*
+ * Get new ID for the new gpio_line source. After idr_alloc() calling
+ * the new source will be freely available into the kernel.
+ */
+ ret = idr_alloc(&gpio_line_idr, gpio_line, 0,
+ GPIO_LINE_MAX_SOURCES, GFP_KERNEL);
+ if (ret < 0) {
+ if (ret == -ENOSPC) {
+ pr_err("%s: too many GPIO lines in the system\n",
+ name);
+ ret = -EBUSY;
+ }
+ goto error_device_create;
+ }
+ gpio_line->id = ret;
+ mutex_unlock(&gpio_line_idr_lock);
+
+ /* Create the device and init the device's data */
+ devt = MKDEV(MAJOR(gpio_line_devt), gpio_line->id);
+ gpio_line->dev = device_create(gpio_line_class, parent, devt, gpio_line,
+ "%s", name);
+ if (IS_ERR(gpio_line->dev)) {
+ dev_err(gpio_line->dev, "unable to create device %s\n", name);
+ ret = PTR_ERR(gpio_line->dev);
+ goto error_idr_remove;
+ }
+ dev_set_drvdata(gpio_line->dev, gpio_line);
+
+ /* Init the gpio_line data */
+ gpio_line->gpiod = gpiod;
+ gpio_line->name = name;
+
+ return gpio_line;
+
+error_idr_remove:
+ mutex_lock(&gpio_line_idr_lock);
+ idr_remove(&gpio_line_idr, gpio_line->id);
+
+error_device_create:
+ mutex_unlock(&gpio_line_idr_lock);
+ kfree(gpio_line);
+
+ return ERR_PTR(ret);
+}
+
+static int gpio_line_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct fwnode_handle *child;
+ struct gpio_line_device *gpio_line;
+ int ret;
+
+ device_for_each_child_node(dev, child) {
+ struct device_node *np = to_of_node(child);
+ const char *name;
+ enum gpiod_flags flags;
+ struct gpio_desc *gpiod;
+
+ ret = fwnode_property_read_string(child, "line-name", &name);
+ if (ret && IS_ENABLED(CONFIG_OF) && np)
+ name = np->name;
+ if (!name) {
+ dev_err(dev,
+ "name property not defined or invalid!\n");
+ goto skip;
+ }
+
+ flags = GPIOD_ASIS;
+ if (of_property_read_bool(np, "input"))
+ flags = GPIOD_IN;
+ else if (of_property_read_bool(np, "output-low"))
+ flags = GPIOD_OUT_LOW;
+ else if (of_property_read_bool(np, "output-high"))
+ flags = GPIOD_OUT_HIGH;
+ gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL, child,
+ flags, name);
+ if (IS_ERR(gpiod)) {
+ dev_err(dev, "gpios property not defined!\n");
+ goto skip;
+ }
+
+ gpio_line = gpio_line_create_entry(name, gpiod, dev);
+ if (IS_ERR(gpio_line))
+ goto skip;
+
+ /* Success, go to the next child */
+ dev_info(gpio_line->dev, "GPIO%d added as %s\n",
+ desc_to_gpio(gpiod),
+ flags == GPIOD_ASIS ? "as-is" :
+ flags == GPIOD_OUT_HIGH ? "output-high" :
+ flags == GPIOD_OUT_LOW ? "output-low" :
+ flags == GPIOD_IN ? "input" : "unknow!");
+ continue;
+
+skip: /* Error, skip the child */
+ fwnode_handle_put(child);
+ dev_err(dev, "failed to register GPIO lines interface\n");
+ }
+
+ return 0;
+}
+
+static const struct of_device_id of_gpio_gpio_line_match[] = {
+ { .compatible = "gpio-line", },
+ { /* sentinel */ }
+};
+
+static struct platform_driver gpio_line_gpio_driver = {
+ .driver = {
+ .name = "gpio-line",
+ .of_match_table = of_gpio_gpio_line_match,
+ },
+};
+
+builtin_platform_driver_probe(gpio_line_gpio_driver, gpio_line_gpio_probe);
+
+/*
+ * Module stuff
+ */
+
+static int __init gpiolib_line_init(void)
+{
+ /* Create the new class */
+ gpio_line_class = class_create(THIS_MODULE, "line");
+ if (!gpio_line_class) {
+ printk(KERN_ERR "gpio_line: failed to create class\n");
+ return -ENOMEM;
+ }
+ gpio_line_class->dev_groups = gpio_line_groups;
+
+ return 0;
+}
+
+postcore_initcall(gpiolib_line_init);
