Re: [U-Boot] [PATCH 3/7] dm: at91: Add driver model support for atmel GPIO driver

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 sjg@chromium.org

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