All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support
@ 2020-12-15 17:23 Nicolas Saenz Julienne
  2020-12-15 17:23 ` [PATCH v3 01/12] rpi: Add identifier for the new RPi400 Nicolas Saenz Julienne
                   ` (11 more replies)
  0 siblings, 12 replies; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

This series could be split into at least two or even three parts, but I
kept it as is for now as it contains all the changes needed in order to
have u-boot working on the new Raspberry Pi 400 and Raspberry Pi Compute
Module 4.

There are core changes, specifically with regard to cpu to bus address
space translations. So far we had relied on hard-coded values but RPi
needs per device translations as it has at least three distinct bus
address spaces with different offsets. So it's a good opportunity to
implement bus translations the right way by parsing DT's dma-ranges.

Here's a more concrete example of what we're dealing with:

 - On a RPi4, SoC version BCM2711C0 with 8GB of memory

	[0x0 0x200000000]   [0x200000000 0x400000000]  [0xc0000000 0x100000000]  [0x00000000 0x100000000]
	 phys/cpu address       PCIe bus address           Legacy peripheral           emmc2 address
	     space                   space                   address space                space

 - On a RPi4, SoC version BCM2711C0 with 4GB of memory

	[0x0 0x100000000]   [0x100000000 0x200000000]  [0xc0000000 0x100000000]  [0x00000000 0x100000000]
	  phys/cpu address      PCIe bus address          Legacy peripheral            emmc2 address
	    space                  space                    address space                space

- On a RPi4, SoC version BCM2711B0 with 8GB of memory (bus can only access the
  lower 3GB of memory because of a SoC routing bug)

	[0x0 0x200000000]   [0x00000000 0xC0000000]   [0xc0000000 0x100000000]
	 phys/cpu address      PCIe bus address          Legacy peripheral
	    space                  space                   address space

---

Changes since v2:
 - Test builds not broken with buildman
 - Add tests to all DM changes
 - Make code conditional with config option
 - Correct OF refcount
 - Add config changes
 - Address small changes as per reviews

Changes since v1:
 - Fix some issues in 'dm: Introduce xxx_get_dma_range()'
 - Fix some typos in commit messages
 - Change DTB file name for RPi400
 - Address Matthias' comments

Nicolas Saenz Julienne (12):
  rpi: Add identifier for the new RPi400
  rpi: Add identifier for the new CM4
  pci: pcie-brcmstb: Fix inbound window configurations
  dm: Introduce xxx_get_dma_range()
  dm: test: Add test case for dev_get_dma_ranges()
  dm: Introduce DMA constraints into the core device model
  dm: test: Add test case for dev->dma_offset
  dm: Introduce dev_phys_to_bus()/dev_bus_to_phys()
  dm: test: Add test case for dev_phys_to_bus()/dev_bus_to_phys()
  xhci: translate virtual addresses into the bus's address space
  mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys()
  configs: rpi4: Enable DM_DMA across all RPi4 configurations

 arch/sandbox/dts/test.dts          | 21 ++++++++
 board/raspberrypi/rpi/rpi.c        | 10 ++++
 common/fdt_support.c               | 73 ++++++++++++++++++++++++++++
 configs/rpi_4_32b_defconfig        |  1 +
 configs/rpi_4_defconfig            |  1 +
 configs/rpi_arm64_defconfig        |  1 +
 configs/sandbox64_defconfig        |  1 +
 configs/sandbox_defconfig          |  1 +
 configs/sandbox_flattree_defconfig |  1 +
 configs/sandbox_spl_defconfig      |  1 +
 drivers/core/Kconfig               | 10 ++++
 drivers/core/device.c              | 35 ++++++++++++++
 drivers/core/of_addr.c             | 78 ++++++++++++++++++++++++++++++
 drivers/core/ofnode.c              |  9 ++++
 drivers/core/read.c                |  6 +++
 drivers/mmc/sdhci.c                |  7 +--
 drivers/pci/pcie_brcmstb.c         | 12 ++---
 drivers/usb/host/xhci-mem.c        | 45 +++++++++--------
 drivers/usb/host/xhci-ring.c       | 11 +++--
 drivers/usb/host/xhci.c            |  4 +-
 include/dm/device.h                |  3 ++
 include/dm/of_addr.h               | 17 +++++++
 include/dm/ofnode.h                | 16 ++++++
 include/dm/read.h                  | 21 ++++++++
 include/fdt_support.h              | 14 ++++++
 include/mmc.h                      | 10 ++++
 include/phys2bus.h                 | 16 ++++++
 include/usb/xhci.h                 | 22 ++++++++-
 test/dm/Makefile                   |  2 +
 test/dm/core.c                     | 30 ++++++++++++
 test/dm/phys2bus.c                 | 36 ++++++++++++++
 test/dm/read.c                     | 49 +++++++++++++++++++
 32 files changed, 526 insertions(+), 38 deletions(-)
 create mode 100644 test/dm/phys2bus.c
 create mode 100644 test/dm/read.c

-- 
2.29.2

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 01/12] rpi: Add identifier for the new RPi400
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-15 17:23 ` [PATCH v3 02/12] rpi: Add identifier for the new CM4 Nicolas Saenz Julienne
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

The Raspberry Pi Foundation released the new RPi400 which we want to
detect, so we can enable Ethernet on it and know the correct device tree
file name.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

---

Changes since v1:
 - The RPi Foundation introduced a RPi400 specific device tree, so use
   that file name instead of the fallback (RPi4b).

 board/raspberrypi/rpi/rpi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 6b1fa5fc14..b66698e4a9 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -157,6 +157,11 @@ static const struct rpi_model rpi_models_new_scheme[] = {
 		DTB_DIR "bcm2711-rpi-4-b.dtb",
 		true,
 	},
+	[0x13] = {
+		"400",
+		DTB_DIR "bcm2711-rpi-400.dtb",
+		true,
+	},
 };
 
 static const struct rpi_model rpi_models_old_scheme[] = {
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 02/12] rpi: Add identifier for the new CM4
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
  2020-12-15 17:23 ` [PATCH v3 01/12] rpi: Add identifier for the new RPi400 Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-15 17:23 ` [PATCH v3 03/12] pci: pcie-brcmstb: Fix inbound window configurations Nicolas Saenz Julienne
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

The Raspberry Pi Foundation released the new Compute Module 4 which we
want to detect, so we can enable Ethernet on it and know the correct
device tree file name.

Note that this sets the Ethernet option to true since the official CM4
IO board has an Ethernet port. But that might not be the case when using
custom ones.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 board/raspberrypi/rpi/rpi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index b66698e4a9..abcf41a5a8 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -162,6 +162,11 @@ static const struct rpi_model rpi_models_new_scheme[] = {
 		DTB_DIR "bcm2711-rpi-400.dtb",
 		true,
 	},
+	[0x14] = {
+		"Compute Module 4",
+		DTB_DIR "bcm2711-rpi-cm4.dtb",
+		true,
+	},
 };
 
 static const struct rpi_model rpi_models_old_scheme[] = {
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 03/12] pci: pcie-brcmstb: Fix inbound window configurations
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
  2020-12-15 17:23 ` [PATCH v3 01/12] rpi: Add identifier for the new RPi400 Nicolas Saenz Julienne
  2020-12-15 17:23 ` [PATCH v3 02/12] rpi: Add identifier for the new CM4 Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-15 17:23 ` [PATCH v3 04/12] dm: Introduce xxx_get_dma_range() Nicolas Saenz Julienne
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

So far we've assumed a fixed configuration for inbound windows as we had
a single user for this controller. But the controller's DMA constraints
were improved starting with BCM2711's B1 revision of the SoC, notably
available in CM4 and Pi400. They allow for wider inbound windows. We can
now cover the whole address space, whereas before we where limited to
the lower 3GB.

This information is passed to us through DT's 'dma-ranges' property and
it's specially important for us to honor it since some interactions with
the board's co-processor assume we're doing so (specifically the XHCI
firmware load operation, which is handled by the co-processor after
u-boot has correctly configured the PCIe controller).

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 drivers/pci/pcie_brcmstb.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/pcie_brcmstb.c b/drivers/pci/pcie_brcmstb.c
index dade79e9c8..f6e8ad0d0a 100644
--- a/drivers/pci/pcie_brcmstb.c
+++ b/drivers/pci/pcie_brcmstb.c
@@ -432,6 +432,7 @@ static int brcm_pcie_probe(struct udevice *dev)
 	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
 	struct brcm_pcie *pcie = dev_get_priv(dev);
 	void __iomem *base = pcie->base;
+	struct pci_region region;
 	bool ssc_good = false;
 	int num_out_wins = 0;
 	u64 rc_bar2_offset, rc_bar2_size;
@@ -468,13 +469,10 @@ static int brcm_pcie_probe(struct udevice *dev)
 			MISC_CTRL_SCB_ACCESS_EN_MASK |
 			MISC_CTRL_CFG_READ_UR_MODE_MASK |
 			MISC_CTRL_MAX_BURST_SIZE_128);
-	/*
-	 * TODO: When support for other SoCs than BCM2711 is added we may
-	 * need to use the base address and size(s) provided in the dma-ranges
-	 * property.
-	 */
-	rc_bar2_offset = 0;
-	rc_bar2_size = 0xc0000000;
+
+	pci_get_dma_regions(dev, &region, 0);
+	rc_bar2_offset = region.bus_start - region.phys_start;
+	rc_bar2_size = 1ULL << fls64(region.size - 1);
 
 	tmp = lower_32_bits(rc_bar2_offset);
 	u32p_replace_bits(&tmp, brcm_pcie_encode_ibar_size(rc_bar2_size),
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 04/12] dm: Introduce xxx_get_dma_range()
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
                   ` (2 preceding siblings ...)
  2020-12-15 17:23 ` [PATCH v3 03/12] pci: pcie-brcmstb: Fix inbound window configurations Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-19  2:28   ` Simon Glass
  2020-12-15 17:23 ` [PATCH v3 05/12] dm: test: Add test case for dev_get_dma_ranges() Nicolas Saenz Julienne
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

Add the following functions to get a specific device's DMA ranges:
 - dev_get_dma_range()
 - ofnode_get_dma_range()
 - of_get_dma_range()
 - fdt_get_dma_range()
They are specially useful in oder to be able validate a physical address
space range into a bus's and to convert addresses from and to address
spaces.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

---
Changes since v2:
 - Return ENOENT instead of ENODEV
 - Refcount OF nodes

Changes since v1:
 - Fix wrong arguments in of_get_dma_range()'s call to of_translate_dma_address()
 - Fix build in SPL/TPL and no LIBFDT supprt
 - Add missing declaration in 'core/read.c'
 - Address Matthias' comments

 common/fdt_support.c   | 73 +++++++++++++++++++++++++++++++++++++++
 drivers/core/of_addr.c | 78 ++++++++++++++++++++++++++++++++++++++++++
 drivers/core/ofnode.c  |  9 +++++
 drivers/core/read.c    |  6 ++++
 include/dm/of_addr.h   | 17 +++++++++
 include/dm/ofnode.h    | 16 +++++++++
 include/dm/read.h      | 21 ++++++++++++
 include/fdt_support.h  | 14 ++++++++
 8 files changed, 234 insertions(+)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 5ae75df3c6..bf855d26c8 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -1342,6 +1342,79 @@ u64 fdt_translate_dma_address(const void *blob, int node_offset,
 	return __of_translate_address(blob, node_offset, in_addr, "dma-ranges");
 }
 
+int fdt_get_dma_range(const void *blob, int node, phys_addr_t *cpu,
+		      dma_addr_t *bus, u64 *size)
+{
+	bool found_dma_ranges = false;
+	struct of_bus *bus_node;
+	const fdt32_t *ranges;
+	int na, ns, pna, pns;
+	int parent = node;
+	int ret = 0;
+	int len;
+
+	/* Find the closest dma-ranges property */
+	while (parent >= 0) {
+		ranges = fdt_getprop(blob, parent, "dma-ranges", &len);
+
+		/* Ignore empty ranges, they imply no translation required */
+		if (ranges && len > 0)
+			break;
+
+		/* Once we find 'dma-ranges', then a missing one is an error */
+		if (found_dma_ranges && !ranges) {
+			ret = -EINVAL;
+			goto out;
+		}
+
+		if (ranges)
+			found_dma_ranges = true;
+
+		parent = fdt_parent_offset(blob, parent);
+	}
+
+	if (!ranges || parent < 0) {
+		debug("no dma-ranges found for node %s\n",
+		      fdt_get_name(blob, node, NULL));
+		ret = -ENOENT;
+		goto out;
+	}
+
+	/* switch to that node */
+	node = parent;
+	parent = fdt_parent_offset(blob, node);
+	if (parent < 0) {
+		printf("Found dma-ranges in root node, shoudln't happen\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Get the address sizes both for the bus and its parent */
+	bus_node = of_match_bus(blob, node);
+	bus_node->count_cells(blob, node, &na, &ns);
+	if (!OF_CHECK_COUNTS(na, ns)) {
+		printf("%s: Bad cell count for %s\n", __FUNCTION__,
+		       fdt_get_name(blob, node, NULL));
+		return -EINVAL;
+		goto out;
+	}
+
+	bus_node = of_match_bus(blob, parent);
+	bus_node->count_cells(blob, parent, &pna, &pns);
+	if (!OF_CHECK_COUNTS(pna, pns)) {
+		printf("%s: Bad cell count for %s\n", __FUNCTION__,
+		       fdt_get_name(blob, parent, NULL));
+		return -EINVAL;
+		goto out;
+	}
+
+	*bus = fdt_read_number(ranges, na);
+	*cpu = fdt_translate_dma_address(blob, node, ranges + na);
+	*size = fdt_read_number(ranges + na + pna, ns);
+out:
+	return ret;
+}
+
 /**
  * fdt_node_offset_by_compat_reg: Find a node that matches compatiable and
  * who's reg property matches a physical cpu address
diff --git a/drivers/core/of_addr.c b/drivers/core/of_addr.c
index ca34d84922..703bc3e3f5 100644
--- a/drivers/core/of_addr.c
+++ b/drivers/core/of_addr.c
@@ -325,6 +325,84 @@ u64 of_translate_dma_address(const struct device_node *dev, const __be32 *in_add
 	return __of_translate_address(dev, in_addr, "dma-ranges");
 }
 
+int of_get_dma_range(const struct device_node *dev, phys_addr_t *cpu,
+		     dma_addr_t *bus, u64 *size)
+{
+	bool found_dma_ranges = false;
+	struct device_node *parent;
+	struct of_bus *bus_node;
+	int na, ns, pna, pns;
+	const __be32 *ranges;
+	int ret = 0;
+	int len;
+
+	/* Find the closest dma-ranges property */
+	dev = of_node_get(dev);
+	while (dev) {
+		ranges = of_get_property(dev, "dma-ranges", &len);
+
+		/* Ignore empty ranges, they imply no translation required */
+		if (ranges && len > 0)
+			break;
+
+		/* Once we find 'dma-ranges', then a missing one is an error */
+		if (found_dma_ranges && !ranges) {
+			ret = -EINVAL;
+			goto out;
+		}
+
+		if (ranges)
+			found_dma_ranges = true;
+
+		parent = of_get_parent(dev);
+		of_node_put(dev);
+		dev = parent;
+	}
+
+	if (!dev || !ranges) {
+		debug("no dma-ranges found for node %s\n",
+		      of_node_full_name(dev));
+		ret = -ENOENT;
+		goto out;
+	}
+
+	/* switch to that node */
+	parent = of_get_parent(dev);
+	if (!parent) {
+		printf("Found dma-ranges in root node, shoudln't happen\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Get the address sizes both for the bus and its parent */
+	bus_node = of_match_bus((struct device_node*)dev);
+	bus_node->count_cells(dev, &na, &ns);
+	if (!OF_CHECK_COUNTS(na, ns)) {
+		printf("Bad cell count for %s\n", of_node_full_name(dev));
+		return -EINVAL;
+		goto out_parent;
+	}
+
+	bus_node = of_match_bus(parent);
+	bus_node->count_cells(parent, &pna, &pns);
+	if (!OF_CHECK_COUNTS(pna, pns)) {
+		printf("Bad cell count for %s\n", of_node_full_name(parent));
+		return -EINVAL;
+		goto out_parent;
+	}
+
+	*bus = of_read_number(ranges, na);
+	*cpu = of_translate_dma_address(dev, ranges + na);
+	*size = of_read_number(ranges + na + pna, ns);
+
+out_parent:
+	of_node_put(parent);
+out:
+	of_node_put(dev);
+	return ret;
+}
+
+
 static int __of_address_to_resource(const struct device_node *dev,
 		const __be32 *addrp, u64 size, unsigned int flags,
 		const char *name, struct resource *r)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index a68076bf35..15470d4875 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -911,6 +911,15 @@ u64 ofnode_translate_dma_address(ofnode node, const fdt32_t *in_addr)
 		return fdt_translate_dma_address(gd->fdt_blob, ofnode_to_offset(node), in_addr);
 }
 
+int ofnode_get_dma_range(ofnode node, phys_addr_t *cpu, dma_addr_t *bus, u64 *size)
+{
+	if (ofnode_is_np(node))
+		return of_get_dma_range(ofnode_to_np(node), cpu, bus, size);
+	else
+		return fdt_get_dma_range(gd->fdt_blob, ofnode_to_offset(node),
+					 cpu, bus, size);
+}
+
 int ofnode_device_is_compatible(ofnode node, const char *compat)
 {
 	if (ofnode_is_np(node))
diff --git a/drivers/core/read.c b/drivers/core/read.c
index 076125824c..0994fbd979 100644
--- a/drivers/core/read.c
+++ b/drivers/core/read.c
@@ -338,6 +338,12 @@ u64 dev_translate_dma_address(const struct udevice *dev, const fdt32_t *in_addr)
 	return ofnode_translate_dma_address(dev_ofnode(dev), in_addr);
 }
 
+int dev_get_dma_range(const struct udevice *dev, phys_addr_t *cpu,
+		      dma_addr_t *bus, u64 *size)
+{
+	return ofnode_get_dma_range(dev_ofnode(dev), cpu, bus, size);
+}
+
 int dev_read_alias_highest_id(const char *stem)
 {
 	if (of_live_active())
diff --git a/include/dm/of_addr.h b/include/dm/of_addr.h
index 3fa1ffce81..ee21d5cf4f 100644
--- a/include/dm/of_addr.h
+++ b/include/dm/of_addr.h
@@ -44,6 +44,23 @@ u64 of_translate_address(const struct device_node *no, const __be32 *in_addr);
  */
 u64 of_translate_dma_address(const struct device_node *no, const __be32 *in_addr);
 
+
+/**
+ * of_get_dma_range() - get dma-ranges for a specific DT node
+ *
+ * Get DMA ranges for a specifc node, this is useful to perform bus->cpu and
+ * cpu->bus address translations
+ *
+ * @param blob		Pointer to device tree blob
+ * @param node_offset	Node DT offset
+ * @param cpu		Pointer to variable storing the range's cpu address
+ * @param bus		Pointer to variable storing the range's bus address
+ * @param size		Pointer to variable storing the range's size
+ * @return translated DMA address or OF_BAD_ADDR on error
+ */
+int of_get_dma_range(const struct device_node *dev, phys_addr_t *cpu,
+		     dma_addr_t *bus, u64 *size);
+
 /**
  * of_get_address() - obtain an address from a node
  *
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index ced7f6ffb2..dc3dd84d9f 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -939,6 +939,22 @@ u64 ofnode_translate_address(ofnode node, const fdt32_t *in_addr);
  */
 u64 ofnode_translate_dma_address(ofnode node, const fdt32_t *in_addr);
 
+/**
+ * ofnode_get_dma_range() - get dma-ranges for a specific DT node
+ *
+ * Get DMA ranges for a specifc node, this is useful to perform bus->cpu and
+ * cpu->bus address translations
+ *
+ * @param blob		Pointer to device tree blob
+ * @param node_offset	Node DT offset
+ * @param cpu		Pointer to variable storing the range's cpu address
+ * @param bus		Pointer to variable storing the range's bus address
+ * @param size		Pointer to variable storing the range's size
+ * @return translated DMA address or OF_BAD_ADDR on error
+ */
+int ofnode_get_dma_range(ofnode node, phys_addr_t *cpu, dma_addr_t *bus,
+			 u64 *size);
+
 /**
  * ofnode_device_is_compatible() - check if the node is compatible with compat
  *
diff --git a/include/dm/read.h b/include/dm/read.h
index 0585eb1228..46e902c57d 100644
--- a/include/dm/read.h
+++ b/include/dm/read.h
@@ -664,6 +664,21 @@ u64 dev_translate_address(const struct udevice *dev, const fdt32_t *in_addr);
 u64 dev_translate_dma_address(const struct udevice *dev,
 			      const fdt32_t *in_addr);
 
+/**
+ * dev_get_dma_range() - Get a device's DMA constraints
+ *
+ * Provide the address bases and size of the linear mapping between the CPU and
+ * a device's BUS address space.
+ *
+ * @dev: device giving the context in which to translate the DMA address
+ * @cpu: base address for CPU's view of memory
+ * @bus: base address for BUS's view of memory
+ * @size: size of the address space
+ * @return 0 if ok, negative on error
+ */
+int dev_get_dma_range(const struct udevice *dev, phys_addr_t *cpu,
+		      dma_addr_t *bus, u64 *size);
+
 /**
  * dev_read_alias_highest_id - Get highest alias id for the given stem
  * @stem:	Alias stem to be examined
@@ -1004,6 +1019,12 @@ static inline u64 dev_translate_dma_address(const struct udevice *dev,
 	return ofnode_translate_dma_address(dev_ofnode(dev), in_addr);
 }
 
+static inline int dev_get_dma_range(const struct udevice *dev, phys_addr_t *cpu,
+				    dma_addr_t *bus, u64 *size)
+{
+	return ofnode_get_dma_range(dev_ofnode(dev), cpu, bus, size);
+}
+
 static inline int dev_read_alias_highest_id(const char *stem)
 {
 	if (!CONFIG_IS_ENABLED(OF_LIBFDT))
diff --git a/include/fdt_support.h b/include/fdt_support.h
index dbbac0fb6a..46eb1dbbb2 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -260,6 +260,20 @@ u64 fdt_translate_address(const void *blob, int node_offset,
 u64 fdt_translate_dma_address(const void *blob, int node_offset,
 			      const __be32 *in_addr);
 
+/**
+ * Get DMA ranges for a specifc node, this is useful to perform bus->cpu and
+ * cpu->bus address translations
+ *
+ * @param blob		Pointer to device tree blob
+ * @param node_offset	Node DT offset
+ * @param cpu		Pointer to variable storing the range's cpu address
+ * @param bus		Pointer to variable storing the range's bus address
+ * @param size		Pointer to variable storing the range's size
+ * @return translated DMA address or OF_BAD_ADDR on error
+ */
+int fdt_get_dma_range(const void *blob, int node_offset, phys_addr_t *cpu,
+		      dma_addr_t *bus, u64 *size);
+
 int fdt_node_offset_by_compat_reg(void *blob, const char *compat,
 					phys_addr_t compat_off);
 int fdt_alloc_phandle(void *blob);
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 05/12] dm: test: Add test case for dev_get_dma_ranges()
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
                   ` (3 preceding siblings ...)
  2020-12-15 17:23 ` [PATCH v3 04/12] dm: Introduce xxx_get_dma_range() Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-19  2:28   ` Simon Glass
  2020-12-15 17:23 ` [PATCH v3 06/12] dm: Introduce DMA constraints into the core device model Nicolas Saenz Julienne
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

Introduce some new nodes in sandbox's test device-tree and dm tests in
order to validate dev_get_dma_range().

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 arch/sandbox/dts/test.dts | 17 ++++++++++++++
 test/dm/Makefile          |  1 +
 test/dm/read.c            | 49 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)
 create mode 100644 test/dm/read.c

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index f3b766271d..3a3b51f83b 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -292,6 +292,23 @@
 		test5-gpios = <&gpio_a 19>;
 	};
 
+	mmio-bus at 0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "denx,u-boot-test-bus";
+		dma-ranges = <0x10000000 0x00000000 0x00040000>;
+
+		subnode at 0 {
+			compatible = "denx,u-boot-fdt-test";
+		};
+	};
+
+	mmio-bus at 1 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "denx,u-boot-test-bus";
+	};
+
 	acpi_test1: acpi-test {
 		compatible = "denx,u-boot-acpi-test";
 		acpi-ssdt-test-data = "ab";
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 46e076ed09..5c52d8b6ea 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_UT_DM) += test-fdt.o
 obj-$(CONFIG_UT_DM) += test-uclass.o
 
 obj-$(CONFIG_UT_DM) += core.o
+obj-$(CONFIG_UT_DM) += read.o
 ifneq ($(CONFIG_SANDBOX),)
 obj-$(CONFIG_ACPIGEN) += acpi.o
 obj-$(CONFIG_ACPIGEN) += acpigen.o
diff --git a/test/dm/read.c b/test/dm/read.c
new file mode 100644
index 0000000000..090eee0544
--- /dev/null
+++ b/test/dm/read.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020 Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <dm/device.h>
+#include <dm/ofnode.h>
+#include <dm/root.h>
+#include <dm/test.h>
+#include <dm/uclass-internal.h>
+#include <test/ut.h>
+
+static int dm_test_dma_ranges(struct unit_test_state *uts)
+{
+	struct udevice *dev;
+	phys_addr_t cpu;
+	dma_addr_t bus;
+	ofnode node;
+	u64 size;
+
+	/* dma-ranges are on the device's node */
+	node = ofnode_path("/mmio-bus at 0");
+	ut_assert(ofnode_valid(node));
+	ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_BUS, node, &dev));
+	ut_assert(!dev_get_dma_range(dev, &cpu, &bus, &size));
+	ut_asserteq_64(0x40000, size);
+	ut_asserteq_64(0x0, cpu);
+	ut_asserteq_64(0x10000000, bus);
+
+	/* dma-ranges are on the bus' node */
+	node = ofnode_path("/mmio-bus at 0/subnode at 0");
+	ut_assert(ofnode_valid(node));
+	ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_FDT, node, &dev));
+	ut_assert(!dev_get_dma_range(dev, &cpu, &bus, &size));
+	ut_asserteq_64(0x40000, size);
+	ut_asserteq_64(0x0, cpu);
+	ut_asserteq_64(0x10000000, bus);
+
+	/* No dma-ranges available */
+	node = ofnode_path("/mmio-bus at 1");
+	ut_assert(ofnode_valid(node));
+	ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_BUS, node, &dev));
+	ut_asserteq(-ENOENT, dev_get_dma_range(dev, &cpu, &bus, &size));
+
+	return 0;
+}
+DM_TEST(dm_test_dma_ranges, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 06/12] dm: Introduce DMA constraints into the core device model
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
                   ` (4 preceding siblings ...)
  2020-12-15 17:23 ` [PATCH v3 05/12] dm: test: Add test case for dev_get_dma_ranges() Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-19  2:28   ` Simon Glass
  2020-12-15 17:23 ` [PATCH v3 07/12] dm: test: Add test case for dev->dma_offset Nicolas Saenz Julienne
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

Calculating the DMA offset between a bus address space and CPU's every
time we call phys_to_bus() and bus_to_phys() isn't ideal performance
wise, as it implies traversing the device tree from the device's node up
to the root. Since this information is static and available before the
device's initialization, parse it before the probe call an provide the
DMA offset in 'struct udevice' for the address translation code to use
it.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

---
Changes since v2:
 - Return/Fail on error
 - Add config option
 - use ulong instead for u64 for dev->dma_offset

Changes since v1:
 - Update commit message so as to explain better the reasoning behind
   this

 drivers/core/Kconfig  | 10 ++++++++++
 drivers/core/device.c | 35 +++++++++++++++++++++++++++++++++++
 include/dm/device.h   |  3 +++
 3 files changed, 48 insertions(+)

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index ffae6f9795..153be25351 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -113,6 +113,16 @@ config SPL_DM_SEQ_ALIAS
 	  numbered devices (e.g. serial0 = &serial0). This feature can be
 	  disabled if it is not required, to save code space in SPL.
 
+config DM_DMA
+	bool "Support per device DMA constraints"
+	depends on DM
+	default n
+	help
+	  Enable this to extract per-device DMA constraints, only supported on
+	  device-tree systems for now. This is needed in order translate
+	  addresses on systems where different buses have different views of
+	  the physical address space.
+
 config REGMAP
 	bool "Support register maps"
 	depends on DM
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 4b3dcb3b37..21285432d8 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -421,6 +421,37 @@ fail:
 	return ret;
 }
 
+#if CONFIG_IS_ENABLED(DM_DMA)
+static int device_get_dma_constraints(struct udevice *dev)
+{
+	struct udevice *parent = dev->parent;
+	phys_addr_t cpu = 0;
+	dma_addr_t bus = 0;
+	u64 size = 0;
+	int ret;
+
+	if (!parent || !dev_of_valid(parent))
+		return 0;
+
+	/*
+	 * We start parsing for dma-ranges from the device's bus node. This is
+	 * specially important on nested buses.
+	 */
+	ret = dev_get_dma_range(parent, &cpu, &bus, &size);
+	/* Don't return an error if no 'dma-ranges' were found */
+	if (ret && ret != -ENOENT) {
+		dm_warn("%s: failed to get DMA range, %d\n", dev->name, ret);
+		return ret;
+	}
+
+	dev->dma_offset = cpu - bus;
+
+	return 0;
+}
+#else
+static int device_get_dma_constraints(struct udevice *dev) { return 0; }
+#endif
+
 int device_probe(struct udevice *dev)
 {
 	const struct driver *drv;
@@ -482,6 +513,10 @@ int device_probe(struct udevice *dev)
 			goto fail;
 	}
 
+	ret = device_get_dma_constraints(dev);
+	if (ret)
+		goto fail;
+
 	ret = uclass_pre_probe_device(dev);
 	if (ret)
 		goto fail;
diff --git a/include/dm/device.h b/include/dm/device.h
index 5bef484247..9126dc00fe 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -161,6 +161,9 @@ struct udevice {
 #ifdef CONFIG_DEVRES
 	struct list_head devres_head;
 #endif
+#ifdef CONFIG_DM_DMA
+	ulong dma_offset;
+#endif
 };
 
 /* Maximum sequence number supported */
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 07/12] dm: test: Add test case for dev->dma_offset
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
                   ` (5 preceding siblings ...)
  2020-12-15 17:23 ` [PATCH v3 06/12] dm: Introduce DMA constraints into the core device model Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-19  2:28   ` Simon Glass
  2020-12-15 17:23 ` [PATCH v3 08/12] dm: Introduce dev_phys_to_bus()/dev_bus_to_phys() Nicolas Saenz Julienne
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

Add test to validate dev->dma_offset is properly set on devices.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 arch/sandbox/dts/test.dts          |  4 ++++
 configs/sandbox64_defconfig        |  1 +
 configs/sandbox_defconfig          |  1 +
 configs/sandbox_flattree_defconfig |  1 +
 configs/sandbox_spl_defconfig      |  1 +
 test/dm/core.c                     | 30 ++++++++++++++++++++++++++++++
 6 files changed, 38 insertions(+)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 3a3b51f83b..78f0100282 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -307,6 +307,10 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		compatible = "denx,u-boot-test-bus";
+
+		subnode at 0 {
+			compatible = "denx,u-boot-fdt-test";
+		};
 	};
 
 	acpi_test1: acpi-test {
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 5fbbfd7236..2c189a8a00 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -94,6 +94,7 @@ CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
 CONFIG_BOOTP_SEND_HOSTNAME=y
 CONFIG_NETCONSOLE=y
 CONFIG_IP_DEFRAG=y
+CONFIG_DM_DMA=y
 CONFIG_REGMAP=y
 CONFIG_SYSCON=y
 CONFIG_DEVRES=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index f1ec701a9f..ac97c16cef 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -108,6 +108,7 @@ CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
 CONFIG_BOOTP_SEND_HOSTNAME=y
 CONFIG_NETCONSOLE=y
 CONFIG_IP_DEFRAG=y
+CONFIG_DM_DMA=y
 CONFIG_REGMAP=y
 CONFIG_SYSCON=y
 CONFIG_DEVRES=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index edca7f1808..f5923bf713 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -75,6 +75,7 @@ CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
 CONFIG_BOOTP_SEND_HOSTNAME=y
 CONFIG_NETCONSOLE=y
 CONFIG_IP_DEFRAG=y
+CONFIG_DM_DMA=y
 CONFIG_REGMAP=y
 CONFIG_SYSCON=y
 CONFIG_DEVRES=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index 479f0226e3..0a4815770c 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -94,6 +94,7 @@ CONFIG_BOOTP_SEND_HOSTNAME=y
 CONFIG_NETCONSOLE=y
 CONFIG_IP_DEFRAG=y
 CONFIG_SPL_DM=y
+CONFIG_DM_DMA=y
 CONFIG_REGMAP=y
 CONFIG_SPL_REGMAP=y
 CONFIG_SYSCON=y
diff --git a/test/dm/core.c b/test/dm/core.c
index 6f380a574c..998874ffee 100644
--- a/test/dm/core.c
+++ b/test/dm/core.c
@@ -1066,3 +1066,33 @@ static int dm_test_inactive_child(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_inactive_child, UT_TESTF_SCAN_PDATA);
+
+static int dm_test_dma_offset(struct unit_test_state *uts)
+{
+       struct udevice *dev;
+       ofnode node;
+
+       /* Make sure the bus's dma-ranges aren't taken into account here */
+       node = ofnode_path("/mmio-bus at 0");
+       ut_assert(ofnode_valid(node));
+       ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_BUS, node, &dev));
+       ut_asserteq_64(0, dev->dma_offset);
+
+       /* Device behind a bus with dma-ranges */
+       node = ofnode_path("/mmio-bus at 0/subnode at 0");
+       ut_assert(ofnode_valid(node));
+       ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_FDT, node, &dev));
+       ut_asserteq_64(-0x10000000ULL, dev->dma_offset);
+
+       /* This one has no dma-ranges */
+       node = ofnode_path("/mmio-bus at 1");
+       ut_assert(ofnode_valid(node));
+       ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_BUS, node, &dev));
+       node = ofnode_path("/mmio-bus at 1/subnode at 0");
+       ut_assert(ofnode_valid(node));
+       ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_FDT, node, &dev));
+       ut_asserteq_64(0, dev->dma_offset);
+
+       return 0;
+}
+DM_TEST(dm_test_dma_offset, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 08/12] dm: Introduce dev_phys_to_bus()/dev_bus_to_phys()
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
                   ` (6 preceding siblings ...)
  2020-12-15 17:23 ` [PATCH v3 07/12] dm: test: Add test case for dev->dma_offset Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-15 17:23 ` [PATCH v3 09/12] dm: test: Add test case for dev_phys_to_bus()/dev_bus_to_phys() Nicolas Saenz Julienne
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

These functions, instead of relying on hard-coded platform-specific
address translations, make use of the DMA constraints provided by the DM
core. This allows for per-device translations.

We can't yet get rid of the legacy phys_to_bus()/bus_to_phys()
implementations as some of its users are not integrated into the
device model.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Simon Glass <sjg@chromium.org>

---
Changes since v2:
 - Use CONFIG_DM_DMA

 include/phys2bus.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/phys2bus.h b/include/phys2bus.h
index dc9b8e5a25..516d630aea 100644
--- a/include/phys2bus.h
+++ b/include/phys2bus.h
@@ -21,4 +21,20 @@ static inline unsigned long bus_to_phys(unsigned long bus)
 }
 #endif
 
+#if CONFIG_IS_ENABLED(DM_DMA)
+#include <dm/device.h>
+
+static inline dma_addr_t dev_phys_to_bus(struct udevice *dev,
+					 phys_addr_t phys)
+{
+	return phys - dev->dma_offset;
+}
+
+static inline phys_addr_t dev_bus_to_phys(struct udevice *dev,
+					  dma_addr_t bus)
+{
+	return bus + dev->dma_offset;
+}
+#endif
+
 #endif
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 09/12] dm: test: Add test case for dev_phys_to_bus()/dev_bus_to_phys()
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
                   ` (7 preceding siblings ...)
  2020-12-15 17:23 ` [PATCH v3 08/12] dm: Introduce dev_phys_to_bus()/dev_bus_to_phys() Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-19  2:28   ` Simon Glass
  2020-12-15 17:23 ` [PATCH v3 10/12] xhci: translate virtual addresses into the bus's address space Nicolas Saenz Julienne
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

By reusing DT nodes already available in sandbox's test DT introduce a
test to validate dev_phys_to_bus()/dev_bus_to_phys().

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 test/dm/Makefile   |  1 +
 test/dm/phys2bus.c | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)
 create mode 100644 test/dm/phys2bus.c

diff --git a/test/dm/Makefile b/test/dm/Makefile
index 5c52d8b6ea..ac86abaa88 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_UT_DM) += test-uclass.o
 
 obj-$(CONFIG_UT_DM) += core.o
 obj-$(CONFIG_UT_DM) += read.o
+obj-$(CONFIG_UT_DM) += phys2bus.o
 ifneq ($(CONFIG_SANDBOX),)
 obj-$(CONFIG_ACPIGEN) += acpi.o
 obj-$(CONFIG_ACPIGEN) += acpigen.o
diff --git a/test/dm/phys2bus.c b/test/dm/phys2bus.c
new file mode 100644
index 0000000000..06a045c168
--- /dev/null
+++ b/test/dm/phys2bus.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020 Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <phys2bus.h>
+#include <dm/device.h>
+#include <dm/ofnode.h>
+#include <dm/root.h>
+#include <dm/test.h>
+#include <dm/uclass-internal.h>
+#include <test/ut.h>
+
+static int dm_test_phys_to_bus(struct unit_test_state *uts)
+{
+	struct udevice *dev;
+	ofnode node;
+
+	node = ofnode_path("/mmio-bus at 0");
+	ut_assert(ofnode_valid(node));
+	ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_BUS, node, &dev));
+	/* In this case it should be transparent, no dma-ranges in parent bus */
+	ut_asserteq_ptr((void*)0xfffffULL, (void*)dev_phys_to_bus(dev, 0xfffff));
+	ut_asserteq_ptr((void*)0xfffffULL, (void*)(ulong)dev_bus_to_phys(dev, 0xfffff));
+
+	node = ofnode_path("/mmio-bus at 0/subnode at 0");
+	ut_assert(ofnode_valid(node));
+	ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_FDT, node, &dev));
+	ut_asserteq_ptr((void*)0x100fffffULL, (void*)dev_phys_to_bus(dev, 0xfffff));
+	ut_asserteq_ptr((void*)0xfffffULL, (void*)(ulong)dev_bus_to_phys(dev, 0x100fffff));
+
+	return 0;
+}
+DM_TEST(dm_test_phys_to_bus, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 10/12] xhci: translate virtual addresses into the bus's address space
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
                   ` (8 preceding siblings ...)
  2020-12-15 17:23 ` [PATCH v3 09/12] dm: test: Add test case for dev_phys_to_bus()/dev_bus_to_phys() Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-19  2:28   ` Simon Glass
  2020-12-15 17:23 ` [PATCH v3 11/12] mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys() Nicolas Saenz Julienne
  2020-12-15 17:23 ` [PATCH v3 12/12] configs: rpi4: Enable DM_DMA across all RPi4 configurations Nicolas Saenz Julienne
  11 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

So far we've been content with passing physical addresses when
configuring memory addresses into XHCI controllers, but not all
platforms have buses with transparent mappings. Specifically the
Raspberry Pi 4 might introduce an offset to memory accesses incoming
from its PCIe port.

Introduce xhci_virt_to_bus() and xhci_bus_to_virt() to cater with these
limitations, and make sure we don't break non DM users.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 drivers/usb/host/xhci-mem.c  | 45 +++++++++++++++++++-----------------
 drivers/usb/host/xhci-ring.c | 11 +++++----
 drivers/usb/host/xhci.c      |  4 ++--
 include/usb/xhci.h           | 22 +++++++++++++++++-
 4 files changed, 54 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index b002d6f166..83147d51b5 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -110,7 +110,7 @@ static void xhci_scratchpad_free(struct xhci_ctrl *ctrl)
 
 	ctrl->dcbaa->dev_context_ptrs[0] = 0;
 
-	free((void *)(uintptr_t)le64_to_cpu(ctrl->scratchpad->sp_array[0]));
+	free(xhci_bus_to_virt(ctrl, le64_to_cpu(ctrl->scratchpad->sp_array[0])));
 	free(ctrl->scratchpad->sp_array);
 	free(ctrl->scratchpad);
 	ctrl->scratchpad = NULL;
@@ -216,8 +216,8 @@ static void *xhci_malloc(unsigned int size)
  * @param link_trbs	flag to indicate whether to link the trbs or NOT
  * @return none
  */
-static void xhci_link_segments(struct xhci_segment *prev,
-				struct xhci_segment *next, bool link_trbs)
+static void xhci_link_segments(struct xhci_ctrl *ctrl, struct xhci_segment *prev,
+			       struct xhci_segment *next, bool link_trbs)
 {
 	u32 val;
 	u64 val_64 = 0;
@@ -226,7 +226,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
 		return;
 	prev->next = next;
 	if (link_trbs) {
-		val_64 = virt_to_phys(next->trbs);
+		val_64 = xhci_virt_to_bus(ctrl, next->trbs);
 		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
 			cpu_to_le64(val_64);
 
@@ -304,7 +304,8 @@ static struct xhci_segment *xhci_segment_alloc(void)
  * @param link_trbs	flag to indicate whether to link the trbs or NOT
  * @return pointer to the newly created RING
  */
-struct xhci_ring *xhci_ring_alloc(unsigned int num_segs, bool link_trbs)
+struct xhci_ring *xhci_ring_alloc(struct xhci_ctrl *ctrl, unsigned int num_segs,
+				  bool link_trbs)
 {
 	struct xhci_ring *ring;
 	struct xhci_segment *prev;
@@ -327,12 +328,12 @@ struct xhci_ring *xhci_ring_alloc(unsigned int num_segs, bool link_trbs)
 		next = xhci_segment_alloc();
 		BUG_ON(!next);
 
-		xhci_link_segments(prev, next, link_trbs);
+		xhci_link_segments(ctrl, prev, next, link_trbs);
 
 		prev = next;
 		num_segs--;
 	}
-	xhci_link_segments(prev, ring->first_seg, link_trbs);
+	xhci_link_segments(ctrl, prev, ring->first_seg, link_trbs);
 	if (link_trbs) {
 		/* See section 4.9.2.1 and 6.4.4.1 */
 		prev->trbs[TRBS_PER_SEGMENT-1].link.control |=
@@ -354,6 +355,7 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
 	struct xhci_hccr *hccr = ctrl->hccr;
 	struct xhci_hcor *hcor = ctrl->hcor;
 	struct xhci_scratchpad *scratchpad;
+	uint64_t val_64;
 	int num_sp;
 	uint32_t page_size;
 	void *buf;
@@ -371,8 +373,9 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
 	scratchpad->sp_array = xhci_malloc(num_sp * sizeof(u64));
 	if (!scratchpad->sp_array)
 		goto fail_sp2;
-	ctrl->dcbaa->dev_context_ptrs[0] =
-		cpu_to_le64((uintptr_t)scratchpad->sp_array);
+
+	val_64 = xhci_virt_to_bus(ctrl, scratchpad->sp_array);
+	ctrl->dcbaa->dev_context_ptrs[0] = cpu_to_le64(val_64);
 
 	xhci_flush_cache((uintptr_t)&ctrl->dcbaa->dev_context_ptrs[0],
 		sizeof(ctrl->dcbaa->dev_context_ptrs[0]));
@@ -393,8 +396,8 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
 	xhci_flush_cache((uintptr_t)buf, num_sp * page_size);
 
 	for (i = 0; i < num_sp; i++) {
-		uintptr_t ptr = (uintptr_t)buf + i * page_size;
-		scratchpad->sp_array[i] = cpu_to_le64(ptr);
+		val_64 = xhci_virt_to_bus(ctrl, buf + i * page_size);
+		scratchpad->sp_array[i] = cpu_to_le64(val_64);
 	}
 
 	xhci_flush_cache((uintptr_t)scratchpad->sp_array,
@@ -484,9 +487,9 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
 	}
 
 	/* Allocate endpoint 0 ring */
-	virt_dev->eps[0].ring = xhci_ring_alloc(1, true);
+	virt_dev->eps[0].ring = xhci_ring_alloc(ctrl, 1, true);
 
-	byte_64 = virt_to_phys(virt_dev->out_ctx->bytes);
+	byte_64 = xhci_virt_to_bus(ctrl, virt_dev->out_ctx->bytes);
 
 	/* Point to output device context in dcbaa. */
 	ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
@@ -522,15 +525,15 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 		return -ENOMEM;
 	}
 
-	val_64 = virt_to_phys(ctrl->dcbaa);
+	val_64 = xhci_virt_to_bus(ctrl, ctrl->dcbaa);
 	/* Set the pointer in DCBAA register */
 	xhci_writeq(&hcor->or_dcbaap, val_64);
 
 	/* Command ring control pointer register initialization */
-	ctrl->cmd_ring = xhci_ring_alloc(1, true);
+	ctrl->cmd_ring = xhci_ring_alloc(ctrl, 1, true);
 
 	/* Set the address in the Command Ring Control register */
-	trb_64 = virt_to_phys(ctrl->cmd_ring->first_seg->trbs);
+	trb_64 = xhci_virt_to_bus(ctrl, ctrl->cmd_ring->first_seg->trbs);
 	val_64 = xhci_readq(&hcor->or_crcr);
 	val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
 		(trb_64 & (u64) ~CMD_RING_RSVD_BITS) |
@@ -551,7 +554,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 	ctrl->ir_set = &ctrl->run_regs->ir_set[0];
 
 	/* Event ring does not maintain link TRB */
-	ctrl->event_ring = xhci_ring_alloc(ERST_NUM_SEGS, false);
+	ctrl->event_ring = xhci_ring_alloc(ctrl, ERST_NUM_SEGS, false);
 	ctrl->erst.entries = xhci_malloc(sizeof(struct xhci_erst_entry) *
 					 ERST_NUM_SEGS);
 
@@ -560,8 +563,8 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 	for (val = 0, seg = ctrl->event_ring->first_seg;
 			val < ERST_NUM_SEGS;
 			val++) {
-		trb_64 = virt_to_phys(seg->trbs);
 		struct xhci_erst_entry *entry = &ctrl->erst.entries[val];
+		trb_64 = xhci_virt_to_bus(ctrl, seg->trbs);
 		entry->seg_addr = cpu_to_le64(trb_64);
 		entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
 		entry->rsvd = 0;
@@ -570,7 +573,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 	xhci_flush_cache((uintptr_t)ctrl->erst.entries,
 			 ERST_NUM_SEGS * sizeof(struct xhci_erst_entry));
 
-	deq = virt_to_phys(ctrl->event_ring->dequeue);
+	deq = xhci_virt_to_bus(ctrl, ctrl->event_ring->dequeue);
 
 	/* Update HC event ring dequeue pointer */
 	xhci_writeq(&ctrl->ir_set->erst_dequeue,
@@ -585,7 +588,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 	/* this is the event ring segment table pointer */
 	val_64 = xhci_readq(&ctrl->ir_set->erst_base);
 	val_64 &= ERST_PTR_MASK;
-	val_64 |= virt_to_phys(ctrl->erst.entries) & ~ERST_PTR_MASK;
+	val_64 |= xhci_virt_to_bus(ctrl, ctrl->erst.entries) & ~ERST_PTR_MASK;
 
 	xhci_writeq(&ctrl->ir_set->erst_base, val_64);
 
@@ -848,7 +851,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 	/* EP 0 can handle "burst" sizes of 1, so Max Burst Size field is 0 */
 	ep0_ctx->ep_info2 |= cpu_to_le32(MAX_BURST(0) | ERROR_COUNT(3));
 
-	trb_64 = virt_to_phys(virt_dev->eps[0].ring->first_seg->trbs);
+	trb_64 = xhci_virt_to_bus(ctrl, virt_dev->eps[0].ring->first_seg->trbs);
 	ep0_ctx->deq = cpu_to_le64(trb_64 | virt_dev->eps[0].ring->cycle_state);
 
 	/*
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 13065d7ca9..5a6ad383cd 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -275,10 +275,13 @@ void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, u32 slot_id,
 			u32 ep_index, trb_type cmd)
 {
 	u32 fields[4];
-	u64 val_64 = virt_to_phys(ptr);
+	u64 val_64 = 0;
 
 	BUG_ON(prepare_ring(ctrl, ctrl->cmd_ring, EP_STATE_RUNNING));
 
+	if (ptr)
+		val_64 = xhci_virt_to_bus(ctrl, ptr);
+
 	fields[0] = lower_32_bits(val_64);
 	fields[1] = upper_32_bits(val_64);
 	fields[2] = 0;
@@ -401,7 +404,7 @@ void xhci_acknowledge_event(struct xhci_ctrl *ctrl)
 
 	/* Inform the hardware */
 	xhci_writeq(&ctrl->ir_set->erst_dequeue,
-		    virt_to_phys(ctrl->event_ring->dequeue) | ERST_EHB);
+		    xhci_virt_to_bus(ctrl, ctrl->event_ring->dequeue) | ERST_EHB);
 }
 
 /**
@@ -579,7 +582,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 	u64 addr;
 	int ret;
 	u32 trb_fields[4];
-	u64 val_64 = virt_to_phys(buffer);
+	u64 val_64 = xhci_virt_to_bus(ctrl, buffer);
 
 	debug("dev=%p, pipe=%lx, buffer=%p, length=%d\n",
 		udev, pipe, buffer, length);
@@ -872,7 +875,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	if (length > 0) {
 		if (req->requesttype & USB_DIR_IN)
 			field |= TRB_DIR_IN;
-		buf_64 = virt_to_phys(buffer);
+		buf_64 = xhci_virt_to_bus(ctrl, buffer);
 
 		trb_fields[0] = lower_32_bits(buf_64);
 		trb_fields[1] = upper_32_bits(buf_64);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 7080f8fabe..d27ac01c83 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -604,7 +604,7 @@ static int xhci_set_configuration(struct usb_device *udev)
 		ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
 
 		/* Allocate the ep rings */
-		virt_dev->eps[ep_index].ring = xhci_ring_alloc(1, true);
+		virt_dev->eps[ep_index].ring = xhci_ring_alloc(ctrl, 1, true);
 		if (!virt_dev->eps[ep_index].ring)
 			return -ENOMEM;
 
@@ -628,7 +628,7 @@ static int xhci_set_configuration(struct usb_device *udev)
 			cpu_to_le32(MAX_BURST(max_burst) |
 			ERROR_COUNT(err_count));
 
-		trb_64 = virt_to_phys(virt_dev->eps[ep_index].ring->enqueue);
+		trb_64 = xhci_virt_to_bus(ctrl, virt_dev->eps[ep_index].ring->enqueue);
 		ep_ctx[ep_index]->deq = cpu_to_le64(trb_64 |
 				virt_dev->eps[ep_index].ring->cycle_state);
 
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index e1d382369a..b87210b9ba 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -16,6 +16,7 @@
 #ifndef HOST_XHCI_H_
 #define HOST_XHCI_H_
 
+#include <phys2bus.h>
 #include <reset.h>
 #include <asm/types.h>
 #include <asm/cache.h>
@@ -1250,7 +1251,8 @@ int xhci_check_maxpacket(struct usb_device *udev);
 void xhci_flush_cache(uintptr_t addr, u32 type_len);
 void xhci_inval_cache(uintptr_t addr, u32 type_len);
 void xhci_cleanup(struct xhci_ctrl *ctrl);
-struct xhci_ring *xhci_ring_alloc(unsigned int num_segs, bool link_trbs);
+struct xhci_ring *xhci_ring_alloc(struct xhci_ctrl *ctrl, unsigned int num_segs,
+				  bool link_trbs);
 int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id);
 int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 		  struct xhci_hcor *hcor);
@@ -1278,4 +1280,22 @@ extern struct dm_usb_ops xhci_usb_ops;
 
 struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev);
 
+static inline dma_addr_t xhci_virt_to_bus(struct xhci_ctrl *ctrl, void *addr)
+{
+#if CONFIG_IS_ENABLED(DM_DMA)
+	return dev_phys_to_bus(ctrl->dev, virt_to_phys(addr));
+#else
+	return phys_to_bus(virt_to_phys(addr));
+#endif
+}
+
+static inline void *xhci_bus_to_virt(struct xhci_ctrl *ctrl, dma_addr_t addr)
+{
+#if CONFIG_IS_ENABLED(DM_DMA)
+	return phys_to_virt(dev_bus_to_phys(ctrl->dev, addr));
+#else
+	return phys_to_virt(bus_to_phys(addr));
+#endif
+}
+
 #endif /* HOST_XHCI_H_ */
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 11/12] mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys()
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
                   ` (9 preceding siblings ...)
  2020-12-15 17:23 ` [PATCH v3 10/12] xhci: translate virtual addresses into the bus's address space Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-19  2:28   ` Simon Glass
  2020-12-15 17:23 ` [PATCH v3 12/12] configs: rpi4: Enable DM_DMA across all RPi4 configurations Nicolas Saenz Julienne
  11 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

This will allow us to use DM variants of phys_to_bus()/bus_to_phys()
when relevant.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 drivers/mmc/sdhci.c |  7 ++++---
 include/mmc.h       | 10 ++++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 0628934312..2086d7cdb1 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -19,7 +19,6 @@
 #include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
-#include <phys2bus.h>
 
 static void sdhci_reset(struct sdhci_host *host, u8 mask)
 {
@@ -103,7 +102,8 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
 					  mmc_get_dma_dir(data));
 
 	if (host->flags & USE_SDMA) {
-		sdhci_writel(host, phys_to_bus((ulong)host->start_addr),
+		sdhci_writel(host,
+			     mmc_phys_to_bus(host->mmc, (ulong)host->start_addr),
 				SDHCI_DMA_ADDRESS);
 	}
 #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
@@ -162,7 +162,8 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
 				start_addr &=
 				~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1);
 				start_addr += SDHCI_DEFAULT_BOUNDARY_SIZE;
-				sdhci_writel(host, phys_to_bus((ulong)start_addr),
+				sdhci_writel(host,
+					     mmc_phys_to_bus(host->mmc, (ulong)start_addr),
 					     SDHCI_DMA_ADDRESS);
 			}
 		}
diff --git a/include/mmc.h b/include/mmc.h
index 1d377e0281..5fe1ef1dfc 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -15,6 +15,7 @@
 #include <linux/compiler.h>
 #include <linux/dma-direction.h>
 #include <part.h>
+#include <phys2bus.h>
 
 struct bd_info;
 
@@ -977,4 +978,13 @@ static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
 	return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 }
 
+static inline dma_addr_t mmc_phys_to_bus(struct mmc *mmc, phys_addr_t addr)
+{
+#if CONFIG_IS_ENABLED(DM_DMA)
+	return dev_phys_to_bus(mmc->dev, addr);
+#else
+	return phys_to_bus(addr);
+#endif
+}
+
 #endif /* _MMC_H_ */
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 12/12] configs: rpi4: Enable DM_DMA across all RPi4 configurations
  2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
                   ` (10 preceding siblings ...)
  2020-12-15 17:23 ` [PATCH v3 11/12] mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys() Nicolas Saenz Julienne
@ 2020-12-15 17:23 ` Nicolas Saenz Julienne
  2020-12-19  2:28   ` Simon Glass
  11 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-15 17:23 UTC (permalink / raw)
  To: u-boot

The DM_DMA option is needed in order to translate physical address into
bus addresses on a per-device basis.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 configs/rpi_4_32b_defconfig | 1 +
 configs/rpi_4_defconfig     | 1 +
 configs/rpi_arm64_defconfig | 1 +
 3 files changed, 3 insertions(+)

diff --git a/configs/rpi_4_32b_defconfig b/configs/rpi_4_32b_defconfig
index 5ddd838fd5..0a5d3ff8cd 100644
--- a/configs/rpi_4_32b_defconfig
+++ b/configs/rpi_4_32b_defconfig
@@ -22,6 +22,7 @@ CONFIG_OF_BOARD=y
 CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
+CONFIG_DM_DMA=y
 CONFIG_DFU_MMC=y
 CONFIG_DM_KEYBOARD=y
 CONFIG_DM_MMC=y
diff --git a/configs/rpi_4_defconfig b/configs/rpi_4_defconfig
index 2590d0a696..b61cdc05f1 100644
--- a/configs/rpi_4_defconfig
+++ b/configs/rpi_4_defconfig
@@ -22,6 +22,7 @@ CONFIG_OF_BOARD=y
 CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
+CONFIG_DM_DMA=y
 CONFIG_DFU_MMC=y
 CONFIG_DM_KEYBOARD=y
 CONFIG_DM_MMC=y
diff --git a/configs/rpi_arm64_defconfig b/configs/rpi_arm64_defconfig
index 2639219a1a..4125a1feba 100644
--- a/configs/rpi_arm64_defconfig
+++ b/configs/rpi_arm64_defconfig
@@ -20,6 +20,7 @@ CONFIG_CMD_FS_UUID=y
 CONFIG_OF_BOARD=y
 CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
 CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
+CONFIG_DM_DMA=y
 CONFIG_DM_KEYBOARD=y
 CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 04/12] dm: Introduce xxx_get_dma_range()
  2020-12-15 17:23 ` [PATCH v3 04/12] dm: Introduce xxx_get_dma_range() Nicolas Saenz Julienne
@ 2020-12-19  2:28   ` Simon Glass
  2020-12-21 13:03     ` Nicolas Saenz Julienne
  0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2020-12-19  2:28 UTC (permalink / raw)
  To: u-boot

Hi Nicolas,

On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Add the following functions to get a specific device's DMA ranges:
>  - dev_get_dma_range()
>  - ofnode_get_dma_range()
>  - of_get_dma_range()
>  - fdt_get_dma_range()
> They are specially useful in oder to be able validate a physical address
> space range into a bus's and to convert addresses from and to address
> spaces.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>
> ---
> Changes since v2:
>  - Return ENOENT instead of ENODEV
>  - Refcount OF nodes
>
> Changes since v1:
>  - Fix wrong arguments in of_get_dma_range()'s call to of_translate_dma_address()
>  - Fix build in SPL/TPL and no LIBFDT supprt
>  - Add missing declaration in 'core/read.c'
>  - Address Matthias' comments
>
>  common/fdt_support.c   | 73 +++++++++++++++++++++++++++++++++++++++
>  drivers/core/of_addr.c | 78 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/core/ofnode.c  |  9 +++++
>  drivers/core/read.c    |  6 ++++
>  include/dm/of_addr.h   | 17 +++++++++
>  include/dm/ofnode.h    | 16 +++++++++
>  include/dm/read.h      | 21 ++++++++++++
>  include/fdt_support.h  | 14 ++++++++
>  8 files changed, 234 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

I don't suppose it is worth writing a version that uses the ofnode API
and thus reduce code size? Probably not since if livetree is enabled,
I doubt we would ever call the flattree one. Also it is nice to have
the same livetree code as Linux, where possible.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 05/12] dm: test: Add test case for dev_get_dma_ranges()
  2020-12-15 17:23 ` [PATCH v3 05/12] dm: test: Add test case for dev_get_dma_ranges() Nicolas Saenz Julienne
@ 2020-12-19  2:28   ` Simon Glass
  0 siblings, 0 replies; 31+ messages in thread
From: Simon Glass @ 2020-12-19  2:28 UTC (permalink / raw)
  To: u-boot

On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Introduce some new nodes in sandbox's test device-tree and dm tests in
> order to validate dev_get_dma_range().
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  arch/sandbox/dts/test.dts | 17 ++++++++++++++
>  test/dm/Makefile          |  1 +
>  test/dm/read.c            | 49 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+)
>  create mode 100644 test/dm/read.c
>

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 06/12] dm: Introduce DMA constraints into the core device model
  2020-12-15 17:23 ` [PATCH v3 06/12] dm: Introduce DMA constraints into the core device model Nicolas Saenz Julienne
@ 2020-12-19  2:28   ` Simon Glass
  0 siblings, 0 replies; 31+ messages in thread
From: Simon Glass @ 2020-12-19  2:28 UTC (permalink / raw)
  To: u-boot

Hi Nicolas,

On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Calculating the DMA offset between a bus address space and CPU's every
> time we call phys_to_bus() and bus_to_phys() isn't ideal performance
> wise, as it implies traversing the device tree from the device's node up
> to the root. Since this information is static and available before the
> device's initialization, parse it before the probe call an provide the
> DMA offset in 'struct udevice' for the address translation code to use
> it.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>
> ---
> Changes since v2:
>  - Return/Fail on error
>  - Add config option
>  - use ulong instead for u64 for dev->dma_offset
>
> Changes since v1:
>  - Update commit message so as to explain better the reasoning behind
>    this
>
>  drivers/core/Kconfig  | 10 ++++++++++
>  drivers/core/device.c | 35 +++++++++++++++++++++++++++++++++++
>  include/dm/device.h   |  3 +++
>  3 files changed, 48 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

nits below

>
> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> index ffae6f9795..153be25351 100644
> --- a/drivers/core/Kconfig
> +++ b/drivers/core/Kconfig
> @@ -113,6 +113,16 @@ config SPL_DM_SEQ_ALIAS
>           numbered devices (e.g. serial0 = &serial0). This feature can be
>           disabled if it is not required, to save code space in SPL.
>
> +config DM_DMA
> +       bool "Support per device DMA constraints"

per-device

> +       depends on DM
> +       default n
> +       help
> +         Enable this to extract per-device DMA constraints, only supported on
> +         device-tree systems for now. This is needed in order translate
> +         addresses on systems where different buses have different views of
> +         the physical address space.
> +
>  config REGMAP
>         bool "Support register maps"
>         depends on DM
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 4b3dcb3b37..21285432d8 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -421,6 +421,37 @@ fail:
>         return ret;
>  }
>
> +#if CONFIG_IS_ENABLED(DM_DMA)

I feel you could use

if (CONFIG_IS_ENABLED)

in the function below (returning at the start) and avoid an #ifdef?

To do that you'll need access macros - you can see things like that in
device.h and asm-generic/global_data.h

> +static int device_get_dma_constraints(struct udevice *dev)

Please add a comment to describe this function.

> +{
> +       struct udevice *parent = dev->parent;
> +       phys_addr_t cpu = 0;
> +       dma_addr_t bus = 0;
> +       u64 size = 0;
> +       int ret;
> +
> +       if (!parent || !dev_of_valid(parent))
> +               return 0;
> +
> +       /*
> +        * We start parsing for dma-ranges from the device's bus node. This is
> +        * specially important on nested buses.
> +        */
> +       ret = dev_get_dma_range(parent, &cpu, &bus, &size);
> +       /* Don't return an error if no 'dma-ranges' were found */
> +       if (ret && ret != -ENOENT) {
> +               dm_warn("%s: failed to get DMA range, %d\n", dev->name, ret);
> +               return ret;
> +       }
> +
> +       dev->dma_offset = cpu - bus;
> +
> +       return 0;
> +}
> +#else
> +static int device_get_dma_constraints(struct udevice *dev) { return 0; }
> +#endif
> +
>  int device_probe(struct udevice *dev)
>  {
>         const struct driver *drv;
> @@ -482,6 +513,10 @@ int device_probe(struct udevice *dev)
>                         goto fail;
>         }
>
> +       ret = device_get_dma_constraints(dev);
> +       if (ret)
> +               goto fail;
> +
>         ret = uclass_pre_probe_device(dev);
>         if (ret)
>                 goto fail;
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 5bef484247..9126dc00fe 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -161,6 +161,9 @@ struct udevice {
>  #ifdef CONFIG_DEVRES
>         struct list_head devres_head;
>  #endif
> +#ifdef CONFIG_DM_DMA
> +       ulong dma_offset;

Needs a comment

> +#endif
>  };
>
>  /* Maximum sequence number supported */
> --
> 2.29.2
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 07/12] dm: test: Add test case for dev->dma_offset
  2020-12-15 17:23 ` [PATCH v3 07/12] dm: test: Add test case for dev->dma_offset Nicolas Saenz Julienne
@ 2020-12-19  2:28   ` Simon Glass
  2020-12-21 13:36     ` Nicolas Saenz Julienne
  0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2020-12-19  2:28 UTC (permalink / raw)
  To: u-boot

On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Add test to validate dev->dma_offset is properly set on devices.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  arch/sandbox/dts/test.dts          |  4 ++++
>  configs/sandbox64_defconfig        |  1 +
>  configs/sandbox_defconfig          |  1 +
>  configs/sandbox_flattree_defconfig |  1 +
>  configs/sandbox_spl_defconfig      |  1 +
>  test/dm/core.c                     | 30 ++++++++++++++++++++++++++++++
>  6 files changed, 38 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

We normally use ut_assertok() instead of ut_assert(!xxx), since 0 means OK.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 09/12] dm: test: Add test case for dev_phys_to_bus()/dev_bus_to_phys()
  2020-12-15 17:23 ` [PATCH v3 09/12] dm: test: Add test case for dev_phys_to_bus()/dev_bus_to_phys() Nicolas Saenz Julienne
@ 2020-12-19  2:28   ` Simon Glass
  2020-12-21 13:47     ` Nicolas Saenz Julienne
  0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2020-12-19  2:28 UTC (permalink / raw)
  To: u-boot

On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> By reusing DT nodes already available in sandbox's test DT introduce a
> test to validate dev_phys_to_bus()/dev_bus_to_phys().
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  test/dm/Makefile   |  1 +
>  test/dm/phys2bus.c | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+)
>  create mode 100644 test/dm/phys2bus.c

Reviewed-by: Simon Glass <sjg@chromium.org>

comment below

>
> diff --git a/test/dm/Makefile b/test/dm/Makefile
> index 5c52d8b6ea..ac86abaa88 100644
> --- a/test/dm/Makefile
> +++ b/test/dm/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_UT_DM) += test-uclass.o
>
>  obj-$(CONFIG_UT_DM) += core.o
>  obj-$(CONFIG_UT_DM) += read.o
> +obj-$(CONFIG_UT_DM) += phys2bus.o
>  ifneq ($(CONFIG_SANDBOX),)
>  obj-$(CONFIG_ACPIGEN) += acpi.o
>  obj-$(CONFIG_ACPIGEN) += acpigen.o
> diff --git a/test/dm/phys2bus.c b/test/dm/phys2bus.c
> new file mode 100644
> index 0000000000..06a045c168
> --- /dev/null
> +++ b/test/dm/phys2bus.c
> @@ -0,0 +1,36 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2020 Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <phys2bus.h>
> +#include <dm/device.h>
> +#include <dm/ofnode.h>
> +#include <dm/root.h>
> +#include <dm/test.h>
> +#include <dm/uclass-internal.h>
> +#include <test/ut.h>
> +
> +static int dm_test_phys_to_bus(struct unit_test_state *uts)
> +{
> +       struct udevice *dev;
> +       ofnode node;
> +
> +       node = ofnode_path("/mmio-bus at 0");
> +       ut_assert(ofnode_valid(node));
> +       ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_BUS, node, &dev));
> +       /* In this case it should be transparent, no dma-ranges in parent bus */
> +       ut_asserteq_ptr((void*)0xfffffULL, (void*)dev_phys_to_bus(dev, 0xfffff));

ut_asserteq_addr() might be better here? Not sure.

> +       ut_asserteq_ptr((void*)0xfffffULL, (void*)(ulong)dev_bus_to_phys(dev, 0xfffff));
> +
> +       node = ofnode_path("/mmio-bus at 0/subnode at 0");
> +       ut_assert(ofnode_valid(node));
> +       ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_FDT, node, &dev));
> +       ut_asserteq_ptr((void*)0x100fffffULL, (void*)dev_phys_to_bus(dev, 0xfffff));
> +       ut_asserteq_ptr((void*)0xfffffULL, (void*)(ulong)dev_bus_to_phys(dev, 0x100fffff));
> +
> +       return 0;
> +}
> +DM_TEST(dm_test_phys_to_bus, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
> --
> 2.29.2
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 10/12] xhci: translate virtual addresses into the bus's address space
  2020-12-15 17:23 ` [PATCH v3 10/12] xhci: translate virtual addresses into the bus's address space Nicolas Saenz Julienne
@ 2020-12-19  2:28   ` Simon Glass
  2020-12-21 13:51     ` Nicolas Saenz Julienne
  0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2020-12-19  2:28 UTC (permalink / raw)
  To: u-boot

Hi Nicolas,

On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> So far we've been content with passing physical addresses when
> configuring memory addresses into XHCI controllers, but not all
> platforms have buses with transparent mappings. Specifically the
> Raspberry Pi 4 might introduce an offset to memory accesses incoming
> from its PCIe port.
>
> Introduce xhci_virt_to_bus() and xhci_bus_to_virt() to cater with these
> limitations, and make sure we don't break non DM users.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  drivers/usb/host/xhci-mem.c  | 45 +++++++++++++++++++-----------------
>  drivers/usb/host/xhci-ring.c | 11 +++++----
>  drivers/usb/host/xhci.c      |  4 ++--
>  include/usb/xhci.h           | 22 +++++++++++++++++-
>  4 files changed, 54 insertions(+), 28 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

nits below

[..]

> diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> index e1d382369a..b87210b9ba 100644
> --- a/include/usb/xhci.h
> +++ b/include/usb/xhci.h
> @@ -16,6 +16,7 @@
>  #ifndef HOST_XHCI_H_
>  #define HOST_XHCI_H_
>
> +#include <phys2bus.h>
>  #include <reset.h>
>  #include <asm/types.h>
>  #include <asm/cache.h>
> @@ -1250,7 +1251,8 @@ int xhci_check_maxpacket(struct usb_device *udev);
>  void xhci_flush_cache(uintptr_t addr, u32 type_len);
>  void xhci_inval_cache(uintptr_t addr, u32 type_len);
>  void xhci_cleanup(struct xhci_ctrl *ctrl);
> -struct xhci_ring *xhci_ring_alloc(unsigned int num_segs, bool link_trbs);
> +struct xhci_ring *xhci_ring_alloc(struct xhci_ctrl *ctrl, unsigned int num_segs,
> +                                 bool link_trbs);

Can you please add a comment while you are here?

>  int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id);
>  int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>                   struct xhci_hcor *hcor);
> @@ -1278,4 +1280,22 @@ extern struct dm_usb_ops xhci_usb_ops;
>
>  struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev);
>
> +static inline dma_addr_t xhci_virt_to_bus(struct xhci_ctrl *ctrl, void *addr)
> +{
> +#if CONFIG_IS_ENABLED(DM_DMA)

Can this use if() ?

> +       return dev_phys_to_bus(ctrl->dev, virt_to_phys(addr));
> +#else
> +       return phys_to_bus(virt_to_phys(addr));
> +#endif
> +}
> +
> +static inline void *xhci_bus_to_virt(struct xhci_ctrl *ctrl, dma_addr_t addr)
> +{
> +#if CONFIG_IS_ENABLED(DM_DMA)
> +       return phys_to_virt(dev_bus_to_phys(ctrl->dev, addr));
> +#else
> +       return phys_to_virt(bus_to_phys(addr));
> +#endif
> +}
> +
>  #endif /* HOST_XHCI_H_ */
> --
> 2.29.2
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 11/12] mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys()
  2020-12-15 17:23 ` [PATCH v3 11/12] mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys() Nicolas Saenz Julienne
@ 2020-12-19  2:28   ` Simon Glass
  2020-12-21 14:23     ` Nicolas Saenz Julienne
  0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2020-12-19  2:28 UTC (permalink / raw)
  To: u-boot

On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> This will allow us to use DM variants of phys_to_bus()/bus_to_phys()
> when relevant.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  drivers/mmc/sdhci.c |  7 ++++---
>  include/mmc.h       | 10 ++++++++++
>  2 files changed, 14 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>


>
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 0628934312..2086d7cdb1 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -19,7 +19,6 @@
>  #include <linux/bitops.h>
>  #include <linux/delay.h>
>  #include <linux/dma-mapping.h>
> -#include <phys2bus.h>
>
>  static void sdhci_reset(struct sdhci_host *host, u8 mask)
>  {
> @@ -103,7 +102,8 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
>                                           mmc_get_dma_dir(data));
>
>         if (host->flags & USE_SDMA) {
> -               sdhci_writel(host, phys_to_bus((ulong)host->start_addr),
> +               sdhci_writel(host,
> +                            mmc_phys_to_bus(host->mmc, (ulong)host->start_addr),
>                                 SDHCI_DMA_ADDRESS);
>         }
>  #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
> @@ -162,7 +162,8 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
>                                 start_addr &=
>                                 ~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1);
>                                 start_addr += SDHCI_DEFAULT_BOUNDARY_SIZE;
> -                               sdhci_writel(host, phys_to_bus((ulong)start_addr),
> +                               sdhci_writel(host,
> +                                            mmc_phys_to_bus(host->mmc, (ulong)start_addr),
>                                              SDHCI_DMA_ADDRESS);
>                         }
>                 }
> diff --git a/include/mmc.h b/include/mmc.h
> index 1d377e0281..5fe1ef1dfc 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -15,6 +15,7 @@
>  #include <linux/compiler.h>
>  #include <linux/dma-direction.h>
>  #include <part.h>
> +#include <phys2bus.h>
>
>  struct bd_info;
>
> @@ -977,4 +978,13 @@ static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
>         return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>  }
>
> +static inline dma_addr_t mmc_phys_to_bus(struct mmc *mmc, phys_addr_t addr)
> +{
> +#if CONFIG_IS_ENABLED(DM_DMA)

if() ?

Also makes me wonder if dev_phys_to_bus() should call phys_to_bus() if
DM_DMA is not enabled?

> +       return dev_phys_to_bus(mmc->dev, addr);
> +#else
> +       return phys_to_bus(addr);
> +#endif
> +}
> +
>  #endif /* _MMC_H_ */
> --
> 2.29.2
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 12/12] configs: rpi4: Enable DM_DMA across all RPi4 configurations
  2020-12-15 17:23 ` [PATCH v3 12/12] configs: rpi4: Enable DM_DMA across all RPi4 configurations Nicolas Saenz Julienne
@ 2020-12-19  2:28   ` Simon Glass
  0 siblings, 0 replies; 31+ messages in thread
From: Simon Glass @ 2020-12-19  2:28 UTC (permalink / raw)
  To: u-boot

On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> The DM_DMA option is needed in order to translate physical address into
> bus addresses on a per-device basis.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  configs/rpi_4_32b_defconfig | 1 +
>  configs/rpi_4_defconfig     | 1 +
>  configs/rpi_arm64_defconfig | 1 +
>  3 files changed, 3 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 04/12] dm: Introduce xxx_get_dma_range()
  2020-12-19  2:28   ` Simon Glass
@ 2020-12-21 13:03     ` Nicolas Saenz Julienne
  2020-12-21 16:47       ` Simon Glass
  0 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-21 13:03 UTC (permalink / raw)
  To: u-boot

Hi Simon, thanks for the review.

On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> Hi Nicolas,
> 
> On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > 
> > Add the following functions to get a specific device's DMA ranges:
> > ?- dev_get_dma_range()
> > ?- ofnode_get_dma_range()
> > ?- of_get_dma_range()
> > ?- fdt_get_dma_range()
> > They are specially useful in oder to be able validate a physical address
> > space range into a bus's and to convert addresses from and to address
> > spaces.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > 
> > ---
> > Changes since v2:
> > ?- Return ENOENT instead of ENODEV
> > ?- Refcount OF nodes
> > 
> > Changes since v1:
> > ?- Fix wrong arguments in of_get_dma_range()'s call to of_translate_dma_address()
> > ?- Fix build in SPL/TPL and no LIBFDT supprt
> > ?- Add missing declaration in 'core/read.c'
> > ?- Address Matthias' comments
> > 
> > ?common/fdt_support.c   | 73 +++++++++++++++++++++++++++++++++++++++
> > ?drivers/core/of_addr.c | 78 ++++++++++++++++++++++++++++++++++++++++++
> > ?drivers/core/ofnode.c  |  9 +++++
> > ?drivers/core/read.c    |  6 ++++
> > ?include/dm/of_addr.h   | 17 +++++++++
> > ?include/dm/ofnode.h    | 16 +++++++++
> > ?include/dm/read.h      | 21 ++++++++++++
> > ?include/fdt_support.h  | 14 ++++++++
> > ?8 files changed, 234 insertions(+)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> I don't suppose it is worth writing a version that uses the ofnode API
> and thus reduce code size? Probably not since if livetree is enabled,
> I doubt we would ever call the flattree one. Also it is nice to have
> the same livetree code as Linux, where possible.

As far as the logic is concerned we'd be able to mimic what linux does so it
shouldn't be a problem.

But I'd be forced to port low level DT code to ofnode. Notably 'of_match_bus()'
and 'struct of_bus', which IMO have no place in ofnode: it seems to me that
ofnode is an abstraction geared towards simplifying DT consumers. This wouldn't
provide much benefit to them, with the downside of divering from upstream
Linux's implementation.

Regards,
Nicolas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201221/038bbee0/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 07/12] dm: test: Add test case for dev->dma_offset
  2020-12-19  2:28   ` Simon Glass
@ 2020-12-21 13:36     ` Nicolas Saenz Julienne
  0 siblings, 0 replies; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-21 13:36 UTC (permalink / raw)
  To: u-boot

On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > 
> > Add test to validate dev->dma_offset is properly set on devices.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> > ?arch/sandbox/dts/test.dts          |  4 ++++
> > ?configs/sandbox64_defconfig        |  1 +
> > ?configs/sandbox_defconfig          |  1 +
> > ?configs/sandbox_flattree_defconfig |  1 +
> > ?configs/sandbox_spl_defconfig      |  1 +
> > ?test/dm/core.c                     | 30 ++++++++++++++++++++++++++++++
> > ?6 files changed, 38 insertions(+)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> We normally use ut_assertok() instead of ut_assert(!xxx), since 0 means OK.

Will fix this across all tests.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201221/7fb5f24f/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 09/12] dm: test: Add test case for dev_phys_to_bus()/dev_bus_to_phys()
  2020-12-19  2:28   ` Simon Glass
@ 2020-12-21 13:47     ` Nicolas Saenz Julienne
  2020-12-21 16:47       ` Simon Glass
  0 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-21 13:47 UTC (permalink / raw)
  To: u-boot

On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > 
> > By reusing DT nodes already available in sandbox's test DT introduce a
> > test to validate dev_phys_to_bus()/dev_bus_to_phys().
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> > ?test/dm/Makefile   |  1 +
> > ?test/dm/phys2bus.c | 36 ++++++++++++++++++++++++++++++++++++
> > ?2 files changed, 37 insertions(+)
> > ?create mode 100644 test/dm/phys2bus.c
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> comment below
> 
> > 
> > diff --git a/test/dm/Makefile b/test/dm/Makefile
> > index 5c52d8b6ea..ac86abaa88 100644
> > --- a/test/dm/Makefile
> > +++ b/test/dm/Makefile
> > @@ -16,6 +16,7 @@ obj-$(CONFIG_UT_DM) += test-uclass.o
> > 
> > ?obj-$(CONFIG_UT_DM) += core.o
> > ?obj-$(CONFIG_UT_DM) += read.o
> > +obj-$(CONFIG_UT_DM) += phys2bus.o
> > ?ifneq ($(CONFIG_SANDBOX),)
> > ?obj-$(CONFIG_ACPIGEN) += acpi.o
> > ?obj-$(CONFIG_ACPIGEN) += acpigen.o
> > diff --git a/test/dm/phys2bus.c b/test/dm/phys2bus.c
> > new file mode 100644
> > index 0000000000..06a045c168
> > --- /dev/null
> > +++ b/test/dm/phys2bus.c
> > @@ -0,0 +1,36 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2020 Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <phys2bus.h>
> > +#include <dm/device.h>
> > +#include <dm/ofnode.h>
> > +#include <dm/root.h>
> > +#include <dm/test.h>
> > +#include <dm/uclass-internal.h>
> > +#include <test/ut.h>
> > +
> > +static int dm_test_phys_to_bus(struct unit_test_state *uts)
> > +{
> > +       struct udevice *dev;
> > +       ofnode node;
> > +
> > +       node = ofnode_path("/mmio-bus at 0");
> > +       ut_assert(ofnode_valid(node));
> > +       ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_BUS, node, &dev));
> > +       /* In this case it should be transparent, no dma-ranges in parent bus */
> > +       ut_asserteq_ptr((void*)0xfffffULL, (void*)dev_phys_to_bus(dev, 0xfffff));
> 
> ut_asserteq_addr() might be better here? Not sure.

That function uses map_to_sysmem() under the hood, which AFAIR didn't play well
with these addresses.

Regards,
Nicolas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201221/3cd972b1/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 10/12] xhci: translate virtual addresses into the bus's address space
  2020-12-19  2:28   ` Simon Glass
@ 2020-12-21 13:51     ` Nicolas Saenz Julienne
  0 siblings, 0 replies; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-21 13:51 UTC (permalink / raw)
  To: u-boot

On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> Hi Nicolas,
> 
> On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > 
> > So far we've been content with passing physical addresses when
> > configuring memory addresses into XHCI controllers, but not all
> > platforms have buses with transparent mappings. Specifically the
> > Raspberry Pi 4 might introduce an offset to memory accesses incoming
> > from its PCIe port.
> > 
> > Introduce xhci_virt_to_bus() and xhci_bus_to_virt() to cater with these
> > limitations, and make sure we don't break non DM users.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> > ?drivers/usb/host/xhci-mem.c  | 45 +++++++++++++++++++-----------------
> > ?drivers/usb/host/xhci-ring.c | 11 +++++----
> > ?drivers/usb/host/xhci.c      |  4 ++--
> > ?include/usb/xhci.h           | 22 +++++++++++++++++-
> > ?4 files changed, 54 insertions(+), 28 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> nits below
> 
> [..]
> 
> > diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> > index e1d382369a..b87210b9ba 100644
> > --- a/include/usb/xhci.h
> > +++ b/include/usb/xhci.h
> > @@ -16,6 +16,7 @@
> > ?#ifndef HOST_XHCI_H_
> > ?#define HOST_XHCI_H_
> > 
> > +#include <phys2bus.h>
> > ?#include <reset.h>
> > ?#include <asm/types.h>
> > ?#include <asm/cache.h>
> > @@ -1250,7 +1251,8 @@ int xhci_check_maxpacket(struct usb_device *udev);
> > ?void xhci_flush_cache(uintptr_t addr, u32 type_len);
> > ?void xhci_inval_cache(uintptr_t addr, u32 type_len);
> > ?void xhci_cleanup(struct xhci_ctrl *ctrl);
> > -struct xhci_ring *xhci_ring_alloc(unsigned int num_segs, bool link_trbs);
> > +struct xhci_ring *xhci_ring_alloc(struct xhci_ctrl *ctrl, unsigned int num_segs,
> > +                                 bool link_trbs);
> 
> Can you please add a comment while you are here?

The comment is in the xhci-mem.c. Should I move it here?

> > ?int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id);
> > ?int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
> > ??????????????????struct xhci_hcor *hcor);
> > @@ -1278,4 +1280,22 @@ extern struct dm_usb_ops xhci_usb_ops;
> > 
> > ?struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev);
> > 
> > +static inline dma_addr_t xhci_virt_to_bus(struct xhci_ctrl *ctrl, void *addr)
> > +{
> > +#if CONFIG_IS_ENABLED(DM_DMA)
> 
> Can this use if() ?

Yes, will do.

> > +       return dev_phys_to_bus(ctrl->dev, virt_to_phys(addr));
> > +#else
> > +       return phys_to_bus(virt_to_phys(addr));
> > +#endif
> > +}
> > +
> > +static inline void *xhci_bus_to_virt(struct xhci_ctrl *ctrl, dma_addr_t addr)
> > +{
> > +#if CONFIG_IS_ENABLED(DM_DMA)
> > +       return phys_to_virt(dev_bus_to_phys(ctrl->dev, addr));
> > +#else
> > +       return phys_to_virt(bus_to_phys(addr));
> > +#endif
> > +}
> > +
> > ?#endif /* HOST_XHCI_H_ */
> > --
> > 2.29.2
> > 
> 
> Regards,
> Simon



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201221/3508f4d8/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 11/12] mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys()
  2020-12-19  2:28   ` Simon Glass
@ 2020-12-21 14:23     ` Nicolas Saenz Julienne
  2020-12-21 16:47       ` Simon Glass
  0 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-21 14:23 UTC (permalink / raw)
  To: u-boot

On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > 
> > This will allow us to use DM variants of phys_to_bus()/bus_to_phys()
> > when relevant.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> > ?drivers/mmc/sdhci.c |  7 ++++---
> > ?include/mmc.h       | 10 ++++++++++
> > ?2 files changed, 14 insertions(+), 3 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> 
> > 
> > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> > index 0628934312..2086d7cdb1 100644
> > --- a/drivers/mmc/sdhci.c
> > +++ b/drivers/mmc/sdhci.c
> > @@ -19,7 +19,6 @@
> > ?#include <linux/bitops.h>
> > ?#include <linux/delay.h>
> > ?#include <linux/dma-mapping.h>
> > -#include <phys2bus.h>
> > 
> > ?static void sdhci_reset(struct sdhci_host *host, u8 mask)
> > ?{
> > @@ -103,7 +102,8 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
> > ??????????????????????????????????????????mmc_get_dma_dir(data));
> > 
> > ????????if (host->flags & USE_SDMA) {
> > -               sdhci_writel(host, phys_to_bus((ulong)host->start_addr),
> > +               sdhci_writel(host,
> > +                            mmc_phys_to_bus(host->mmc, (ulong)host->start_addr),
> > ????????????????????????????????SDHCI_DMA_ADDRESS);
> > ????????}
> > ?#if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
> > @@ -162,7 +162,8 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
> > ????????????????????????????????start_addr &=
> > ????????????????????????????????~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1);
> > ????????????????????????????????start_addr += SDHCI_DEFAULT_BOUNDARY_SIZE;
> > -                               sdhci_writel(host, phys_to_bus((ulong)start_addr),
> > +                               sdhci_writel(host,
> > +                                            mmc_phys_to_bus(host->mmc, (ulong)start_addr),
> > ?????????????????????????????????????????????SDHCI_DMA_ADDRESS);
> > ????????????????????????}
> > ????????????????}
> > diff --git a/include/mmc.h b/include/mmc.h
> > index 1d377e0281..5fe1ef1dfc 100644
> > --- a/include/mmc.h
> > +++ b/include/mmc.h
> > @@ -15,6 +15,7 @@
> > ?#include <linux/compiler.h>
> > ?#include <linux/dma-direction.h>
> > ?#include <part.h>
> > +#include <phys2bus.h>
> > 
> > ?struct bd_info;
> > 
> > @@ -977,4 +978,13 @@ static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
> > ????????return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
> > ?}
> > 
> > +static inline dma_addr_t mmc_phys_to_bus(struct mmc *mmc, phys_addr_t addr)
> > +{
> > +#if CONFIG_IS_ENABLED(DM_DMA)
> 
> if() ?

Noted.

> Also makes me wonder if dev_phys_to_bus() should call phys_to_bus() if
> DM_DMA is not enabled?

I'd still need some sort per-subsystem macro to cater for !DM configurations
where the device pointer isn't defined in the controller struct. Which IMO
isn't much of an enhancement to this approach.

What I'd like to have is some sort of subsystem independent way of doing
'controller->dev' regardless of CONFIG_DM (falling back to NULL when not
enabled). The easy way would be defining the variable regardless of CONFIG_DM,
but I guess that isn't ideal WRT image sizes.

Regards,
Nicolas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201221/6fb3c151/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 11/12] mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys()
  2020-12-21 14:23     ` Nicolas Saenz Julienne
@ 2020-12-21 16:47       ` Simon Glass
  2020-12-21 19:52         ` Nicolas Saenz Julienne
  0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2020-12-21 16:47 UTC (permalink / raw)
  To: u-boot

Hi Nicolas,

On Mon, 21 Dec 2020 at 07:23, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> > On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> > <nsaenzjulienne@suse.de> wrote:
> > >
> > > This will allow us to use DM variants of phys_to_bus()/bus_to_phys()
> > > when relevant.
> > >
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > ---
> > >  drivers/mmc/sdhci.c |  7 ++++---
> > >  include/mmc.h       | 10 ++++++++++
> > >  2 files changed, 14 insertions(+), 3 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> >
> > >
> > > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> > > index 0628934312..2086d7cdb1 100644
> > > --- a/drivers/mmc/sdhci.c
> > > +++ b/drivers/mmc/sdhci.c
> > > @@ -19,7 +19,6 @@
> > >  #include <linux/bitops.h>
> > >  #include <linux/delay.h>
> > >  #include <linux/dma-mapping.h>
> > > -#include <phys2bus.h>
> > >
> > >  static void sdhci_reset(struct sdhci_host *host, u8 mask)
> > >  {
> > > @@ -103,7 +102,8 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
> > >                                           mmc_get_dma_dir(data));
> > >
> > >         if (host->flags & USE_SDMA) {
> > > -               sdhci_writel(host, phys_to_bus((ulong)host->start_addr),
> > > +               sdhci_writel(host,
> > > +                            mmc_phys_to_bus(host->mmc, (ulong)host->start_addr),
> > >                                 SDHCI_DMA_ADDRESS);
> > >         }
> > >  #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
> > > @@ -162,7 +162,8 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
> > >                                 start_addr &=
> > >                                 ~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1);
> > >                                 start_addr += SDHCI_DEFAULT_BOUNDARY_SIZE;
> > > -                               sdhci_writel(host, phys_to_bus((ulong)start_addr),
> > > +                               sdhci_writel(host,
> > > +                                            mmc_phys_to_bus(host->mmc, (ulong)start_addr),
> > >                                              SDHCI_DMA_ADDRESS);
> > >                         }
> > >                 }
> > > diff --git a/include/mmc.h b/include/mmc.h
> > > index 1d377e0281..5fe1ef1dfc 100644
> > > --- a/include/mmc.h
> > > +++ b/include/mmc.h
> > > @@ -15,6 +15,7 @@
> > >  #include <linux/compiler.h>
> > >  #include <linux/dma-direction.h>
> > >  #include <part.h>
> > > +#include <phys2bus.h>
> > >
> > >  struct bd_info;
> > >
> > > @@ -977,4 +978,13 @@ static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
> > >         return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
> > >  }
> > >
> > > +static inline dma_addr_t mmc_phys_to_bus(struct mmc *mmc, phys_addr_t addr)
> > > +{
> > > +#if CONFIG_IS_ENABLED(DM_DMA)
> >
> > if() ?
>
> Noted.
>
> > Also makes me wonder if dev_phys_to_bus() should call phys_to_bus() if
> > DM_DMA is not enabled?
>
> I'd still need some sort per-subsystem macro to cater for !DM configurations
> where the device pointer isn't defined in the controller struct. Which IMO
> isn't much of an enhancement to this approach.

We don't need to worry about non-DM for new features. In fact we don't
generally accept code that adds new features to non-DM code. It is
deprecated and slated for removal, so it just prolongs the migration.

>
> What I'd like to have is some sort of subsystem independent way of doing
> 'controller->dev' regardless of CONFIG_DM (falling back to NULL when not
> enabled). The easy way would be defining the variable regardless of CONFIG_DM,
> but I guess that isn't ideal WRT image sizes.

Well, see above!

Regards,
Simon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 09/12] dm: test: Add test case for dev_phys_to_bus()/dev_bus_to_phys()
  2020-12-21 13:47     ` Nicolas Saenz Julienne
@ 2020-12-21 16:47       ` Simon Glass
  0 siblings, 0 replies; 31+ messages in thread
From: Simon Glass @ 2020-12-21 16:47 UTC (permalink / raw)
  To: u-boot

Hi Nicolas,

On Mon, 21 Dec 2020 at 06:47, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> > On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> > <nsaenzjulienne@suse.de> wrote:
> > >
> > > By reusing DT nodes already available in sandbox's test DT introduce a
> > > test to validate dev_phys_to_bus()/dev_bus_to_phys().
> > >
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > ---
> > >  test/dm/Makefile   |  1 +
> > >  test/dm/phys2bus.c | 36 ++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 37 insertions(+)
> > >  create mode 100644 test/dm/phys2bus.c
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > comment below
> >
> > >
> > > diff --git a/test/dm/Makefile b/test/dm/Makefile
> > > index 5c52d8b6ea..ac86abaa88 100644
> > > --- a/test/dm/Makefile
> > > +++ b/test/dm/Makefile
> > > @@ -16,6 +16,7 @@ obj-$(CONFIG_UT_DM) += test-uclass.o
> > >
> > >  obj-$(CONFIG_UT_DM) += core.o
> > >  obj-$(CONFIG_UT_DM) += read.o
> > > +obj-$(CONFIG_UT_DM) += phys2bus.o
> > >  ifneq ($(CONFIG_SANDBOX),)
> > >  obj-$(CONFIG_ACPIGEN) += acpi.o
> > >  obj-$(CONFIG_ACPIGEN) += acpigen.o
> > > diff --git a/test/dm/phys2bus.c b/test/dm/phys2bus.c
> > > new file mode 100644
> > > index 0000000000..06a045c168
> > > --- /dev/null
> > > +++ b/test/dm/phys2bus.c
> > > @@ -0,0 +1,36 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (c) 2020 Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <dm.h>
> > > +#include <phys2bus.h>
> > > +#include <dm/device.h>
> > > +#include <dm/ofnode.h>
> > > +#include <dm/root.h>
> > > +#include <dm/test.h>
> > > +#include <dm/uclass-internal.h>
> > > +#include <test/ut.h>
> > > +
> > > +static int dm_test_phys_to_bus(struct unit_test_state *uts)
> > > +{
> > > +       struct udevice *dev;
> > > +       ofnode node;
> > > +
> > > +       node = ofnode_path("/mmio-bus at 0");
> > > +       ut_assert(ofnode_valid(node));
> > > +       ut_assert(!uclass_get_device_by_ofnode(UCLASS_TEST_BUS, node, &dev));
> > > +       /* In this case it should be transparent, no dma-ranges in parent bus */
> > > +       ut_asserteq_ptr((void*)0xfffffULL, (void*)dev_phys_to_bus(dev, 0xfffff));
> >
> > ut_asserteq_addr() might be better here? Not sure.
>
> That function uses map_to_sysmem() under the hood, which AFAIR didn't play well
> with these addresses.

Well it might be worth checking that again. This is a nop on
everything but sandbox[1]. With sandbox we actually want to print the
address as an address, not a pointer, since the address makes sense
(it is in U-Boot's memory map) but the pointer is whatever the OS
assigned the U-Boot executable.

Regards,
Simon

[1] I see Risc-V uses it to map SDRAM which seems like a bug

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 04/12] dm: Introduce xxx_get_dma_range()
  2020-12-21 13:03     ` Nicolas Saenz Julienne
@ 2020-12-21 16:47       ` Simon Glass
  0 siblings, 0 replies; 31+ messages in thread
From: Simon Glass @ 2020-12-21 16:47 UTC (permalink / raw)
  To: u-boot

Hi Nicolas,

On Mon, 21 Dec 2020 at 06:03, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Hi Simon, thanks for the review.
>
> On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> > Hi Nicolas,
> >
> > On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> > <nsaenzjulienne@suse.de> wrote:
> > >
> > > Add the following functions to get a specific device's DMA ranges:
> > >  - dev_get_dma_range()
> > >  - ofnode_get_dma_range()
> > >  - of_get_dma_range()
> > >  - fdt_get_dma_range()
> > > They are specially useful in oder to be able validate a physical address
> > > space range into a bus's and to convert addresses from and to address
> > > spaces.
> > >
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > >
> > > ---
> > > Changes since v2:
> > >  - Return ENOENT instead of ENODEV
> > >  - Refcount OF nodes
> > >
> > > Changes since v1:
> > >  - Fix wrong arguments in of_get_dma_range()'s call to of_translate_dma_address()
> > >  - Fix build in SPL/TPL and no LIBFDT supprt
> > >  - Add missing declaration in 'core/read.c'
> > >  - Address Matthias' comments
> > >
> > >  common/fdt_support.c   | 73 +++++++++++++++++++++++++++++++++++++++
> > >  drivers/core/of_addr.c | 78 ++++++++++++++++++++++++++++++++++++++++++
> > >  drivers/core/ofnode.c  |  9 +++++
> > >  drivers/core/read.c    |  6 ++++
> > >  include/dm/of_addr.h   | 17 +++++++++
> > >  include/dm/ofnode.h    | 16 +++++++++
> > >  include/dm/read.h      | 21 ++++++++++++
> > >  include/fdt_support.h  | 14 ++++++++
> > >  8 files changed, 234 insertions(+)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > I don't suppose it is worth writing a version that uses the ofnode API
> > and thus reduce code size? Probably not since if livetree is enabled,
> > I doubt we would ever call the flattree one. Also it is nice to have
> > the same livetree code as Linux, where possible.
>
> As far as the logic is concerned we'd be able to mimic what linux does so it
> shouldn't be a problem.
>
> But I'd be forced to port low level DT code to ofnode. Notably 'of_match_bus()'
> and 'struct of_bus', which IMO have no place in ofnode: it seems to me that
> ofnode is an abstraction geared towards simplifying DT consumers. This wouldn't
> provide much benefit to them, with the downside of divering from upstream
> Linux's implementation.

Yes that sounds right to me, thanks.

Regards,
Simon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 11/12] mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys()
  2020-12-21 16:47       ` Simon Glass
@ 2020-12-21 19:52         ` Nicolas Saenz Julienne
  2020-12-21 20:23           ` Simon Glass
  0 siblings, 1 reply; 31+ messages in thread
From: Nicolas Saenz Julienne @ 2020-12-21 19:52 UTC (permalink / raw)
  To: u-boot

On Mon, 2020-12-21 at 09:47 -0700, Simon Glass wrote:
> Hi Nicolas,
> 
> On Mon, 21 Dec 2020 at 07:23, Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > 
> > On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> > > On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> > > <nsaenzjulienne@suse.de> wrote:
> > > > 
> > > > This will allow us to use DM variants of phys_to_bus()/bus_to_phys()
> > > > when relevant.
> > > > 
> > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > > ---
> > > > ?drivers/mmc/sdhci.c |  7 ++++---
> > > > ?include/mmc.h       | 10 ++++++++++
> > > > ?2 files changed, 14 insertions(+), 3 deletions(-)
> > > 
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > 
> > > 
> > > > 
> > > > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> > > > index 0628934312..2086d7cdb1 100644
> > > > --- a/drivers/mmc/sdhci.c
> > > > +++ b/drivers/mmc/sdhci.c
> > > > @@ -19,7 +19,6 @@
> > > > ?#include <linux/bitops.h>
> > > > ?#include <linux/delay.h>
> > > > ?#include <linux/dma-mapping.h>
> > > > -#include <phys2bus.h>
> > > > 
> > > > ?static void sdhci_reset(struct sdhci_host *host, u8 mask)
> > > > ?{
> > > > @@ -103,7 +102,8 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
> > > > ??????????????????????????????????????????mmc_get_dma_dir(data));
> > > > 
> > > > ????????if (host->flags & USE_SDMA) {
> > > > -               sdhci_writel(host, phys_to_bus((ulong)host->start_addr),
> > > > +               sdhci_writel(host,
> > > > +                            mmc_phys_to_bus(host->mmc, (ulong)host->start_addr),
> > > > ????????????????????????????????SDHCI_DMA_ADDRESS);
> > > > ????????}
> > > > ?#if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
> > > > @@ -162,7 +162,8 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
> > > > ????????????????????????????????start_addr &=
> > > > ????????????????????????????????~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1);
> > > > ????????????????????????????????start_addr += SDHCI_DEFAULT_BOUNDARY_SIZE;
> > > > -                               sdhci_writel(host, phys_to_bus((ulong)start_addr),
> > > > +                               sdhci_writel(host,
> > > > +                                            mmc_phys_to_bus(host->mmc, (ulong)start_addr),
> > > > ?????????????????????????????????????????????SDHCI_DMA_ADDRESS);
> > > > ????????????????????????}
> > > > ????????????????}
> > > > diff --git a/include/mmc.h b/include/mmc.h
> > > > index 1d377e0281..5fe1ef1dfc 100644
> > > > --- a/include/mmc.h
> > > > +++ b/include/mmc.h
> > > > @@ -15,6 +15,7 @@
> > > > ?#include <linux/compiler.h>
> > > > ?#include <linux/dma-direction.h>
> > > > ?#include <part.h>
> > > > +#include <phys2bus.h>
> > > > 
> > > > ?struct bd_info;
> > > > 
> > > > @@ -977,4 +978,13 @@ static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
> > > > ????????return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
> > > > ?}
> > > > 
> > > > +static inline dma_addr_t mmc_phys_to_bus(struct mmc *mmc, phys_addr_t addr)
> > > > +{
> > > > +#if CONFIG_IS_ENABLED(DM_DMA)
> > > 
> > > if() ?
> > 
> > Noted.
> > 
> > > Also makes me wonder if dev_phys_to_bus() should call phys_to_bus() if
> > > DM_DMA is not enabled?
> > 
> > I'd still need some sort per-subsystem macro to cater for !DM configurations
> > where the device pointer isn't defined in the controller struct. Which IMO
> > isn't much of an enhancement to this approach.
> 
> We don't need to worry about non-DM for new features. In fact we don't
> generally accept code that adds new features to non-DM code. It is
> deprecated and slated for removal, so it just prolongs the migration.

Understood, so let's not call the plain phys_to_bus() and assume a 1:1 address
translation on non DM_DMA users (for both mmc and xhci). I'd say it's pretty
safe to do anyway, as it seems phys2bus.h was introduced specially for the RPi
family.

There still are some rogue users of the non dev dependent phys_to_bus() API in
RPi platform code. I'll try to clean that up in a subsequent series.

Regards,
Nicolas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201221/e1d87d8d/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 11/12] mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys()
  2020-12-21 19:52         ` Nicolas Saenz Julienne
@ 2020-12-21 20:23           ` Simon Glass
  0 siblings, 0 replies; 31+ messages in thread
From: Simon Glass @ 2020-12-21 20:23 UTC (permalink / raw)
  To: u-boot

Hi Nicolas,

On Mon, 21 Dec 2020 at 12:52, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> On Mon, 2020-12-21 at 09:47 -0700, Simon Glass wrote:
> > Hi Nicolas,
> >
> > On Mon, 21 Dec 2020 at 07:23, Nicolas Saenz Julienne
> > <nsaenzjulienne@suse.de> wrote:
> > >
> > > On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> > > > On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> > > > <nsaenzjulienne@suse.de> wrote:
> > > > >
> > > > > This will allow us to use DM variants of phys_to_bus()/bus_to_phys()
> > > > > when relevant.
> > > > >
> > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > > > ---
> > > > >  drivers/mmc/sdhci.c |  7 ++++---
> > > > >  include/mmc.h       | 10 ++++++++++
> > > > >  2 files changed, 14 insertions(+), 3 deletions(-)
> > > >
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > >
> > > >
> > > > >
> > > > > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> > > > > index 0628934312..2086d7cdb1 100644
> > > > > --- a/drivers/mmc/sdhci.c
> > > > > +++ b/drivers/mmc/sdhci.c
> > > > > @@ -19,7 +19,6 @@
> > > > >  #include <linux/bitops.h>
> > > > >  #include <linux/delay.h>
> > > > >  #include <linux/dma-mapping.h>
> > > > > -#include <phys2bus.h>
> > > > >
> > > > >  static void sdhci_reset(struct sdhci_host *host, u8 mask)
> > > > >  {
> > > > > @@ -103,7 +102,8 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
> > > > >                                           mmc_get_dma_dir(data));
> > > > >
> > > > >         if (host->flags & USE_SDMA) {
> > > > > -               sdhci_writel(host, phys_to_bus((ulong)host->start_addr),
> > > > > +               sdhci_writel(host,
> > > > > +                            mmc_phys_to_bus(host->mmc, (ulong)host->start_addr),
> > > > >                                 SDHCI_DMA_ADDRESS);
> > > > >         }
> > > > >  #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
> > > > > @@ -162,7 +162,8 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
> > > > >                                 start_addr &=
> > > > >                                 ~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1);
> > > > >                                 start_addr += SDHCI_DEFAULT_BOUNDARY_SIZE;
> > > > > -                               sdhci_writel(host, phys_to_bus((ulong)start_addr),
> > > > > +                               sdhci_writel(host,
> > > > > +                                            mmc_phys_to_bus(host->mmc, (ulong)start_addr),
> > > > >                                              SDHCI_DMA_ADDRESS);
> > > > >                         }
> > > > >                 }
> > > > > diff --git a/include/mmc.h b/include/mmc.h
> > > > > index 1d377e0281..5fe1ef1dfc 100644
> > > > > --- a/include/mmc.h
> > > > > +++ b/include/mmc.h
> > > > > @@ -15,6 +15,7 @@
> > > > >  #include <linux/compiler.h>
> > > > >  #include <linux/dma-direction.h>
> > > > >  #include <part.h>
> > > > > +#include <phys2bus.h>
> > > > >
> > > > >  struct bd_info;
> > > > >
> > > > > @@ -977,4 +978,13 @@ static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
> > > > >         return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
> > > > >  }
> > > > >
> > > > > +static inline dma_addr_t mmc_phys_to_bus(struct mmc *mmc, phys_addr_t addr)
> > > > > +{
> > > > > +#if CONFIG_IS_ENABLED(DM_DMA)
> > > >
> > > > if() ?
> > >
> > > Noted.
> > >
> > > > Also makes me wonder if dev_phys_to_bus() should call phys_to_bus() if
> > > > DM_DMA is not enabled?
> > >
> > > I'd still need some sort per-subsystem macro to cater for !DM configurations
> > > where the device pointer isn't defined in the controller struct. Which IMO
> > > isn't much of an enhancement to this approach.
> >
> > We don't need to worry about non-DM for new features. In fact we don't
> > generally accept code that adds new features to non-DM code. It is
> > deprecated and slated for removal, so it just prolongs the migration.
>
> Understood, so let's not call the plain phys_to_bus() and assume a 1:1 address
> translation on non DM_DMA users (for both mmc and xhci). I'd say it's pretty
> safe to do anyway, as it seems phys2bus.h was introduced specially for the RPi
> family.

That sounds right to me.

>
> There still are some rogue users of the non dev dependent phys_to_bus() API in
> RPi platform code. I'll try to clean that up in a subsequent series.

OK ta.

Regards,
Simon

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2020-12-21 20:23 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 17:23 [PATCH v3 00/12] Raspberry Pi 400/Compute Module 4 support Nicolas Saenz Julienne
2020-12-15 17:23 ` [PATCH v3 01/12] rpi: Add identifier for the new RPi400 Nicolas Saenz Julienne
2020-12-15 17:23 ` [PATCH v3 02/12] rpi: Add identifier for the new CM4 Nicolas Saenz Julienne
2020-12-15 17:23 ` [PATCH v3 03/12] pci: pcie-brcmstb: Fix inbound window configurations Nicolas Saenz Julienne
2020-12-15 17:23 ` [PATCH v3 04/12] dm: Introduce xxx_get_dma_range() Nicolas Saenz Julienne
2020-12-19  2:28   ` Simon Glass
2020-12-21 13:03     ` Nicolas Saenz Julienne
2020-12-21 16:47       ` Simon Glass
2020-12-15 17:23 ` [PATCH v3 05/12] dm: test: Add test case for dev_get_dma_ranges() Nicolas Saenz Julienne
2020-12-19  2:28   ` Simon Glass
2020-12-15 17:23 ` [PATCH v3 06/12] dm: Introduce DMA constraints into the core device model Nicolas Saenz Julienne
2020-12-19  2:28   ` Simon Glass
2020-12-15 17:23 ` [PATCH v3 07/12] dm: test: Add test case for dev->dma_offset Nicolas Saenz Julienne
2020-12-19  2:28   ` Simon Glass
2020-12-21 13:36     ` Nicolas Saenz Julienne
2020-12-15 17:23 ` [PATCH v3 08/12] dm: Introduce dev_phys_to_bus()/dev_bus_to_phys() Nicolas Saenz Julienne
2020-12-15 17:23 ` [PATCH v3 09/12] dm: test: Add test case for dev_phys_to_bus()/dev_bus_to_phys() Nicolas Saenz Julienne
2020-12-19  2:28   ` Simon Glass
2020-12-21 13:47     ` Nicolas Saenz Julienne
2020-12-21 16:47       ` Simon Glass
2020-12-15 17:23 ` [PATCH v3 10/12] xhci: translate virtual addresses into the bus's address space Nicolas Saenz Julienne
2020-12-19  2:28   ` Simon Glass
2020-12-21 13:51     ` Nicolas Saenz Julienne
2020-12-15 17:23 ` [PATCH v3 11/12] mmc: Introduce mmc_phys_to_bus()/mmc_bus_to_phys() Nicolas Saenz Julienne
2020-12-19  2:28   ` Simon Glass
2020-12-21 14:23     ` Nicolas Saenz Julienne
2020-12-21 16:47       ` Simon Glass
2020-12-21 19:52         ` Nicolas Saenz Julienne
2020-12-21 20:23           ` Simon Glass
2020-12-15 17:23 ` [PATCH v3 12/12] configs: rpi4: Enable DM_DMA across all RPi4 configurations Nicolas Saenz Julienne
2020-12-19  2:28   ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.