Skip to contentSkip to conversation

dm: Introduce driver model for Atmel at91

PATCH New Simon Glass 2 participants 20 messages Oct 6, 2014 – Oct 23, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover dm: Introduce driver model for Atmel at91

This series adds driver model support for serial and GPIO for the Atmel AT91 series. The at91sam9260-based Snapper 9260 is converted over as an example.

For both drivers the old behaviour is retained, so driver model becomes option, not mandatory.

This series is available at u-boot-dm/atmel-working.

Simon Glass (7):
dm: at91: Correct text base for snapper9260
dm: at91: Move snapper9260 to generic baord
dm: at91: Add driver model support for atmel GPIO driver
dm: at91: Add platform data for GPIO on at91sam9260-based boards
dm: at91: Refactor serial driver slightly for driver model
dm: at91: Add driver model support for the serial driver
dm: at91: Convert snapper9260 to use driver model

 arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c |  14 ++
 arch/arm/include/asm/arch-at91/at91_serial.h      |  15 ++
 arch/arm/include/asm/arch-at91/at91sam9260.h      |   4 +-
 arch/arm/include/asm/arch-at91/gpio.h             |   6 +
 board/bluewater/snapper9260/snapper9260.c         |  18 +-
 drivers/gpio/at91_gpio.c                          | 221 +++++++++++++++++-----
 drivers/serial/atmel_usart.c                      | 117 +++++++++++-
 include/configs/snapper9260.h                     |  12 +-
 8 files changed, 349 insertions(+), 58 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-at91/at91_serial.h
Signature · 1 line
2.1.0.rc2.206.gedb03e5
#1 lore raw

PATCH 1/7 dm: at91: Correct text base for snapper9260

The value should be 0x23f00000. Fix it.

---

 include/configs/snapper9260.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/configs/snapper9260.h +1 -1
index 1ebee71..adb09c2 100644
@@ -15,7 +15,7 @@
 #include <asm/hardware.h>
 #include <linux/sizes.h>
 
-#define CONFIG_SYS_TEXT_BASE		0x20000000
+#define CONFIG_SYS_TEXT_BASE		0x23f00000
 
 /* ARM asynchronous clock */
 #define CONFIG_SYS_AT91_MAIN_CLOCK	18432000 /* External Crystal, in Hz */
Signature · 1 line
2.1.0.rc2.206.gedb03e5
#2 lore raw

PATCH 1/7 dm: at91: Correct text base for snapper9260

Dear Simon,

Quoted 13 lines · Simon Glass

The value should be 0x23f00000. Fix it.

include/configs/snapper9260.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

(quoted diff) +0 -0

b/include/configs/snapper9260.h index 1ebee71..adb09c2 100644 --- a/include/configs/snapper9260.h +++ b/include/configs/snapper9260.h

(quoted diff) +0 -0
@@ -15,7 +15,7 @@ #include <asm/hardware.h> #include

<linux/sizes.h>

Quoted 2 lines

-#define CONFIG_SYS_TEXT_BASE 0x20000000 +#define
CONFIG_SYS_TEXT_BASE 0x23f00000

I wonder why this is required. It depends on the first stage loader snapper9260 uses and I guess the first value is the right one used in the factory programmed loader.
Nevertheless other at91 boards use 0x21f00000 due to relocation issues on boards with less RAM. It seems the snapper9260 has just 64MiB of RAM, this means just 1MiB of space for relocated u-boot, malloc arena, ... This was exactly the reason for other at91 boards to switch to 0x21f00000 as base address.

Best regards

Andreas Bie?mann

#3 lore raw

PATCH 1/7 dm: at91: Correct text base for snapper9260

It also seems the maintainer of snapper9260 is not reachable. Simon, do you own such a board?

Best regards

Andreas Bie?mann

Quoted 32 lines · Andreas Bie?mann

Dear Simon,

Quoted 13 lines · Simon Glass

The value should be 0x23f00000. Fix it.

include/configs/snapper9260.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

(quoted diff) +0 -0

b/include/configs/snapper9260.h index 1ebee71..adb09c2 100644 --- a/include/configs/snapper9260.h +++ b/include/configs/snapper9260.h

(quoted diff) +0 -0
@@ -15,7 +15,7 @@ #include <asm/hardware.h> #include

<linux/sizes.h>

Quoted 2 lines

-#define CONFIG_SYS_TEXT_BASE 0x20000000 +#define
CONFIG_SYS_TEXT_BASE 0x23f00000

I wonder why this is required. It depends on the first stage loader snapper9260 uses and I guess the first value is the right one used in the factory programmed loader.
Nevertheless other at91 boards use 0x21f00000 due to relocation issues on boards with less RAM. It seems the snapper9260 has just 64MiB of RAM, this means just 1MiB of space for relocated u-boot, malloc arena, ... This was exactly the reason for other at91 boards to switch to 0x21f00000 as base address.

Best regards

Andreas Bie?mann

#4 lore raw

PATCH 1/7 dm: at91: Correct text base for snapper9260

Hi,

Quoted 3 lines · Andreas Bie?mann

It also seems the maintainer of snapper9260 is not reachable. Simon, do you own such a board?

Yes I have one. The base address is to make the maximum amount of room for loading the kernel, etc.

Regards,
Simon

Quoted 38 lines

Best regards

Andreas Bie?mann

Quoted 32 lines · Andreas Bie?mann

Dear Simon,

Quoted 13 lines · Simon Glass

The value should be 0x23f00000. Fix it.

include/configs/snapper9260.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

(quoted diff) +0 -0

b/include/configs/snapper9260.h index 1ebee71..adb09c2 100644 --- a/include/configs/snapper9260.h +++ b/include/configs/snapper9260.h

(quoted diff) +0 -0
@@ -15,7 +15,7 @@ #include <asm/hardware.h> #include

<linux/sizes.h>

Quoted 2 lines

-#define CONFIG_SYS_TEXT_BASE 0x20000000 +#define
CONFIG_SYS_TEXT_BASE 0x23f00000

I wonder why this is required. It depends on the first stage loader snapper9260 uses and I guess the first value is the right one used in the factory programmed loader.
Nevertheless other at91 boards use 0x21f00000 due to relocation issues on boards with less RAM. It seems the snapper9260 has just 64MiB of RAM, this means just 1MiB of space for relocated u-boot, malloc arena, ... This was exactly the reason for other at91 boards to switch to 0x21f00000 as base address.

Best regards

Andreas Bie?mann

#5 lore raw

PATCH 1/7 dm: at91: Correct text base for snapper9260

Hi,

Quoted 9 lines · Simon Glass

Hi,

Quoted 3 lines · On Oct 22, 2014 11:10 PM, "Andreas Bie?mann" <andreas.devel at googlemail.com…

It also seems the maintainer of snapper9260 is not reachable. Simon, do you own such a board?

Yes I have one.

so would you take maintainer-ship?

Quoted 2 lines

The base address is to make the maximum amount of room for loading the kernel, etc.

Well, the base address is just used for the temporary copy of u-boot inside RAM. After copy from media to RAM we will relocate to a runtime defined address. This address depends on some variable parameters which may also be defined at runtime. We had the problem that in some cases the calculated relocation address collates with running u-boot in RAM. The chance to hit this issue is high when you just have room of 1 MiB (which must hold two copies of u-boot!)

Best regards

Andreas Bie?mann

#6 lore raw

PATCH 1/7 dm: at91: Correct text base for snapper9260

Hi Andreas,

Quoted 8 lines · Andreas Bie?mann

Hi,

Quoted 5 lines · Simon Glass

Hi,

Quoted 1 line · On Oct 22, 2014 11:10 PM, "Andreas Bie?mann" <andreas.devel at googlemail.com…
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

It also seems the maintainer of snapper9260 is not reachable. Simon, do you own such a board?

Yes I have one.

so would you take maintainer-ship?

OK.

Quoted 11 lines
Quoted 3 lines

The base address is to make the maximum amount of room for loading the kernel, etc.

Well, the base address is just used for the temporary copy of u-boot inside RAM. After copy from media to RAM we will relocate to a runtime defined address. This address depends on some variable parameters which may also be defined at runtime. We had the problem that in some cases the calculated relocation address collates with running u-boot in RAM. The chance to hit this issue is high when you just have room of 1 MiB (which must hold two copies of u-boot!)

Ah yes of course :-) I'm not sure about the pre-boot but will see if I can change it.

Regards,
Simon

#7 lore raw

PATCH 2/7 dm: at91: Move snapper9260 to generic baord

In reply to Simon Glass

This works correctly, so switch it over before the deadline.

---

 include/configs/snapper9260.h | 1 +
 1 file changed, 1 insertion(+)
include/configs/snapper9260.h +1 -0
index adb09c2..fdb6cca 100644
@@ -20,6 +20,7 @@
 /* ARM asynchronous clock */
 #define CONFIG_SYS_AT91_MAIN_CLOCK	18432000 /* External Crystal, in Hz */
 #define CONFIG_SYS_AT91_SLOW_CLOCK	32768
+#define CONFIG_SYS_GENERIC_BOARD
 
 /* CPU */
 #define CONFIG_ARCH_CPU_INIT
Signature · 1 line
2.1.0.rc2.206.gedb03e5
#8 lore raw

PATCH 3/7 dm: at91: Add driver model support for atmel GPIO driver

In reply to Simon Glass

Modify this driver to support driver model, with platform data required to determine the GPIOs that it controls.

---

 arch/arm/include/asm/arch-at91/gpio.h |   6 +
 drivers/gpio/at91_gpio.c              | 221 +++++++++++++++++++++++++++-------
 2 files changed, 183 insertions(+), 44 deletions(-)
arch/arm/include/asm/arch-at91/gpio.h +6 -0
index 7121388..6d2a7b7 100644
@@ -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 */
drivers/gpio/at91_gpio.c +177 -44

295 lines not on this page · raw message

Signature · 1 line
2.1.0.rc2.206.gedb03e5
#9 lore raw

PATCH 3/7 dm: at91: Add driver model support for atmel GPIO driver

Dear Simon,

Quoted 66 lines · Simon Glass

Modify this driver to support driver model, with platform data required to determine the GPIOs that it controls.

---

 arch/arm/include/asm/arch-at91/gpio.h |   6 +
 drivers/gpio/at91_gpio.c              | 221 +++++++++++++++++++++++++++-------
 2 files changed, 183 insertions(+), 44 deletions(-)
arch/arm/include/asm/arch-at91/gpio.h +6 -0
index 7121388..6d2a7b7 100644
@@ -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 */
drivers/gpio/at91_gpio.c +17 -2
index 6517af1..822ba8c 100644
@@ -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;

drivers/gpio/at91_gpio.c static struct at91_port *at91_pio_get_port(unsigned port) · line 62

Quoted 10 lines
drivers/gpio/at91_gpio.c +1 -8
 
-	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?

Quoted 6 lines
drivers/gpio/at91_gpio.c +1 -0
+		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)
 }

drivers/gpio/at91_gpio.c int at91_set_d_periph(unsigned port, unsigned pin, int use_pullup) · line 191

Quoted 10 lines
drivers/gpio/at91_gpio.c +8 -0
 #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?

Quoted 64 lines
drivers/gpio/at91_gpio.c +33 -11
+}
+#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;

drivers/gpio/at91_gpio.c int at91_set_pio_input(unsigned port, u32 pin, int use_pullup) · line 245

Quoted 10 lines
drivers/gpio/at91_gpio.c +1 -9
-		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.

Quoted 156 lines
drivers/gpio/at91_gpio.c +106 -14
+		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;

drivers/gpio/at91_gpio.c int gpio_set_value(unsigned gpio, int value) · line 530

Quoted 10 lines
drivers/gpio/at91_gpio.c +10 -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

#10 lore raw

PATCH 4/7 dm: at91: Add platform data for GPIO on at91sam9260-based boards

In reply to Simon Glass

These boards all have the same GPIO arrangement, so add some common platform data that can be used by all boards. Remove the configs which are no longer required.

---

 arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c | 14 ++++++++++++++
 arch/arm/include/asm/arch-at91/at91sam9260.h      |  4 +++-
 2 files changed, 17 insertions(+), 1 deletion(-)
arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c +14 -0
index cae4abc..5edcfc2 100644
@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <asm/io.h>
 #include <asm/arch/at91_common.h>
 #include <asm/arch/at91_pmc.h>
@@ -207,3 +208,16 @@ void at91_mci_hw_init(void)
 #endif
 }
 #endif
+
+/* Platform data for the GPIOs */
+static const struct at91_port_platdata at91sam9260_plat[] = {
+	{ ATMEL_BASE_PIOA, "A" },
+	{ ATMEL_BASE_PIOB, "B" },
+	{ ATMEL_BASE_PIOC, "C" },
+};
+
+U_BOOT_DEVICES(at91sam9260_gpios) = {
+	{ "gpio_at91", &at91sam9260_plat[0] },
+	{ "gpio_at91", &at91sam9260_plat[1] },
+	{ "gpio_at91", &at91sam9260_plat[2] },
+};
arch/arm/include/asm/arch-at91/at91sam9260.h +3 -1
index 2e902ee..4d46def 100644
@@ -136,9 +136,11 @@
 /*
  * Other misc defines
  */
+#ifndef CONFIG_DM_GPIO
 #define ATMEL_PIO_PORTS		3		/* these SoCs have 3 PIO */
-#define ATMEL_PMC_UHP		AT91SAM926x_PMC_UHP
 #define ATMEL_BASE_PIO		ATMEL_BASE_PIOA
+#endif
+#define ATMEL_PMC_UHP		AT91SAM926x_PMC_UHP
 
 /*
  * SoC specific defines
Signature · 1 line
2.1.0.rc2.206.gedb03e5
#11 lore raw

PATCH 4/7 dm: at91: Add platform data for GPIO on at91sam9260-based boards

Hi Simon,

Quoted 24 lines · Simon Glass

These boards all have the same GPIO arrangement, so add some common platform data that can be used by all boards. Remove the configs which are no longer required.

---

 arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c | 14 ++++++++++++++
 arch/arm/include/asm/arch-at91/at91sam9260.h      |  4 +++-
 2 files changed, 17 insertions(+), 1 deletion(-)
arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c +1 -0
index cae4abc..5edcfc2 100644
@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <asm/io.h>
 #include <asm/arch/at91_common.h>
 #include <asm/arch/at91_pmc.h>

arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c void at91_mci_hw_init(void) · line 216

Quoted 9 lines
arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c +6 -0
@@ -207,3 +208,16 @@ void at91_mci_hw_init(void)
 #endif
 }
 #endif
+
+/* Platform data for the GPIOs */
+static const struct at91_port_platdata at91sam9260_plat[] = {
+	{ ATMEL_BASE_PIOA, "A" },
+	{ ATMEL_BASE_PIOB, "B" },
+	{ ATMEL_BASE_PIOC, "C" },

data sheet call the parts 'Px', sam9260 has PA0..PA31, PB0..PB31 and PC0..PC31, shouldn't we use that name here too?

Quoted 15 lines
arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c +7 -0
+};
+
+U_BOOT_DEVICES(at91sam9260_gpios) = {
+	{ "gpio_at91", &at91sam9260_plat[0] },
+	{ "gpio_at91", &at91sam9260_plat[1] },
+	{ "gpio_at91", &at91sam9260_plat[2] },
+};
arch/arm/include/asm/arch-at91/at91sam9260.h +0 -0
index 2e902ee..4d46def 100644
@@ -136,9 +136,11 @@
 /*
  * Other misc defines

arch/arm/include/asm/arch-at91/at91sam9260.h line 146

Quoted 10 lines
arch/arm/include/asm/arch-at91/at91sam9260.h +3 -1
  */
+#ifndef CONFIG_DM_GPIO
 #define ATMEL_PIO_PORTS		3		/* these SoCs have 3 PIO */
-#define ATMEL_PMC_UHP		AT91SAM926x_PMC_UHP
 #define ATMEL_BASE_PIO		ATMEL_BASE_PIOA
+#endif
+#define ATMEL_PMC_UHP		AT91SAM926x_PMC_UHP
 
 /*
  * SoC specific defines

Best regards

Andreas Bie?mann

#12 lore raw

PATCH 5/7 dm: at91: Refactor serial driver slightly for driver model

In reply to Simon Glass

Before adding driver model support, split out a few of the functions so that they can be used by the driver model code.

---

 drivers/serial/atmel_usart.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)
drivers/serial/atmel_usart.c +23 -9
index 8f0e348..ce36e99 100644
@@ -19,9 +19,9 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static void atmel_serial_setbrg(void)
+static void atmel_serial_setbrg_internal(atmel_usart3_t *usart, int id,
+					 int baudrate)
 {
-	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
 	unsigned long divisor;
 	unsigned long usart_hz;
 
@@ -30,15 +30,13 @@ static void atmel_serial_setbrg(void)
 	 * Baud Rate = --------------
 	 *                16 * CD
 	 */
-	usart_hz = get_usart_clk_rate(CONFIG_USART_ID);
-	divisor = (usart_hz / 16 + gd->baudrate / 2) / gd->baudrate;
+	usart_hz = get_usart_clk_rate(id);
+	divisor = (usart_hz / 16 + baudrate / 2) / baudrate;
 	writel(USART3_BF(CD, divisor), &usart->brgr);
 }
 
-static int atmel_serial_init(void)
+static void atmel_serial_init_internal(atmel_usart3_t *usart)
 {
-	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
-
 	/*
 	 * Just in case: drain transmitter register
 	 * 1000us is enough for baudrate >= 9600
@@ -47,9 +45,10 @@ static int atmel_serial_init(void)
 		__udelay(1000);
 
 	writel(USART3_BIT(RSTRX) | USART3_BIT(RSTTX), &usart->cr);
+}
 
-	serial_setbrg();
-
+static void atmel_serial_activate(atmel_usart3_t *usart)
+{
 	writel((USART3_BF(USART_MODE, USART3_USART_MODE_NORMAL)
 			   | USART3_BF(USCLKS, USART3_USCLKS_MCK)
 			   | USART3_BF(CHRL, USART3_CHRL_8)
@@ -59,6 +58,21 @@ static int atmel_serial_init(void)
 	writel(USART3_BIT(RXEN) | USART3_BIT(TXEN), &usart->cr);
 	/* 100us is enough for the new settings to be settled */
 	__udelay(100);
+}
+
+static void atmel_serial_setbrg(void)
+{
+	atmel_serial_setbrg_internal((atmel_usart3_t *)CONFIG_USART_BASE,
+				     CONFIG_USART_ID, gd->baudrate);
+}
+
+static int atmel_serial_init(void)
+{
+	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
+
+	atmel_serial_init_internal(usart);
+	serial_setbrg();
+	atmel_serial_activate(usart);
 
 	return 0;
 }
Signature · 1 line
2.1.0.rc2.206.gedb03e5
#13 lore raw

PATCH 5/7 dm: at91: Refactor serial driver slightly for driver model

Quoted 4 lines · Simon Glass

Before adding driver model support, split out a few of the functions so that they can be used by the driver model code.

Quoted 4 lines

---

 drivers/serial/atmel_usart.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)
#14 lore raw

PATCH 6/7 dm: at91: Add driver model support for the serial driver

In reply to Simon Glass

Add driver model support while retaining the existing legacy code. This allows the driver to support boards that have converted to driver model as well as those that have not.

---

 arch/arm/include/asm/arch-at91/at91_serial.h | 15 +++++
 drivers/serial/atmel_usart.c                 | 85 ++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-at91/at91_serial.h
arch/arm/include/asm/arch-at91/at91_serial.h new file +15 -0
new file mode 100644
index 0000000..9f78ed4
@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2014 Google, Inc
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#ifndef _AT91_SERIAL_H
+#define _AT91_SERIAL_H
+
+/* Information about a serial port */
+struct at91_serial_platdata {
+	uint32_t base_addr;
+};
+
+#endif
drivers/serial/atmel_usart.c +85 -0

105 lines not on this page · raw message

Signature · 1 line
2.1.0.rc2.206.gedb03e5
#15 lore raw

PATCH 6/7 dm: at91: Add driver model support for the serial driver

Hi Simon,

Quoted 40 lines · Simon Glass

Add driver model support while retaining the existing legacy code. This allows the driver to support boards that have converted to driver model as well as those that have not.

---

 arch/arm/include/asm/arch-at91/at91_serial.h | 15 +++++
 drivers/serial/atmel_usart.c                 | 85 ++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-at91/at91_serial.h
arch/arm/include/asm/arch-at91/at91_serial.h new file +15 -0
new file mode 100644
index 0000000..9f78ed4
@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2014 Google, Inc
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#ifndef _AT91_SERIAL_H
+#define _AT91_SERIAL_H
+
+/* Information about a serial port */
+struct at91_serial_platdata {
+	uint32_t base_addr;
+};
+
+#endif
drivers/serial/atmel_usart.c +0 -0
index ce36e99..d3e2ade 100644
@@ -7,11 +7,17 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */

drivers/serial/atmel_usart.c line 18

Quoted 10 lines
drivers/serial/atmel_usart.c +4 -0
 #include <common.h>
+#include <dm.h>
+#include <errno.h>
 #include <watchdog.h>
 #include <serial.h>
 #include <linux/compiler.h>
 
 #include <asm/io.h>
+#ifdef CONFIG_DM_SERIAL
+/* Should this be made common with avr32? */

yes, we should do so. Therefore I vote for atmel_serial.h.

Quoted 77 lines
drivers/serial/atmel_usart.c +63 -0
+#include <asm/arch/at91_serial.h>
+#endif
 #include <asm/arch/clk.h>
 #include <asm/arch/hardware.h>
 
@@ -60,6 +66,7 @@ static void atmel_serial_activate(atmel_usart3_t *usart)
 	__udelay(100);
 }
 
+#ifndef CONFIG_DM_SERIAL
 static void atmel_serial_setbrg(void)
 {
 	atmel_serial_setbrg_internal((atmel_usart3_t *)CONFIG_USART_BASE,
@@ -123,3 +130,81 @@ __weak struct serial_device *default_serial_console(void)
 {
 	return &atmel_serial_drv;
 }
+#endif
+
+#ifdef CONFIG_DM_SERIAL
+
+struct at91_serial_priv {
+	atmel_usart3_t *usart;
+};
+
+int at91_serial_setbrg(struct udevice *dev, int baudrate)
+{
+	struct at91_serial_priv *priv = dev_get_priv(dev);
+
+	atmel_serial_setbrg_internal(priv->usart, 0 /* ignored */, baudrate);
+	atmel_serial_activate(priv->usart);
+
+	return 0;
+}
+
+static int at91_serial_getc(struct udevice *dev)
+{
+	struct at91_serial_priv *priv = dev_get_priv(dev);
+
+	if (!(readl(&priv->usart->csr) & USART3_BIT(RXRDY)))
+		return -EAGAIN;
+
+	return readl(&priv->usart->rhr);
+}
+
+static int at91_serial_putc(struct udevice *dev, const char ch)
+{
+	struct at91_serial_priv *priv = dev_get_priv(dev);
+
+	if (!(readl(&priv->usart->csr) & USART3_BIT(TXRDY)))
+		return -EAGAIN;
+
+	writel(ch, &priv->usart->thr);
+
+	return 0;
+}
+
+static int at91_serial_pending(struct udevice *dev, bool input)
+{
+	struct at91_serial_priv *priv = dev_get_priv(dev);
+	uint32_t csr = readl(&priv->usart->csr);
+
+	if (input)
+		return csr & USART3_BIT(RXRDY) ? 1 : 0;
+	else
+		return csr & USART3_BIT(TXEMPTY) ? 0 : 1;
+}
+
+static const struct dm_serial_ops at91_serial_ops = {
+	.putc = at91_serial_putc,
+	.pending = at91_serial_pending,
+	.getc = at91_serial_getc,
+	.setbrg = at91_serial_setbrg,
+};
+
+static int at91_serial_probe(struct udevice *dev)
+{

drivers/serial/atmel_usart.c __weak struct serial_device *default_serial_console(void) · line 202

Quoted 10 lines
drivers/serial/atmel_usart.c +10 -0
+	struct at91_serial_platdata *plat = dev->platdata;
+	struct at91_serial_priv *priv = dev_get_priv(dev);
+
+	priv->usart = (atmel_usart3_t *)plat->base_addr;
+	atmel_serial_init_internal(priv->usart);
+
+	return 0;
+}
+
+U_BOOT_DRIVER(serial_at91) = {

Please name it 'atmel' rather than 'at91' at91 is just one (legacy) product family, there is also avr32 (ok, outdated) but new and growing 'sama5'.

drivers/serial/atmel_usart.c __weak struct serial_device *default_serial_console(void) · line 210

Quoted 9 lines
drivers/serial/atmel_usart.c +8 -0
+	.name	= "serial_at91",
+	.id	= UCLASS_SERIAL,
+	.probe = at91_serial_probe,
+	.ops	= &at91_serial_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+	.priv_auto_alloc_size	= sizeof(struct at91_serial_priv),
+};
+#endif

The patch looks good to me, but I wonder if changing the baudrate is working as expected. Did you test this?

Best regards

Andreas Bie?mann

#16 lore raw

PATCH 7/7 dm: at91: Convert snapper9260 to use driver model

In reply to Simon Glass

Convert this at91sam9260-based board to use driver model. This should serve as an example for other similar boards. Serial and GPIO are supported so far.

---

 board/bluewater/snapper9260/snapper9260.c | 18 ++++++++++++++++--
 include/configs/snapper9260.h             |  9 ++++++++-
 2 files changed, 24 insertions(+), 3 deletions(-)
board/bluewater/snapper9260/snapper9260.c +16 -2
index bfde129..b98359f 100644
@@ -9,12 +9,15 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <asm/io.h>
+#include <asm/gpio.h>
 #include <asm/arch/at91sam9260_matrix.h>
 #include <asm/arch/at91sam9_smc.h>
 #include <asm/arch/at91_common.h>
 #include <asm/arch/at91_pmc.h>
 #include <asm/arch/gpio.h>
+#include <asm/arch/at91_serial.h>
 #include <net.h>
 #include <netdev.h>
 #include <i2c.h>
@@ -95,10 +98,12 @@ static void nand_hw_init(void)
 	       &smc->cs[3].mode);
 
 	/* Configure RDY/BSY */
-	at91_set_gpio_input(CONFIG_SYS_NAND_READY_PIN, 1);
+	gpio_request(CONFIG_SYS_NAND_READY_PIN, "nand_rdy");
+	gpio_direction_input(CONFIG_SYS_NAND_READY_PIN);
 
 	/* Enable NandFlash */
-	at91_set_gpio_output(CONFIG_SYS_NAND_ENABLE_PIN, 1);
+	gpio_request(CONFIG_SYS_NAND_ENABLE_PIN, "nand_ce");
+	gpio_direction_output(CONFIG_SYS_NAND_ENABLE_PIN, 1);
 }
 
 int board_init(void)
@@ -140,3 +145,12 @@ int dram_init(void)
 void reset_phy(void)
 {
 }
+
+static struct at91_serial_platdata at91sam9260_serial_plat = {
+	.base_addr = ATMEL_BASE_DBGU,
+};
+
+U_BOOT_DEVICE(at91sam9260_serial) = {
+	.name	= "serial_at91",
+	.platdata = &at91sam9260_serial_plat,
+};
include/configs/snapper9260.h +8 -1
index fdb6cca..251ef07 100644
@@ -21,6 +21,11 @@
 #define CONFIG_SYS_AT91_MAIN_CLOCK	18432000 /* External Crystal, in Hz */
 #define CONFIG_SYS_AT91_SLOW_CLOCK	32768
 #define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_DM
+#define CONFIG_CMD_DM
+#define CONFIG_DM_GPIO
+#define CONFIG_DM_SERIAL
+#define CONFIG_SYS_MALLOC_F_LEN		(1 << 10)
 
 /* CPU */
 #define CONFIG_ARCH_CPU_INIT
@@ -85,8 +90,10 @@
 
 /* UARTs/Serial console */
 #define CONFIG_ATMEL_USART
+#ifndef CONFIG_DM_SERIAL
 #define CONFIG_USART_BASE		ATMEL_BASE_DBGU
 #define CONFIG_USART_ID			ATMEL_ID_SYS
+#endif
 #define CONFIG_BAUDRATE			115200
 #define CONFIG_SYS_PROMPT		"Snapper> "
 
@@ -160,7 +167,7 @@
 #define CONFIG_CMD_DHCP
 #define CONFIG_CMD_FAT
 #define CONFIG_CMD_I2C
-#undef CONFIG_CMD_GPIO
+#define CONFIG_CMD_GPIO
 #define CONFIG_CMD_USB
 #define CONFIG_CMD_MII
 #define CONFIG_CMD_NAND
Signature · 1 line
2.1.0.rc2.206.gedb03e5
#17 lore raw

PATCH 7/7 dm: at91: Convert snapper9260 to use driver model

Quoted 5 lines · Simon Glass

Convert this at91sam9260-based board to use driver model. This should serve as an example for other similar boards. Serial and GPIO are supported so far.

Quoted 5 lines

---

 board/bluewater/snapper9260/snapper9260.c | 18 ++++++++++++++++--
 include/configs/snapper9260.h             |  9 ++++++++-
 2 files changed, 24 insertions(+), 3 deletions(-)
#18 lore raw

cover dm: Introduce driver model for Atmel at91

In reply to Simon Glass

Hi,

Quoted 7 lines · Simon Glass

This series adds driver model support for serial and GPIO for the Atmel AT91 series. The at91sam9260-based Snapper 9260 is converted over as an example.

For both drivers the old behaviour is retained, so driver model becomes option, not mandatory.

This series is available at u-boot-dm/atmel-working.

Are there any comments on this series please?

Quoted 22 lines

Simon Glass (7):
dm: at91: Correct text base for snapper9260
dm: at91: Move snapper9260 to generic baord
dm: at91: Add driver model support for atmel GPIO driver
dm: at91: Add platform data for GPIO on at91sam9260-based boards
dm: at91: Refactor serial driver slightly for driver model
dm: at91: Add driver model support for the serial driver
dm: at91: Convert snapper9260 to use driver model

 arch/arm/cpu/arm926ejs/at91/at91sam9260_devices.c |  14 ++
 arch/arm/include/asm/arch-at91/at91_serial.h      |  15 ++
 arch/arm/include/asm/arch-at91/at91sam9260.h      |   4 +-
 arch/arm/include/asm/arch-at91/gpio.h             |   6 +
 board/bluewater/snapper9260/snapper9260.c         |  18 +-
 drivers/gpio/at91_gpio.c                          | 221 +++++++++++++++++-----
 drivers/serial/atmel_usart.c                      | 117 +++++++++++-
 include/configs/snapper9260.h                     |  12 +-
 8 files changed, 349 insertions(+), 58 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-at91/at91_serial.h
Quoted 3 lines

--
2.1.0.rc2.206.gedb03e5

Regards,
Simon

#19 lore raw

cover dm: Introduce driver model for Atmel at91

Hi Simon,

Quoted 6 lines · Simon Glass

Hi,

Quoted 3 lines · Simon Glass

This series adds driver model support for serial and GPIO for the Atmel AT91 series. The at91sam9260-based Snapper 9260 is converted over as an example.

Quoted 6 lines
Quoted 4 lines

For both drivers the old behaviour is retained, so driver model becomes option, not mandatory.

This series is available at u-boot-dm/atmel-working.

Are there any comments on this series please?

done ;)

Regards

Andreas Bie?mann