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

From: Sameer Pujar <spujar@nvidia.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	broonie@kernel.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, thierry.reding@gmail.com,
	catalin.marinas@arm.com, will@kernel.org, perex@perex.cz,
	tiwai@suse.com
Cc: jonathanh@nvidia.com, alsa-devel@alsa-project.org,
	devicetree@vger.kernel.org, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/6] ASoC: tegra: Add binding doc for OPE module
Date: Fri, 20 May 2022 09:49:26 +0530	[thread overview]
Message-ID: <90680cfb-c611-63cd-ab5f-5afb86c91cec@nvidia.com> (raw)
In-Reply-To: <049173a1-0fa6-510b-9169-ebe869b8a3b3@linaro.org>

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@nvidia.com>
>> +  - Mohan Kumar <mkumard@nvidia.com>
>> +  - Sameer Pujar <spujar@nvidia.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"

>
>> +
>> +  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@702d8000 {
> I would suggest generic node name, if it is possible.

May be "processing-engine@xxx" ?

> peq@702d8100 {
> Generic node name, so this is "equalizer", I guess?

Yes, I can use this name.