Dear Simon,

On 06.10.14 20:14, Simon Glass wrote:
> Modify this driver to support driver model, with platform data required to
> determine the GPIOs that it controls.
> 
> Signed-off-by: Simon Glass <[email protected]>
> ---
> 
>  arch/arm/include/asm/arch-at91/gpio.h |   6 +
>  drivers/gpio/at91_gpio.c              | 221 
> +++++++++++++++++++++++++++-------
>  2 files changed, 183 insertions(+), 44 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-at91/gpio.h 
> b/arch/arm/include/asm/arch-at91/gpio.h
> index 7121388..6d2a7b7 100644
> --- a/arch/arm/include/asm/arch-at91/gpio.h
> +++ b/arch/arm/include/asm/arch-at91/gpio.h
> @@ -253,4 +253,10 @@ static inline unsigned at91_gpio_to_pin(unsigned gpio)
>       return gpio % 32;
>  }
>  
> +/* Platform data for each GPIO port */
> +struct at91_port_platdata {
> +     uint32_t base_addr;
> +     const char *bank_name;
> +};
> +
>  #endif /* __ASM_ARCH_AT91_GPIO_H */
> diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c
> index 6517af1..822ba8c 100644
> --- a/drivers/gpio/at91_gpio.c
> +++ b/drivers/gpio/at91_gpio.c
> @@ -10,11 +10,14 @@
>  
>  #include <config.h>
>  #include <common.h>
> +#include <dm.h>
>  #include <asm/io.h>
>  #include <linux/sizes.h>
> +#include <asm/gpio.h>
>  #include <asm/arch/hardware.h>
>  #include <asm/arch/at91_pio.h>
> -#include <asm/arch/gpio.h>
> +
> +#define GPIO_PER_BANK        32
>  
>  static struct at91_port *at91_pio_get_port(unsigned port)
>  {
> @@ -39,19 +42,25 @@ static struct at91_port *at91_pio_get_port(unsigned port)
>       }
>  }
>  
> +static void at91_set_port_pullup(struct at91_port *at91_port, unsigned 
> offset,
> +                              int use_pullup)
> +{
> +     u32 mask;
> +
> +     mask = 1 << offset;
> +     if (use_pullup)
> +             writel(mask, &at91_port->puer);
> +     else
> +             writel(mask, &at91_port->pudr);
> +     writel(mask, &at91_port->per);
> +}
> +
>  int at91_set_pio_pullup(unsigned port, unsigned pin, int use_pullup)
>  {
>       struct at91_port *at91_port = at91_pio_get_port(port);
> -     u32 mask;
>  
> -     if (at91_port && (pin < 32)) {
> -             mask = 1 << pin;
> -             if (use_pullup)
> -                     writel(1 << pin, &at91_port->puer);
> -             else
> -                     writel(1 << pin, &at91_port->pudr);
> -             writel(mask, &at91_port->per);
> -     }
> +     if (at91_port && (pin < 32))

could we please use the newly defined GPIO_PER_BANK here and elsewhere?

> +             at91_set_port_pullup(at91_port, pin, use_pullup);
>  
>       return 0;
>  }
> @@ -172,6 +181,29 @@ int at91_set_d_periph(unsigned port, unsigned pin, int 
> use_pullup)
>  }
>  #endif
>  
> +#ifdef CONFIG_DM_GPIO
> +static bool at91_get_port_output(struct at91_port *at91_port, int offset)
> +{
> +     u32 mask, val;
> +
> +     mask = 1 << offset;
> +     val = readl(&at91_port->osr);
> +     return val & mask ? true : false;

Nitpick, wouldn't just 'return val & mask' work here? It would be
implicit conversion, but isn't the construct here also implicit conversion?

> +}
> +#endif
> +
> +static void at91_set_port_input(struct at91_port *at91_port, int offset,
> +                             int use_pullup)
> +{
> +     u32 mask;
> +
> +     mask = 1 << offset;
> +     writel(mask, &at91_port->idr);
> +     at91_set_port_pullup(at91_port, offset, use_pullup);
> +     writel(mask, &at91_port->odr);
> +     writel(mask, &at91_port->per);
> +}
> +
>  /*
>   * mux the pin to the gpio controller (instead of "A" or "B" peripheral), and
>   * configure it for an input.
> @@ -179,19 +211,29 @@ int at91_set_d_periph(unsigned port, unsigned pin, int 
> use_pullup)
>  int at91_set_pio_input(unsigned port, u32 pin, int use_pullup)
>  {
>       struct at91_port *at91_port = at91_pio_get_port(port);
> -     u32 mask;
>  
> -     if (at91_port && (pin < 32)) {
> -             mask = 1 << pin;
> -             writel(mask, &at91_port->idr);
> -             at91_set_pio_pullup(port, pin, use_pullup);
> -             writel(mask, &at91_port->odr);
> -             writel(mask, &at91_port->per);
> -     }
> +     if (at91_port && (pin < 32))
> +             at91_set_port_input(at91_port, pin, use_pullup);
>  
>       return 0;
>  }
>  
> +static void at91_set_port_output(struct at91_port *at91_port, int offset,
> +                              int value)
> +{
> +     u32 mask;
> +
> +     mask = 1 << offset;
> +     writel(mask, &at91_port->idr);
> +     writel(mask, &at91_port->pudr);
> +     if (value)
> +             writel(mask, &at91_port->sodr);
> +     else
> +             writel(mask, &at91_port->codr);
> +     writel(mask, &at91_port->oer);
> +     writel(mask, &at91_port->per);
> +}
> +
>  /*
>   * mux the pin to the gpio controller (instead of "A" or "B" peripheral),
>   * and configure it for an output.
> @@ -199,19 +241,9 @@ int at91_set_pio_input(unsigned port, u32 pin, int 
> use_pullup)
>  int at91_set_pio_output(unsigned port, u32 pin, int value)
>  {
>       struct at91_port *at91_port = at91_pio_get_port(port);
> -     u32 mask;
>  
> -     if (at91_port && (port < ATMEL_PIO_PORTS) && (pin < 32)) {
> -             mask = 1 << pin;
> -             writel(mask, &at91_port->idr);
> -             writel(mask, &at91_port->pudr);
> -             if (value)
> -                     writel(mask, &at91_port->sodr);
> -             else
> -                     writel(mask, &at91_port->codr);
> -             writel(mask, &at91_port->oer);
> -             writel(mask, &at91_port->per);
> -     }
> +     if (at91_port && (port < ATMEL_PIO_PORTS) && (pin < 32))

I think the port check here is a leftover. It should be handled
correctly inside at91_pio_get_port() and therefore could be removed here.

> +             at91_set_port_output(at91_port, pin, value);
>  
>       return 0;
>  }
> @@ -321,41 +353,54 @@ int at91_set_pio_multi_drive(unsigned port, unsigned 
> pin, int is_on)
>       return 0;
>  }
>  
> +static void at91_set_port_value(struct at91_port *at91_port, int offset,
> +                             int value)
> +{
> +     u32 mask;
> +
> +     mask = 1 << offset;
> +     if (value)
> +             writel(mask, &at91_port->sodr);
> +     else
> +             writel(mask, &at91_port->codr);
> +}
> +
>  /*
>   * assuming the pin is muxed as a gpio output, set its value.
>   */
>  int at91_set_pio_value(unsigned port, unsigned pin, int value)
>  {
>       struct at91_port *at91_port = at91_pio_get_port(port);
> -     u32 mask;
>  
> -     if (at91_port && (pin < 32)) {
> -             mask = 1 << pin;
> -             if (value)
> -                     writel(mask, &at91_port->sodr);
> -             else
> -                     writel(mask, &at91_port->codr);
> -     }
> +     if (at91_port && (pin < 32))
> +             at91_set_port_value(at91_port, pin, value);
>  
>       return 0;
>  }
>  
> +static int at91_get_port_value(struct at91_port *at91_port, int offset)
> +{
> +     u32 pdsr = 0, mask;
> +
> +     mask = 1 << offset;
> +     pdsr = readl(&at91_port->pdsr) & mask;
> +
> +     return pdsr != 0;
> +}
>  /*
>   * read the pin's value (works even if it's not muxed as a gpio).
>   */
>  int at91_get_pio_value(unsigned port, unsigned pin)
>  {
>       struct at91_port *at91_port = at91_pio_get_port(port);
> -     u32 pdsr = 0, mask;
>  
> -     if (at91_port && (pin < 32)) {
> -             mask = 1 << pin;
> -             pdsr = readl(&at91_port->pdsr) & mask;
> -     }
> +     if (at91_port && (pin < 32))
> +             return at91_get_port_value(at91_port, pin);
>  
> -     return pdsr != 0;
> +     return 0;
>  }
>  
> +#ifndef CONFIG_DM_GPIO
>  /* Common GPIO API */
>  
>  int gpio_request(unsigned gpio, const char *label)
> @@ -395,3 +440,91 @@ int gpio_set_value(unsigned gpio, int value)
>  
>       return 0;
>  }
> +#endif
> +
> +#ifdef CONFIG_DM_GPIO
> +
> +struct at91_port_priv {
> +     struct at91_port *regs;
> +};
> +
> +/* set GPIO pin 'gpio' as an input */
> +static int at91_gpio_direction_input(struct udevice *dev, unsigned offset)
> +{
> +     struct at91_port_priv *port = dev_get_platdata(dev);
> +
> +     at91_set_port_input(port->regs, offset, 0);
> +
> +     return 0;
> +}
> +
> +/* set GPIO pin 'gpio' as an output, with polarity 'value' */
> +static int at91_gpio_direction_output(struct udevice *dev, unsigned offset,
> +                                    int value)
> +{
> +     struct at91_port_priv *port = dev_get_platdata(dev);
> +
> +     at91_set_port_output(port->regs, offset, value);
> +
> +     return 0;
> +}
> +
> +/* read GPIO IN value of pin 'gpio' */
> +static int at91_gpio_get_value(struct udevice *dev, unsigned offset)
> +{
> +     struct at91_port_priv *port = dev_get_platdata(dev);
> +
> +     return at91_get_port_value(port->regs, offset);
> +}
> +
> +/* write GPIO OUT value to pin 'gpio' */
> +static int at91_gpio_set_value(struct udevice *dev, unsigned offset,
> +                            int value)
> +{
> +     struct at91_port_priv *port = dev_get_platdata(dev);
> +
> +     at91_set_port_value(port->regs, offset, value);
> +
> +     return 0;
> +}
> +
> +static int at91_gpio_get_function(struct udevice *dev, unsigned offset)
> +{
> +     struct at91_port_priv *port = dev_get_platdata(dev);
> +
> +     /* GPIOF_FUNC is not implemented yet */
> +     if (at91_get_port_output(port->regs, offset))
> +             return GPIOF_OUTPUT;
> +     else
> +             return GPIOF_INPUT;
> +}
> +
> +static const struct dm_gpio_ops gpio_at91_ops = {
> +     .direction_input        = at91_gpio_direction_input,
> +     .direction_output       = at91_gpio_direction_output,
> +     .get_value              = at91_gpio_get_value,
> +     .set_value              = at91_gpio_set_value,
> +     .get_function           = at91_gpio_get_function,
> +};
> +
> +static int at91_gpio_probe(struct udevice *dev)
> +{
> +     struct at91_port_priv *port = dev_get_priv(dev);
> +     struct at91_port_platdata *plat = dev_get_platdata(dev);
> +     struct gpio_dev_priv *uc_priv = dev->uclass_priv;
> +
> +     uc_priv->bank_name = plat->bank_name;
> +     uc_priv->gpio_count = GPIO_PER_BANK;
> +     port->regs = (struct at91_port *)plat->base_addr;
> +
> +     return 0;
> +}
> +
> +U_BOOT_DRIVER(gpio_at91) = {
> +     .name   = "gpio_at91",
> +     .id     = UCLASS_GPIO,
> +     .ops    = &gpio_at91_ops,
> +     .probe  = at91_gpio_probe,
> +     .priv_auto_alloc_size = sizeof(struct at91_port_priv),
> +};
> +#endif
> 

Best regards

Andreas Bießmann
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to