From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Tue, 17 Feb 2015 22:01:58 -0700
Subject: [U-Boot] [PATCH 1/4] dm: gpio: extend gpio api by
	dm_gpio_set_pull()
In-Reply-To: <1424178544-28632-2-git-send-email-p.marczak@samsung.com>
References: <1424178544-28632-1-git-send-email-p.marczak@samsung.com>
 <1424178544-28632-2-git-send-email-p.marczak@samsung.com>
Message-ID: <CAPnjgZ2dVkiMBr=m92WBomw1QBt7HSa58Upp1R6kbknORkPxCA@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

+Stephen who might have an opinion on this.

Hi Przemyslaw,

On 17 February 2015 at 06:09, Przemyslaw Marczak <p.marczak@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@samsung.com>
> CC: Simon Glass <sjg@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