All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roger Quadros <rogerq@ti.com>
To: Ladislav Michl <ladis@linux-mips.org>, Tony Lindgren <tony@atomide.com>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>,
	Boris Brezillon <boris.brezillon@free-electrons.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	linux-omap@vger.kernel.org, linux-mtd@lists.infradead.org
Subject: Re: [PATCH v3 5/7] mtd: onenand: omap2: Unify OMAP2 and OMAP3 DMA implementation
Date: Tue, 14 Nov 2017 16:47:46 +0200	[thread overview]
Message-ID: <f44d5df7-fd46-4873-3deb-e6b9ad916ab0@ti.com> (raw)
In-Reply-To: <20171111125054.4gkttjltqrgjqtwt@lenoch>

On 11/11/17 14:50, Ladislav Michl wrote:
> On Fri, Nov 10, 2017 at 10:29:38AM -0800, Tony Lindgren wrote:
>> I just made sure things keep working with Peter's changes, no additional
>> patches. So the dma is barely used at all.
> 
> It seemed suspicious to me looking here:
> https://github.com/omap-audio/linux-audio/commit/52e914395afe31a7401b35814b6dabf3ef46a052#diff-1662a4cc544d322b68c547cfdc99448cR312
> as wait_for_completion_timeout is returning time remaining and zero on
> timeout :-)
> Fixed in next version and also changed to wait_for_completion_io_timeout
> to get proper sched stats.
> Btw, I think we should stop DMA on timeout as we are unmapping DMA region
> later and it might not behave nicely if we do so while DMA is eventually
> running (that's fixed too).
> 
> I made Peter's patches part of next version and enabled DMA unconditionally.

That's cool, but I'd still hold from making it default in first go.
It could have more testing and benchmarking.

> It works nicely on:
> OF: fdt: Machine model: IGEPv2 Rev. C (TI OMAP AM/DM37x)
> OMAP3630/DM3730 ES1.2 (l2cache iva sgx neon isp 192mhz_clk)
> ...
> omap-gpmc 6e000000.gpmc: GPMC revision 5.0
> gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
> omap2-onenand 30000000.onenand: initializing on CS0 (0x30000000), va e0080000, DMA mode
> Muxed OneNAND(DDP) 512MB 1.8V 16-bit (0x58)
> OneNAND version = 0x0031
> Scanning device for bad blocks
> OneNAND eraseblock 597 is an initial bad block
> OneNAND eraseblock 1159 is an initial bad block
> OneNAND eraseblock 2812 is an initial bad block
> omap2-onenand 30000000.onenand: optimized timings for 83 MHz
> 2 ofpart partitions found on MTD device 30000000.onenand
> Creating 2 MTD partitions on "30000000.onenand":
> 0x000000000000-0x000000080000 : "SPL"
> 0x000000080000-0x000020000000 : "UBI"
> ...
> 
> Based on this experiment, I'll delay v4 a bit and leave DMA enabled by
> default. We are brave enough and want some testing, right?
> It would be nice if someone could provide some benchmarks using PIO
> and DMA mode. I do not expect any dramatic change, but kernel latencies
> should decrease when doing flash I/O.

There should be noticeable difference between both when the CPU is loaded.

> 
> Based on above I also think R/B pin should be in ordinary GPIO mode.
> Perhaps using external DMA request is somehow posible, but it is not clear
> to me how after yet another reading of SDMA, GPMC and OneNAND documentation,
> so I'll happily leave it for others.

At least on NAND we use R/B as a plain GPIO that is polled till it shows
ready during NAND erase/program cycles.
e.g. see nand_wait() in nand/nand_base.c
and omap_dev_ready() in nand/omap2.c

So ready checking could be done similarly in onenand core?

> 
> Btw, using R/B pin would elimitate latencies caused by busy looping while
> waiting for onenand command to complete. Nice todo for someone with a lot
> of spare time :-)
> 
> Best regards,
> 	ladis
> 

-- 
cheers,
-roger

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
From: Roger Quadros <rogerq@ti.com>
To: Ladislav Michl <ladis@linux-mips.org>, Tony Lindgren <tony@atomide.com>
Cc: <linux-mtd@lists.infradead.org>, <linux-omap@vger.kernel.org>,
	Peter Ujfalusi <peter.ujfalusi@ti.com>,
	Boris Brezillon <boris.brezillon@free-electrons.com>,
	Kyungmin Park <kyungmin.park@samsung.com>
Subject: Re: [PATCH v3 5/7] mtd: onenand: omap2: Unify OMAP2 and OMAP3 DMA implementation
Date: Tue, 14 Nov 2017 16:47:46 +0200	[thread overview]
Message-ID: <f44d5df7-fd46-4873-3deb-e6b9ad916ab0@ti.com> (raw)
In-Reply-To: <20171111125054.4gkttjltqrgjqtwt@lenoch>

On 11/11/17 14:50, Ladislav Michl wrote:
> On Fri, Nov 10, 2017 at 10:29:38AM -0800, Tony Lindgren wrote:
>> I just made sure things keep working with Peter's changes, no additional
>> patches. So the dma is barely used at all.
> 
> It seemed suspicious to me looking here:
> https://github.com/omap-audio/linux-audio/commit/52e914395afe31a7401b35814b6dabf3ef46a052#diff-1662a4cc544d322b68c547cfdc99448cR312
> as wait_for_completion_timeout is returning time remaining and zero on
> timeout :-)
> Fixed in next version and also changed to wait_for_completion_io_timeout
> to get proper sched stats.
> Btw, I think we should stop DMA on timeout as we are unmapping DMA region
> later and it might not behave nicely if we do so while DMA is eventually
> running (that's fixed too).
> 
> I made Peter's patches part of next version and enabled DMA unconditionally.

That's cool, but I'd still hold from making it default in first go.
It could have more testing and benchmarking.

> It works nicely on:
> OF: fdt: Machine model: IGEPv2 Rev. C (TI OMAP AM/DM37x)
> OMAP3630/DM3730 ES1.2 (l2cache iva sgx neon isp 192mhz_clk)
> ...
> omap-gpmc 6e000000.gpmc: GPMC revision 5.0
> gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000
> omap2-onenand 30000000.onenand: initializing on CS0 (0x30000000), va e0080000, DMA mode
> Muxed OneNAND(DDP) 512MB 1.8V 16-bit (0x58)
> OneNAND version = 0x0031
> Scanning device for bad blocks
> OneNAND eraseblock 597 is an initial bad block
> OneNAND eraseblock 1159 is an initial bad block
> OneNAND eraseblock 2812 is an initial bad block
> omap2-onenand 30000000.onenand: optimized timings for 83 MHz
> 2 ofpart partitions found on MTD device 30000000.onenand
> Creating 2 MTD partitions on "30000000.onenand":
> 0x000000000000-0x000000080000 : "SPL"
> 0x000000080000-0x000020000000 : "UBI"
> ...
> 
> Based on this experiment, I'll delay v4 a bit and leave DMA enabled by
> default. We are brave enough and want some testing, right?
> It would be nice if someone could provide some benchmarks using PIO
> and DMA mode. I do not expect any dramatic change, but kernel latencies
> should decrease when doing flash I/O.

There should be noticeable difference between both when the CPU is loaded.

> 
> Based on above I also think R/B pin should be in ordinary GPIO mode.
> Perhaps using external DMA request is somehow posible, but it is not clear
> to me how after yet another reading of SDMA, GPMC and OneNAND documentation,
> so I'll happily leave it for others.

At least on NAND we use R/B as a plain GPIO that is polled till it shows
ready during NAND erase/program cycles.
e.g. see nand_wait() in nand/nand_base.c
and omap_dev_ready() in nand/omap2.c

So ready checking could be done similarly in onenand core?

> 
> Btw, using R/B pin would elimitate latencies caused by busy looping while
> waiting for onenand command to complete. Nice todo for someone with a lot
> of spare time :-)
> 
> Best regards,
> 	ladis
> 

-- 
cheers,
-roger

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

  parent reply	other threads:[~2017-11-14 14:47 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-09  9:11 [PATCH v3 0/7] OMAP2+ OneNAND driver update Ladislav Michl
2017-11-09  9:11 ` Ladislav Michl
2017-11-09  9:12 ` [PATCH v3 1/7] memory: omap-gpmc: Refactor OneNAND support Ladislav Michl
2017-11-09  9:12   ` Ladislav Michl
2017-11-09 17:56   ` Tony Lindgren
2017-11-09 17:56     ` Tony Lindgren
2017-11-09 18:10     ` Ladislav Michl
2017-11-09 18:10       ` Ladislav Michl
2017-11-09 18:26       ` Tony Lindgren
2017-11-09 18:26         ` Tony Lindgren
2017-11-09 18:34         ` Ladislav Michl
2017-11-09 18:34           ` Ladislav Michl
2017-11-09 18:48           ` Tony Lindgren
2017-11-09 18:48             ` Tony Lindgren
2017-11-09 19:10             ` Ladislav Michl
2017-11-09 19:10               ` Ladislav Michl
2017-11-09 21:59               ` Tony Lindgren
2017-11-09 21:59                 ` Tony Lindgren
2017-11-09 22:26                 ` Ladislav Michl
2017-11-09 22:26                   ` Ladislav Michl
2017-11-10  8:12             ` Roger Quadros
2017-11-10  8:12               ` Roger Quadros
2017-11-09  9:13 ` [PATCH v3 2/7] mtd: onenand: omap2: Remove regulator support Ladislav Michl
2017-11-09  9:13   ` Ladislav Michl
2017-11-10  8:17   ` Roger Quadros
2017-11-10  8:17     ` Roger Quadros
2017-11-09  9:14 ` [PATCH v3 3/7] mtd: onenand: omap2: Remove skip initial unlocking support Ladislav Michl
2017-11-09  9:14   ` Ladislav Michl
2017-11-10  8:18   ` Roger Quadros
2017-11-10  8:18     ` Roger Quadros
2017-11-09  9:14 ` [PATCH v3 4/7] mtd: onenand: omap2: Remove partitioning support from platform data Ladislav Michl
2017-11-09  9:14   ` Ladislav Michl
2017-11-10  8:19   ` Roger Quadros
2017-11-10  8:19     ` Roger Quadros
2017-11-10  9:48     ` Ladislav Michl
2017-11-10  9:48       ` Ladislav Michl
2017-11-09  9:15 ` [PATCH v3 5/7] mtd: onenand: omap2: Unify OMAP2 and OMAP3 DMA implementation Ladislav Michl
2017-11-09  9:15   ` Ladislav Michl
2017-11-10  8:21   ` Roger Quadros
2017-11-10  8:21     ` Roger Quadros
2017-11-10  9:51     ` Ladislav Michl
2017-11-10  9:51       ` Ladislav Michl
2017-11-10 15:26       ` Tony Lindgren
2017-11-10 15:26         ` Tony Lindgren
2017-11-10 18:19         ` Ladislav Michl
2017-11-10 18:19           ` Ladislav Michl
2017-11-10 18:29           ` Tony Lindgren
2017-11-10 18:29             ` Tony Lindgren
2017-11-11 12:50             ` Ladislav Michl
2017-11-11 12:50               ` Ladislav Michl
2017-11-13 20:10               ` Peter Ujfalusi
2017-11-13 20:10                 ` Peter Ujfalusi
2017-11-14 14:47               ` Roger Quadros [this message]
2017-11-14 14:47                 ` Roger Quadros
2017-11-14 15:03                 ` Ladislav Michl
2017-11-14 15:03                   ` Ladislav Michl
2017-11-10  8:25   ` Peter Ujfalusi
2017-11-10  8:25     ` Peter Ujfalusi
2017-11-10 10:04     ` Ladislav Michl
2017-11-10 10:04       ` Ladislav Michl
2017-11-10 15:24       ` Tony Lindgren
2017-11-10 15:24         ` Tony Lindgren
2017-11-10 18:26         ` Ladislav Michl
2017-11-10 18:26           ` Ladislav Michl
2017-11-10 18:48           ` Tony Lindgren
2017-11-10 18:48             ` Tony Lindgren
2017-11-10 21:39         ` Ladislav Michl
2017-11-10 21:39           ` Ladislav Michl
2017-11-14 21:53           ` Tony Lindgren
2017-11-14 21:53             ` Tony Lindgren
2017-11-14 22:32             ` Ladislav Michl
2017-11-14 22:32               ` Ladislav Michl
2017-11-15  2:11               ` Tony Lindgren
2017-11-15  2:11                 ` Tony Lindgren
2017-11-13  8:22         ` Peter Ujfalusi
2017-11-13  8:22           ` Peter Ujfalusi
2017-11-13 12:15           ` Ladislav Michl
2017-11-13 12:15             ` Ladislav Michl
2017-11-13 14:36             ` Peter Ujfalusi
2017-11-13 14:36               ` Peter Ujfalusi
2017-11-13 15:23               ` Tony Lindgren
2017-11-13 15:23                 ` Tony Lindgren
2017-11-13 15:27               ` Ladislav Michl
2017-11-13 15:27                 ` Ladislav Michl
2017-11-14 15:05                 ` Roger Quadros
2017-11-14 15:05                   ` Roger Quadros
2017-11-14 15:22                   ` Ladislav Michl
2017-11-14 15:22                     ` Ladislav Michl
2017-11-09  9:16 ` [PATCH v3 6/7] mtd: onenand: omap2: Do not make delay for GPIO OMAP3 specific Ladislav Michl
2017-11-09  9:16   ` Ladislav Michl
2017-11-09  9:17 ` [PATCH v3 7/7] mtd: onenand: omap2: Configure driver from DT Ladislav Michl
2017-11-09  9:17   ` Ladislav Michl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f44d5df7-fd46-4873-3deb-e6b9ad916ab0@ti.com \
    --to=rogerq@ti.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=kyungmin.park@samsung.com \
    --cc=ladis@linux-mips.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=peter.ujfalusi@ti.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.