[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8e93f459-53e0-ee39-96cc-2c9e51a72547@ti.com>
Date: Wed, 7 Dec 2022 11:39:56 +0530
From: Vignesh Raghavendra <vigneshr@...com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Peter Ujfalusi <peter.ujfalusi@...il.com>,
Vinod Koul <vkoul@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
CC: <dmaengine@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/5] dt-bindings: dma: ti: k3-bcdma: Add bindings for
BCDMA CSI RX
Hi Krzysztof,
On 06/12/22 14:02, Krzysztof Kozlowski wrote:
> On 06/12/2022 05:35, Vignesh Raghavendra wrote:
>> AM62A SoC has a dedicated BCDMA that serves Camera Serial Interface
>> (CSI) IP. Add new compatible for the same. Unlike system
>> BCDMA, this instance only has RX DMA channels and lack TX or block copy
>> channel. Thus make those properties optional. Additionally CSI RX has
>> independent power domain, add the binding for the same.
>>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@...com>
>> ---
>> .../devicetree/bindings/dma/ti/k3-bcdma.yaml | 87 ++++++++++++++-----
>> 1 file changed, 63 insertions(+), 24 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
>> index 08627d91e607..d7b5adbb9b2e 100644
>> --- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
>> +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
>> @@ -32,9 +32,66 @@ allOf:
>> - $ref: /schemas/dma/dma-controller.yaml#
>> - $ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
>>
>
> When adding if:then:, please move entire allOf after "required:" part.
Sure, will do
>
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: ti,am62a-dmss-bcdma-csirx
>> + then:
>> + properties:
>> + ti,sci-rm-range-bchan: false
>> + ti,sci-rm-range-tchan: false
>> +
>> + reg:
>> + maxItems: 3
>> +
>> + reg-names:
>> + items:
>> + - const: gcfg
>> + - const: rchanrt
>> + - const: ringrt
>
> With my changes further this can be only "maxItems: 3"
Yes, but wont that mean any of the 3 reg-names out of the 5? Would it
not be better to further restrict specifically to above 3 reg-names (as
thats how the IP is)
>
>> +
>> + required:
>> + - compatible
>> + - "#dma-cells"
>> + - reg
>> + - reg-names
>> + - msi-parent
>> + - ti,sci
>> + - ti,sci-dev-id
>> + - ti,sci-rm-range-rchan
>> + - power-domains
>> +
>> + else:
>> + properties:
>> + reg:
>> + maxItems: 5
>> +
>> + reg-names:
>> + items:
>> + - const: gcfg
>> + - const: bchanrt
>> + - const: rchanrt
>> + - const: tchanrt
>> + - const: ringrt
>
> With my changes further this can be only "minItems: 5"
Ok.
>
>> +
>> + required:
>> + - compatible
>> + - "#dma-cells"
>> + - reg
>> + - reg-names
>> + - msi-parent
>> + - ti,sci
>> + - ti,sci-dev-id
>> + - ti,sci-rm-range-bchan
>> + - ti,sci-rm-range-tchan
>> + - ti,sci-rm-range-rchan
>> +
>> properties:
>> compatible:
>> - const: ti,am64-dmss-bcdma
>> + enum:
>> + - ti,am64-dmss-bcdma
>> + - ti,am62a-dmss-bcdma-csirx
>
> Keep some order, e.g. alphabetical. This reduces later conflicts on
> simultaneous edits.
Will fix!
>
>>
>> "#dma-cells":
>> const: 3
>> @@ -65,19 +122,13 @@ properties:
>>
>> cell 3: ASEL value for the channel
>>
>> - reg:
>> - maxItems: 5
>
> Keep it here with widest constrains - minItems: 3, maxItems: 5
>
>> -
>> - reg-names:
>> - items:
>> - - const: gcfg
>> - - const: bchanrt
>> - - const: rchanrt
>> - - const: tchanrt
>> - - const: ringrt
>
> Keep the list here with minItems: 3
>
>> -
>> msi-parent: true
>>
>> + power-domains:
>> + description:
>> + Power domain if available
>> + maxItems: 1
>> +
>> ti,asel:
>> $ref: /schemas/types.yaml#/definitions/uint32
>> description: ASEL value for non slave channels
>> @@ -115,18 +166,6 @@ properties:
>> items:
>> maximum: 0x3f
>>
>> -required:
>> - - compatible
>> - - "#dma-cells"
>> - - reg
>> - - reg-names
>> - - msi-parent
>> - - ti,sci
>> - - ti,sci-dev-id
>> - - ti,sci-rm-range-bchan
>> - - ti,sci-rm-range-tchan
>> - - ti,sci-rm-range-rchan
>
> Keep required here. Customize it if needed in if:then:else.
Got it, will fix accordingly...
>
>> -
>> unevaluatedProperties: false
>>
>> examples:
>
> Best regards,
> Krzysztof
>
Thanks for the review!
--
Regards
Vignesh
Powered by blists - more mailing lists