[U-Boot] [PATCH 1/4] dm: gpio: extend gpio api by dm_gpio_set_pull()

[Simon Glass]

+Stephen who might have an opinion on this.

Hi Przemyslaw,

On 17 February 2015 at 06:09, Przemyslaw Marczak <p.marczak at samsung.com> wrote:
> This commits extends:
> - dm gpio ops by: 'set_pull' call
> - dm gpio uclass by: dm_gpio_set_pull() function
>
> The pull mode is not defined so should be driver specific.

It's good to implement this, but I think you should try to have a
standard interface. You could define the options you want to support
and pass in a standard value.

Otherwise we are not really providing a driver abstraction, only an interface.

>
> Signed-off-by: Przemyslaw Marczak <p.marczak at samsung.com>
> CC: Simon Glass <sjg at chromium.org>
> ---
>  drivers/gpio/gpio-uclass.c | 12 ++++++++++++
>  include/asm-generic/gpio.h | 12 ++++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> index a69bbd2..48b31c2 100644
> --- a/drivers/gpio/gpio-uclass.c
> +++ b/drivers/gpio/gpio-uclass.c
> @@ -321,6 +321,18 @@ int dm_gpio_set_value(struct gpio_desc *desc, int value)
>         return 0;
>  }
>
> +int dm_gpio_set_pull(struct gpio_desc *desc, int pull)
> +{
> +       int ret;
> +
> +       ret = check_reserved(desc, "set_pull");
> +       if (ret)
> +               return ret;
> +
> +       gpio_get_ops(desc->dev)->set_pull(desc->dev, desc->offset, pull);

Should return this value.

> +       return 0;
> +}
> +
>  int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
>  {
>         struct udevice *dev = desc->dev;
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index 3b96b82..7e0d426 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -241,6 +241,7 @@ struct dm_gpio_ops {
>                                 int value);
>         int (*get_value)(struct udevice *dev, unsigned offset);
>         int (*set_value)(struct udevice *dev, unsigned offset, int value);
> +       int (*set_pull)(struct udevice *dev, unsigned offset, int pull);
>         /**
>          * get_function() Get the GPIO function
>          *
> @@ -479,6 +480,7 @@ int gpio_free_list_nodev(struct gpio_desc *desc, int count);
>
>  /**
>   * dm_gpio_get_value() - Get the value of a GPIO
> +
>   *
>   * This is the driver model version of the existing gpio_get_value() function
>   * and should be used instead of that.
> @@ -495,6 +497,16 @@ int dm_gpio_get_value(struct gpio_desc *desc);
>  int dm_gpio_set_value(struct gpio_desc *desc, int value);
>
>  /**
> + * dm_gpio_set_pull() - Set the pull-up/down value of a GPIO
> + *
> + * @desc:      GPIO description containing device, offset and flags,
> + *             previously returned by gpio_request_by_name()
> + * @pull:      GPIO pull value - driver specific
> + * @return 0 on success or -ve on error
> +*/
> +int dm_gpio_set_pull(struct gpio_desc *desc, int pull);
> +
> +/**
>   * dm_gpio_set_dir() - Set the direction for a GPIO
>   *
>   * This sets up the direction according tot the provided flags. It will do
> --
> 1.9.1
>

Regards,
Simon