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] [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

Powered by Openwall GNU/*/Linux Powered by OpenVZ