diff mbox series

[11/16] dmaengine: ti: k3-psil-j721e: Add entry for CSI2RX

Message ID 20210330173348.30135-12-p.yadav@ti.com
State Superseded
Headers show
Series CSI2RX support on J721E | expand

Commit Message

Pratyush Yadav March 30, 2021, 5:33 p.m. UTC
The CSI2RX subsystem uses PSI-L DMA to transfer frames to memory. It can
have up to 32 threads but the current driver only supports using one. So
add an entry for that one thread.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 drivers/dma/ti/k3-psil-j721e.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Péter Ujfalusi April 4, 2021, 1:24 p.m. UTC | #1
Hi Pratyush,

On 3/30/21 8:33 PM, Pratyush Yadav wrote:
> The CSI2RX subsystem uses PSI-L DMA to transfer frames to memory. It can
> have up to 32 threads but the current driver only supports using one. So
> add an entry for that one thread.

If you are absolutely sure that the other threads are not going to be
used, then:
Acked-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>

but I would consider adding the other threads if there is a chance that
the cs2rx will need to support it in the future.

> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
>  drivers/dma/ti/k3-psil-j721e.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/dma/ti/k3-psil-j721e.c b/drivers/dma/ti/k3-psil-j721e.c
> index 7580870ed746..19ffa31e6dc6 100644
> --- a/drivers/dma/ti/k3-psil-j721e.c
> +++ b/drivers/dma/ti/k3-psil-j721e.c
> @@ -58,6 +58,14 @@
>  		},					\
>  	}
>  
> +#define PSIL_CSI2RX(x)					\
> +	{						\
> +		.thread_id = x,				\
> +		.ep_config = {				\
> +			.ep_type = PSIL_EP_NATIVE,	\
> +		},					\
> +	}
> +
>  /* PSI-L source thread IDs, used for RX (DMA_DEV_TO_MEM) */
>  static struct psil_ep j721e_src_ep_map[] = {
>  	/* SA2UL */
> @@ -138,6 +146,8 @@ static struct psil_ep j721e_src_ep_map[] = {
>  	PSIL_PDMA_XY_PKT(0x4707),
>  	PSIL_PDMA_XY_PKT(0x4708),
>  	PSIL_PDMA_XY_PKT(0x4709),
> +	/* CSI2RX */
> +	PSIL_CSI2RX(0x4940),
>  	/* CPSW9 */
>  	PSIL_ETHERNET(0x4a00),
>  	/* CPSW0 */
>
Pratyush Yadav April 6, 2021, 3:09 p.m. UTC | #2
On 04/04/21 04:24PM, Péter Ujfalusi wrote:
> Hi Pratyush,
> 
> On 3/30/21 8:33 PM, Pratyush Yadav wrote:
> > The CSI2RX subsystem uses PSI-L DMA to transfer frames to memory. It can
> > have up to 32 threads but the current driver only supports using one. So
> > add an entry for that one thread.
> 
> If you are absolutely sure that the other threads are not going to be
> used, then:

The opposite in fact. I do expect other threads to be used in the 
future. But the current driver can only use one so I figured it is 
better to add just the thread that is currently needed and then I can 
always add the rest later.

Why does this have to be a one-and-done deal? Is there anything wrong 
with adding the other threads when the driver can actually use them?

> Acked-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>
> 
> but I would consider adding the other threads if there is a chance that
> the cs2rx will need to support it in the future.
> 
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > ---
> >  drivers/dma/ti/k3-psil-j721e.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/dma/ti/k3-psil-j721e.c b/drivers/dma/ti/k3-psil-j721e.c
> > index 7580870ed746..19ffa31e6dc6 100644
> > --- a/drivers/dma/ti/k3-psil-j721e.c
> > +++ b/drivers/dma/ti/k3-psil-j721e.c
> > @@ -58,6 +58,14 @@
> >  		},					\
> >  	}
> >  
> > +#define PSIL_CSI2RX(x)					\
> > +	{						\
> > +		.thread_id = x,				\
> > +		.ep_config = {				\
> > +			.ep_type = PSIL_EP_NATIVE,	\
> > +		},					\
> > +	}
> > +
> >  /* PSI-L source thread IDs, used for RX (DMA_DEV_TO_MEM) */
> >  static struct psil_ep j721e_src_ep_map[] = {
> >  	/* SA2UL */
> > @@ -138,6 +146,8 @@ static struct psil_ep j721e_src_ep_map[] = {
> >  	PSIL_PDMA_XY_PKT(0x4707),
> >  	PSIL_PDMA_XY_PKT(0x4708),
> >  	PSIL_PDMA_XY_PKT(0x4709),
> > +	/* CSI2RX */
> > +	PSIL_CSI2RX(0x4940),
> >  	/* CPSW9 */
> >  	PSIL_ETHERNET(0x4a00),
> >  	/* CPSW0 */
> > 
> 
> -- 
> Péter
Péter Ujfalusi April 6, 2021, 3:33 p.m. UTC | #3
On 4/6/21 6:09 PM, Pratyush Yadav wrote:
> On 04/04/21 04:24PM, Péter Ujfalusi wrote:
>> Hi Pratyush,
>>
>> On 3/30/21 8:33 PM, Pratyush Yadav wrote:
>>> The CSI2RX subsystem uses PSI-L DMA to transfer frames to memory. It can
>>> have up to 32 threads but the current driver only supports using one. So
>>> add an entry for that one thread.
>>
>> If you are absolutely sure that the other threads are not going to be
>> used, then:
> 
> The opposite in fact. I do expect other threads to be used in the 
> future. But the current driver can only use one so I figured it is 
> better to add just the thread that is currently needed and then I can 
> always add the rest later.
> 
> Why does this have to be a one-and-done deal? Is there anything wrong 
> with adding the other threads when the driver can actually use them?

You can skip CCing DMAengine (and me ;) ). Less subsystems is the better
when sending patches...

> 
>> Acked-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>
>>
>> but I would consider adding the other threads if there is a chance that
>> the cs2rx will need to support it in the future.
>>
>>> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
>>> ---
>>>  drivers/dma/ti/k3-psil-j721e.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/dma/ti/k3-psil-j721e.c b/drivers/dma/ti/k3-psil-j721e.c
>>> index 7580870ed746..19ffa31e6dc6 100644
>>> --- a/drivers/dma/ti/k3-psil-j721e.c
>>> +++ b/drivers/dma/ti/k3-psil-j721e.c
>>> @@ -58,6 +58,14 @@
>>>  		},					\
>>>  	}
>>>  
>>> +#define PSIL_CSI2RX(x)					\
>>> +	{						\
>>> +		.thread_id = x,				\
>>> +		.ep_config = {				\
>>> +			.ep_type = PSIL_EP_NATIVE,	\
>>> +		},					\
>>> +	}
>>> +
>>>  /* PSI-L source thread IDs, used for RX (DMA_DEV_TO_MEM) */
>>>  static struct psil_ep j721e_src_ep_map[] = {
>>>  	/* SA2UL */
>>> @@ -138,6 +146,8 @@ static struct psil_ep j721e_src_ep_map[] = {
>>>  	PSIL_PDMA_XY_PKT(0x4707),
>>>  	PSIL_PDMA_XY_PKT(0x4708),
>>>  	PSIL_PDMA_XY_PKT(0x4709),
>>> +	/* CSI2RX */
>>> +	PSIL_CSI2RX(0x4940),
>>>  	/* CPSW9 */
>>>  	PSIL_ETHERNET(0x4a00),
>>>  	/* CPSW0 */
>>>
>>
>> -- 
>> Péter
>
Pratyush Yadav April 6, 2021, 4:55 p.m. UTC | #4
On 06/04/21 06:33PM, Péter Ujfalusi wrote:
> 
> 
> On 4/6/21 6:09 PM, Pratyush Yadav wrote:
> > On 04/04/21 04:24PM, Péter Ujfalusi wrote:
> >> Hi Pratyush,
> >>
> >> On 3/30/21 8:33 PM, Pratyush Yadav wrote:
> >>> The CSI2RX subsystem uses PSI-L DMA to transfer frames to memory. It can
> >>> have up to 32 threads but the current driver only supports using one. So
> >>> add an entry for that one thread.
> >>
> >> If you are absolutely sure that the other threads are not going to be
> >> used, then:
> > 
> > The opposite in fact. I do expect other threads to be used in the 
> > future. But the current driver can only use one so I figured it is 
> > better to add just the thread that is currently needed and then I can 
> > always add the rest later.
> > 
> > Why does this have to be a one-and-done deal? Is there anything wrong 
> > with adding the other threads when the driver can actually use them?
> 
> You can skip CCing DMAengine (and me ;) ). Less subsystems is the better
> when sending patches...

I'm a bit confused here. If you are no longer interested in maintaining 
the TI DMA drivers then that's fine, I can skip CCing you. But the patch 
is still relevant to the dmaengine list so why should I skip CCing it? 
And if I don't CC the dmaengine list then on which list would I get 
comments/reviews for the patch?

> 
> > 
> >> Acked-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>
> >>
> >> but I would consider adding the other threads if there is a chance that
> >> the cs2rx will need to support it in the future.
> >>
> >>> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> >>> ---
> >>>  drivers/dma/ti/k3-psil-j721e.c | 10 ++++++++++
> >>>  1 file changed, 10 insertions(+)
> >>>
> >>> diff --git a/drivers/dma/ti/k3-psil-j721e.c b/drivers/dma/ti/k3-psil-j721e.c
> >>> index 7580870ed746..19ffa31e6dc6 100644
> >>> --- a/drivers/dma/ti/k3-psil-j721e.c
> >>> +++ b/drivers/dma/ti/k3-psil-j721e.c
> >>> @@ -58,6 +58,14 @@
> >>>  		},					\
> >>>  	}
> >>>  
> >>> +#define PSIL_CSI2RX(x)					\
> >>> +	{						\
> >>> +		.thread_id = x,				\
> >>> +		.ep_config = {				\
> >>> +			.ep_type = PSIL_EP_NATIVE,	\
> >>> +		},					\
> >>> +	}
> >>> +
> >>>  /* PSI-L source thread IDs, used for RX (DMA_DEV_TO_MEM) */
> >>>  static struct psil_ep j721e_src_ep_map[] = {
> >>>  	/* SA2UL */
> >>> @@ -138,6 +146,8 @@ static struct psil_ep j721e_src_ep_map[] = {
> >>>  	PSIL_PDMA_XY_PKT(0x4707),
> >>>  	PSIL_PDMA_XY_PKT(0x4708),
> >>>  	PSIL_PDMA_XY_PKT(0x4709),
> >>> +	/* CSI2RX */
> >>> +	PSIL_CSI2RX(0x4940),
> >>>  	/* CPSW9 */
> >>>  	PSIL_ETHERNET(0x4a00),
> >>>  	/* CPSW0 */
> >>>
> >>
> >> -- 
> >> Péter
> > 
> 
> -- 
> Péter
Pratyush Yadav April 6, 2021, 5:10 p.m. UTC | #5
On 06/04/21 10:25PM, Pratyush Yadav wrote:
> On 06/04/21 06:33PM, Péter Ujfalusi wrote:
> > 
> > 
> > On 4/6/21 6:09 PM, Pratyush Yadav wrote:
> > > On 04/04/21 04:24PM, Péter Ujfalusi wrote:
> > >> Hi Pratyush,
> > >>
> > >> On 3/30/21 8:33 PM, Pratyush Yadav wrote:
> > >>> The CSI2RX subsystem uses PSI-L DMA to transfer frames to memory. It can
> > >>> have up to 32 threads but the current driver only supports using one. So
> > >>> add an entry for that one thread.
> > >>
> > >> If you are absolutely sure that the other threads are not going to be
> > >> used, then:
> > > 
> > > The opposite in fact. I do expect other threads to be used in the 
> > > future. But the current driver can only use one so I figured it is 
> > > better to add just the thread that is currently needed and then I can 
> > > always add the rest later.
> > > 
> > > Why does this have to be a one-and-done deal? Is there anything wrong 
> > > with adding the other threads when the driver can actually use them?
> > 
> > You can skip CCing DMAengine (and me ;) ). Less subsystems is the better
> > when sending patches...
> 
> I'm a bit confused here. If you are no longer interested in maintaining 
> the TI DMA drivers then that's fine, I can skip CCing you. But the patch 
> is still relevant to the dmaengine list so why should I skip CCing it? 
> And if I don't CC the dmaengine list then on which list would I get 
> comments/reviews for the patch?

Ignore this. Got your point. Will do it in v2.
Péter Ujfalusi April 6, 2021, 7:13 p.m. UTC | #6
On 4/6/21 8:10 PM, Pratyush Yadav wrote:
> On 06/04/21 10:25PM, Pratyush Yadav wrote:
>> On 06/04/21 06:33PM, Péter Ujfalusi wrote:
>>>
>>>
>>> On 4/6/21 6:09 PM, Pratyush Yadav wrote:
>>>> On 04/04/21 04:24PM, Péter Ujfalusi wrote:
>>>>> Hi Pratyush,
>>>>>
>>>>> On 3/30/21 8:33 PM, Pratyush Yadav wrote:
>>>>>> The CSI2RX subsystem uses PSI-L DMA to transfer frames to memory. It can
>>>>>> have up to 32 threads but the current driver only supports using one. So
>>>>>> add an entry for that one thread.
>>>>>
>>>>> If you are absolutely sure that the other threads are not going to be
>>>>> used, then:
>>>>
>>>> The opposite in fact. I do expect other threads to be used in the 
>>>> future. But the current driver can only use one so I figured it is 
>>>> better to add just the thread that is currently needed and then I can 
>>>> always add the rest later.
>>>>
>>>> Why does this have to be a one-and-done deal? Is there anything wrong 
>>>> with adding the other threads when the driver can actually use them?
>>>
>>> You can skip CCing DMAengine (and me ;) ). Less subsystems is the better
>>> when sending patches...
>>
>> I'm a bit confused here. If you are no longer interested in maintaining 
>> the TI DMA drivers then that's fine, I can skip CCing you. But the patch 
>> is still relevant to the dmaengine list so why should I skip CCing it? 
>> And if I don't CC the dmaengine list then on which list would I get 
>> comments/reviews for the patch?
> 
> Ignore this. Got your point. Will do it in v2.

If you know that you will be adding the other threads in the near future
then do it with one patch. When you add the support in the csi2rx driver
then you don't need to change the DMAengine files, thus no need to CC
dmaengine or me, thus you need to gather less acked-bys, thus the time
to mainline might be shorter.
diff mbox series

Patch

diff --git a/drivers/dma/ti/k3-psil-j721e.c b/drivers/dma/ti/k3-psil-j721e.c
index 7580870ed746..19ffa31e6dc6 100644
--- a/drivers/dma/ti/k3-psil-j721e.c
+++ b/drivers/dma/ti/k3-psil-j721e.c
@@ -58,6 +58,14 @@ 
 		},					\
 	}
 
+#define PSIL_CSI2RX(x)					\
+	{						\
+		.thread_id = x,				\
+		.ep_config = {				\
+			.ep_type = PSIL_EP_NATIVE,	\
+		},					\
+	}
+
 /* PSI-L source thread IDs, used for RX (DMA_DEV_TO_MEM) */
 static struct psil_ep j721e_src_ep_map[] = {
 	/* SA2UL */
@@ -138,6 +146,8 @@  static struct psil_ep j721e_src_ep_map[] = {
 	PSIL_PDMA_XY_PKT(0x4707),
 	PSIL_PDMA_XY_PKT(0x4708),
 	PSIL_PDMA_XY_PKT(0x4709),
+	/* CSI2RX */
+	PSIL_CSI2RX(0x4940),
 	/* CPSW9 */
 	PSIL_ETHERNET(0x4a00),
 	/* CPSW0 */