Hi Mario, On 23 May 2018 at 06:10, Mario Six <[email protected]> wrote: > Add test infrastructure and tests for the AXI uclass. > > Signed-off-by: Mario Six <[email protected]> > > --- > > 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 <[email protected]> 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, [email protected] > + * > + * 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, [email protected] > + * > + * 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 _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
