| Message ID | 20210330173348.30135-12-p.yadav@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | CSI2RX support on J721E | expand |
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 */ >
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
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 >
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
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.
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 --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 */
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(+)