On Thu, Nov 03, 2022 at 02:38:57PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> DMA parent devices can define separate DMA busses via the "dma-ranges"
> and "#address-cells" and "#size-cells" properties. If the DMA bus has
> different cell counts than its parent, this can cause the translation
> of DMA address to fails (e.g. truncation from 2 to 1 address cells).
My assumption in this case was that the parent cell sizes should be
increased to 2 cells. That tends to be what people want to do anyways
(64-bit everywhere on 64-bit CPUs).
> Avoid this by stopping to search for DMA parents when a parent without
> a "dma-ranges" property is encountered. Also, since it is the DMA parent
> that defines the DMA bus, use the bus' cell counts instead of its parent
> cell counts.
We treat no 'dma-ranges' as equivalent to 'dma-ranges;'. IIRC, the spec
even says that because I hit that case.
Is this going to work for 'dma-device' with something like this?:
bus@0 {
dma-ranges = <...>;
child-bus@... {
dma-device@... {
};
};
};
>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
> Changes in v10:
> - new patch to avoid address truncation when traversing a bus hierarchy
> with mismatching #address-cells properties
>
> Example from Tegra194 (redacted for clarity):
>
> reserved-memory {
> #address-cells = <2>;
> #size-cells = <2>;
> ranges;
>
> framebuffer@0,0 {
> compatible = "framebuffer";
> reg = <0x2 0x57320000 0x0 0x00800000>;
> iommu-addresses = <&dc0 0x2 0x57320000 0x0 0x00800000>;
> };
> };
>
> bus@0 {
> /* truncation happens here */
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x0 0x0 0x40000000>;
>
> mc: memory-controller@2c00000 {
> #address-cells = <2>;
> #size-cells = <2>;
I think this is wrong. The parent should have more or equal number of
cells.
> /*
> * memory controller provides access to 512 GiB
> * of system RAM (root of the DMA bus)
> */
> dma-ranges = <0x0 0x0 0x0 0x80 0x0>;
> };
>
> host1x@13e00000 {
> display-hub@15200000 {
> display@15200000 {
> interconnect-names = "dma-mem", ...;
> interconnects = <&mc ...>;
> memory-region = <&fb>;
> };
> };
> };
> };
>
> During DMA address translation, the framebuffer address (0x257320000)
> will first be translated to the DMA parent's DMA bus, which yields the
> same value. After that, the current translation code will switch to the
> control bus of bus@0 and then the address will be truncated to
> 0x57320000 due to #address-cells = <1>.
>
> The idea of this patch is to interrupt DMA address translation at &mc
> because it is the root of the DMA bus (i.e. its parent does not have a
> dma-ranges property) so that the control bus' #address-cells doesn't
> truncate the DMA address.
>
> drivers/of/address.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 14f137a21b0c..e2f45bdbc41a 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -475,6 +475,7 @@ static u64 __of_translate_address(struct device_node *dev,
> const __be32 *in_addr, const char *rprop,
> struct device_node **host)
> {
> + bool dma = rprop && !strcmp(rprop, "dma-ranges");
> struct device_node *parent = NULL;
> struct of_bus *bus, *pbus;
> __be32 addr[OF_MAX_ADDR_CELLS];
> @@ -494,7 +495,12 @@ static u64 __of_translate_address(struct device_node *dev,
> bus = of_match_bus(parent);
>
> /* Count address cells & copy address locally */
> - bus->count_cells(dev, &na, &ns);
> + if (dma) {
> + na = of_bus_n_addr_cells(parent);
> + ns = of_bus_n_size_cells(parent);
> + } else {
> + bus->count_cells(dev, &na, &ns);
> + }
> if (!OF_CHECK_COUNTS(na, ns)) {
> pr_debug("Bad cell count for %pOF\n", dev);
> goto bail;
> @@ -515,7 +521,7 @@ static u64 __of_translate_address(struct device_node *dev,
> parent = get_parent(dev);
>
> /* If root, we have finished */
> - if (parent == NULL) {
> + if (parent == NULL || (dma && !of_get_property(parent, "dma-ranges", NULL))) {
> pr_debug("reached root node\n");
> result = of_read_number(addr, na);
> break;
> @@ -536,7 +542,12 @@ static u64 __of_translate_address(struct device_node *dev,
>
> /* Get new parent bus and counts */
> pbus = of_match_bus(parent);
> - pbus->count_cells(dev, &pna, &pns);
> + if (dma) {
> + pna = of_bus_n_addr_cells(parent);
> + pns = of_bus_n_size_cells(parent);
> + } else {
> + pbus->count_cells(dev, &pna, &pns);
> + }
> if (!OF_CHECK_COUNTS(pna, pns)) {
> pr_err("Bad cell count for %pOF\n", dev);
> break;
> --
> 2.38.1
>
>
