NAND DMA prefetch has been broken for awhile and seems to have only
worked for SDMA based devices
This patchset fixes DMA prefetch to work on both EDMA and SDMA devices
Test on:
am335x gp evm
am437x gp evm
am37x gp evm
This rev is pretty much the same as v2 which was blocked due to
dependencies to Roger's update GPMC/NAND rework.
This updated rev removes this dependency.
Also it fixes an issue that was introduced when the eDMA driver was
recently updated.
Links to rev 3 patchset:
https://patchwork.ozlabs.org/patch/595631/
https://patchwork.ozlabs.org/patch/595626/
https://patchwork.ozlabs.org/patch/595628/
https://patchwork.ozlabs.org/patch/595630/
https://patchwork.ozlabs.org/patch/595627/
https://patchwork.ozlabs.org/patch/595629/
Links to rev 2 patchset:
https://patchwork.kernel.org/patch/7408691/
https://patchwork.kernel.org/patch/7408681/
https://patchwork.kernel.org/patch/7408661/
https://patchwork.kernel.org/patch/7408641/
https://patchwork.kernel.org/patch/7408621/
Franklin S Cooper Jr (7):
ARM: OMAP2+: gpmc-nand: Set omap2-nand's parent dev to GPMC dev
ARM: dts: am33xx: Fix GPMC dma properties
ARM: dts: am437x: Fix GPMC dma properties
mtd: nand: omap2: Support parsing dma channel information from DT
mtd: nand: omap2: Start dma request before enabling prefetch
mtd: nand: omap2: Fix high memory dma prefetch transfer
ARM: OMAP2+: Update GPMC and NAND DT binding documentation
Documentation/devicetree/bindings/bus/ti-gpmc.txt | 7 +++++-
.../devicetree/bindings/mtd/gpmc-nand.txt | 2 +-
arch/arm/boot/dts/am33xx.dtsi | 2 +-
arch/arm/boot/dts/am4372.dtsi | 2 +-
arch/arm/mach-omap2/gpmc-nand.c | 16 ++++++++++++-
drivers/mtd/nand/omap2.c | 27 +++++++++-------------
6 files changed, 35 insertions(+), 21 deletions(-)
--
2.7.0
This patch updates the GPMC's DT DMA property to reflect the updated eDMA
bindings.
Fixes: cce1ee000187 ("ARM: DTS: am437x: Use the new DT bindings for the eDMA3")
Signed-off-by: Franklin S Cooper Jr <[email protected]>
---
Version 4 changes:
Split into its own commit. Use proper Fixes syntax.
arch/arm/boot/dts/am4372.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index 92068fb..2878b04 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -884,7 +884,7 @@
gpmc: gpmc@50000000 {
compatible = "ti,am3352-gpmc";
ti,hwmods = "gpmc";
- dmas = <&edma 52>;
+ dmas = <&edma 52 0>;
dma-names = "rxtx";
clocks = <&l3s_gclk>;
clock-names = "fck";
--
2.7.0
Based on DMA documentation and testing using high memory buffer when
doing dma transfers can lead to various issues including kernel
panics.
To workaround this simply use cpu copy. The amount of high memory
buffers used are very uncommon so no noticeable performance hit should
be seen.
Signed-off-by: Franklin S Cooper Jr <[email protected]>
---
drivers/mtd/nand/omap2.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 0863a83..22b0112 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -467,17 +467,8 @@ static inline int omap_nand_dma_transfer(struct mtd_info *mtd, void *addr,
int ret;
u32 val;
- if (addr >= high_memory) {
- struct page *p1;
-
- if (((size_t)addr & PAGE_MASK) !=
- ((size_t)(addr + len - 1) & PAGE_MASK))
- goto out_copy;
- p1 = vmalloc_to_page(addr);
- if (!p1)
- goto out_copy;
- addr = page_address(p1) + ((size_t)addr & ~PAGE_MASK);
- }
+ if (addr >= high_memory)
+ goto out_copy;
sg_init_one(&sg, addr, len);
n = dma_map_sg(info->dma->device->dev, &sg, 1, dir);
@@ -534,6 +525,7 @@ out_copy:
else
is_write == 0 ? omap_read_buf8(mtd, (u_char *) addr, len)
: omap_write_buf8(mtd, (u_char *) addr, len);
+
return 0;
}
--
2.7.0
Add additional details to the GPMC NAND documentation to clarify
what is needed to enable NAND DMA prefetch.
Signed-off-by: Franklin S Cooper Jr <[email protected]>
---
Documentation/devicetree/bindings/bus/ti-gpmc.txt | 7 ++++++-
Documentation/devicetree/bindings/mtd/gpmc-nand.txt | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
index 704be93..ae6388a 100644
--- a/Documentation/devicetree/bindings/bus/ti-gpmc.txt
+++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
@@ -33,6 +33,10 @@ Required properties:
As this will change in the future, filling correct
values here is a requirement.
+Required properties when using NAND prefetch dma:
+ - dmas GPMC NAND prefetch dma channel
+ - dma-names Must be set to "rxtx"
+
Timing properties for child nodes. All are optional and default to 0.
- gpmc,sync-clk-ps: Minimum clock period for synchronous mode, in picoseconds
@@ -119,7 +123,8 @@ Example for an AM33xx board:
ti,hwmods = "gpmc";
reg = <0x50000000 0x2000>;
interrupts = <100>;
-
+ dmas = <&edma 52 0>;
+ dma-names = "rxtx";
gpmc,num-cs = <8>;
gpmc,num-waitpins = <2>;
#address-cells = <2>;
diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
index fb733c4..7cb9dc56 100644
--- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
@@ -35,7 +35,7 @@ Optional properties:
"prefetch-polled" Prefetch polled mode (default)
"polled" Polled mode, without prefetch
- "prefetch-dma" Prefetch enabled sDMA mode
+ "prefetch-dma" Prefetch enabled DMA mode
"prefetch-irq" Prefetch enabled irq mode
- elm_id: <deprecated> use "ti,elm-id" instead
--
2.7.0
The prefetch engine sends a dma request once a FIFO threshold has
been met. No other requests are received until the previous request
is handled.
Starting an edma transfer (dma_async_issue_pending) results in any
previous event for the dma channel to be cleared. Therefore, starting
the prefetch engine before initiating the dma transfer may result in
the prefetch triggering a dma request but instead of it being handled
it can end up being cleared. This will result in a hang since the code
will continue to wait for the dma request to complete.
By initiating the dma request before enabling the prefetch engine this
race condition is avoided and no dma request are missed/cleared.
Signed-off-by: Franklin S Cooper Jr <[email protected]>
---
drivers/mtd/nand/omap2.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 95bc636..0863a83 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -497,6 +497,11 @@ static inline int omap_nand_dma_transfer(struct mtd_info *mtd, void *addr,
tx->callback_param = &info->comp;
dmaengine_submit(tx);
+ init_completion(&info->comp);
+
+ /* setup and start DMA using dma_addr */
+ dma_async_issue_pending(info->dma);
+
/* configure and start prefetch transfer */
ret = omap_prefetch_enable(info->gpmc_cs,
PREFETCH_FIFOTHRESHOLD_MAX, 0x1, len, is_write, info);
@@ -504,10 +509,6 @@ static inline int omap_nand_dma_transfer(struct mtd_info *mtd, void *addr,
/* PFPW engine is busy, use cpu copy method */
goto out_copy_unmap;
- init_completion(&info->comp);
- dma_async_issue_pending(info->dma);
-
- /* setup and start DMA using dma_addr */
wait_for_completion(&info->comp);
tim = 0;
limit = (loops_per_jiffy * msecs_to_jiffies(OMAP_NAND_TIMEOUT_MS));
--
2.7.0
Switch from dma_request_channel to allow passing dma channel
information from DT rather than hardcoding a value.
Also provide a handle to the GPMC's dev so it can be used to parse the DMA
channel information within the GPMC's DT node.
Signed-off-by: Franklin S Cooper Jr <[email protected]>
---
Version 4 changes:
Pass parent instead of new variable from the platform data
drivers/mtd/nand/omap2.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index c553f78..95bc636 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1731,7 +1731,9 @@ static int omap_nand_probe(struct platform_device *pdev)
dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
sig = OMAP24XX_DMA_GPMC;
- info->dma = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+ info->dma = dma_request_slave_channel_compat(mask,
+ omap_dma_filter_fn, &sig, pdev->dev.parent, "rxtx");
+
if (!info->dma) {
dev_err(&pdev->dev, "DMA engine request failed\n");
err = -ENXIO;
--
2.7.0
This patch updates the GPMC's DT DMA property to reflect the updated eDMA
bindings.
Fixes: b5e509066074 ("ARM: DTS: am33xx: Use the new DT bindings for the eDMA3")
Signed-off-by: Franklin S Cooper Jr <[email protected]>
---
Version 4 changes:
Split in its own commit. Uses correct syntax for Fixes
arch/arm/boot/dts/am33xx.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 1fafaad..97471d6 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -860,7 +860,7 @@
ti,no-idle-on-init;
reg = <0x50000000 0x2000>;
interrupts = <100>;
- dmas = <&edma 52>;
+ dmas = <&edma 52 0>;
dma-names = "rxtx";
gpmc,num-cs = <7>;
gpmc,num-waitpins = <2>;
--
2.7.0
The dma channel information is located within the GPMC node which is the
NAND's parent node. The NAND driver requires a handle to the GPMC's dev
to properly parse the DMA properties. Therefore, set the NAND's parent dev
to the GPMC's dev so it can be referenced within the driver.
Signed-off-by: Franklin S Cooper Jr <[email protected]>
---
Version 4 changes:
Instead of storing the GPMC dev in a new property simply grab a reference
to it and set omap2-nand's dev.parent to it.
arch/arm/mach-omap2/gpmc-nand.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index 72918c4..77e453c 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -15,6 +15,7 @@
#include <linux/omap-gpmc.h>
#include <linux/mtd/nand.h>
#include <linux/platform_data/mtd-nand-omap2.h>
+#include <linux/of_platform.h>
#include <asm/mach/flash.h>
@@ -77,6 +78,9 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
int err = 0;
struct gpmc_settings s;
struct platform_device *pdev;
+ struct platform_device *gpmc_dev;
+ struct device_node *gpmc_node;
+
struct resource gpmc_nand_res[] = {
{ .flags = IORESOURCE_MEM, },
{ .flags = IORESOURCE_IRQ, },
@@ -134,8 +138,18 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
if (pdev) {
err = platform_device_add_resources(pdev, gpmc_nand_res,
ARRAY_SIZE(gpmc_nand_res));
- if (!err)
+ if (!err) {
pdev->dev.platform_data = gpmc_nand_data;
+
+ gpmc_node = of_get_parent(gpmc_nand_data->of_node);
+
+ if (gpmc_node) {
+ gpmc_dev = of_find_device_by_node(gpmc_node);
+
+ if (gpmc_dev)
+ pdev->dev.parent = &gpmc_dev->dev;
+ }
+ }
} else {
err = -ENOMEM;
}
--
2.7.0
Franklin,
On 11/03/16 01:56, Franklin S Cooper Jr wrote:
> The dma channel information is located within the GPMC node which is the
> NAND's parent node. The NAND driver requires a handle to the GPMC's dev
> to properly parse the DMA properties. Therefore, set the NAND's parent dev
> to the GPMC's dev so it can be referenced within the driver.
>
> Signed-off-by: Franklin S Cooper Jr <[email protected]>
> ---
> Version 4 changes:
> Instead of storing the GPMC dev in a new property simply grab a reference
> to it and set omap2-nand's dev.parent to it.
>
> arch/arm/mach-omap2/gpmc-nand.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
> index 72918c4..77e453c 100644
> --- a/arch/arm/mach-omap2/gpmc-nand.c
> +++ b/arch/arm/mach-omap2/gpmc-nand.c
> @@ -15,6 +15,7 @@
> #include <linux/omap-gpmc.h>
> #include <linux/mtd/nand.h>
> #include <linux/platform_data/mtd-nand-omap2.h>
> +#include <linux/of_platform.h>
>
> #include <asm/mach/flash.h>
>
> @@ -77,6 +78,9 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
> int err = 0;
> struct gpmc_settings s;
> struct platform_device *pdev;
> + struct platform_device *gpmc_dev;
> + struct device_node *gpmc_node;
> +
> struct resource gpmc_nand_res[] = {
> { .flags = IORESOURCE_MEM, },
> { .flags = IORESOURCE_IRQ, },
> @@ -134,8 +138,18 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
> if (pdev) {
> err = platform_device_add_resources(pdev, gpmc_nand_res,
> ARRAY_SIZE(gpmc_nand_res));
> - if (!err)
> + if (!err) {
> pdev->dev.platform_data = gpmc_nand_data;
> +
> + gpmc_node = of_get_parent(gpmc_nand_data->of_node);
I'm afraid that we can't use this method as we want to restrict
gpmc_nand_init() to non-DT boots.
> +
> + if (gpmc_node) {
> + gpmc_dev = of_find_device_by_node(gpmc_node);
> +
> + if (gpmc_dev)
> + pdev->dev.parent = &gpmc_dev->dev;
> + }
> + }
> } else {
> err = -ENOMEM;
> }
>
cheers,
-roger
+Peter,
On 11/03/16 01:56, Franklin S Cooper Jr wrote:
> This patch updates the GPMC's DT DMA property to reflect the updated eDMA
> bindings.
>
> Fixes: b5e509066074 ("ARM: DTS: am33xx: Use the new DT bindings for the eDMA3")
>
> Signed-off-by: Franklin S Cooper Jr <[email protected]>
Acked-by: Roger Quadros <[email protected]>
cheers,
-roger
> ---
> Version 4 changes:
> Split in its own commit. Uses correct syntax for Fixes
>
> arch/arm/boot/dts/am33xx.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 1fafaad..97471d6 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -860,7 +860,7 @@
> ti,no-idle-on-init;
> reg = <0x50000000 0x2000>;
> interrupts = <100>;
> - dmas = <&edma 52>;
> + dmas = <&edma 52 0>;
> dma-names = "rxtx";
> gpmc,num-cs = <7>;
> gpmc,num-waitpins = <2>;
>
+Peter
On 11/03/16 01:56, Franklin S Cooper Jr wrote:
> This patch updates the GPMC's DT DMA property to reflect the updated eDMA
> bindings.
>
> Fixes: cce1ee000187 ("ARM: DTS: am437x: Use the new DT bindings for the eDMA3")
>
> Signed-off-by: Franklin S Cooper Jr <[email protected]>
Acked-by: Roger Quadros <[email protected]>
cheers,
-roger
> ---
> Version 4 changes:
> Split into its own commit. Use proper Fixes syntax.
>
> arch/arm/boot/dts/am4372.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> index 92068fb..2878b04 100644
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi
> @@ -884,7 +884,7 @@
> gpmc: gpmc@50000000 {
> compatible = "ti,am3352-gpmc";
> ti,hwmods = "gpmc";
> - dmas = <&edma 52>;
> + dmas = <&edma 52 0>;
> dma-names = "rxtx";
> clocks = <&l3s_gclk>;
> clock-names = "fck";
>
On 03/11/2016 01:56 AM, Franklin S Cooper Jr wrote:
> This patch updates the GPMC's DT DMA property to reflect the updated eDMA
> bindings.
>
> Fixes: b5e509066074 ("ARM: DTS: am33xx: Use the new DT bindings for the eDMA3")
I have double checked the switch over to the new eDMA bindings when I sent it
and there were no gpmc node existed back then afaik.
Acked-by: Peter Ujfalusi <[email protected]>
>
> Signed-off-by: Franklin S Cooper Jr <[email protected]>
> ---
> Version 4 changes:
> Split in its own commit. Uses correct syntax for Fixes
>
> arch/arm/boot/dts/am33xx.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 1fafaad..97471d6 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -860,7 +860,7 @@
> ti,no-idle-on-init;
> reg = <0x50000000 0x2000>;
> interrupts = <100>;
> - dmas = <&edma 52>;
> + dmas = <&edma 52 0>;
> dma-names = "rxtx";
> gpmc,num-cs = <7>;
> gpmc,num-waitpins = <2>;
>
--
P?ter
Franklin,
On 11/03/16 01:56, Franklin S Cooper Jr wrote:
> NAND DMA prefetch has been broken for awhile and seems to have only
> worked for SDMA based devices
>
> This patchset fixes DMA prefetch to work on both EDMA and SDMA devices
>
> Test on:
> am335x gp evm
> am437x gp evm
> am37x gp evm
>
> This rev is pretty much the same as v2 which was blocked due to
> dependencies to Roger's update GPMC/NAND rework.
>
> This updated rev removes this dependency.
>
> Also it fixes an issue that was introduced when the eDMA driver was
> recently updated.
>
> Links to rev 3 patchset:
> https://patchwork.ozlabs.org/patch/595631/
> https://patchwork.ozlabs.org/patch/595626/
> https://patchwork.ozlabs.org/patch/595628/
> https://patchwork.ozlabs.org/patch/595630/
> https://patchwork.ozlabs.org/patch/595627/
> https://patchwork.ozlabs.org/patch/595629/
>
> Links to rev 2 patchset:
> https://patchwork.kernel.org/patch/7408691/
> https://patchwork.kernel.org/patch/7408681/
> https://patchwork.kernel.org/patch/7408661/
> https://patchwork.kernel.org/patch/7408641/
> https://patchwork.kernel.org/patch/7408621/
>
> Franklin S Cooper Jr (7):
> ARM: OMAP2+: gpmc-nand: Set omap2-nand's parent dev to GPMC dev
> ARM: dts: am33xx: Fix GPMC dma properties
> ARM: dts: am437x: Fix GPMC dma properties
Can you please fix the gpmc nodes in dm816x.dtsi and dm814x.dtsi as well?
You can use linux-next tree or [1] to see the latest changes to these files
that will end up in v4.6 and base your changes on that.
[1] Tony's omap-for-v4.6/dt
https://git.kernel.org/cgit/linux/kernel/git/tmlind/linux-omap.git/log/?h=omap-for-v4.6/dt
> mtd: nand: omap2: Support parsing dma channel information from DT
> mtd: nand: omap2: Start dma request before enabling prefetch
> mtd: nand: omap2: Fix high memory dma prefetch transfer
> ARM: OMAP2+: Update GPMC and NAND DT binding documentation
>
> Documentation/devicetree/bindings/bus/ti-gpmc.txt | 7 +++++-
> .../devicetree/bindings/mtd/gpmc-nand.txt | 2 +-
> arch/arm/boot/dts/am33xx.dtsi | 2 +-
> arch/arm/boot/dts/am4372.dtsi | 2 +-
> arch/arm/mach-omap2/gpmc-nand.c | 16 ++++++++++++-
> drivers/mtd/nand/omap2.c | 27 +++++++++-------------
> 6 files changed, 35 insertions(+), 21 deletions(-)
>
cheers,
-roger
On 03/11/2016 01:56 AM, Franklin S Cooper Jr wrote:
> This patch updates the GPMC's DT DMA property to reflect the updated eDMA
> bindings.
>
> Fixes: cce1ee000187 ("ARM: DTS: am437x: Use the new DT bindings for the eDMA3")
I have also converted all users of the edma back when I sent, I guess the gpmc
DMA binding went in parallel...
Acked-by: Peter Ujfalusi <[email protected]>
>
> Signed-off-by: Franklin S Cooper Jr <[email protected]>
> ---
> Version 4 changes:
> Split into its own commit. Use proper Fixes syntax.
>
> arch/arm/boot/dts/am4372.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> index 92068fb..2878b04 100644
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi
> @@ -884,7 +884,7 @@
> gpmc: gpmc@50000000 {
> compatible = "ti,am3352-gpmc";
> ti,hwmods = "gpmc";
> - dmas = <&edma 52>;
> + dmas = <&edma 52 0>;
> dma-names = "rxtx";
> clocks = <&l3s_gclk>;
> clock-names = "fck";
>
--
P?ter
On 03/11/2016 07:52 AM, Roger Quadros wrote:
> Franklin,
>
> On 11/03/16 01:56, Franklin S Cooper Jr wrote:
>> The dma channel information is located within the GPMC node which is the
>> NAND's parent node. The NAND driver requires a handle to the GPMC's dev
>> to properly parse the DMA properties. Therefore, set the NAND's parent dev
>> to the GPMC's dev so it can be referenced within the driver.
>>
>> Signed-off-by: Franklin S Cooper Jr <[email protected]>
>> ---
>> Version 4 changes:
>> Instead of storing the GPMC dev in a new property simply grab a reference
>> to it and set omap2-nand's dev.parent to it.
>>
>> arch/arm/mach-omap2/gpmc-nand.c | 16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
>> index 72918c4..77e453c 100644
>> --- a/arch/arm/mach-omap2/gpmc-nand.c
>> +++ b/arch/arm/mach-omap2/gpmc-nand.c
>> @@ -15,6 +15,7 @@
>> #include <linux/omap-gpmc.h>
>> #include <linux/mtd/nand.h>
>> #include <linux/platform_data/mtd-nand-omap2.h>
>> +#include <linux/of_platform.h>
>>
>> #include <asm/mach/flash.h>
>>
>> @@ -77,6 +78,9 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
>> int err = 0;
>> struct gpmc_settings s;
>> struct platform_device *pdev;
>> + struct platform_device *gpmc_dev;
>> + struct device_node *gpmc_node;
>> +
>> struct resource gpmc_nand_res[] = {
>> { .flags = IORESOURCE_MEM, },
>> { .flags = IORESOURCE_IRQ, },
>> @@ -134,8 +138,18 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
>> if (pdev) {
>> err = platform_device_add_resources(pdev, gpmc_nand_res,
>> ARRAY_SIZE(gpmc_nand_res));
>> - if (!err)
>> + if (!err) {
>> pdev->dev.platform_data = gpmc_nand_data;
>> +
>> + gpmc_node = of_get_parent(gpmc_nand_data->of_node);
> I'm afraid that we can't use this method as we want to restrict
> gpmc_nand_init() to non-DT boots.
The only users of the parent GPMC driver are already using
DT. The gpmc_probe_nand_child function in the GPMC driver
which calls gpmc_nand_init is already DT only.
The only other caller to gpmc_nand_init is board-flash.c.
The driver doesn't utilize xfer_type to even switch to any
other modes including DMA prefetch mode. Looking at it
closer there isn't a dev from some kind of parent for me to
pass along. Board_nand_init which calls gpmc_nand_init just
takes raw NAND values with no relation to its parent.
With that being said are you ok with leaving it as is?
>
>> +
>> + if (gpmc_node) {
>> + gpmc_dev = of_find_device_by_node(gpmc_node);
>> +
>> + if (gpmc_dev)
>> + pdev->dev.parent = &gpmc_dev->dev;
>> + }
>> + }
>> } else {
>> err = -ENOMEM;
>> }
>>
> cheers,
> -roger
Franklin, Tony,
On 11/03/16 17:39, Franklin S Cooper Jr. wrote:
>
>
> On 03/11/2016 07:52 AM, Roger Quadros wrote:
>> Franklin,
>>
>> On 11/03/16 01:56, Franklin S Cooper Jr wrote:
>>> The dma channel information is located within the GPMC node which is the
>>> NAND's parent node. The NAND driver requires a handle to the GPMC's dev
>>> to properly parse the DMA properties. Therefore, set the NAND's parent dev
>>> to the GPMC's dev so it can be referenced within the driver.
>>>
>>> Signed-off-by: Franklin S Cooper Jr <[email protected]>
>>> ---
>>> Version 4 changes:
>>> Instead of storing the GPMC dev in a new property simply grab a reference
>>> to it and set omap2-nand's dev.parent to it.
>>>
>>> arch/arm/mach-omap2/gpmc-nand.c | 16 +++++++++++++++-
>>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
>>> index 72918c4..77e453c 100644
>>> --- a/arch/arm/mach-omap2/gpmc-nand.c
>>> +++ b/arch/arm/mach-omap2/gpmc-nand.c
>>> @@ -15,6 +15,7 @@
>>> #include <linux/omap-gpmc.h>
>>> #include <linux/mtd/nand.h>
>>> #include <linux/platform_data/mtd-nand-omap2.h>
>>> +#include <linux/of_platform.h>
>>>
>>> #include <asm/mach/flash.h>
>>>
>>> @@ -77,6 +78,9 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
>>> int err = 0;
>>> struct gpmc_settings s;
>>> struct platform_device *pdev;
>>> + struct platform_device *gpmc_dev;
>>> + struct device_node *gpmc_node;
>>> +
>>> struct resource gpmc_nand_res[] = {
>>> { .flags = IORESOURCE_MEM, },
>>> { .flags = IORESOURCE_IRQ, },
>>> @@ -134,8 +138,18 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
>>> if (pdev) {
>>> err = platform_device_add_resources(pdev, gpmc_nand_res,
>>> ARRAY_SIZE(gpmc_nand_res));
>>> - if (!err)
>>> + if (!err) {
>>> pdev->dev.platform_data = gpmc_nand_data;
>>> +
>>> + gpmc_node = of_get_parent(gpmc_nand_data->of_node);
>> I'm afraid that we can't use this method as we want to restrict
>> gpmc_nand_init() to non-DT boots.
>
> The only users of the parent GPMC driver are already using
> DT. The gpmc_probe_nand_child function in the GPMC driver
> which calls gpmc_nand_init is already DT only.
>
> The only other caller to gpmc_nand_init is board-flash.c.
> The driver doesn't utilize xfer_type to even switch to any
> other modes including DMA prefetch mode. Looking at it
> closer there isn't a dev from some kind of parent for me to
> pass along. Board_nand_init which calls gpmc_nand_init just
> takes raw NAND values with no relation to its parent.
>
>
> With that being said are you ok with leaving it as is?
I think it is OK to assume that NAND DMA won't work with legacy boot.
Tony any objections? I see that board-ldp.c is the only legacy user
of NAND. When can we drop support for it?
I want to keep gpmc_nand_init() as it is and don't want to add any
device tree specific calls here.
So I think it is still best if you rebase your series on top of [1]
so that you are assured NAND controller's parent is the GPMC device
in the DT case without requiring the $subject patch.
[1] https://lkml.org/lkml/2016/2/19/599
The series has been Acked by all maintainers and will go in v4.6
cheers,
-roger
On 03/11/2016 08:02 AM, Roger Quadros wrote:
> Franklin,
>
> On 11/03/16 01:56, Franklin S Cooper Jr wrote:
>> NAND DMA prefetch has been broken for awhile and seems to have only
>> worked for SDMA based devices
>>
>> This patchset fixes DMA prefetch to work on both EDMA and SDMA devices
>>
>> Test on:
>> am335x gp evm
>> am437x gp evm
>> am37x gp evm
>>
>> This rev is pretty much the same as v2 which was blocked due to
>> dependencies to Roger's update GPMC/NAND rework.
>>
>> This updated rev removes this dependency.
>>
>> Also it fixes an issue that was introduced when the eDMA driver was
>> recently updated.
>>
>> Links to rev 3 patchset:
>> https://patchwork.ozlabs.org/patch/595631/
>> https://patchwork.ozlabs.org/patch/595626/
>> https://patchwork.ozlabs.org/patch/595628/
>> https://patchwork.ozlabs.org/patch/595630/
>> https://patchwork.ozlabs.org/patch/595627/
>> https://patchwork.ozlabs.org/patch/595629/
>>
>> Links to rev 2 patchset:
>> https://patchwork.kernel.org/patch/7408691/
>> https://patchwork.kernel.org/patch/7408681/
>> https://patchwork.kernel.org/patch/7408661/
>> https://patchwork.kernel.org/patch/7408641/
>> https://patchwork.kernel.org/patch/7408621/
>>
>> Franklin S Cooper Jr (7):
>> ARM: OMAP2+: gpmc-nand: Set omap2-nand's parent dev to GPMC dev
>> ARM: dts: am33xx: Fix GPMC dma properties
>> ARM: dts: am437x: Fix GPMC dma properties
> Can you please fix the gpmc nodes in dm816x.dtsi and dm814x.dtsi as well?
> You can use linux-next tree or [1] to see the latest changes to these files
> that will end up in v4.6 and base your changes on that.
>
> [1] Tony's omap-for-v4.6/dt
> https://git.kernel.org/cgit/linux/kernel/git/tmlind/linux-omap.git/log/?h=omap-for-v4.6/dt
Sorry I didn't bother looking at dm816 and dm814 dtsi files
since I assumed it didn't use edma since the recent edma
updates didn't make updates for those devices. My mistake.
I'll update those dtsi files.
>> mtd: nand: omap2: Support parsing dma channel information from DT
>> mtd: nand: omap2: Start dma request before enabling prefetch
>> mtd: nand: omap2: Fix high memory dma prefetch transfer
>> ARM: OMAP2+: Update GPMC and NAND DT binding documentation
>>
>> Documentation/devicetree/bindings/bus/ti-gpmc.txt | 7 +++++-
>> .../devicetree/bindings/mtd/gpmc-nand.txt | 2 +-
>> arch/arm/boot/dts/am33xx.dtsi | 2 +-
>> arch/arm/boot/dts/am4372.dtsi | 2 +-
>> arch/arm/mach-omap2/gpmc-nand.c | 16 ++++++++++++-
>> drivers/mtd/nand/omap2.c | 27 +++++++++-------------
>> 6 files changed, 35 insertions(+), 21 deletions(-)
>>
> cheers,
> -roger
On Thu, Mar 10, 2016 at 05:56:43PM -0600, Franklin S Cooper Jr wrote:
> Add additional details to the GPMC NAND documentation to clarify
> what is needed to enable NAND DMA prefetch.
>
> Signed-off-by: Franklin S Cooper Jr <[email protected]>
> ---
> Documentation/devicetree/bindings/bus/ti-gpmc.txt | 7 ++++++-
> Documentation/devicetree/bindings/mtd/gpmc-nand.txt | 2 +-
> 2 files changed, 7 insertions(+), 2 deletions(-)
Acked-by: Rob Herring <[email protected]>
Hi Franklin,
On Thu, 10 Mar 2016 17:56:42 -0600
Franklin S Cooper Jr <[email protected]> wrote:
> Based on DMA documentation and testing using high memory buffer when
> doing dma transfers can lead to various issues including kernel
> panics.
I guess it all comes from the vmalloced buffer case, which are not
guaranteed to be physically contiguous (one of the DMA requirement,
unless you have an iommu).
>
> To workaround this simply use cpu copy. The amount of high memory
> buffers used are very uncommon so no noticeable performance hit should
> be seen.
Hm, that's not necessarily true. UBI and UBIFS allocate their buffers
using vmalloc (vmalloced buffers fall in the high_memory region), and
those are likely to be dis-contiguous if you have NANDs with pages > 4k.
I recently posted patches to ease sg_table creation from any kind of
virtual address [1][2]. Can you try them and let me know if it fixes
your problem?
Thanks,
Boris
[1]https://lkml.org/lkml/2016/3/8/276
[2]https://lkml.org/lkml/2016/3/8/277
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
* Roger Quadros <[email protected]> [160314 02:16]:
>
> I think it is OK to assume that NAND DMA won't work with legacy boot.
> Tony any objections? I see that board-ldp.c is the only legacy user
> of NAND. When can we drop support for it?
Sorry for the delay, just nocied this one. Let's not touch the
board-*.c files until we just remove them except for fixes.
It's OK to assume legacy booting does not support DMA.
Regards,
Tony
* Peter Ujfalusi <[email protected]> [160311 06:01]:
> On 03/11/2016 01:56 AM, Franklin S Cooper Jr wrote:
> > This patch updates the GPMC's DT DMA property to reflect the updated eDMA
> > bindings.
> >
> > Fixes: b5e509066074 ("ARM: DTS: am33xx: Use the new DT bindings for the eDMA3")
>
> I have double checked the switch over to the new eDMA bindings when I sent it
> and there were no gpmc node existed back then afaik.
>
> Acked-by: Peter Ujfalusi <[email protected]>
Applying patches 2 & 3 into omap-for-v4.6/fixes thanks.
Tony
On 03/21/2016 10:04 AM, Boris Brezillon wrote:
> Hi Franklin,
>
> On Thu, 10 Mar 2016 17:56:42 -0600
> Franklin S Cooper Jr <[email protected]> wrote:
>
>> Based on DMA documentation and testing using high memory buffer when
>> doing dma transfers can lead to various issues including kernel
>> panics.
>
> I guess it all comes from the vmalloced buffer case, which are not
> guaranteed to be physically contiguous (one of the DMA requirement,
> unless you have an iommu).
>
>>
>> To workaround this simply use cpu copy. The amount of high memory
>> buffers used are very uncommon so no noticeable performance hit should
>> be seen.
>
> Hm, that's not necessarily true. UBI and UBIFS allocate their buffers
> using vmalloc (vmalloced buffers fall in the high_memory region), and
> those are likely to be dis-contiguous if you have NANDs with pages > 4k.
>
> I recently posted patches to ease sg_table creation from any kind of
> virtual address [1][2]. Can you try them and let me know if it fixes
> your problem?
It looks like you won't be going forward with your patchset based on
this thread [1]. I can probably reword the patch description to avoid
implying that it is uncommon to run into high mem buffers. Also DMA with
NAND prefetch suffers from a reduction of performance compared to CPU
polling with prefetch. This is largely due to the significant over head
required to read such a small amount of data at a time. The
optimizations I've worked on all revolved around reducing the cycles
spent before executing the DMA request. Trying to make a high memory
buffer able to be used by the DMA adds significant amount of cycles and
your better off just using the cpu for performance reasons.
[1]https://lkml.org/lkml/2016/4/4/346
>
> Thanks,
>
> Boris
>
> [1]https://lkml.org/lkml/2016/3/8/276
> [2]https://lkml.org/lkml/2016/3/8/277
>
>
Hi Franklin,
On Wed, 13 Apr 2016 15:08:12 -0500
"Franklin S Cooper Jr." <[email protected]> wrote:
>
>
> On 03/21/2016 10:04 AM, Boris Brezillon wrote:
> > Hi Franklin,
> >
> > On Thu, 10 Mar 2016 17:56:42 -0600
> > Franklin S Cooper Jr <[email protected]> wrote:
> >
> >> Based on DMA documentation and testing using high memory buffer when
> >> doing dma transfers can lead to various issues including kernel
> >> panics.
> >
> > I guess it all comes from the vmalloced buffer case, which are not
> > guaranteed to be physically contiguous (one of the DMA requirement,
> > unless you have an iommu).
> >
> >>
> >> To workaround this simply use cpu copy. The amount of high memory
> >> buffers used are very uncommon so no noticeable performance hit should
> >> be seen.
> >
> > Hm, that's not necessarily true. UBI and UBIFS allocate their buffers
> > using vmalloc (vmalloced buffers fall in the high_memory region), and
> > those are likely to be dis-contiguous if you have NANDs with pages > 4k.
> >
> > I recently posted patches to ease sg_table creation from any kind of
> > virtual address [1][2]. Can you try them and let me know if it fixes
> > your problem?
>
> It looks like you won't be going forward with your patchset based on
> this thread [1].
Nope. According to Russell it's unsafe to do that.
> I can probably reword the patch description to avoid
> implying that it is uncommon to run into high mem buffers. Also DMA with
> NAND prefetch suffers from a reduction of performance compared to CPU
> polling with prefetch. This is largely due to the significant over head
> required to read such a small amount of data at a time. The
> optimizations I've worked on all revolved around reducing the cycles
> spent before executing the DMA request. Trying to make a high memory
> buffer able to be used by the DMA adds significant amount of cycles and
> your better off just using the cpu for performance reasons.
Okay.
One comment though, why not using virt_addr_valid() instead of
addr >= high_memory here?
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On 04/13/2016 03:24 PM, Boris Brezillon wrote:
> Hi Franklin,
>
> On Wed, 13 Apr 2016 15:08:12 -0500
> "Franklin S Cooper Jr." <[email protected]> wrote:
>
>>
>>
>> On 03/21/2016 10:04 AM, Boris Brezillon wrote:
>>> Hi Franklin,
>>>
>>> On Thu, 10 Mar 2016 17:56:42 -0600
>>> Franklin S Cooper Jr <[email protected]> wrote:
>>>
>>>> Based on DMA documentation and testing using high memory buffer when
>>>> doing dma transfers can lead to various issues including kernel
>>>> panics.
>>>
>>> I guess it all comes from the vmalloced buffer case, which are not
>>> guaranteed to be physically contiguous (one of the DMA requirement,
>>> unless you have an iommu).
>>>
>>>>
>>>> To workaround this simply use cpu copy. The amount of high memory
>>>> buffers used are very uncommon so no noticeable performance hit should
>>>> be seen.
>>>
>>> Hm, that's not necessarily true. UBI and UBIFS allocate their buffers
>>> using vmalloc (vmalloced buffers fall in the high_memory region), and
>>> those are likely to be dis-contiguous if you have NANDs with pages > 4k.
>>>
>>> I recently posted patches to ease sg_table creation from any kind of
>>> virtual address [1][2]. Can you try them and let me know if it fixes
>>> your problem?
>>
>> It looks like you won't be going forward with your patchset based on
>> this thread [1].
>
> Nope. According to Russell it's unsafe to do that.
>
>> I can probably reword the patch description to avoid
>> implying that it is uncommon to run into high mem buffers. Also DMA with
>> NAND prefetch suffers from a reduction of performance compared to CPU
>> polling with prefetch. This is largely due to the significant over head
>> required to read such a small amount of data at a time. The
>> optimizations I've worked on all revolved around reducing the cycles
>> spent before executing the DMA request. Trying to make a high memory
>> buffer able to be used by the DMA adds significant amount of cycles and
>> your better off just using the cpu for performance reasons.
>
> Okay.
> One comment though, why not using virt_addr_valid() instead of
> addr >= high_memory here?
I had no reason other than simply using the approach used in the driver
already. Virt_addr_valid looks like it will work so I'll make the switch
after testing it.
>
> Best Regards,
>
> Boris
>
>