Hi Simon,
On Fri, May 25, 2018 at 4:41 AM, Simon Glass sjg@chromium.org wrote:
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?
Yes, sure, will be fixed in v3.
+{
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.
Yeah, coverity probably won't like that. I'll put the casts in the case statements for v3.
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
Will be done for v3.
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?
Yes, no problem. Will be fixed in v3.
+/**
- 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
Best regards, Mario