gpio: Add comments on single direction chips

A patch from Ricardo got me thinking about some gpio chip
semantics so let's drop in some comments to make things
more clear around that.

Cc: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index cabe1b460458b8f66a43aa90e3d7eaf680c12969..4ce402f9fc69247443ff123a3b2111e8635f78d4 100644 (file)
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2548,20 +2548,31 @@ int gpiod_direction_input(struct gpio_desc *desc)
        VALIDATE_DESC(desc);
        chip = desc->gdev->chip;
 
+       /*
+        * It is legal to have no .get() and .direction_input() specified if
+        * the chip is output-only, but you can't specify .direction_input()
+        * and not support the .get() operation, that doesn't make sense.
+        */
        if (!chip->get && chip->direction_input) {
                gpiod_warn(desc,
-                       "%s: missing get() and direction_input() operations\n",
-                       __func__);
+                          "%s: missing get() but have direction_input()\n",
+                          __func__);
                return -EIO;
        }
 
+       /*
+        * If we have a .direction_input() callback, things are simple,
+        * just call it. Else we are some input-only chip so try to check the
+        * direction (if .get_direction() is supported) else we silently
+        * assume we are in input mode after this.
+        */
        if (chip->direction_input) {
                status = chip->direction_input(chip, gpio_chip_hwgpio(desc));
        } else if (chip->get_direction &&
                  (chip->get_direction(chip, gpio_chip_hwgpio(desc)) != 1)) {
                gpiod_warn(desc,
-                       "%s: missing direction_input() operation\n",
-                       __func__);
+                          "%s: missing direction_input() operation and line is output\n",
+                          __func__);
                return -EIO;
        }
        if (status == 0)
@@ -2587,16 +2598,22 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
        int val = !!value;
        int ret = 0;
 
+       /*
+        * It's OK not to specify .direction_output() if the gpiochip is
+        * output-only, but if there is then not even a .set() operation it
+        * is pretty tricky to drive the output line.
+        */
        if (!gc->set && !gc->direction_output) {
                gpiod_warn(desc,
-                      "%s: missing set() and direction_output() operations\n",
-                      __func__);
+                          "%s: missing set() and direction_output() operations\n",
+                          __func__);
                return -EIO;
        }
 
        if (gc->direction_output) {
                ret = gc->direction_output(gc, gpio_chip_hwgpio(desc), val);
        } else {
+               /* Check that we are in output mode if we can */
                if (gc->get_direction &&
                    gc->get_direction(gc, gpio_chip_hwgpio(desc))) {
                        gpiod_warn(desc,
@@ -2604,6 +2621,10 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
                                __func__);
                        return -EIO;
                }
+               /*
+                * If we can't actively set the direction, we are some
+                * output-only chip, so just drive the output as desired.
+                */
                gc->set(gc, gpio_chip_hwgpio(desc), val);
        }
 

diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index d8dcd0e44cab843c68f2a5178024d271b83508bb..f6b95734073f856b0852533e86d6b95304f36a97 100644 (file)
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -178,9 +178,13 @@ static inline struct gpio_irq_chip *to_gpio_irq_chip(struct irq_chip *chip)
  * @free: optional hook for chip-specific deactivation, such as
  *     disabling module power and clock; may sleep
  * @get_direction: returns direction for signal "offset", 0=out, 1=in,
- *     (same as GPIOF_DIR_XXX), or negative error
+ *     (same as GPIOF_DIR_XXX), or negative error.
+ *     It is recommended to always implement this function, even on
+ *     input-only or output-only gpio chips.
  * @direction_input: configures signal "offset" as input, or returns error
+ *     This can be omitted on input-only or output-only gpio chips.
  * @direction_output: configures signal "offset" as output, or returns error
+ *     This can be omitted on input-only or output-only gpio chips.
  * @get: returns value for signal "offset", 0=low, 1=high, or negative error
  * @get_multiple: reads values for multiple signals defined by "mask" and
  *     stores them in "bits", returns 0 on success or negative error