On 01/09/2015 10:34 AM, Rob Herring wrote:
On Thu, Jan 8, 2015 at 4:24 PM, Arnd Bergmann<arnd@xxxxxxxx> wrote:
On Thursday 08 January 2015 14:26:36 Murali Karicheri wrote:
On 01/08/2015 03:40 AM, Arnd Bergmann wrote:
On Wednesday 07 January 2015 17:37:56 Rob Herring wrote:
On Wed, Jan 7, 2015 at 12:49 PM, Murali Karicheri<m-karicheri2@xxxxxx> wrote:
+ ret = of_dma_get_range(np,&dma_addr,&paddr,&size);
+ if (ret< 0) {
+ dma_addr = offset = 0;
+ size = dev->coherent_dma_mask + 1;
If coherent_dma_mask is DMA_BIT_MASK(64), then you will overflow and
have a size of 0. There may also be a problem when the mask is only
32-bit type.
The mask is always a 64-bit type, it's not optional. But you are right,
the 64-bit mask case is broken, so I guess we have to fix it differently
by always passing the smaller value into arch_setup_dma_ops and
adapting that function instead.
Arnd,
What is the smaller value you are referring to in the below code?
between *dev->dma_mask and size from DT? But overflow can still happen
when size is to be calculated in arch_setup_dma_ops() for Non DT case or
when DT size is configured to be equivalent of DMA_BIT_MASK(64) + 1. Can
we discuss the code change you have in mind when you get a chance?
I meant changing every function that the size values gets passed into
to take a mask like 0xffffffff instead of a size like 0x100000000, so
we can represent a 64-bit capable bus correctly.
Or you could special case a size of 0 to mean all/max? I'm not sure if
we need to handle size=0 for other reasons beyond just wrong DT data.
This means we also need to adapt the value returned from of_dma_get_range.
A minor complication here is that the DT properties sometimes already
contain the mask value, in particular when we want to represent a
full mapping like
bus {
#address-cells =<1>;
#size-cells =<1>;
dma-ranges =<0 0 0xffffffff>; /* all 4 GB, DMA_BIT_MASK(32) */
This is wrong though, right? The DT should be size. Certainly, this
could be a valid size, but that would not make the mask 0xfffffffe. We
would still want it to be 0xffffffff.
We could do a fixup for these cases adding 1 if bit 0 is set (or not
subtracting 1 if we want the mask).
Rob
Arnd, Rob, et all,
Do we have preference one way or other for the size format? If we need
to follow the mask format, all of the calling functions below and the
arm_iommu_create_mapping() has to change as well to use this changed format.
drivers/gpu/drm/rockchip/rockchip_drm_drv.c: mapping =
arm_iommu_create_mapping(&platform_bus_type, 0x00000000,
drivers/gpu/drm/exynos/exynos_drm_iommu.c: mapping =
arm_iommu_create_mapping(&platform_bus_type, priv->da_start,
drivers/media/platform/omap3isp/isp.c: mapping =
arm_iommu_create_mapping(&platform_bus_type, SZ_1G, SZ_2G);
drivers/iommu/shmobile-iommu.c: mapping =
arm_iommu_create_mapping(&platform_bus_type, 0,
drivers/iommu/ipmmu-vmsa.c: mapping =
arm_iommu_create_mapping(&platform_bus_type,
So IMO, keeping current convention of size and taking care of exception
in DT handling is the right thing to do instead of changing all of the
above functions. i.e in of_dma_configure(), if DT provide a mask for
size (lsb set), we will check that and add 1 to it. Only case in DTS
that I can see this usage is
arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi: dma-ranges = <0x80 0x0
0x80 0x0 0x7f 0xffffffff>;
arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi: dma-ranges = <0x43000000
0x80 0x0 0x80 0x0 0x7f 0xffffffff>;
This logic should take care of setting the size to ox80_00000000 for
these cases. if dma_coherent_mask is set to max of u64, then this will
result in a zero size (both DT case and non DT case). So treat a size of
zero as error being overflow.
Also arm_iommu_create_mapping() currently accept a size of type size_t
which means, this function expect the size to be max of 0xffffffff. So
in arm_setup_iommu_dma_ops(), we need to check if size if >0xffffffff
and return an error. If in future this function support u64 for size,
this check can be removed.
I will update the series with this change and post it if we have an
agreement on this. Please repond.
Thanks
--
Murali Karicheri
Linux Kernel, Texas Instruments
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
