arm: Add PL031 test

PATCH Superseded Alexander Graf 2 participants 3 messages Jul 25, 2019

Subsystems (2)

KVM KVM arm64

lore

Share & tools

lore

Alexander Graf

author

2019-07-25 13:10 UTC #0 lore raw

PATCH v4 1/1 arm: Add PL031 test

This patch adds a unit test for the PL031 RTC that is used in the virt machine. It just pokes basic functionality. I've mostly written it to familiarize myself with the device, but I suppose having the test around does not hurt, as it also exercises the GIC SPI interrupt path.

Signed-off-by Alexander Graf <[email protected]>

Reviewed-by Andrew Jones <[email protected]>

---

v1 -> v2:

- Use FDT to find base, irq and existence
- Put isb after timer read
- Use dist_base for gicv3

v2 -> v3

- Enable compilation on 32bit ARM target
- Use ioremap

v3 -> v4:

- Use dt_pbus_translate_node()
- Make irq_triggered volatile

---
 arm/Makefile.common |   1 +
 arm/pl031.c         | 260 ++++++++++++++++++++++++++++++++++++++++++++
 lib/arm/asm/gic.h   |   1 +
 3 files changed, 262 insertions(+)
 create mode 100644 arm/pl031.c

arm/Makefile.common +1 -0

index f0c4b5d..b8988f2 100644
@@ -11,6 +11,7 @@ tests-common += $(TEST_DIR)/pmu.flat
 tests-common += $(TEST_DIR)/gic.flat
 tests-common += $(TEST_DIR)/psci.flat
 tests-common += $(TEST_DIR)/sieve.flat
+tests-common += $(TEST_DIR)/pl031.flat
 
 tests-all = $(tests-common) $(tests)
 all: directories $(tests-all)

arm/pl031.c new file +260 -0

260 lines not on this page · raw message

lib/arm/asm/gic.h +1 -0

index f6dfb90..1fc10a0 100644
@@ -41,6 +41,7 @@
 #include <asm/gic-v3.h>
 
 #define PPI(irq)			((irq) + 16)
+#define SPI(irq)			((irq) + GIC_FIRST_SPI)
 
 #ifndef __ASSEMBLY__
 #include <asm/cpumask.h>

Signature · 1 line

2.17.1

Andrew Jones

2019-07-25 13:25 UTC #1 lore raw

Quoted 23 lines · Alexander Graf

This patch adds a unit test for the PL031 RTC that is used in the virt machine. It just pokes basic functionality. I've mostly written it to familiarize myself with the device, but I suppose having the test around does not hurt, as it also exercises the GIC SPI interrupt path.
Signed-off-by Alexander Graf <[email protected]>
Reviewed-by Andrew Jones <[email protected]>
---
v1 -> v2:
- Use FDT to find base, irq and existence
- Put isb after timer read
- Use dist_base for gicv3
v2 -> v3
- Enable compilation on 32bit ARM target
- Use ioremap
v3 -> v4:

Quoted 8 lines

- Use dt_pbus_translate_node()
- Make irq_triggered volatile

---
 arm/Makefile.common |   1 +
 arm/pl031.c         | 260 ++++++++++++++++++++++++++++++++++++++++++++
 lib/arm/asm/gic.h   |   1 +
 3 files changed, 262 insertions(+)
 create mode 100644 arm/pl031.c

Thanks for the new version. I have a new nit (below), but my r-b stands with or without making another change.

[...]

Quoted 9 lines

arm/pl031.c new file +9 -0

+static int rtc_fdt_init(void)
+{
+	const struct fdt_property *prop;
+	const void *fdt = dt_fdt();
+	struct dt_pbus_reg base;
+	int node, len;
+	u32 *data;
+
+	node = fdt_node_offset_by_compatible(fdt, -1, "arm,pl031");

arm/pl031.c line 238

Quoted 10 lines

arm/pl031.c new file +10 -0

+	if (node < 0)
+		return -1;
+
+	prop = fdt_get_property(fdt, node, "interrupts", &len);
+	assert(prop && len == (3 * sizeof(u32)));
+	data = (u32 *)prop->data;
+	assert(data[0] == 0); /* SPI */
+	pl031_irq = SPI(fdt32_to_cpu(data[1]));
+
+	assert(!dt_pbus_translate_node(node, 0, &base));

We prefer to do something like

ret = dt_pbus_translate_node(node, 0, &base);
assert(!ret);

than the above, just in case we ever compiled with assert() defined as a no-op. But the probability of doing that is pretty close to zero.

arm/pl031.c line 242

Quoted 4 lines

arm/pl031.c new file +4 -0

+	pl031 = ioremap(base.addr, base.size);
+
+	return 0;
+}

Thanks,
drew

Alexander Graf

author

2019-07-25 13:41 UTC #2 lore raw

Quoted 60 lines · Andrew Jones

Quoted 23 lines · Alexander Graf
This patch adds a unit test for the PL031 RTC that is used in the virt machine. It just pokes basic functionality. I've mostly written it to familiarize myself with the device, but I suppose having the test around does not hurt, as it also exercises the GIC SPI interrupt path.
Signed-off-by Alexander Graf <[email protected]>
Reviewed-by Andrew Jones <[email protected]>
---
v1 -> v2:
- Use FDT to find base, irq and existence
- Put isb after timer read
- Use dist_base for gicv3
v2 -> v3
- Enable compilation on 32bit ARM target
- Use ioremap
v3 -> v4:
Quoted 8 lines
- Use dt_pbus_translate_node()
- Make irq_triggered volatile
---
  arm/Makefile.common |   1 +
  arm/pl031.c         | 260 ++++++++++++++++++++++++++++++++++++++++++++
  lib/arm/asm/gic.h   |   1 +
  3 files changed, 262 insertions(+)
create mode 100644 arm/pl031.c
Thanks for the new version. I have a new nit (below), but my r-b stands with or without making another change.
[...]
Quoted 9 lines
arm/pl031.c new file +9 -0
+static int rtc_fdt_init(void)
+{
+	const struct fdt_property *prop;
+	const void *fdt = dt_fdt();
+	struct dt_pbus_reg base;
+	int node, len;
+	u32 *data;
+
+	node = fdt_node_offset_by_compatible(fdt, -1, "arm,pl031");
arm/pl031.c line 238
Quoted 10 lines
arm/pl031.c new file +10 -0
+	if (node < 0)
+		return -1;
+
+	prop = fdt_get_property(fdt, node, "interrupts", &len);
+	assert(prop && len == (3 * sizeof(u32)));
+	data = (u32 *)prop->data;
+	assert(data[0] == 0); /* SPI */
+	pl031_irq = SPI(fdt32_to_cpu(data[1]));
+
+	assert(!dt_pbus_translate_node(node, 0, &base));
We prefer to do something like

Quoted 5 lines

  ret = dt_pbus_translate_node(node, 0, &base);
  assert(!ret);
than the above, just in case we ever compiled with assert() defined as a no-op. But the probability of doing that is pretty close to zero.

Yeah, but before someone wastes an hour of debugging on this later, let's fix it right away. Thanks for catching it! :)

Alex