Hi Simon,

On Fri, Mar 30, 2018 at 10:41 AM, Simon Glass <s...@chromium.org> wrote:
> Hi Mario,
>
> On 28 March 2018 at 20:40, Mario Six <mario....@gdsys.cc> wrote:
>> Add test infrastructure and tests for the AXI uclass.
>>
>> Signed-off-by: Mario Six <mario....@gdsys.cc>
>> ---
>>  drivers/axi/Kconfig           |   6 +++
>>  drivers/axi/Makefile          |   3 ++
>>  drivers/axi/axi-emul-uclass.c |  68 ++++++++++++++++++++++++
>>  drivers/axi/axi_sandbox.c     |  61 ++++++++++++++++++++++
>>  drivers/axi/sandbox_store.c   | 118 
>> ++++++++++++++++++++++++++++++++++++++++++
>>  include/axi.h                 |  51 ++++++++++++++++++
>>  include/dm/uclass-id.h        |   1 +
>>  7 files changed, 308 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
>>
>> diff --git a/drivers/axi/Kconfig b/drivers/axi/Kconfig
>> index 19e1b7fd2f..a0333782cf 100644
>> --- a/drivers/axi/Kconfig
>> +++ b/drivers/axi/Kconfig
>> @@ -21,4 +21,10 @@ config IHS_AXI
>>           Support for IHS AXI bus on a gdsys IHS FPGA used to communicate 
>> with
>>           IP cores in the FPGA (e.g. video transmitter cores).
>>
>> +config AXI_SANDBOX
>> +       bool "Enable AXI sandbox driver"
>> +       depends on DM
>> +       help
>> +         Support the AXI emulation for the sandbox environment.
>
> Please spell out abbreviations
>
>> +
>>  endif
>> diff --git a/drivers/axi/Makefile b/drivers/axi/Makefile
>> index 18d9380e9b..66b6c5a28f 100644
>> --- a/drivers/axi/Makefile
>> +++ b/drivers/axi/Makefile
>> @@ -7,3 +7,6 @@
>>
>>  obj-$(CONFIG_AXI) += axi-uclass.o
>>  obj-$(CONFIG_IHS_AXI) += ihs_axi.o
>> +obj-$(CONFIG_SANDBOX) += axi-emul-uclass.o
>> +obj-$(CONFIG_SANDBOX) += sandbox_store.o
>> +obj-$(CONFIG_AXI_SANDBOX) += axi_sandbox.o
>> diff --git a/drivers/axi/axi-emul-uclass.c b/drivers/axi/axi-emul-uclass.c
>> new file mode 100644
>> index 0000000000..bf60c40518
>> --- /dev/null
>> +++ b/drivers/axi/axi-emul-uclass.c
>> @@ -0,0 +1,68 @@
>> +/*
>> + * (C) Copyright 2018
>> + * Mario Six, Guntermann & Drunck GmbH, mario....@gdsys.cc
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <axi.h>
>> +#include <dm.h>
>> +#include <dm/device-internal.h>
>> +
>> +int axi_sandbox_get_emul(struct udevice *bus, ulong address, enum 
>> axi_size_t size,
>> +                        struct udevice **emulp)
>> +{
>> +       struct udevice *dev;
>> +       u32 reg[2];
>> +       uint offset;
>> +
>> +       switch (size) {
>> +       case AXI_SIZE_8:
>> +               offset = 1;
>> +               break;
>> +       case AXI_SIZE_16:
>> +               offset = 2;
>> +               break;
>> +       case AXI_SIZE_32:
>> +               offset = 4;
>> +               break;
>> +       default:
>> +               offset = 0;
>> +       }
>> +
>> +       for (device_find_first_child(bus, &dev);
>> +            dev;
>> +            device_find_next_child(&dev)) {
>> +               int ret;
>> +
>> +               ret = dev_read_u32_array(dev, "reg", reg, ARRAY_SIZE(reg));
>> +               if (ret)
>> +                       continue;
>> +
>> +               if (address >= reg[0] && address <= reg[0] + reg[1] - 
>> offset) {
>> +                       if (device_probe(dev))
>> +                               return -ENODEV;
>> +
>> +                       *emulp = dev;
>> +                       return 0;
>> +               }
>> +       }
>> +
>> +       return -ENODEV;
>> +}
>> +
>> +int axi_get_store(struct udevice *dev, u8 **store)
>> +{
>> +       struct axi_emul_ops *ops = axi_emul_get_ops(dev);
>> +
>> +       if (!ops->get_store)
>> +               return -ENOSYS;
>> +
>> +       return ops->get_store(dev, store);
>> +}
>> +
>> +UCLASS_DRIVER(axi_emul) = {
>> +       .id             = UCLASS_AXI_EMUL,
>> +       .name           = "axi_emul",
>> +};
>> diff --git a/drivers/axi/axi_sandbox.c b/drivers/axi/axi_sandbox.c
>> new file mode 100644
>> index 0000000000..b0f24bdd9d
>> --- /dev/null
>> +++ b/drivers/axi/axi_sandbox.c
>> @@ -0,0 +1,61 @@
>> +/*
>> + * (C) Copyright 2018
>> + * Mario Six, Guntermann & Drunck GmbH, mario....@gdsys.cc
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <axi.h>
>> +#include <dm.h>
>> +
>> +int axi_sandbox_read(struct udevice *bus, ulong address, void *data,
>> +                    enum axi_size_t size)
>> +{
>> +       struct axi_emul_ops *ops;
>> +       struct udevice *emul;
>> +       int ret;
>> +
>> +       ret = axi_sandbox_get_emul(bus, address, size, &emul);
>> +       if (ret)
>> +               return ret == -ENODEV ? 0 : ret;
>
> Please add a comment as to why you are doing this.
>
>> +       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;
>> +
>> +       ret = axi_sandbox_get_emul(bus, address, size, &emul);
>> +       if (ret)
>> +               return ret == -ENODEV ? 0 : ret;
>> +       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 0000000000..059e612459
>> --- /dev/null
>> +++ b/drivers/axi/sandbox_store.c
>> @@ -0,0 +1,118 @@
>> +/*
>> + * (C) Copyright 2018
>> + * Mario Six, Guntermann & Drunck GmbH, mario....@gdsys.cc
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <axi.h>
>> +#include <dm.h>
>> +#include <asm/unaligned.h>
>> +
>> +struct sandbox_store_priv {
>> +       u8 *store;
>> +};
>> +
>> +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);
>> +       u8 *p8 = data;
>> +       u16 *p16 = data;
>> +       u32 *p32 = data;
>> +
>> +       switch (size) {
>> +       case AXI_SIZE_8:
>> +               *p8 = priv->store[address];
>> +               break;
>> +       case AXI_SIZE_16:
>> +               *p16 = get_unaligned_be16(priv->store + address);
>> +               break;
>> +       case AXI_SIZE_32:
>> +               *p32 = get_unaligned_be32(priv->store + address);
>
> Do you really need unaligned access? Maybe you could require that
> accesses be aligned?
>
>> +               break;
>> +       }
>> +
>> +       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);
>> +       u8 *p8 = data;
>> +       u16 *p16 = data;
>> +       u32 *p32 = data;
>> +
>> +       switch (size) {
>> +       case AXI_SIZE_8:
>> +               priv->store[address] = *p8;
>> +               break;
>> +       case AXI_SIZE_16:
>> +               put_unaligned_be16(*p16, priv->store + address);
>> +               break;
>> +       case AXI_SIZE_32:
>> +               put_unaligned_be32(*p32, priv->store + address);
>> +               break;
>> +       }
>> +
>> +       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);
>> +
>> +       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 317e931a6c..d0a3e09b9a 100644
>> --- a/include/axi.h
>> +++ b/include/axi.h
>> @@ -72,4 +72,55 @@ 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 memory of a emulated AXI
>> +        *               device
>
> Please explain why someone would want this, and what a storage pointer is
>
>> +        *
>> +        * @dev:        Emulated AXI device to get the storage pointer for.
>> +        * @store:      Pointer to the internal storage of the emulated AXI
>
> Suggest storep so it is clear that it is a pointer to the value.
>
>> +        *              device.
>> +        * @return 0 if OK, -ve on error.
>> +        */
>> +       int (*get_store)(struct udevice *dev, u8 **store);
>> +};
>> +
>> +#define axi_emul_get_ops(dev)  ((struct axi_emul_ops *)(dev)->driver->ops)
>> +
>> +/**
>> + * axi_get_store() - Get address of internal memory of a emulated AXI device
>> + *
>> + * @dev:       Emulated AXI device to get the storage pointer for.
>> + * @store:     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 **store);
>> +
>> +int axi_sandbox_get_emul(struct udevice *bus, ulong address, uint length,
>> +                        struct udevice **emulp);
>
> comment
>
>>  #endif
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index 9857796e54..04faebdde9 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -23,6 +23,7 @@ enum uclass_id {
>>         UCLASS_I2C_EMUL,        /* sandbox I2C device emulator */
>>         UCLASS_PCI_EMUL,        /* sandbox PCI device emulator */
>>         UCLASS_USB_EMUL,        /* sandbox USB bus device emulator */
>> +       UCLASS_AXI_EMUL,        /* sandbox AXI bus device emulator */
>>         UCLASS_SIMPLE_BUS,      /* bus with child devices */
>>
>>         /* U-Boot uclasses start here - in alphabetical order */
>> --
>> 2.16.1
>>
>
> Regards,
> Simon
>

I'll address all problems in v2. Thanks for reviewing!

Best regards,

Mario
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to