Hi Mario,
On 23 May 2018 at 06:10, Mario Six mario.six@gdsys.cc wrote:
Add test infrastructure and tests for the AXI uclass.
Signed-off-by: Mario Six mario.six@gdsys.cc
v1 -> v2:
- Spelled out abbreviations in Kconfig help
- Expanded emulation documentation
- Renamed storage pointer to storep
- Expanded AXI emulator usage documentation
- Switched AXI emulator to aligned access
drivers/axi/Kconfig | 7 +++ drivers/axi/Makefile | 3 ++ drivers/axi/axi-emul-uclass.c | 68 ++++++++++++++++++++++++++ drivers/axi/axi_sandbox.c | 77 +++++++++++++++++++++++++++++ drivers/axi/sandbox_store.c | 110 ++++++++++++++++++++++++++++++++++++++++++ include/axi.h | 84 ++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + 7 files changed, 350 insertions(+) create mode 100644 drivers/axi/axi-emul-uclass.c create mode 100644 drivers/axi/axi_sandbox.c create mode 100644 drivers/axi/sandbox_store.c
Reviewed-by: Simon Glass sjg@chromium.org
Some nits below.
diff --git a/drivers/axi/axi_sandbox.c b/drivers/axi/axi_sandbox.c new file mode 100644 index 00000000000..7a485b114bf --- /dev/null +++ b/drivers/axi/axi_sandbox.c @@ -0,0 +1,77 @@ +/*
- (C) Copyright 2018
- Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <axi.h> +#include <dm.h>
+/*
- This driver implements a AXI bus for the sandbox architecture for testing
- purposes.
- The bus forwards every access to it to a special AXI emulation device (which
- it gets via the axi_emul_get_ops function) that implements a simple
- read/write storage.
- The emulator device must still be contained in the device tree in the usual
- way, since configuration data for the storage is read from the DT.
- */
+int axi_sandbox_read(struct udevice *bus, ulong address, void *data,
enum axi_size_t size)
Can this and the write() function below be static?
+{
struct axi_emul_ops *ops;struct udevice *emul;int ret;/* Get emulator device */ret = axi_sandbox_get_emul(bus, address, size, &emul);if (ret)return ret == -ENODEV ? 0 : ret;/* Forward all reads to the AXI emulator */ops = axi_emul_get_ops(emul);if (!ops || !ops->read)return -ENOSYS;return ops->read(emul, address, data, size);+}
+int axi_sandbox_write(struct udevice *bus, ulong address, void *data,
enum axi_size_t size)+{
struct axi_emul_ops *ops;struct udevice *emul;int ret;/* Get emulator device */ret = axi_sandbox_get_emul(bus, address, size, &emul);if (ret)return ret == -ENODEV ? 0 : ret;/* Forward all writes to the AXI emulator */ops = axi_emul_get_ops(emul);if (!ops || !ops->write)return -ENOSYS;return ops->write(emul, address, data, size);+}
+static const struct udevice_id axi_sandbox_ids[] = {
{ .compatible = "sandbox,axi" },{ /* sentinel */ }+};
+static const struct axi_ops axi_sandbox_ops = {
.read = axi_sandbox_read,.write = axi_sandbox_write,+};
+U_BOOT_DRIVER(axi_sandbox_bus) = {
.name = "axi_sandbox_bus",.id = UCLASS_AXI,.of_match = axi_sandbox_ids,.ops = &axi_sandbox_ops,+}; diff --git a/drivers/axi/sandbox_store.c b/drivers/axi/sandbox_store.c new file mode 100644 index 00000000000..61c7aea22c8 --- /dev/null +++ b/drivers/axi/sandbox_store.c @@ -0,0 +1,110 @@ +/*
- (C) Copyright 2018
- Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <axi.h> +#include <dm.h>
+struct sandbox_store_priv {
u8 *store;+};
+void copy_axi_data(void *src, void *dst, enum axi_size_t size) +{
u8 *dst8 = dst;u16 *dst16 = dst;u32 *dst32 = dst;
Can you put these declarations in the case statements instead? I'm not sure what coverity with think of this aliasing.
switch (size) {case AXI_SIZE_8:*dst8 = *((u8 *)src);break;case AXI_SIZE_16:*dst16 = be16_to_cpu(*((u16 *)src));break;case AXI_SIZE_32:*dst32 = be32_to_cpu(*((u32 *)src));break;}+}
+int sandbox_store_read(struct udevice *dev, ulong address, void *data,
enum axi_size_t size)+{
struct sandbox_store_priv *priv = dev_get_priv(dev);copy_axi_data(priv->store + address, data, size);return 0;+}
+int sandbox_store_write(struct udevice *dev, ulong address, void *data,
enum axi_size_t size)+{
struct sandbox_store_priv *priv = dev_get_priv(dev);copy_axi_data(data, priv->store + address, size);return 0;+}
+int sandbox_store_get_store(struct udevice *dev, u8 **store) +{
struct sandbox_store_priv *priv = dev_get_priv(dev);*store = priv->store;return 0;+}
+static const struct udevice_id sandbox_store_ids[] = {
{ .compatible = "sandbox,sandbox_store" },{ /* sentinel */ }+};
+static const struct axi_emul_ops sandbox_store_ops = {
.read = sandbox_store_read,.write = sandbox_store_write,.get_store = sandbox_store_get_store,+};
+int sandbox_store_probe(struct udevice *dev) +{
struct sandbox_store_priv *priv = dev_get_priv(dev);u32 reg[2];int ret;ret = dev_read_u32_array(dev, "reg", reg, ARRAY_SIZE(reg));if (ret)return -EINVAL;priv->store = calloc(reg[1], 1);
drop blank line
if (!priv->store)return -ENOMEM;return 0;+}
+int sandbox_store_remove(struct udevice *dev) +{
struct sandbox_store_priv *priv = dev_get_priv(dev);free(priv->store);return 0;+}
+U_BOOT_DRIVER(sandbox_axi_store) = {
.name = "sandbox_axi_store",.id = UCLASS_AXI_EMUL,.of_match = sandbox_store_ids,.ops = &sandbox_store_ops,.priv_auto_alloc_size = sizeof(struct sandbox_store_priv),.probe = sandbox_store_probe,.remove = sandbox_store_remove,+}; diff --git a/include/axi.h b/include/axi.h index 317e931a6cf..5b428127f8c 100644 --- a/include/axi.h +++ b/include/axi.h @@ -72,4 +72,88 @@ int axi_read(struct udevice *dev, ulong address, void *data, enum axi_size_t siz
- @return 0 if OK, -ve on error.
*/ int axi_write(struct udevice *dev, ulong address, void *data, enum axi_size_t size);
+struct axi_emul_ops {
/*** read() - Read a single value from a specified address on a AXI bus** @dev: AXI bus to read from.* @address: The address to read from.* @data: Pointer to a variable that takes the data value read* from the address on the AXI bus.* @size: The size of the data to be read.* @return 0 if OK, -ve on error.*/int (*read)(struct udevice *dev, ulong address, void *data, enum axi_size_t size);/*** write() - Write a single value to a specified address on a AXI bus** @dev: AXI bus to write to.* @address: The address to write to.* @data: Pointer to the data value to be written to the address* on the AXI bus.* @size: The size of the data to write.* @return 0 if OK, -ve on error.*/int (*write)(struct udevice *dev, ulong address, void *data, enum axi_size_t size);/*** get_store() - Get address of internal storage of a emulated AXI* device** @dev: Emulated AXI device to get the pointer of the internal* storage for.* @storep: Pointer to the internal storage of the emulated AXI* device.* @return 0 if OK, -ve on error.*/int (*get_store)(struct udevice *dev, u8 **storep);+};
+#define axi_emul_get_ops(dev) ((struct axi_emul_ops *)(dev)->driver->ops)
+/**
- axi_sandbox_get_emul() - Retrieve a pointer to a AXI emulation device
- To test the AXI uclass, we implement a simple AXI emulation device, which is
- a virtual device on a AXI bus that exposes a simple storage interface: When
- reading and writing from the device, the addresses are translated to offsets
- within the device's storage. For write accesses the data is written to the
- specified storage offset, and for read accesses the data is read from the
- specified storage offset.
- A DTS entry might look like this:
- axi: axi@0 {
compatible = "sandbox,axi";
#address-cells = <0x1>;
#size-cells = <0x1>;
store@0 {
compatible = "sandbox,sandbox_store";
reg = <0x0 0x400>;
};
- };
- This function may then be used to retrieve the pointer to the sandbox_store
- emulation device given the AXI bus device.
- */
+int axi_sandbox_get_emul(struct udevice *bus, ulong address, uint length,
struct udevice **emulp);
Can these two function declarations go in arch/sandbox/include/asm perhaps? I don't think they are needed except for sandbox?
+/**
- axi_get_store() - Get address of internal storage of a emulated AXI device
- To preset or read back the contents internal storage of the emulated AXI
- device, this function returns the pointer to the storage. Changes to the
- contents of the storage are reflected when using the AXI read/write API
- methods, and vice versa, so by using this method expected read data can be
- set up in advance, and written data can be checked in unit tests.
- @dev: Emulated AXI device to get the pointer of the internal storage
for.
- @storep: Pointer to the internal storage of the emulated AXI device.
- @return 0 if OK, -ve on error.
- */
+int axi_get_store(struct udevice *dev, u8 **storep);
#endif
Regards, Simon