linux-kernel - Re: [PATCH 1/6] ASoC: tegra: Add binding doc for OPE module

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:   Fri, 20 May 2022 08:51:30 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Sameer Pujar <spujar@...dia.com>, broonie@...nel.org,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        thierry.reding@...il.com, catalin.marinas@....com, will@...nel.org,
        perex@...ex.cz, tiwai@...e.com
Cc:     jonathanh@...dia.com, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/6] ASoC: tegra: Add binding doc for OPE module

On 20/05/2022 06:19, Sameer Pujar wrote:
> Thanks Krzysztof for review.
> 
> 
> On 19-05-2022 17:10, Krzysztof Kozlowski wrote:
>> On 18/05/2022 19:36, Sameer Pujar wrote:
>>> +description: |
>>> +  The Multi Band Dynamic Range Compressor (MBDRC) is part of Output
>>> +  Processing Engine (OPE) which interfaces with Audio Hub (AHUB) via
>>> +  Audio Client Interface (ACIF). MBDRC can be used as a traditional
>>> +  single full band or a dual band or a multi band dynamic processor.
>>> +
>>> +maintainers:
>>> +  - Jon Hunter <jonathanh@...dia.com>
>>> +  - Mohan Kumar <mkumard@...dia.com>
>>> +  - Sameer Pujar <spujar@...dia.com>
>>> +
>>> +properties:
>>> +  $nodename:
>>> +    pattern: "^mbdrc@[0-9a-f]*$"
>> Why? We enforce only generic names in shared schemas and this is neither
>> shared schema nor is it generic name.
> 
> Idea was to keep these node names consistent across DT files and parent 
> node can allow a given list of child nodes with strict checks. Does name 
> like "dynamic-range-compressor@xxx"

The checks are not coming from device node name, but from matching
schema to compatible. Why do you need consistent names across DTS files?
They should be anyway generic but what happens if they differ?

Additionally, the parent schema enforces nodes of children, so if this
is included in other schema, then the change is pointless.

I propose to drop it, unless it is a shared schema for many different
vendors.

>>
>>> +
>>> +  compatible:
>>> +    oneOf:
>>> +      - const: nvidia,tegra210-mbdrc
>>> +      - items:
>>> +          - enum:
>>> +              - nvidia,tegra234-mbdrc
>>> +              - nvidia,tegra194-mbdrc
>>> +              - nvidia,tegra186-mbdrc
>>> +          - const: nvidia,tegra210-mbdrc
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +
>> No need for space
> 
> will remove
> 
> 
>>> +
>>> +  compatible:
>>> +    oneOf:
>>> +      - const: nvidia,tegra210-ope
>>> +      - items:
>>> +          - enum:
>>> +              - nvidia,tegra234-ope
>>> +              - nvidia,tegra194-ope
>>> +              - nvidia,tegra186-ope
>>> +          - const: nvidia,tegra210-ope
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  "#address-cells":
>>> +    const: 1
>>> +
>>> +  "#size-cells":
>>> +    const: 1
>>> +
>>> +  ranges: true
>>> +
>>> +  sound-name-prefix:
>>> +    pattern: "^OPE[1-9]$"
>>> +
>>> +  ports:
>>> +    $ref: /schemas/graph.yaml#/properties/ports
>>> +    properties:
>>> +      port@0:
>>> +        $ref: audio-graph-port.yaml#
>>> +        unevaluatedProperties: false
>>> +        description: |
>> No need for |
> 
> will remove.
> 
> 
>>
>>> +    ope@...d8000 {
>> I would suggest generic node name, if it is possible.
> 
> May be "processing-engine@xxx" ?

Sure.


Best regards,
Krzysztof