Re: [PATCH 1/2] arm64: dts: broadcom: remove an unused property dma-ranges

[Leizhen (ThunderTown)]

On 2020/10/16 15:06, Leizhen (ThunderTown) wrote:
> 
> 
> On 2020/10/14 22:02, Arnd Bergmann wrote:
>> On Wed, Oct 14, 2020 at 3:36 PM Leizhen (ThunderTown)
>> <thunder.leizhen@xxxxxxxxxx> wrote:
>>> On 2020/10/14 15:38, Arnd Bergmann wrote:
>>>> On Wed, Oct 14, 2020 at 5:15 AM Florian Fainelli <f.fainelli@xxxxxxxxx> wrote:
>>>>> On 10/12/2020 11:06 PM, Zhen Lei wrote:
>>>>>> stingray-usb.dtsi is finally included by three dts files:
>>>>>> bcm958802a802x.dts, bcm958742k.dts and bcm958742t.dts. I searched all
>>>>>> these three entire expanded dts files, and each of them contains only one
>>>>>> dma-ranges. No conversion range is specified, so it cannot work properly.
>>>>>> I think this property "dma-ranges" is added by mistake, just remove it.
>>>>>> Otherwise, the following error will be reported when any YAML detection
>>>>>> is performed on arm64.
>>>>>>
>>>>>> arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi:7.3-14: Warning \
>>>>>> (dma_ranges_format): /usb:dma-ranges: empty "dma-ranges" property but \
>>>>>> its #address-cells (1) differs from / (2)
>>>>>> arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi:7.3-14: Warning \
>>>>>> (dma_ranges_format): /usb:dma-ranges: empty "dma-ranges" property but \
>>>>>> its #size-cells (1) differs from / (2)
>>>>>>
>>>>>> Signed-off-by: Zhen Lei <thunder.leizhen@xxxxxxxxxx>
>>>>>
>>>>> This looks fine to me, Scott, Ray do you want to Ack this patch before I
>>>>> take it?
>>>>
>>>> Does it mean that there are no devices on this bus that can do DMA?
>>>>
>>>> Usually there should be a dma-ranges property to identify that DMA
>>>> is possible and what the limits are, though we have failed to enforce
>>>> that.
>>>
>>> Documentation/devicetree/bindings/iommu/iommu.txt +79
>>> When an "iommus" property is specified in a device tree node, the IOMMU will
>>> be used for address translation. If a "dma-ranges" property exists in the
>>> device's parent node it will be ignored. An exception to this rule is if the
>>> referenced IOMMU is disabled, in which case the "dma-ranges" property of the
>>> parent shall take effect.
>>>
>>> The dma-ranges is only required by IOMMU disabled case. And should exist in
>>> the parent node of IOMMU device. But this deleted dma-ranges is under the usb
>>> bus node.
>>
>> The USB hosts here don't use an IOMMU though, right?
> 
> Generally, USB devices are accessed through the IOMMU. However, even in this
> case, dma-ranges is not necessarily required. There are many examples of this
> in arch/arm64/boot/dts/. For example: arch/arm64/boot/dts/arm/juno.dt.yaml.
> 
> Not sure, but maybe I found the answer.
> 
> vi drivers/of/address.c +457
> 457                                               Thus we treat the absence of
> 458          * "ranges" as equivalent to an empty "ranges" property which means
> 459          * a 1:1 translation at that level.
> 
> 466          * This quirk also applies for 'dma-ranges' which frequently exist in
> 467          * child nodes without 'dma-ranges' in the parent nodes. --RobH
> 
> 475         if (ranges == NULL || rlen == 0) {
> 476                 offset = of_read_number(addr, na);
> 477                 memset(addr, 0, pna * 4);
> 478                 pr_debug("empty ranges; 1:1 translation\n");
> 479                 goto finish;
> 480         }
> 
> By the way: At first, I thought that these errors was detected by YAML. Now,
> I found that it was generated by "make dtbs". That's why it was reported by
> any YAML. Thus, the need to fix these errors is even more urgent.
> 
> 
>>
>>>> Also note that the #address-cells=<1> means that any device under
>>>> this bus is assumed to only support 32-bit addressing, and DMA will

Hi, Arnd:
  I known what you mean now. I will rewrite the patch. Thanks.

>>>> have to go through a slow swiotlb in the absence of an IOMMU.
>>>
>>> The dma_alloc_coherent() will allocate memory with GFP_DMA32 flag and
>>> try the 0-4G first. The reserved swiotlb buffer memory is used only
>>> when the allocation failed.
>>
>> The swiotlb is primarily about the streaming mappings with dma_map_*(),
>> which has to copy all data sent to the device. dma_alloc_coherent()
>> is a rare operation and less impacted by DMA limitations.
> 
> OK, I got it.
> 
>>
>>       Arnd
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>> .
>>
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> .
>