Re: [RFC PATCH] mtd: nand: add support for ts72xx

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Nikita Shubin <nikita.shubin@maquefel.me>
Cc: 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: Wed, 27 Sep 2023 17:29:47 +0200	[thread overview]
Message-ID: <20230927172947.33106fe2@xps-13> (raw)
In-Reply-To: <20230927141532.25525-1-nikita.shubin@maquefel.me>

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

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?

Thanks,
Miquèl

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