drivers: Add AXI uclass

Add a driver for the gdsys IHS AXI bus used on IHS FPGAs.

Signed-off-by Mario Six <[email protected]>

---

v1 -> v2:
New in v2

---
 drivers/axi/Kconfig   |  12 ++++
 drivers/axi/Makefile  |   1 +
 drivers/axi/ihs_axi.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 192 insertions(+)
 create mode 100644 drivers/axi/ihs_axi.c

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

drivers/axi/axi_sandbox.c new file +15 -0

new file mode 100644
index 00000000000..7a485b114bf
@@ -0,0 +1,77 @@
+/*
+ * (C) Copyright 2018
+ * Mario Six, Guntermann & Drunck GmbH, mario.six at 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.
+ *

drivers/axi/axi_sandbox.c new file +52 -0

+{
+       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,
+};

drivers/axi/sandbox_store.c new file +10 -0

new file mode 100644
index 00000000000..61c7aea22c8
@@ -0,0 +1,110 @@
+/*
+ * (C) Copyright 2018
+ * Mario Six, Guntermann & Drunck GmbH, mario.six at gdsys.cc
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <axi.h>
+#include <dm.h>

drivers/axi/sandbox_store.c new file +56 -0

+
+       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)
+{

drivers/axi/sandbox_store.c new file +24 -0

+       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,
+};

include/axi.h +58 -0

index 317e931a6cf..5b428127f8c 100644
@@ -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 at 0 {
+ *     compatible = "sandbox,axi";
+ *     #address-cells = <0x1>;
+ *     #size-cells = <0x1>;
+ *     store at 0 {

include/axi.h +7 -0

+/**
+ * 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

Hi Mario,

Quoted 13 lines · Mario Six

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

Quoted 13 lines

---
 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.

Quoted 25 lines

drivers/axi/axi_sandbox.c new file +19 -0

new file mode 100644
index 00000000000..7a485b114bf
@@ -0,0 +1,77 @@
+/*
+ * (C) Copyright 2018
+ * Mario Six, Guntermann & Drunck GmbH, mario.six at 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.
+ *

drivers/axi/axi_sandbox.c line 25

Quoted 6 lines

drivers/axi/axi_sandbox.c new file +6 -0

+ * 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?

Quoted 73 lines

drivers/axi/axi_sandbox.c new file +52 -0

+{
+       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,
+};

drivers/axi/sandbox_store.c new file +15 -0

new file mode 100644
index 00000000000..61c7aea22c8
@@ -0,0 +1,110 @@
+/*
+ * (C) Copyright 2018
+ * Mario Six, Guntermann & Drunck GmbH, mario.six at gdsys.cc
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <axi.h>
+#include <dm.h>
+
+struct sandbox_store_priv {
+       u8 *store;
+};
+

drivers/axi/sandbox_store.c line 20

Quoted 5 lines

drivers/axi/sandbox_store.c new file +5 -0

+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.

Quoted 60 lines

drivers/axi/sandbox_store.c new file +60 -0

+
+       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;
+

drivers/axi/sandbox_store.c line 86

Quoted 6 lines

drivers/axi/sandbox_store.c new file +6 -0

+       ret = dev_read_u32_array(dev, "reg", reg, ARRAY_SIZE(reg));
+       if (ret)
+               return -EINVAL;
+
+       priv->store = calloc(reg[1], 1);
+

drop blank line

Quoted 95 lines

drivers/axi/sandbox_store.c new file +24 -0

+       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,
+};

include/axi.h +63 -0

index 317e931a6cf..5b428127f8c 100644
@@ -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 at 0 {
+ *     compatible = "sandbox,axi";
+ *     #address-cells = <0x1>;
+ *     #size-cells = <0x1>;
+ *     store at 0 {
+ *             compatible = "sandbox,sandbox_store";
+ *             reg = <0x0 0x400>;
+ *     };
+ * };
+ *

include/axi.h int axi_read(struct udevice *dev, ulong address, void *data, enum axi_size_t siz · line 142

Quoted 5 lines

include/axi.h +5 -0

+ * 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?

Quoted 12 lines

include/axi.h +12 -0

+/**
+ * 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.

include/axi.h int axi_read(struct udevice *dev, ulong address, void *data, enum axi_size_t siz · line 159

Quoted 5 lines

include/axi.h +4 -0

+ * @return 0 if OK, -ve on error.
+ */
+int axi_get_store(struct udevice *dev, u8 **storep);
+
 #endif

Regards,
Simon

Add tests for the AXI uclass.

Signed-off-by Mario Six <[email protected]>

---

v1 -> v2:
* Fixed asserts (moved expected values first)

---
 test/dm/Makefile |  1 +
 test/dm/axi.c    | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 test/dm/axi.c

Quoted 6 lines · Mario Six

Add tests for the AXI uclass.

Signed-off-by Mario Six <[email protected]>

---

Quoted 8 lines

v1 -> v2:
* Fixed asserts (moved expected values first)

---
 test/dm/Makefile |  1 +
 test/dm/axi.c    | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 test/dm/axi.c

Reviewed-by Simon Glass <[email protected]>

Can you please use lower-case hex?

Add a command to debug the AXI bus.

Signed-off-by Mario Six <[email protected]>

---

v1 -> v2:
No changes

---
 cmd/Kconfig  |   8 ++
 cmd/Makefile |   1 +
 cmd/axi.c    | 310 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 319 insertions(+)
 create mode 100644 cmd/axi.c

cmd/Kconfig +8 -0

index 38406fcfdac..0f730f7a397 100644
@@ -995,6 +995,14 @@ config CMD_USB_MASS_STORAGE
        help
          USB mass storage support
 
+config CMD_AXI
+       bool "axi"
+       depends on AXI
+       help
+         Enable the command "axi" for accessing AXI (Advanced eXtensible
+         Interface) busses, a on-chip interconnect specification for managing
+         functional blocks in SoC designs, which is also often used in designs
+         involving FPGAs (e.g.  communication with IP cores in Xilinx FPGAs).
 endmenu
 
 

cmd/Makefile +1 -0

index 0d7322ee0a4..cfca11499ec 100644
@@ -145,6 +145,7 @@ obj-$(CONFIG_CMD_ZFS) += zfs.o
 obj-$(CONFIG_CMD_DFU) += dfu.o
 obj-$(CONFIG_CMD_GPT) += gpt.o
 obj-$(CONFIG_CMD_ETHSW) += ethsw.o
+obj-$(CONFIG_CMD_AXI) += axi.o
 
 # Power
 obj-$(CONFIG_CMD_PMIC) += pmic.o

cmd/axi.c new file +46 -0

new file mode 100644
index 00000000000..d8001875e2c
@@ -0,0 +1,310 @@
+/*
+ * (C) Copyright 2016
+ * Dirk Eibach,  Guntermann & Drunck GmbH, dirk.eibach at gdsys.cc
+ *
+ * (C) Copyright 2017
+ * Mario Six,  Guntermann & Drunck GmbH, mario.six at gdsys.cc
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <command.h>
+#include <console.h>
+#include <axi.h>
+
+static struct udevice *axi_cur_bus;
+static uint dp_last_size;
+static uint dp_last_addr;
+static uint dp_last_length = 0x40;
+
+static void show_bus(struct udevice *bus)
+{
+       struct udevice *dev;
+
+       printf("Bus %d:\t%s", bus->req_seq, bus->name);
+       if (device_active(bus))
+               printf("  (active %d)", bus->seq);
+       printf("\n");
+       for (device_find_first_child(bus, &dev);
+            dev;
+            device_find_next_child(&dev))
+               printf("  %s\n", dev->name);
+}
+
+static int do_axi_show_bus(cmd_tbl_t *cmdtp, int flag, int argc,
+                          char * const argv[])
+{
+       if (argc == 1) {
+               /* show all busses */
+               struct udevice *bus;
+
+               for (uclass_first_device(UCLASS_AXI, &bus);
+                    bus;
+                    uclass_next_device(&bus))
+                       show_bus(bus);

cmd/axi.c new file +13 -0

+               if (ret) {
+                       printf("Invalid bus %d: err=%d\n", i, ret);
+                       return CMD_RET_FAILURE;
+               }
+               show_bus(bus);
+       }
+
+       return 0;
+}
+
+static int cmd_axi_set_bus_num(unsigned int busnum)
+{
+       struct udevice *bus;

Hi Mario,

Quoted 9 lines · Mario Six

Add a command to debug the AXI bus.

Signed-off-by Mario Six <[email protected]>

---

v1 -> v2:
No changes

Quoted 6 lines

---
 cmd/Kconfig  |   8 ++
 cmd/Makefile |   1 +
 cmd/axi.c    | 310 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 319 insertions(+)
 create mode 100644 cmd/axi.c

Reviewed-by Simon Glass <[email protected]>

Comments below.

Quoted 88 lines

cmd/Kconfig +8 -0

index 38406fcfdac..0f730f7a397 100644
@@ -995,6 +995,14 @@ config CMD_USB_MASS_STORAGE
        help
          USB mass storage support
 
+config CMD_AXI
+       bool "axi"
+       depends on AXI
+       help
+         Enable the command "axi" for accessing AXI (Advanced eXtensible
+         Interface) busses, a on-chip interconnect specification for managing
+         functional blocks in SoC designs, which is also often used in designs
+         involving FPGAs (e.g.  communication with IP cores in Xilinx FPGAs).
 endmenu
 
 

cmd/Makefile +1 -0

index 0d7322ee0a4..cfca11499ec 100644
@@ -145,6 +145,7 @@ obj-$(CONFIG_CMD_ZFS) += zfs.o
 obj-$(CONFIG_CMD_DFU) += dfu.o
 obj-$(CONFIG_CMD_GPT) += gpt.o
 obj-$(CONFIG_CMD_ETHSW) += ethsw.o
+obj-$(CONFIG_CMD_AXI) += axi.o
 
 # Power
 obj-$(CONFIG_CMD_PMIC) += pmic.o

cmd/axi.c new file +50 -0

new file mode 100644
index 00000000000..d8001875e2c
@@ -0,0 +1,310 @@
+/*
+ * (C) Copyright 2016
+ * Dirk Eibach,  Guntermann & Drunck GmbH, dirk.eibach at gdsys.cc
+ *
+ * (C) Copyright 2017
+ * Mario Six,  Guntermann & Drunck GmbH, mario.six at gdsys.cc
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <command.h>
+#include <console.h>
+#include <axi.h>
+
+static struct udevice *axi_cur_bus;
+static uint dp_last_size;
+static uint dp_last_addr;
+static uint dp_last_length = 0x40;
+
+static void show_bus(struct udevice *bus)
+{
+       struct udevice *dev;
+
+       printf("Bus %d:\t%s", bus->req_seq, bus->name);
+       if (device_active(bus))
+               printf("  (active %d)", bus->seq);
+       printf("\n");
+       for (device_find_first_child(bus, &dev);
+            dev;
+            device_find_next_child(&dev))
+               printf("  %s\n", dev->name);
+}
+
+static int do_axi_show_bus(cmd_tbl_t *cmdtp, int flag, int argc,
+                          char * const argv[])
+{
+       if (argc == 1) {
+               /* show all busses */
+               struct udevice *bus;
+
+               for (uclass_first_device(UCLASS_AXI, &bus);
+                    bus;
+                    uclass_next_device(&bus))
+                       show_bus(bus);
+       } else {
+               int i;
+
+               /* show specific bus */

cmd/axi.c line 56

Quoted 6 lines

cmd/axi.c new file +6 -0

+               i = simple_strtoul(argv[1], NULL, 10);
+
+               struct udevice *bus;
+               int ret;
+
+               ret = uclass_get_device_by_seq(UCLASS_AXI, i, &bus);

Here you probe the bus, whereas above you don't. Is that intended?

Quoted 17 lines

cmd/axi.c new file +17 -0

+               if (ret) {
+                       printf("Invalid bus %d: err=%d\n", i, ret);
+                       return CMD_RET_FAILURE;
+               }
+               show_bus(bus);
+       }
+
+       return 0;
+}
+
+static int cmd_axi_set_bus_num(unsigned int busnum)
+{
+       struct udevice *bus;
+       struct udevice *dummy;
+       int ret;
+
+       /* Make sure that all sequence numbers are initialized */

cmd/axi.c line 79

Quoted 6 lines

cmd/axi.c new file +6 -0

+       for (uclass_first_device(UCLASS_AXI, &dummy);
+            dummy;
+            uclass_next_device(&dummy))
+               ;
+
+       ret = uclass_get_device_by_seq(UCLASS_AXI, busnum, &bus);

Here you probe the bus which seems right to me.

Quoted 9 lines

cmd/axi.c new file +9 -0

+       if (ret) {
+               debug("%s: No bus %d\n", __func__, busnum);
+               return ret;
+       }
+       axi_cur_bus = bus;
+
+       return 0;
+}
+

[..]

Regards,
Simon

drivers/video/logicore_dp_dpcd.h new file +0 -0

new file mode 100644
index 00000000000..68582945514

drivers/video/logicore_dp_dpcd.h new file +3 -0

+
+bool is_link_rate_valid(struct udevice *dev, u8 link_rate)
+{

drivers/video/logicore_dp_tx_regif.h new file +0 -0

new file mode 100644
index 00000000000..c4105605c9b

+/*
+ * PHY_STATUS_ALL_LANES_READY_MASK seems zo be missing lanes 0 and 1 in

Hi Mario,

Please test the patch with ./scripts/checkpatch.pl, there are warnings/ errors reported that need fixing:
...
total: 1 errors, 31 warnings, 26 checks, 2746 lines checked

On Wed, 23 May 2018 14:10:47 +0200
Mario Six mario.six at gdsys.cc wrote:
...

Quoted 6 lines

drivers/video/Makefile +0 -0

index cf7ad281c3b..fa4ac715fcf 100644
@@ -21,6 +21,7 @@ obj-$(CONFIG_VIDEO_IVYBRIDGE_IGD) += ivybridge_igd.o
 

drivers/video/Makefile obj-$(CONFIG_VIDEO_IVYBRIDGE_IGD) += ivybridge_igd.o · line 24

Quoted 3 lines

drivers/video/Makefile +1 -0

 obj-$(CONFIG_ATI_RADEON_FB) += ati_radeon_fb.o videomodes.o
 obj-$(CONFIG_ATMEL_HLCD) += atmel_hlcdfb.o
+obj-$(CONFIG_LOGICORE_DP_TX) += logicore_dp_tx.o

Please add new driver entry sorted alphabetically. I know there are already not sorted entries in the list here, but we shouldn't add more.

...

Quoted 5 lines

drivers/video/logicore_dp_dpcd.h new file +0 -0

new file mode 100644
index 00000000000..68582945514

...

drivers/video/logicore_dp_dpcd.h

Quoted 6 lines

drivers/video/logicore_dp_dpcd.h new file +6 -0

+struct main_stream_attributes {
+     /* Pixel clock of the stream (in Hz). */
+     u32 pixel_clock_hz;
+     /* Miscellaneous stream attributes 0 as specified by the DisplayPort
+      * 1.2 specification.
+      */

Please fix multi-line comment style throughout this patch, we use this style:
/*
* multi-line
* comment
*/

...

drivers/video/logicore_dp_dpcd.h

Quoted 7 lines

drivers/video/logicore_dp_dpcd.h new file +7 -0

+static u32 get_reg(struct udevice *dev, u32 reg)
+{
+     struct dp_tx *dp_tx = dev_get_priv(dev);
+     u32 value = 0;
+     int res;
+
+     // TODO error handling

please no C++ comments.

...

drivers/video/logicore_dp_dpcd.h

Quoted 2 lines

drivers/video/logicore_dp_dpcd.h new file +2 -0

+bool is_connected(struct udevice *dev)
+{

shouldn't it be static?

...

Quoted 3 lines

drivers/video/logicore_dp_dpcd.h new file +3 -0

+
+bool is_link_rate_valid(struct udevice *dev, u8 link_rate)
+{

...

drivers/video/logicore_dp_dpcd.h

Quoted 2 lines

drivers/video/logicore_dp_dpcd.h new file +2 -0

+bool is_lane_count_valid(struct udevice *dev, u8 lane_count)
+{

Are these functions supposed to be used externally? If not, please add static. Otherwise move them to /* external API */ section.

...

drivers/video/logicore_dp_dpcd.h

Quoted 3 lines

drivers/video/logicore_dp_dpcd.h new file +3 -0

+int logicore_dp_tx_enable(struct udevice *dev, int panel_bpp,
+                       const struct display_timing *timing)
+{

should be static?

...

drivers/video/logicore_dp_dpcd.h

Quoted 2 lines

drivers/video/logicore_dp_dpcd.h new file +2 -0

+int logicore_dp_tx_probe(struct udevice *dev)
+{

should be static?

...

drivers/video/logicore_dp_dpcd.h

Quoted 3 lines

drivers/video/logicore_dp_dpcd.h new file +3 -0

+struct dm_display_ops logicore_dp_tx_ops = {
+     .enable = logicore_dp_tx_enable,
+};

should be static, too.

...

Quoted 5 lines

drivers/video/logicore_dp_tx_regif.h new file +0 -0

new file mode 100644
index 00000000000..c4105605c9b

...

drivers/video/logicore_dp_tx_regif.h line 37

Quoted 3 lines

drivers/video/logicore_dp_tx_regif.h new file +3 -0

+     /* core ID */
+     REG_VERSION =                   0x0F8,
+     REG_CORE_ID =                   0x0FC,

here a space and tabs follow '=', please use tabs only.

...

drivers/video/logicore_dp_tx_regif.h line 72

Quoted 2 lines

drivers/video/logicore_dp_tx_regif.h new file +2 -0

+     REG_USER_PIXEL_WIDTH =          0x1B8,
+     REG_USER_DATA_COUNT_PER_LANE =0x1BC,

spaces around '=', checkpatch will report style issues like this.

...

Quoted 2 lines

+/*
+ * PHY_STATUS_ALL_LANES_READY_MASK seems zo be missing lanes 0 and 1 in

s/zo/to ?

Thanks,

Add a uclass for AXI (Advanced eXtensible Interface) busses, and a driver for the gdsys IHS AXI bus on IHS FPGAs.

Signed-off-by Mario Six <[email protected]>

---

v1 -> v2:
* Spelled out all abbreviations in the Kconfig help
* Split commit into uclass addition and driver addition

---
 drivers/Kconfig          |  2 ++
 drivers/Makefile         |  1 +
 drivers/axi/Kconfig      | 13 +++++++++
 drivers/axi/Makefile     |  8 ++++++
 drivers/axi/axi-uclass.c | 40 ++++++++++++++++++++++++++
 include/axi.h            | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h   |  1 +
 7 files changed, 140 insertions(+)
 create mode 100644 drivers/axi/Kconfig
 create mode 100644 drivers/axi/Makefile
 create mode 100644 drivers/axi/axi-uclass.c
 create mode 100644 include/axi.h

include/dm/uclass-id.h +0 -1

        UCLASS_IRQ,             /* Interrupt controller */
        UCLASS_KEYBOARD,        /* Keyboard input device */
        UCLASS_LED,             /* Light-emitting diode (LED) */
--

2.11.0

Quoted 11 lines · Mario Six

Add a uclass for AXI (Advanced eXtensible Interface) busses, and a driver for the gdsys IHS AXI bus on IHS FPGAs.

Signed-off-by Mario Six <[email protected]>

---

v1 -> v2:
* Spelled out all abbreviations in the Kconfig help
* Split commit into uclass addition and driver addition

Quoted 13 lines

---
 drivers/Kconfig          |  2 ++
 drivers/Makefile         |  1 +
 drivers/axi/Kconfig      | 13 +++++++++
 drivers/axi/Makefile     |  8 ++++++
 drivers/axi/axi-uclass.c | 40 ++++++++++++++++++++++++++
 include/axi.h            | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h   |  1 +
 7 files changed, 140 insertions(+)
 create mode 100644 drivers/axi/Kconfig
 create mode 100644 drivers/axi/Makefile
 create mode 100644 drivers/axi/axi-uclass.c
 create mode 100644 include/axi.h

Reviewed-by Simon Glass <[email protected]>

One little nit:

include/dm/uclass-id.h enum uclass_id { · line 46

Quoted 9 lines

include/dm/uclass-id.h +1 -0

index d7f9df3583a..0aad4bc14d7 100644
@@ -43,6 +43,7 @@ enum uclass_id {
        UCLASS_I2C_GENERIC,     /* Generic I2C device */
        UCLASS_I2C_MUX,         /* I2C multiplexer */
        UCLASS_IDE,             /* IDE device */
+       UCLASS_AXI,             /* AXI busses */

AXI bus

Quoted 6 lines

include/dm/uclass-id.h +0 -1

        UCLASS_IRQ,             /* Interrupt controller */
        UCLASS_KEYBOARD,        /* Keyboard input device */
        UCLASS_LED,             /* Light-emitting diode (LED) */
--

2.11.0