Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Date:   Wed, 27 Sep 2023 17:29:47 +0200
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
Message-ID: <20230927172947.33106fe2@xps-13>
In-Reply-To: <20230927141532.25525-1-nikita.shubin@maquefel.me>
References: <20230927141532.25525-1-nikita.shubin@maquefel.me>
Organization: Bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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.
>=20
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
> Hello Miqu=C3=A8l.
>=20
> Can you please take a look on it as RFC, so no need for the whole series =
spinning ?
>=20
> 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 do=
cs=20
>   for this controller, so as legacy had no READCACHE, i mark it as non-su=
pported

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/#md7e5e9=
44a6a08e24f4f1e20068a49f94794ab945

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=20
>   readb_poll_timeout() to something reasonable i experience speed degrada=
tion=20
>   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=C3=A8l