[PATCH 1/2] dt-bindings: dma: ti: k3-bcdma: Add TX channel for AM62A CSIRX BCDMA

[PATCH 1/2] dt-bindings: dma: ti: k3-bcdma: Add TX channel for AM62A CSIRX BCDMA

[Vaishnav Achath]

J722S CSIRX BCDMA is based on AM62A BCDMA and supports CSI TX channels
in addition to currently supported RX channels. Add TX channel
properties as optional properties in the list so that the same
compatible can be reused. K3 UDMA makes use of TCHAN_CNT
capabilities register to identify whether platform supports
TX channels.

Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
---
 Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
index 27b8e1636560..c748f78b313e 100644
--- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
+++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
@@ -138,19 +138,22 @@ allOf:
     then:
       properties:
         ti,sci-rm-range-bchan: false
-        ti,sci-rm-range-tchan: false
 
         reg:
+          minItems: 3
           items:
             - description: BCDMA Control /Status Registers region
             - description: RX Channel Realtime Registers region
             - description: Ring Realtime Registers region
+            - description: TX Channel Realtime Registers region
 
         reg-names:
+          minItems: 3
           items:
             - const: gcfg
             - const: rchanrt
             - const: ringrt
+            - const: tchanrt
 
       required:
         - power-domains
-- 
2.34.1

[PATCH 2/2] dmaengine: ti: k3-udma: Add TX channel data in AM62A CSIRX DMSS

[Vaishnav Achath]

J722S/AM67 uses the same BCDMA CSIRX IP as AM62A, but it supports
TX channels as well in addition to RX. Add the BCDMA TCHAN information
in the am62a_dmss_csi_soc_data so as to support all the platforms in the
family with same compatible. UDMA_CAP2_TCHAN_CNT indicates the presence
of TX channels and it will be 0 for platforms without TX support.

Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
---

CSI2RX capture test results on J722S EVM with IMX219:
https://gist.github.com/vaishnavachath/e2eaed62ee8f53428ee9b830aaa02cc3

 drivers/dma/ti/k3-udma.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index b3f27b3f9209..4130f50979d4 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -4340,6 +4340,8 @@ static struct udma_match_data j721e_mcu_data = {
 
 static struct udma_soc_data am62a_dmss_csi_soc_data = {
 	.oes = {
+		.bcdma_tchan_data = 0x800,
+		.bcdma_tchan_ring = 0xa00,
 		.bcdma_rchan_data = 0xe00,
 		.bcdma_rchan_ring = 0x1000,
 	},
-- 
2.34.1

Re: [PATCH 2/2] dmaengine: ti: k3-udma: Add TX channel data in AM62A CSIRX DMSS

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]

On Mon, Nov 25, 2024 at 02:09:14PM +0530, Vaishnav Achath wrote:
> J722S/AM67 uses the same BCDMA CSIRX IP as AM62A, but it supports
> TX channels as well in addition to RX.

This doesn't make sense. You say that the am62a doesn't have a tx
channel ("but it supports TX as well") but then modify the struct for
the am62a to add a tx channel. Does that not break things on the am62a?


> Add the BCDMA TCHAN information
> in the am62a_dmss_csi_soc_data so as to support all the platforms in the
> family with same compatible. UDMA_CAP2_TCHAN_CNT indicates the presence
> of TX channels and it will be 0 for platforms without TX support.
> 
> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
> ---
> 
> CSI2RX capture test results on J722S EVM with IMX219:
> https://gist.github.com/vaishnavachath/e2eaed62ee8f53428ee9b830aaa02cc3
> 
>  drivers/dma/ti/k3-udma.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
> index b3f27b3f9209..4130f50979d4 100644
> --- a/drivers/dma/ti/k3-udma.c
> +++ b/drivers/dma/ti/k3-udma.c
> @@ -4340,6 +4340,8 @@ static struct udma_match_data j721e_mcu_data = {
>  
>  static struct udma_soc_data am62a_dmss_csi_soc_data = {
>  	.oes = {
> +		.bcdma_tchan_data = 0x800,
> +		.bcdma_tchan_ring = 0xa00,
>  		.bcdma_rchan_data = 0xe00,
>  		.bcdma_rchan_ring = 0x1000,
>  	},
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] dmaengine: ti: k3-udma: Add TX channel data in AM62A CSIRX DMSS

[Vaishnav Achath]

Hi Conor,

On 26/11/24 00:01, Conor Dooley wrote:
> On Mon, Nov 25, 2024 at 02:09:14PM +0530, Vaishnav Achath wrote:
>> J722S/AM67 uses the same BCDMA CSIRX IP as AM62A, but it supports
>> TX channels as well in addition to RX.
> 
> This doesn't make sense. You say that the am62a doesn't have a tx
> channel ("but it supports TX as well") but then modify the struct for
> the am62a to add a tx channel. Does that not break things on the am62a?
> 

Thank you for the review, I have sent a v2 of this series adding new 
compatible as suggested, after looking at it again, the J722S BCDMA CSI
is more similar to J721S2 in terms of having RX and TX support, so 
updated in that way.

The below changes did not really break AM62A since the driver checks 
hardware capability registers (TCHAN_CNT) to detect presence of TX 
channels and then only use the Output Event Steering(OES) data below.

V2:

https://lore.kernel.org/all/20241126125158.37744-1-vaishnav.a@ti.com/

Thanks and Regards,
Vaishnav

> 
>> Add the BCDMA TCHAN information
>> in the am62a_dmss_csi_soc_data so as to support all the platforms in the
>> family with same compatible. UDMA_CAP2_TCHAN_CNT indicates the presence
>> of TX channels and it will be 0 for platforms without TX support.
>>
>> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
>> ---
>>
>> CSI2RX capture test results on J722S EVM with IMX219:
>> https://gist.github.com/vaishnavachath/e2eaed62ee8f53428ee9b830aaa02cc3
>>
>>   drivers/dma/ti/k3-udma.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
>> index b3f27b3f9209..4130f50979d4 100644
>> --- a/drivers/dma/ti/k3-udma.c
>> +++ b/drivers/dma/ti/k3-udma.c
>> @@ -4340,6 +4340,8 @@ static struct udma_match_data j721e_mcu_data = {
>>   
>>   static struct udma_soc_data am62a_dmss_csi_soc_data = {
>>   	.oes = {
>> +		.bcdma_tchan_data = 0x800,
>> +		.bcdma_tchan_ring = 0xa00,
>>   		.bcdma_rchan_data = 0xe00,
>>   		.bcdma_rchan_ring = 0x1000,
>>   	},
>> -- 
>> 2.34.1
>>

Re: [PATCH 1/2] dt-bindings: dma: ti: k3-bcdma: Add TX channel for AM62A CSIRX BCDMA

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]

On Mon, Nov 25, 2024 at 02:09:13PM +0530, Vaishnav Achath wrote:
> J722S CSIRX BCDMA is based on AM62A BCDMA and supports CSI TX channels

There's no specific compatible in this file for a j722s, you should add
one.

> in addition to currently supported RX channels. Add TX channel
> properties as optional properties in the list so that the same
> compatible can be reused. K3 UDMA makes use of TCHAN_CNT
> capabilities register to identify whether platform supports
> TX channels.
> 
> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
> ---
>  Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> index 27b8e1636560..c748f78b313e 100644
> --- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> @@ -138,19 +138,22 @@ allOf:
>      then:
>        properties:
>          ti,sci-rm-range-bchan: false
> -        ti,sci-rm-range-tchan: false
>  
>          reg:
> +          minItems: 3

You need to then constrain maxItems to 3 for all !j722s devices in an
if/then/else to avoid allowing 4 reg entries where it is not valid.

Thanks,
Conor.

>            items:
>              - description: BCDMA Control /Status Registers region
>              - description: RX Channel Realtime Registers region
>              - description: Ring Realtime Registers region
> +            - description: TX Channel Realtime Registers region
>  
>          reg-names:
> +          minItems: 3
>            items:
>              - const: gcfg
>              - const: rchanrt
>              - const: ringrt
> +            - const: tchanrt
>  
>        required:
>          - power-domains
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]