lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sun, 12 Jan 2014 16:44:10 +0900
From:	Alexandre Courbot <gnurou@...il.com>
To:	Philipp Zabel <p.zabel@...gutronix.de>
Cc:	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Rob Landley <rob@...dley.net>, Arnd Bergmann <arnd@...db.de>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	kernel@...gutronix.de
Subject: Re: [RFC PATCH] gpiolib: make gpiod_direction_output take a logical
 value, add gpiod_direction_output_raw

Hi Philipp,

On Tue, Jan 7, 2014 at 8:34 PM, Philipp Zabel <p.zabel@...gutronix.de> wrote:
> The documentation was not clear about whether gpio_direction_output should take
> a logical value or the physical level on the output line, i.e. whether the
> ACTIVE_LOW status would be taken into account.
> This converts gpiod_direction_output to use the logical level and adds a new
> gpiod_direction_output_raw for the raw value.

Thanks for taking the time to craft this! The patch seems to do
exactly what we discussed and looks good to me. Maybe Linus can let us
know what he thinks about it? Unless there are reasons against it, I
think this should be merged ASAP as we want to gpiod API to converge
to something stable ASAP.

Reviewed-by: Alexandre Courbot <acourbot@...dia.com>

>
> Signed-off-by: Philipp Zabel <p.zabel@...gutronix.de>
> ---
>  Documentation/gpio/consumer.txt |  1 +
>  drivers/gpio/gpiolib.c          | 67 +++++++++++++++++++++++++++++------------
>  include/asm-generic/gpio.h      |  2 +-
>  include/linux/gpio/consumer.h   |  7 +++++
>  4 files changed, 57 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt
> index 07c74a3..4dc4809 100644
> --- a/Documentation/gpio/consumer.txt
> +++ b/Documentation/gpio/consumer.txt
> @@ -150,6 +150,7 @@ raw line value:
>         void gpiod_set_raw_value(struct gpio_desc *desc, int value)
>         int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc)
>         void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value)
> +       int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
>
>  The active-low state of a GPIO can also be queried using the following call:
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 85f772c..f04f1e6 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -330,9 +330,9 @@ static ssize_t gpio_direction_store(struct device *dev,
>         if (!test_bit(FLAG_EXPORT, &desc->flags))
>                 status = -EIO;
>         else if (sysfs_streq(buf, "high"))
> -               status = gpiod_direction_output(desc, 1);
> +               status = gpiod_direction_output_raw(desc, 1);
>         else if (sysfs_streq(buf, "out") || sysfs_streq(buf, "low"))
> -               status = gpiod_direction_output(desc, 0);
> +               status = gpiod_direction_output_raw(desc, 0);
>         else if (sysfs_streq(buf, "in"))
>                 status = gpiod_direction_input(desc);
>         else
> @@ -1579,7 +1579,7 @@ int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
>         if (flags & GPIOF_DIR_IN)
>                 err = gpiod_direction_input(desc);
>         else
> -               err = gpiod_direction_output(desc,
> +               err = gpiod_direction_output_raw(desc,
>                                 (flags & GPIOF_INIT_HIGH) ? 1 : 0);
>
>         if (err)
> @@ -1744,28 +1744,13 @@ fail:
>  }
>  EXPORT_SYMBOL_GPL(gpiod_direction_input);
>
> -/**
> - * gpiod_direction_output - set the GPIO direction to input
> - * @desc:      GPIO to set to output
> - * @value:     initial output value of the GPIO
> - *
> - * Set the direction of the passed GPIO to output, such as gpiod_set_value() can
> - * be called safely on it. The initial value of the output must be specified.
> - *
> - * Return 0 in case of success, else an error code.
> - */
> -int gpiod_direction_output(struct gpio_desc *desc, int value)
> +static int _gpiod_direction_output_raw(struct gpio_desc *desc, int value)
>  {
>         unsigned long           flags;
>         struct gpio_chip        *chip;
>         int                     status = -EINVAL;
>         int offset;
>
> -       if (!desc || !desc->chip) {
> -               pr_warn("%s: invalid GPIO\n", __func__);
> -               return -EINVAL;
> -       }
> -
>         /* GPIOs used for IRQs shall not be set as output */
>         if (test_bit(FLAG_USED_AS_IRQ, &desc->flags)) {
>                 gpiod_err(desc,
> @@ -1827,6 +1812,50 @@ fail:
>                 gpiod_dbg(desc, "%s: gpio status %d\n", __func__, status);
>         return status;
>  }
> +
> +/**
> + * gpiod_direction_output_raw - set the GPIO direction to output
> + * @desc:      GPIO to set to output
> + * @value:     initial output value of the GPIO
> + *
> + * Set the direction of the passed GPIO to output, such as gpiod_set_value() can
> + * be called safely on it. The initial value of the output must be specified
> + * as raw value on the physical line without regard for the ACTIVE_LOW status.
> + *
> + * Return 0 in case of success, else an error code.
> + */
> +int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
> +{
> +       if (!desc || !desc->chip) {
> +               pr_warn("%s: invalid GPIO\n", __func__);
> +               return -EINVAL;
> +       }
> +       return _gpiod_direction_output_raw(desc, value);
> +}
> +EXPORT_SYMBOL_GPL(gpiod_direction_output_raw);
> +
> +/**
> + * gpiod_direction_output - set the GPIO direction to input
> + * @desc:      GPIO to set to output
> + * @value:     initial output value of the GPIO
> + *
> + * Set the direction of the passed GPIO to output, such as gpiod_set_value() can
> + * be called safely on it. The initial value of the output must be specified
> + * as the logical value of the GPIO, i.e. taking its ACTIVE_LOW status into
> + * account.
> + *
> + * Return 0 in case of success, else an error code.
> + */
> +int gpiod_direction_output(struct gpio_desc *desc, int value)
> +{
> +       if (!desc || !desc->chip) {
> +               pr_warn("%s: invalid GPIO\n", __func__);
> +               return -EINVAL;
> +       }
> +       if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
> +               value = !value;
> +       return _gpiod_direction_output_raw(desc, value);
> +}
>  EXPORT_SYMBOL_GPL(gpiod_direction_output);
>
>  /**
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index a5f56a0..23e3645 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -69,7 +69,7 @@ static inline int gpio_direction_input(unsigned gpio)
>  }
>  static inline int gpio_direction_output(unsigned gpio, int value)
>  {
> -       return gpiod_direction_output(gpio_to_desc(gpio), value);
> +       return gpiod_direction_output_raw(gpio_to_desc(gpio), value);
>  }
>
>  static inline int gpio_set_debounce(unsigned gpio, unsigned debounce)
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index 4d34dbb..3879943 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -36,6 +36,7 @@ void devm_gpiod_put(struct device *dev, struct gpio_desc *desc);
>  int gpiod_get_direction(const struct gpio_desc *desc);
>  int gpiod_direction_input(struct gpio_desc *desc);
>  int gpiod_direction_output(struct gpio_desc *desc, int value);
> +int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
>
>  /* Value get/set from non-sleeping context */
>  int gpiod_get_value(const struct gpio_desc *desc);
> @@ -121,6 +122,12 @@ static inline int gpiod_direction_output(struct gpio_desc *desc, int value)
>         WARN_ON(1);
>         return -ENOSYS;
>  }
> +static inline int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
> +{
> +       /* GPIO can never have been requested */
> +       WARN_ON(1);
> +       return -ENOSYS;
> +}
>
>
>  static inline int gpiod_get_value(const struct gpio_desc *desc)
> --
> 1.8.5.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ