linux-kernel - Re: [PATCH v5 1/3] dt-bindings: dmaengine: Add dmamux for CV18XX/SG200X series SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 26 Mar 2024 12:31:18 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Inochi Amaoto <inochiama@...look.com>, Vinod Koul <vkoul@...nel.org>,
 Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, Chen Wang <unicorn_wang@...look.com>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>
Cc: Jisheng Zhang <jszhang@...nel.org>, Liu Gui <kenneth.liu@...hgo.com>,
 Jingbao Qiu <qiujingbao.dlmu@...il.com>, dlan@...too.org,
 dmaengine@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v5 1/3] dt-bindings: dmaengine: Add dmamux for
 CV18XX/SG200X series SoC

On 26/03/2024 12:15, Inochi Amaoto wrote:
> On Tue, Mar 26, 2024 at 09:53:09AM +0100, Krzysztof Kozlowski wrote:
>> On 26/03/2024 08:35, Inochi Amaoto wrote:
>>>>> +
>>>>> +required:
>>>>> +  - '#dma-cells'
>>>>> +  - dma-masters
>>>>> +
>>>>
>>>>
>>>> I don't understand what happened here. Previously you had a child and I
>>>> proposed to properly describe it with $ref.
>>>>
>>>> Now, all children are gone. Binding is supposed to be complete. Based on
>>>> your cover letter, this is not complete, but why? What is missing and
>>>> why it cannot be added?
>>>>
>>>
>>> The binding of syscon is removed due to a usb phy subdevices, which needs 
>>> sometime to figure out the actual property. This is why the syscon binding 
>>> is removed.
>>>
>>> I think it is better to use the origianl syscon series to evolve after
>>> the usb phy binding is submitted. The subdevices of syscon may need
>>> much reverse engineering to know its parameters. So at least for now,
>>> the syscon binding is hard to be complete.
>>
>> Some explanation why dma-router is gone would be useful, but fine.
>>
> 
> OK, I will add some comments on the why it is gone.
> 
>>>
>>>>
>>>>> +additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> +  - |
>>>>> +    dma-router {
>>>>> +      compatible = "sophgo,cv1800-dmamux";
>>>>> +      #dma-cells = <2>;
>>>>> +      dma-masters = <&dmac>;
>>>>> +      dma-requests = <8>;
>>>>> +    };
>>>>> diff --git a/include/dt-bindings/dma/cv1800-dma.h b/include/dt-bindings/dma/cv1800-dma.h
>>>>> new file mode 100644
>>>>> index 000000000000..3ce9dac25259
>>>>> --- /dev/null
>>>>> +++ b/include/dt-bindings/dma/cv1800-dma.h
>>>>
>>>> Filename should match bindings filename.
>>>>
>>>
>>> Thanks.
>>>
>>>>
>>>> Anyway, the problem is that it is a dead header. I don't see it being
>>>> used, so it is not a binding.
>>>>
>>>
>>> This header is not used because the dmamux node is not defined at now.
>>
>> In the driver? The binding header is supposed to be used in the driver,
>> otherwise it is not a binding.
>>
> 
> The driver does use this file.

I checked and could not find. Please point me to specific parts of the code.

> 
>>> And considering the limitation of this dmamux, maybe only devices that 
>>> require dma as a must can have the dma assigned. 
>>> Due to the fact, I think it may be a long time to wait for this header
>>> to be used as the binding header.
>>
>> I don't understand. You did not provide a single reason why this is a
>> binding. Reason is: mapping IDs between DTS and driver. Where is this
>> reason?
>>
> 
> It seems like that I misunderstood something. This file provides one-one
> mapping between the dma device id and cpuid, which is both used in the
> dts and driver. For dts, it provides device id and cpu id mapping. And
> for driver, it is used as the directive to tell how to write the mapping
> register.

So where is it? I looked for DMA_TDM0_RX - nothing. Then DMA_I2C1_RX -
nothing. Then any "DMA_" - also looks nothing.

Best regards,
Krzysztof