Patchwork dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1

login
register
mail settings
Submitter Peter Ujfalusi
Date Nov. 19, 2018, 10:40 a.m.
Message ID <20181119104040.12885-1-peter.ujfalusi@ti.com>
Download mbox | patch
Permalink /patch/659659/
State New
Headers show

Comments

Peter Ujfalusi - Nov. 19, 2018, 10:40 a.m.
When the channel is configured for slave operation the LCH_TYPE needs to be
set to LCh-P. For memcpy channels the LCH_TYPE must be set to LCh-2D.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/ti/omap-dma.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
Aaro Koskinen - Nov. 19, 2018, 6:46 p.m.
Hi,

On Mon, Nov 19, 2018 at 12:40:40PM +0200, Peter Ujfalusi wrote:
> When the channel is configured for slave operation the LCH_TYPE needs to be
> set to LCh-P. For memcpy channels the LCH_TYPE must be set to LCh-2D.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

I don't have the documentation, but based on what omap_udc driver (still
using the legacy OMAP DMA API) does this seems to be correct.

I tested the patch on Nokia 770 with MMC and couldn't see any negative
impact.

Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>

A.

> ---
>  drivers/dma/ti/omap-dma.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> index a4a931ddf6f6..a18cfd497f04 100644
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c
> @@ -185,6 +185,10 @@ enum {
>  
>  	CLNK_CTRL_ENABLE_LNK	= BIT(15),
>  
> +	/* OMAP1 only */
> +	LCH_CTRL_LCH_2D		= 0,
> +	LCH_CTRL_LCH_P		= 2,
> +
>  	CDP_DST_VALID_INC	= 0 << 0,
>  	CDP_DST_VALID_RELOAD	= 1 << 0,
>  	CDP_DST_VALID_REUSE	= 2 << 0,
> @@ -529,6 +533,7 @@ static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d)
>  
>  static void omap_dma_start_desc(struct omap_chan *c)
>  {
> +	struct omap_dmadev *od = to_omap_dma_dev(c->vc.chan.device);
>  	struct virt_dma_desc *vd = vchan_next_desc(&c->vc);
>  	struct omap_desc *d;
>  	unsigned cxsa, cxei, cxfi;
> @@ -570,6 +575,12 @@ static void omap_dma_start_desc(struct omap_chan *c)
>  	omap_dma_chan_write(c, CSDP, d->csdp);
>  	omap_dma_chan_write(c, CLNK_CTRL, d->clnk_ctrl);
>  
> +	if (dma_omap1() && !__dma_omap15xx(od->plat->dma_attr)) {
> +		if (is_slave_direction(d->dir))
> +			omap_dma_chan_write(c, LCH_CTRL, LCH_CTRL_LCH_P);
> +		else
> +			omap_dma_chan_write(c, LCH_CTRL, LCH_CTRL_LCH_2D);
> +	}
>  	omap_dma_start_sg(c, d);
>  }
>  
> -- 
> Peter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
Peter Ujfalusi - Nov. 20, 2018, 7:28 a.m.
Aaro,

On 19/11/2018 20.46, Aaro Koskinen wrote:
> Hi,
> 
> On Mon, Nov 19, 2018 at 12:40:40PM +0200, Peter Ujfalusi wrote:
>> When the channel is configured for slave operation the LCH_TYPE needs to be
>> set to LCh-P. For memcpy channels the LCH_TYPE must be set to LCh-2D.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> 
> I don't have the documentation, but based on what omap_udc driver (still
> using the legacy OMAP DMA API) does this seems to be correct.

They are hard to fine, true. From the omap1710 TRM:

Logical channel types (LCh types) supported are:
- LCh-2D for nonsynchronized transfers (memory transfers, 1D and 2D)
- LCh-P for synchronized transfers (mostly peripheral transfers)
- LCh-PD similar to LCh-P but runs on a dedicated physical channel
- LCh-G for graphical transfers/operations
- LCh-D for display transfers

Looking at other part it looks like hat LCH-2D channel mode can happily
service a peripheral. LCH-P supports the same features as LCH-2D, but it
lacks support for Single/Double-indexed addressing mode on the
peripheral port side.

So, this patch might not be needed at all. Can you test the omap_udc
with s/OMAP_DMA_LCH_P/OMAP_DMA_LCH_2D

If USB works, then we can just drop this patch.

Note: if we ever need the port_window support in OMAP1 then we need
double indexing on the peripheral side.

> I tested the patch on Nokia 770 with MMC and couldn't see any negative
> impact.
> 
> Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> 
> A.
> 
>> ---
>>  drivers/dma/ti/omap-dma.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
>> index a4a931ddf6f6..a18cfd497f04 100644
>> --- a/drivers/dma/ti/omap-dma.c
>> +++ b/drivers/dma/ti/omap-dma.c
>> @@ -185,6 +185,10 @@ enum {
>>  
>>  	CLNK_CTRL_ENABLE_LNK	= BIT(15),
>>  
>> +	/* OMAP1 only */
>> +	LCH_CTRL_LCH_2D		= 0,
>> +	LCH_CTRL_LCH_P		= 2,
>> +
>>  	CDP_DST_VALID_INC	= 0 << 0,
>>  	CDP_DST_VALID_RELOAD	= 1 << 0,
>>  	CDP_DST_VALID_REUSE	= 2 << 0,
>> @@ -529,6 +533,7 @@ static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d)
>>  
>>  static void omap_dma_start_desc(struct omap_chan *c)
>>  {
>> +	struct omap_dmadev *od = to_omap_dma_dev(c->vc.chan.device);
>>  	struct virt_dma_desc *vd = vchan_next_desc(&c->vc);
>>  	struct omap_desc *d;
>>  	unsigned cxsa, cxei, cxfi;
>> @@ -570,6 +575,12 @@ static void omap_dma_start_desc(struct omap_chan *c)
>>  	omap_dma_chan_write(c, CSDP, d->csdp);
>>  	omap_dma_chan_write(c, CLNK_CTRL, d->clnk_ctrl);
>>  
>> +	if (dma_omap1() && !__dma_omap15xx(od->plat->dma_attr)) {
>> +		if (is_slave_direction(d->dir))
>> +			omap_dma_chan_write(c, LCH_CTRL, LCH_CTRL_LCH_P);
>> +		else
>> +			omap_dma_chan_write(c, LCH_CTRL, LCH_CTRL_LCH_2D);
>> +	}
>>  	omap_dma_start_sg(c, d);
>>  }
>>  
>> -- 
>> Peter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Aaro Koskinen - Nov. 20, 2018, 9:04 p.m.
Hi,

On Tue, Nov 20, 2018 at 09:28:37AM +0200, Peter Ujfalusi wrote:
> On 19/11/2018 20.46, Aaro Koskinen wrote:
> > On Mon, Nov 19, 2018 at 12:40:40PM +0200, Peter Ujfalusi wrote:
> >> When the channel is configured for slave operation the LCH_TYPE needs to be
> >> set to LCh-P. For memcpy channels the LCH_TYPE must be set to LCh-2D.
> >>
> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > 
> > I don't have the documentation, but based on what omap_udc driver (still
> > using the legacy OMAP DMA API) does this seems to be correct.
> 
> They are hard to fine, true. From the omap1710 TRM:
> 
> Logical channel types (LCh types) supported are:
> - LCh-2D for nonsynchronized transfers (memory transfers, 1D and 2D)
> - LCh-P for synchronized transfers (mostly peripheral transfers)
> - LCh-PD similar to LCh-P but runs on a dedicated physical channel
> - LCh-G for graphical transfers/operations
> - LCh-D for display transfers

(I found a public document "OMAP5912 Multimedia Processor Direct
Memory Access (DMA) Support Reference Guide", documenting these; easy
to confuse with "OMAP5910 Dual-Core Processor System DMA Controller
Reference Guide".)

> Looking at other part it looks like hat LCH-2D channel mode can happily
> service a peripheral. LCH-P supports the same features as LCH-2D, but it
> lacks support for Single/Double-indexed addressing mode on the
> peripheral port side.
> 
> So, this patch might not be needed at all. Can you test the omap_udc
> with s/OMAP_DMA_LCH_P/OMAP_DMA_LCH_2D
> 
> If USB works, then we can just drop this patch.

Unfortunately omap_udc does not seem to work at all anymore with DMA on
my 770 setup. :-(

I had switched to PIO mode in 2015 since the WARNs about legacy DMA
API were too annoying and flooding the console. And now that I tried
using DMA again with g_ether, it doesn't work anymore. The device get's
recognized on host side, but no traffic goes through. Switching back to
PIO makes it to work again.

A.
Peter Ujfalusi - Nov. 22, 2018, 8:31 a.m.
On 20/11/2018 23.04, Aaro Koskinen wrote:
> Hi,
> 
> On Tue, Nov 20, 2018 at 09:28:37AM +0200, Peter Ujfalusi wrote:
>> On 19/11/2018 20.46, Aaro Koskinen wrote:
>>> On Mon, Nov 19, 2018 at 12:40:40PM +0200, Peter Ujfalusi wrote:
>>>> When the channel is configured for slave operation the LCH_TYPE needs to be
>>>> set to LCh-P. For memcpy channels the LCH_TYPE must be set to LCh-2D.
>>>>
>>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>>
>>> I don't have the documentation, but based on what omap_udc driver (still
>>> using the legacy OMAP DMA API) does this seems to be correct.
>>
>> They are hard to fine, true. From the omap1710 TRM:
>>
>> Logical channel types (LCh types) supported are:
>> - LCh-2D for nonsynchronized transfers (memory transfers, 1D and 2D)
>> - LCh-P for synchronized transfers (mostly peripheral transfers)
>> - LCh-PD similar to LCh-P but runs on a dedicated physical channel
>> - LCh-G for graphical transfers/operations
>> - LCh-D for display transfers
> 
> (I found a public document "OMAP5912 Multimedia Processor Direct
> Memory Access (DMA) Support Reference Guide", documenting these; easy
> to confuse with "OMAP5910 Dual-Core Processor System DMA Controller
> Reference Guide".)
> 
>> Looking at other part it looks like hat LCH-2D channel mode can happily
>> service a peripheral. LCH-P supports the same features as LCH-2D, but it
>> lacks support for Single/Double-indexed addressing mode on the
>> peripheral port side.
>>
>> So, this patch might not be needed at all. Can you test the omap_udc
>> with s/OMAP_DMA_LCH_P/OMAP_DMA_LCH_2D
>>
>> If USB works, then we can just drop this patch.
> 
> Unfortunately omap_udc does not seem to work at all anymore with DMA on
> my 770 setup. :-(
> 
> I had switched to PIO mode in 2015 since the WARNs about legacy DMA
> API were too annoying and flooding the console. And now that I tried
> using DMA again with g_ether, it doesn't work anymore. The device get's
> recognized on host side, but no traffic goes through. Switching back to
> PIO makes it to work again.

After some tinkering I got omap_udc working with DMA (not DMAengine,
yet) on 770 - using nfsroot. Configuring the channels to OMAP_DMA_LCH_2D
works as expected.

This patch can be dropped.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Russell King - ARM Linux - Nov. 22, 2018, 10:29 a.m.
On Tue, Nov 20, 2018 at 11:04:06PM +0200, Aaro Koskinen wrote:
> I had switched to PIO mode in 2015 since the WARNs about legacy DMA
> API were too annoying and flooding the console. And now that I tried
> using DMA again with g_ether, it doesn't work anymore. The device get's
> recognized on host side, but no traffic goes through. Switching back to
> PIO makes it to work again.

A solution to that would be to do what the warning message says, and
update the driver to the DMAengine API.

The reason it didn't get updated when the DMAengine conversion happened
is because I don't have hardware for it, so had no way to test, and no
one seemed to know that anyone was using it.  Eventually, the WARN_ON()
was added to try and root out any users and generate interest in
updating the drivers.  Obviously that didn't happen, because people
just worked around the warning rather than saying anything.

I'm afraid we're long past the time that I'd be willing to update the
omap_udc driver now as I've dropped most of my knowledge on that as
it's been four years, and Peter has been looking after OMAP DMAengine
issues since.

Sorry.
Aaro Koskinen - Nov. 22, 2018, 10:01 p.m.
Hi,

On Thu, Nov 22, 2018 at 10:31:31AM +0200, Peter Ujfalusi wrote:
> On 20/11/2018 23.04, Aaro Koskinen wrote:
> > On Tue, Nov 20, 2018 at 09:28:37AM +0200, Peter Ujfalusi wrote:
> >> On 19/11/2018 20.46, Aaro Koskinen wrote:
> >>> On Mon, Nov 19, 2018 at 12:40:40PM +0200, Peter Ujfalusi wrote:
> >>>> When the channel is configured for slave operation the LCH_TYPE needs to be
> >>>> set to LCh-P. For memcpy channels the LCH_TYPE must be set to LCh-2D.
> >>>>
> >>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> >>>
> >>> I don't have the documentation, but based on what omap_udc driver (still
> >>> using the legacy OMAP DMA API) does this seems to be correct.
> >>
> >> They are hard to fine, true. From the omap1710 TRM:
> >>
> >> Logical channel types (LCh types) supported are:
> >> - LCh-2D for nonsynchronized transfers (memory transfers, 1D and 2D)
> >> - LCh-P for synchronized transfers (mostly peripheral transfers)
> >> - LCh-PD similar to LCh-P but runs on a dedicated physical channel
> >> - LCh-G for graphical transfers/operations
> >> - LCh-D for display transfers
> > 
> > (I found a public document "OMAP5912 Multimedia Processor Direct
> > Memory Access (DMA) Support Reference Guide", documenting these; easy
> > to confuse with "OMAP5910 Dual-Core Processor System DMA Controller
> > Reference Guide".)
> > 
> >> Looking at other part it looks like hat LCH-2D channel mode can happily
> >> service a peripheral. LCH-P supports the same features as LCH-2D, but it
> >> lacks support for Single/Double-indexed addressing mode on the
> >> peripheral port side.
> >>
> >> So, this patch might not be needed at all. Can you test the omap_udc
> >> with s/OMAP_DMA_LCH_P/OMAP_DMA_LCH_2D
> >>
> >> If USB works, then we can just drop this patch.
> > 
> > Unfortunately omap_udc does not seem to work at all anymore with DMA on
> > my 770 setup. :-(
> > 
> > I had switched to PIO mode in 2015 since the WARNs about legacy DMA
> > API were too annoying and flooding the console. And now that I tried
> > using DMA again with g_ether, it doesn't work anymore. The device get's
> > recognized on host side, but no traffic goes through. Switching back to
> > PIO makes it to work again.
> 
> After some tinkering I got omap_udc working with DMA (not DMAengine,
> yet) on 770 - using nfsroot. Configuring the channels to OMAP_DMA_LCH_2D
> works as expected.

Would be interesting to know how you got it working with DMA. Which
gadget driver were you using?

I bisected my issue, and got:

commit 387f869d2579e379ee343f5493dcd360be60f5c6 (refs/bisect/bad)
Author: Felipe Balbi <felipe.balbi@linux.intel.com>
Date:   Wed Mar 22 13:25:18 2017 +0200

    usb: gadget: u_ether: conditionally align transfer size

With that reverted, the DMA works OK (and I can also now confirm that
OMAP_DMA_LCH_2D works). I haven't yet checked if we actually need that
quirk in OMAP UDC, or if this is related to RMK's findings of potential
bugs in the driver. Anyway, there is clearly yet another regression.

A.
Peter Ujfalusi - Nov. 23, 2018, 11:49 a.m.
On 22/11/2018 12.29, Russell King - ARM Linux wrote:
> On Tue, Nov 20, 2018 at 11:04:06PM +0200, Aaro Koskinen wrote:
>> I had switched to PIO mode in 2015 since the WARNs about legacy DMA
>> API were too annoying and flooding the console. And now that I tried
>> using DMA again with g_ether, it doesn't work anymore. The device get's
>> recognized on host side, but no traffic goes through. Switching back to
>> PIO makes it to work again.
> 
> A solution to that would be to do what the warning message says, and
> update the driver to the DMAengine API.

Yep, omap_udc is the last user of legacy omap_dma API. It is a slow
progress as I do the conversion in my free time, onenand/omap2 and tusb
was converted not too long ago, let's see how the omap_udc is going to go.

> The reason it didn't get updated when the DMAengine conversion happened
> is because I don't have hardware for it, so had no way to test, and no
> one seemed to know that anyone was using it.  Eventually, the WARN_ON()
> was added to try and root out any users and generate interest in
> updating the drivers.  Obviously that didn't happen, because people
> just worked around the warning rather than saying anything.
> 
> I'm afraid we're long past the time that I'd be willing to update the
> omap_udc driver now as I've dropped most of my knowledge on that as
> it's been four years, and Peter has been looking after OMAP DMAengine
> issues since.
> 
> Sorry.
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Patch

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index a4a931ddf6f6..a18cfd497f04 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -185,6 +185,10 @@  enum {
 
 	CLNK_CTRL_ENABLE_LNK	= BIT(15),
 
+	/* OMAP1 only */
+	LCH_CTRL_LCH_2D		= 0,
+	LCH_CTRL_LCH_P		= 2,
+
 	CDP_DST_VALID_INC	= 0 << 0,
 	CDP_DST_VALID_RELOAD	= 1 << 0,
 	CDP_DST_VALID_REUSE	= 2 << 0,
@@ -529,6 +533,7 @@  static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d)
 
 static void omap_dma_start_desc(struct omap_chan *c)
 {
+	struct omap_dmadev *od = to_omap_dma_dev(c->vc.chan.device);
 	struct virt_dma_desc *vd = vchan_next_desc(&c->vc);
 	struct omap_desc *d;
 	unsigned cxsa, cxei, cxfi;
@@ -570,6 +575,12 @@  static void omap_dma_start_desc(struct omap_chan *c)
 	omap_dma_chan_write(c, CSDP, d->csdp);
 	omap_dma_chan_write(c, CLNK_CTRL, d->clnk_ctrl);
 
+	if (dma_omap1() && !__dma_omap15xx(od->plat->dma_attr)) {
+		if (is_slave_direction(d->dir))
+			omap_dma_chan_write(c, LCH_CTRL, LCH_CTRL_LCH_P);
+		else
+			omap_dma_chan_write(c, LCH_CTRL, LCH_CTRL_LCH_2D);
+	}
 	omap_dma_start_sg(c, d);
 }