mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nikita Shubin <nikita.shubin@maquefel.me>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Linus Walleij <linus.walleij@linaro.org>,
	Jean Delvare <jdelvare@suse.de>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Christophe Kerello <christophe.kerello@foss.st.com>,
	Chris Packham <chris.packham@alliedtelesis.co.nz>,
	Johan Jonker <jbx6244@gmail.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org
Subject: Re: [RFC PATCH] mtd: nand: add support for ts72xx
Date: Sat, 30 Sep 2023 19:06:42 +0300	[thread overview]
Message-ID: <8bbe66a23eb5c8a2404b72d754b1bcb6f4d23867.camel@maquefel.me> (raw)
In-Reply-To: <20230927172947.33106fe2@xps-13>

Hello Miquèl!

On Wed, 2023-09-27 at 17:29 +0200, Miquel Raynal wrote:
> Hi Nikita,
> 
> nikita.shubin@maquefel.me wrote on Wed, 27 Sep 2023 17:15:25 +0300:
> 
> > Technologic Systems has it's own nand controller implementation in
> > CPLD.
> > 
> > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> > ---
> > Hello Miquèl.
> > 
> > Can you please take a look on it as RFC, so no need for the whole
> > series spinning ?
> > 
> > I've got rid of all legacy stuff i think, however:
> 
> I am glad you did, thanks a lot!
> 
> > - look's like i me ts7250 is missing READCACHE and i couldn't find
> > any docs 
> >   for this controller, so as legacy had no READCACHE, i mark it as
> > non-supported
> 
> What NAND device do you use?
> 
> The controller clearly supports it, as it just forwards whatever
> addrs/cmd/data cycle you input. However we are trying to fix the
> cache
> reads support which is unstable. If you use a NAND that is non-JEDEC
> and non-ONFI then please apply this and you will no longer be
> bothered
> by it:
> https://lore.kernel.org/linux-mtd/20230926132725.5d570e1b@xps-13/T/#md7e5e944a6a08e24f4f1e20068a49f94794ab945
> 

You are totally right - controller and chip are different entities, i
somehow forgot about it:

```
# modprobe technologic-nand-controller
nand: device found, Manufacturer ID: 0xec, Chip ID: 0xf1
nand: Samsung NAND 128MiB 3,3V 8-bit
nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
Scanning device for bad blocks
Bad eraseblock 137 at 0x000001120000
Bad eraseblock 310 at 0x0000026c0000
3 fixed-partitions partitions found on MTD device 60000000.nand-
controller
Creating 3 MTD partitions on "60000000.nand-controller":
0x000000000000-0x000000020000 : "TS-BOOTROM"
0x000000020000-0x000007d20000 : "Linux"
0x000007d20000-0x000008000000 : "RedBoot"
```

This looks like Samsung K9F1G08U0D or K9F1G08U0B.

And the patch above totally solves my issues with READCACHE - thank
you!

> If you are using a Micron chip that specifies not supporting ECC
> correction together with sequential cache reads, then it's a problem
> that we will solve soon (you can hardcode
> chip->controller->supported_op.cont_read to 0);
> 
> Otherwise we can discuss it.
> 
> > - legacy wait implementation had no delays - only timeout and if i
> > set 
> >   readb_poll_timeout() to something reasonable i experience speed
> > degradation 
> >   on mtd_speedtest, can it be left as 0 ?
> 
> Looks like the legacy implementation used cond_resched(). The delay
> needs to be observed before first checking for the status, it's a
> delay
> that is described in the spec, if you get the status before you might
> just not see a consistent value. I believe that is worth taking into
> account in your implementation below (don't wait then delay, it's
> not relevant). Can you share the values you've tried and the
> performances you've got?

The numbers are pretty stable, so indeed legacy is a bit faster,
followed up by new one with zero interval.

Legacy speedtest (old version of nand controller):

```
# insmod mtd_speedtest.ko dev=1

=================================================
mtd_speedtest: MTD device: 1
mtd_speedtest: MTD device size 131072000, eraseblock size 131072, page
size 2048, count of eraseblocks 1000, pages per eraseblock 64, OOB size
64
mtd_test: scanning for bad eraseblocks
mtd_test: block 136 is bad
mtd_test: block 309 is bad
mtd_test: scanned 1000 eraseblocks, 2 are bad
mtd_speedtest: testing eraseblock write speed
mtd_speedtest: eraseblock write speed is 3793 KiB/s
mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 3567 KiB/s
mtd_speedtest: testing page write speed
mtd_speedtest: page write speed is 3682 KiB/s
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 3488 KiB/s
mtd_speedtest: testing 2 page write speed
mtd_speedtest: 2 page write speed is 3724 KiB/s
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 3521 KiB/s
mtd_speedtest: Testing erase speed
mtd_speedtest: erase speed is 198709 KiB/s
mtd_speedtest: Testing 2x multi-block erase speed
mtd_speedtest: 2x multi-block erase speed is 200638 KiB/s
mtd_speedtest: Testing 4x multi-block erase speed
mtd_speedtest: 4x multi-block erase speed is 201545 KiB/s
mtd_speedtest: Testing 8x multi-block erase speed
mtd_speedtest: 8x multi-block erase speed is 202341 KiB/s
mtd_speedtest: Testing 16x multi-block erase speed
mtd_speedtest: 16x multi-block erase speed is 202909 KiB/s
mtd_speedtest: Testing 32x multi-block erase speed
mtd_speedtest: 32x multi-block erase speed is 203095 KiB/s
mtd_speedtest: Testing 64x multi-block erase speed
mtd_speedtest: 64x multi-block erase speed is 203482 KiB/s
mtd_speedtest: finished
=================================================
```

New version with interval zero:

```
# insmod mtd_speedtest.ko dev=1

=================================================
mtd_speedtest: MTD device: 1
mtd_speedtest: MTD device size 131072000, eraseblock size 131072, page
size 2048, count of eraseblocks 1000, pages per eraseblock 64, OOB size
64
mtd_test: scanning for bad eraseblocks
mtd_test: block 136 is bad
mtd_test: block 309 is bad
mtd_test: scanned 1000 eraseblocks, 2 are bad
mtd_speedtest: testing eraseblock write speed
mtd_speedtest: eraseblock write speed is 3685 KiB/s
mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 3517 KiB/s
mtd_speedtest: testing page write speed
mtd_speedtest: page write speed is 3592 KiB/s
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 3444 KiB/s
mtd_speedtest: testing 2 page write speed
mtd_speedtest: 2 page write speed is 3608 KiB/s
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 3475 KiB/s
mtd_speedtest: Testing erase speed
mtd_speedtest: erase speed is 194499 KiB/s
mtd_speedtest: Testing 2x multi-block erase speed
mtd_speedtest: 2x multi-block erase speed is 196316 KiB/s
mtd_speedtest: Testing 4x multi-block erase speed
mtd_speedtest: 4x multi-block erase speed is 198305 KiB/s
mtd_speedtest: Testing 8x multi-block erase speed
mtd_speedtest: 8x multi-block erase speed is 199263 KiB/s
mtd_speedtest: Testing 16x multi-block erase speed
mtd_speedtest: 16x multi-block erase speed is 199548 KiB/s
mtd_speedtest: Testing 32x multi-block erase speed
mtd_speedtest: 32x multi-block erase speed is 200221 KiB/s
mtd_speedtest: Testing 64x multi-block erase speed
mtd_speedtest: 64x multi-block erase speed is 200579 KiB/s
mtd_speedtest: finished
=================================================
```

New version with interval 100:

```
# insmod mtd_speedtest.ko dev=1

=================================================
mtd_speedtest: MTD device: 1
mtd_speedtest: MTD device size 131072000, eraseblock size 131072, page
size 2048, count of eraseblocks 1000, pages per eraseblock 64, OOB size
64
mtd_test: scanning for bad eraseblocks
mtd_test: block 136 is bad
mtd_test: block 309 is bad
mtd_test: scanned 1000 eraseblocks, 2 are bad
mtd_speedtest: testing eraseblock write speed
mtd_speedtest: eraseblock write speed is 2722 KiB/s
mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 2175 KiB/s
mtd_speedtest: testing page write speed
mtd_speedtest: page write speed is 2598 KiB/s
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 2070 KiB/s
mtd_speedtest: testing 2 page write speed
mtd_speedtest: 2 page write speed is 2627 KiB/s
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 2106 KiB/s
mtd_speedtest: Testing erase speed
mtd_speedtest: erase speed is 175851 KiB/s
mtd_speedtest: Testing 2x multi-block erase speed
mtd_speedtest: 2x multi-block erase speed is 181582 KiB/s
mtd_speedtest: Testing 4x multi-block erase speed
mtd_speedtest: 4x multi-block erase speed is 181579 KiB/s
mtd_speedtest: Testing 8x multi-block erase speed
mtd_speedtest: 8x multi-block erase speed is 182735 KiB/s
mtd_speedtest: Testing 16x multi-block erase speed
mtd_speedtest: 16x multi-block erase speed is 183589 KiB/s
mtd_speedtest: Testing 32x multi-block erase speed
mtd_speedtest: 32x multi-block erase speed is 183003 KiB/s
mtd_speedtest: Testing 64x multi-block erase speed
mtd_speedtest: 64x multi-block erase speed is 183460 KiB/s
mtd_speedtest: finished
=================================================
```

Also providing version with zero interval and "if (instr->delay_ns)"
dropped - it's optional as far i understood:

```
# insmod mtd_speedtest.ko dev=1

=================================================
mtd_speedtest: MTD device: 1
mtd_speedtest: MTD device size 131072000, eraseblock size 131072, page
size 2048, count of eraseblocks 1000, pages per eraseblock 64, OOB size
64
mtd_test: scanning for bad eraseblocks
mtd_test: block 136 is bad
mtd_test: block 309 is bad
mtd_test: scanned 1000 eraseblocks, 2 are bad
mtd_speedtest: testing eraseblock write speed
mtd_speedtest: eraseblock write speed is 3695 KiB/s
mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 3532 KiB/s
mtd_speedtest: testing page write speed
mtd_speedtest: page write speed is 3593 KiB/s
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 3457 KiB/s
mtd_speedtest: testing 2 page write speed
mtd_speedtest: 2 page write speed is 3640 KiB/s
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 3488 KiB/s
mtd_speedtest: Testing erase speed
mtd_speedtest: erase speed is 195451 KiB/s
mtd_speedtest: Testing 2x multi-block erase speed
mtd_speedtest: 2x multi-block erase speed is 198538 KiB/s
mtd_speedtest: Testing 4x multi-block erase speed
mtd_speedtest: 4x multi-block erase speed is 199509 KiB/s
mtd_speedtest: Testing 8x multi-block erase speed
mtd_speedtest: 8x multi-block erase speed is 199871 KiB/s
mtd_speedtest: Testing 16x multi-block erase speed
mtd_speedtest: 16x multi-block erase speed is 200075 KiB/s
mtd_speedtest: Testing 32x multi-block erase speed
mtd_speedtest: 32x multi-block erase speed is 200683 KiB/s
mtd_speedtest: Testing 64x multi-block erase speed
mtd_speedtest: 64x multi-block erase speed is 200850 KiB/s
mtd_speedtest: finished
=================================================

```


> 
> Thanks,
> Miquèl
> 


  reply	other threads:[~2023-09-30 16:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-27 14:15 Nikita Shubin
2023-09-27 15:29 ` Miquel Raynal
2023-09-30 16:06   ` Nikita Shubin [this message]
2023-10-02  9:30     ` Miquel Raynal

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=8bbe66a23eb5c8a2404b72d754b1bcb6f4d23867.camel@maquefel.me \
    --to=nikita.shubin@maquefel.me \
    --cc=arnd@arndb.de \
    --cc=chris.packham@alliedtelesis.co.nz \
    --cc=christophe.kerello@foss.st.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=jbx6244@gmail.com \
    --cc=jdelvare@suse.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=neil.armstrong@linaro.org \
    --cc=r.czerwinski@pengutronix.de \
    --cc=richard@nod.at \
    --cc=vigneshr@ti.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®