[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <93a7e063-02bb-4052-a3ed-a543a038c3ba@kernel.org>
Date: Mon, 24 Jun 2024 13:08:43 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Shresth Prasad <shresthprasad7@...il.com>
Cc: vkoul@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, dmaengine@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, skhan@...uxfoundation.org,
javier.carrasco.cruz@...il.com
Subject: Re: [PATCH] dt-bindings: dma: mv-xor-v2: Convert to dtschema
On 24/06/2024 12:14, Shresth Prasad wrote:
> On Mon, Jun 24, 2024 at 10:47 AM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>>
>> On 23/06/2024 14:45, Shresth Prasad wrote:
>>> Convert txt bindings of Marvell XOR v2 engines to dtschema to allow
>>> for validation.
>>>
>>> Signed-off-by: Shresth Prasad <shresthprasad7@...il.com>
>>> ---
>>> Tested against `marvell/armada-7040-db.dtb`, `marvell/armada-7040-mochabin.dtb`
>>> and `marvell/armada-8080-db.dtb`
>>>
>>> .../bindings/dma/marvell,xor-v2.yaml | 69 +++++++++++++++++++
>>> .../devicetree/bindings/dma/mv-xor-v2.txt | 28 --------
>>> 2 files changed, 69 insertions(+), 28 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/dma/marvell,xor-v2.yaml
>>> delete mode 100644 Documentation/devicetree/bindings/dma/mv-xor-v2.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/dma/marvell,xor-v2.yaml b/Documentation/devicetree/bindings/dma/marvell,xor-v2.yaml
>>> new file mode 100644
>>> index 000000000000..3d7481c1917e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/dma/marvell,xor-v2.yaml
>>> @@ -0,0 +1,69 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/dma/marvell,xor-v2.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Marvell XOR v2 engines
>>> +
>>> +maintainers:
>>> + - Vinod Koul <vkoul@...nel.org>
>>
>> Should be rather platform maintainer.
>>
>>> +
>>> +properties:
>>> + compatible:
>>> + contains:
>>
>> This cannot be unspecific. Drop contains.
>>
>>> + enum:
>>> + - marvell,armada-7k-xor
>>> + - marvell,xor-v2
>>> +
>>> + reg:
>>> + items:
>>> + - description: DMA registers location and length
>>> + - description: global registers location and length
>>
>> Drop "location and length", redundant.
>>
>>> +
>>> + clocks:
>>> + minItems: 1
>>> + maxItems: 2
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: core
>>> + - const: reg
>>
>> This does not match number of items in clocks:
>
> I'm not sure what you mean, the original txt stated that `clock-names`
> is only required if there are two `clocks`.
Exactly. It said "required", not "disallowed for 1 clock case". You
basically made it impossible to use for one case, so standard reply:
these should be always in sync.
>
>>
>>> +
>>> + msi-parent:
>>> + description:
>>> + Phandle to the MSI-capable interrupt controller used for
>>> + interrupts.
>>> + maxItems: 1
>>> +
>>> + dma-coherent: true
>>
>> This was not present in the binding and commit msg did not explain why
>> this is needed. Are devices really DMA coherent?
>
> Sorry about that, I added this because all the nodes I looked at
> contained `dma-coherent`.
>
>>
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - msi-parent
>>> + - dma-coherent
>>> +
>>> +if:
>>
>> Put it under allOf: in this place.
>>
>>> + required:
>>> + - clocks
>>
>> This does not work and does not make much sense. Probably you want to
>> list the items per variant?
>>
>>
>>> + properties:
>>> + clocks:
>>> + minItems: 2
>>> + maxItems: 2
>>
>> Instead list and describe the items.
>>
>
> I did it this way to allow for `clock-names` to only be required if there
> are two `clocks` present. Is there another way I should be doing this?
Why number of clocks would mean you need clock-names? Why does it
matter? If the driver is taking second clock by name, it does not mean
second clock name can be anything for other cases.
Best regards,
Krzysztof
Powered by blists - more mailing lists