Skip to contentSkip to conversation

Wrong "nollp" DW DMAC parameter value on ARC SDP.

DISCUSSION Eugeniy Paltsev 2 participants 8 messages Aug 12, 2016 – Aug 16, 2016

Subsystems (3)
dmaengine Synopsys ARC architecture Synopsys DesignWare DMAC driver
lore
Share & tools
lore

Eugeniy Paltsev

author
#0 lore raw

Hi,

"nollp" parameter defines if DW DMAC channel supports multi block transfer or not.

It is calculated in runtime, but differently depending on on availability of pdata. If pdata is absent "nollp" is calculated using autoconfig hardware registers. Otherwise "nollp" is calculated using the next code construction:
channel_writel(dwc, LLP, DWC_LLP_LOC(0xffffffff)); dwc->nollp = DWC_LLP_LOC(channel_readl(dwc, LLP)) == 0; channel_writel(dwc, LLP, 0);

I realized that these methods give different results.
For example on ARC AXS101 SDP in case of using autoconfig "nollp" was calculated as "true" (and DMAC works fine),?
otherwise "nollp" was calculated as "false" (and DMAC doesn't work).

So I'm wondering how the code in question really works?

reviewer
#1 lore raw
Quoted 13 lines · Eugeniy Paltsev

Hi,

"nollp" parameter defines if DW DMAC channel supports multi block transfer or not.

It is calculated in runtime, but differently depending on on availability of pdata. If pdata is absent "nollp" is calculated using autoconfig hardware registers. Otherwise "nollp" is calculated using the next code construction:
channel_writel(dwc, LLP, DWC_LLP_LOC(0xffffffff)); dwc->nollp = DWC_LLP_LOC(channel_readl(dwc, LLP)) == 0; channel_writel(dwc, LLP, 0);

Quoted 4 lines

I realized that these methods give different results.
For example on ARC AXS101 SDP in case of using autoconfig "nollp" was calculated as "true" (and DMAC works fine),?
otherwise "nollp" was calculated as "false" (and DMAC doesn't work).

Can you show out what the value you read back?

Quoted 5 lines

So I'm wondering how the code in question really works? From DW AHB DMAC databook I wasn't able to find anything relevant to this tricky implementation. Could you please clarify a little but what happens here?

"Table 4-1:
...
Hardcode Channel x LLP register to 0?
...
Description: If set to 1, hardcodes channel x Linked List Pointer register to 0 (LLPx.LOC == 0), ..."

Quoted 3 lines

Maybe we should add "nollp" field in pdata structure and receive it from pdata/device tree (like we use "is_private" or "is_memcpu" fields)

Yeah, perhaps we can remove that trick since we need this flag to be set on Intel Quark which might have the same issue as your case [1].

[1]?http://www.spinics.net/lists/linux-serial/msg22948.html

Signature · 2 lines
Andy Shevchenko <andriy.shevchenko at linux.intel.com>
Intel Finland Oy
author
#2 lore raw
Quoted 17 lines · Andy Shevchenko
Quoted 16 lines · Eugeniy Paltsev

Hi,

"nollp" parameter defines if DW DMAC channel supports multi block transfer or not.

It is calculated in runtime, but differently depending on on availability of pdata. If pdata is absent "nollp" is calculated using
autoconfig hardware registers. Otherwise "nollp" is calculated using
the next code construction:
channel_writel(dwc, LLP, DWC_LLP_LOC(0xffffffff)); dwc->nollp = DWC_LLP_LOC(channel_readl(dwc, LLP)) == 0; channel_writel(dwc, LLP, 0);

Quoted 7 lines
Quoted 6 lines

I realized that these methods give different results. For example on ARC AXS101 SDP in case of using autoconfig "nollp" was
calculated as "true" (and DMAC works fine),?
otherwise "nollp" was calculated as "false" (and DMAC doesn't work).

Can you show out what the value you read back?

channel_readl(dwc, LLP) return 0xfffffffc

Quoted 15 lines
Quoted 6 lines

So I'm wondering how the code in question really works? From DW AHB DMAC databook I wasn't able to find anything relevant to
this tricky implementation. Could you please clarify a little but what
happens here?

"Table 4-1:
...
Hardcode Channel x LLP register to 0?
...
Description: If set to 1, hardcodes channel x Linked List Pointer register to 0 (LLPx.LOC == 0), ..."

Quoted 1 line
Quoted 9 lines
Quoted 3 lines

Maybe we should add "nollp" field in pdata structure and receive it from pdata/device tree (like we use "is_private" or "is_memcpu" fields)

Yeah, perhaps we can remove that trick since we need this flag to be set
on Intel Quark which might have the same issue as your case [1].

[1]?http://www.spinics.net/lists/linux-serial/msg22948.html

In which tree I can find this patch applied,?so I may base my work on it?

Signature · 1 line
?Paltsev Eugeniy
reviewer
#3 lore raw
Quoted 26 lines · Eugeniy Paltsev
Quoted 25 lines · Andy Shevchenko
Quoted 23 lines · Eugeniy Paltsev

Hi,

"nollp" parameter defines if DW DMAC channel supports multi block transfer or not.

It is calculated in runtime, but differently depending on on availability of pdata. If pdata is absent "nollp" is calculated using
autoconfig hardware registers. Otherwise "nollp" is calculated using
the next code construction:
channel_writel(dwc, LLP, DWC_LLP_LOC(0xffffffff)); dwc->nollp = DWC_LLP_LOC(channel_readl(dwc, LLP)) == 0; channel_writel(dwc, LLP, 0);

I realized that these methods give different results. For example on ARC AXS101 SDP in case of using autoconfig "nollp" was
calculated as "true" (and DMAC works fine),?
otherwise "nollp" was calculated as "false" (and DMAC doesn't work).

Quoted 3 lines
Quoted 1 line

Can you show out what the value you read back?

channel_readl(dwc, LLP) return 0xfffffffc

Nice.

Oh, forgot to ask, what are the DW_PARAMS and DWC_PARAMS[x] are on the same hardware?

I assume we are talking about that one which has no hardware LLP support.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

So I'm wondering how the code in question really works? From DW AHB DMAC databook I wasn't able to find anything relevant to
this tricky implementation. Could you please clarify a little but what
happens here?

Quoted 6 lines
Quoted 6 lines

"Table 4-1:
...
Hardcode Channel x LLP register to 0?
...
Description: If set to 1, hardcodes channel x Linked List Pointer register to 0 (LLPx.LOC == 0), ..."

So, any comment on this one? I suppose you may have an access to some internal Synopsys documentation which might shed a light. Or maybe I missed something else which should be considered.

Quoted 8 lines
Quoted 8 lines
Quoted 4 lines

Maybe we should add "nollp" field in pdata structure and receive it
from pdata/device tree (like we use "is_private" or "is_memcpu" fields)

Yeah, perhaps we can remove that trick since we need this flag to be set
on Intel Quark which might have the same issue as your case [1].

Quoted 5 lines
Quoted 2 lines

[1]?http://www.spinics.net/lists/linux-serial/msg22948.html

In which tree I can find this patch applied,?so I may base my work on it?

The series is under review. I'm preparing v10, so, I would like to re- make this patch with regarding to your input.

For now I would prefer just to remove the trick, but I still wonder what the circumstances are to bring it not working.

Signature · 2 lines
Andy Shevchenko <andriy.shevchenko at linux.intel.com>
Intel Finland Oy
author
#4 lore raw
Quoted 32 lines · Andy Shevchenko
Quoted 31 lines · Eugeniy Paltsev

?

Quoted 29 lines · Andy Shevchenko

?
?

Quoted 26 lines · Eugeniy Paltsev

?
?
?
Hi,
?
"nollp" parameter defines if DW DMAC channel supports multi block
transfer or not.
?
It is calculated in runtime, but differently depending on on availability of pdata. If pdata is absent "nollp" is calculated using
autoconfig hardware registers. Otherwise "nollp" is calculated using
the next code construction:
channel_writel(dwc, LLP, DWC_LLP_LOC(0xffffffff)); dwc->nollp = DWC_LLP_LOC(channel_readl(dwc, LLP)) == 0; channel_writel(dwc, LLP, 0);
?
I realized that these methods give different results. For example on ARC AXS101 SDP in case of using autoconfig "nollp"
was
calculated as "true" (and DMAC works fine),? otherwise "nollp" was calculated as "false" (and DMAC doesn't work).

Quoted 6 lines
Quoted 2 lines
Quoted 1 line

Can you show out what the value you read back?

channel_readl(dwc, LLP) return 0xfffffffc

Nice.
?
Oh, forgot to ask, what are the DW_PARAMS and DWC_PARAMS[x] are on the same hardware?

DW_PARAMS: 0x38280b0c
DWC_PARAMS[0]: 0x4926d300
DWC_PARAMS[1]: 0x4926d300
DWC_PARAMS[2]: 0x4926d300
DWC_PARAMS[3]: 0x4926d300

Quoted 2 lines

I assume we are talking about that one which has no hardware LLP support.

Yep.

Quoted 17 lines
Quoted 17 lines

?

Quoted 16 lines

?

Quoted 9 lines

?
So I'm wondering how the code in question really works? From DW AHB DMAC databook I wasn't able to find anything relevant
to
this tricky implementation. Could you please clarify a little but
what
happens here?

"Table 4-1:
...
Hardcode Channel x LLP register to 0?
...
Description: If set to 1, hardcodes channel x Linked List Pointer register to 0 (LLPx.LOC == 0), ..."

Quoted 3 lines

So, any comment on this one? I suppose you may have an access to some internal Synopsys documentation which might shed a light. Or maybe I missed something else which should be considered.

Looks like this code based on idea, what if DMA ip-core doesn't have? LLP support it will not have LLP registers. It is not necessarily true.

Quoted 19 lines
Quoted 16 lines

?

Quoted 7 lines

?

Quoted 6 lines

?
Maybe we should add "nollp" field in pdata structure and receive
it
from pdata/device tree (like we use "is_private" or "is_memcpu" fields)

Quoted 7 lines

Yeah, perhaps we can remove that trick since we need this flag to be
set
on Intel Quark which might have the same issue as your case [1]. ?
[1] http://www.spinics.net/lists/linux-serial/msg22948.html
?

Quoted 3 lines

In which tree I can find this patch applied, so I may base my work on
it?

Quoted 8 lines

The series is under review. I'm preparing v10, so, I would like to re-
make this patch with regarding to your input.
?
For now I would prefer just to remove the trick, but I still wonder what
the circumstances are to bring it not working.
?

Please add this code to read "is_memcpy" and "is_nollp" property? from device tree. ----------------------------->8------------------------------

drivers/dma/dw/platform.c +0 -0
index 5bda0eb..2712602 100644
@@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev)

????????if (of_property_read_bool(np, "is_private")) ????????????????pdata->is_private = true;
?
+???????if (of_property_read_bool(np, "is_memcpy")) +???????????????pdata->is_memcpy = true;
+
+???????if (of_property_read_bool(np, "is_nollp")) +???????????????pdata->is_nollp = true;
+
????????if (!of_property_read_u32(np, "chan_allocation_order", &tmp)) ????????????????pdata->chan_allocation_order = (unsigned char)tmp; ? ----------------------------->8------------------------------ --?
?Paltsev Eugeniy

reviewer
#5 lore raw
Quoted 24 lines · Eugeniy Paltsev
Quoted 23 lines · Andy Shevchenko

?

Quoted 22 lines
Quoted 22 lines
Quoted 22 lines

"nollp" parameter defines if DW DMAC channel supports multi block
transfer or not.
?
It is calculated in runtime, but differently depending on on availability of pdata. If pdata is absent "nollp" is calculated
using
autoconfig hardware registers. Otherwise "nollp" is calculated using
the next code construction:
channel_writel(dwc, LLP, DWC_LLP_LOC(0xffffffff)); dwc->nollp = DWC_LLP_LOC(channel_readl(dwc, LLP)) == 0; channel_writel(dwc, LLP, 0);
?
I realized that these methods give different results. For example on ARC AXS101 SDP in case of using autoconfig "nollp"
was
calculated as "true" (and DMAC works fine),? otherwise "nollp" was calculated as "false" (and DMAC doesn't work).

Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
Quoted 1 line

Can you show out what the value you read back?

channel_readl(dwc, LLP) return 0xfffffffc

Nice.
?
Oh, forgot to ask, what are the DW_PARAMS and DWC_PARAMS[x] are on the same hardware?

DW_PARAMS: 0x38280b0c

Wow, you have two masters with number 3 and 4? (Just out of curiosity)

Quoted 4 lines

DWC_PARAMS[0]: 0x4926d300
DWC_PARAMS[1]: 0x4926d300
DWC_PARAMS[2]: 0x4926d300
DWC_PARAMS[3]: 0x4926d300

A-ha, seems you have CHx_HC_LLP == 0. That may explain. ?

Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
Quoted 8 lines

So I'm wondering how the code in question really works? From DW AHB DMAC databook I wasn't able to find anything relevant
to
this tricky implementation. Could you please clarify a little but
what
happens here?

"Table 4-1:
...
Hardcode Channel x LLP register to 0?
...
Description: If set to 1, hardcodes channel x Linked List Pointer
register to 0 (LLPx.LOC == 0), ..."

Quoted 7 lines
Quoted 4 lines

So, any comment on this one? I suppose you may have an access to some
internal Synopsys documentation which might shed a light. Or maybe I missed something else which should be considered.

Looks like this code based on idea, what if DMA ip-core doesn't have? LLP support it will not have LLP registers. It is not necessarily true.

Yep.

Consider to review?
http://www.spinics.net/lists/dmaengine/msg10682.html

Quoted 18 lines
Quoted 1 line

?

Please add this code to read "is_memcpy" and "is_nollp" property? from device tree. ----------------------------->8------------------------------

drivers/dma/dw/platform.c +0 -0
index 5bda0eb..2712602 100644
@@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev)
 ????????if (of_property_read_bool(np, "is_private"))
 ????????????????pdata->is_private = true;

?
+???????if (of_property_read_bool(np, "is_memcpy")) +???????????????pdata->is_memcpy = true;

drivers/dma/dw/platform.c +4 -0
+
+???????if (of_property_read_bool(np, "is_nollp"))
+???????????????pdata->is_nollp = true;
+
Quoted 2 lines

????????if (!of_property_read_u32(np, "chan_allocation_order", &tmp)) ????????????????pdata->chan_allocation_order = (unsigned char)tmp;

Please, make this as a patch and submit either separately, or with your series mentioning that it has a dependency to my 8250 LPSS series.

Signature · 2 lines
Andy Shevchenko <andriy.shevchenko at linux.intel.com>
Intel Finland Oy
author Reviewed-by
#6 lore raw
Quoted 77 lines · Andy Shevchenko
Quoted 32 lines · Eugeniy Paltsev
Quoted 30 lines · Andy Shevchenko

?

Quoted 28 lines
Quoted 28 lines
Quoted 28 lines

"nollp" parameter defines if DW DMAC channel supports multi block
transfer or not.
?
It is calculated in runtime, but differently depending on on
availability of pdata. If pdata is absent "nollp" is calculated
using
autoconfig hardware registers. Otherwise "nollp" is calculated
using
the next code construction:
channel_writel(dwc, LLP, DWC_LLP_LOC(0xffffffff)); dwc->nollp = DWC_LLP_LOC(channel_readl(dwc, LLP)) == 0; channel_writel(dwc, LLP, 0);
?
I realized that these methods give different results. For example on ARC AXS101 SDP in case of using autoconfig "nollp"
was
calculated as "true" (and DMAC works fine),? otherwise "nollp" was calculated as "false" (and DMAC doesn't
work).

Quoted 8 lines
Quoted 7 lines
Quoted 2 lines
Quoted 1 line

Can you show out what the value you read back?

channel_readl(dwc, LLP) return 0xfffffffc

Nice.
?
Oh, forgot to ask, what are the DW_PARAMS and DWC_PARAMS[x] are on
the same hardware?

DW_PARAMS: 0x38280b0c

Wow, you have two masters with number 3 and 4? (Just out of curiosity)

Quoted 5 lines

DWC_PARAMS[0]: 0x4926d300
DWC_PARAMS[1]: 0x4926d300
DWC_PARAMS[2]: 0x4926d300
DWC_PARAMS[3]: 0x4926d300

A-ha, seems you have CHx_HC_LLP == 0. That may explain. ?

Quoted 26 lines
Quoted 26 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines

So I'm wondering how the code in question really works? From DW AHB DMAC databook I wasn't able to find anything relevant
to
this tricky implementation. Could you please clarify a little
but
what
happens here?

Quoted 7 lines
Quoted 7 lines

"Table 4-1:
...
Hardcode Channel x LLP register to 0?
...
Description: If set to 1, hardcodes channel x Linked List Pointer
register to 0 (LLPx.LOC == 0), ..."

So, any comment on this one? I suppose you may have an access to some
internal Synopsys documentation which might shed a light. Or maybe I
missed something else which should be considered.

Quoted 8 lines
Quoted 4 lines

Looks like this code based on idea, what if DMA ip-core doesn't have?
LLP support it will not have LLP registers. It is not necessarily true.

Yep.

Consider to review?
http://www.spinics.net/lists/dmaengine/msg10682.html

Looks good to me.

Quoted 19 lines
Quoted 19 lines
Quoted 4 lines

?

Please add this code to read "is_memcpy" and "is_nollp" property? from device tree. ----------------------------->8------------------------------

drivers/dma/dw/platform.c +0 -0
index 5bda0eb..2712602 100644
@@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev)
 ????????if (of_property_read_bool(np, "is_private"))
 ????????????????pdata->is_private = true;

?
+???????if (of_property_read_bool(np, "is_memcpy")) +???????????????pdata->is_memcpy = true;
+
+???????if (of_property_read_bool(np, "is_nollp"))

Quoted 9 lines
Quoted 5 lines

+???????????????pdata->is_nollp = true;
+
????????if (!of_property_read_u32(np, "chan_allocation_order", &tmp))
????????????????pdata->chan_allocation_order = (unsigned char)tmp;

Please, make this as a patch and submit either separately, or with your
series mentioning that it has a dependency to my 8250 LPSS series.

Reviewed-by Eugeniy Platsev <Eugeniy.Paltsev at synopsys.com>
reviewer
#7 lore raw
Quoted 8 lines · Eugeniy Paltsev
Quoted 4 lines · Andy Shevchenko

On Mon, 2016-08-15@11:10 +0000, Eugeniy Paltsev wrote:

Consider to review?
http://www.spinics.net/lists/dmaengine/msg10682.html

Looks good to me.

Reviewed-by Eugeniy Platsev <Eugeniy.Paltsev at synopsys.com>

I assume that was related to my patch. Please, send this as an answer to that thread.

Signature · 2 lines
Andy Shevchenko <andriy.shevchenko at linux.intel.com>
Intel Finland Oy