From: Marek Vasut <marex@denx.de>
To: hs@denx.de, u-boot@lists.denx.de
Cc: "Jagan Teki" <jagan@amarulasolutions.com>,
"Marek Behún" <marek.behun@nic.cz>,
"Miquel Raynal" <miquel.raynal@bootlin.com>,
"Pali Rohár" <pali@kernel.org>,
"Patrice Chotard" <patrice.chotard@foss.st.com>,
"Patrick Delaunay" <patrick.delaunay@st.com>,
"Priyanka Jain" <priyanka.jain@nxp.com>,
"Simon Glass" <sjg@chromium.org>
Subject: Re: [PATCH] mtd: spi-nor: Fix SF MTDIDS when registering multiple MTDs with DM enabled
Date: Wed, 15 Sep 2021 07:59:55 +0200 [thread overview]
Message-ID: <fa7f2e07-30c3-dc7b-64ce-7d971c56d2cc@denx.de> (raw)
In-Reply-To: <58930899-7190-9c36-6377-cc0c41d1800a@denx.de>
On 9/15/21 6:23 AM, Heiko Schocher wrote:
> Hi Marek,
>
> On 15.09.21 01:06, Marek Vasut wrote:
>> The flash->mtd.name used to be nor%d before, now it is the type of the
>> SPI NOR like e.g. mt25ql02g. It is possible to find plenty of examples
>> of the former in U-Boot configs by searching for MTDIDS.*nor.*spi, while
>> the later is ambiguous if there are multiple flashes of the same type in
>> the system and breaks existing environments.
>>
>> This does no longer get recognized when running 'mtdparts' for example:
>> CONFIG_MTDIDS_DEFAULT="nor0=47040000.spi.0"
>>
>> Fix this by setting the correct mtd.name to nor%d.
>>
>> Fixes: b7f060565e3 ("mtd: spi-nor: allow registering multiple MTDs when DM is enabled")
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Heiko Schocher <hs@denx.de>
>> Cc: Jagan Teki <jagan@amarulasolutions.com>
>> Cc: Marek Behún <marek.behun@nic.cz>
>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>> Cc: Pali Rohár <pali@kernel.org>
>> Cc: Patrice Chotard <patrice.chotard@foss.st.com>
>> Cc: Patrick Delaunay <patrick.delaunay@st.com>
>> Cc: Priyanka Jain <priyanka.jain@nxp.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>> drivers/mtd/spi/sf_mtd.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> Seem fixes the same problem as Patrick already posted here:
>
> https://patchwork.ozlabs.org/project/uboot/patch/20210913095742.v2.1.I73dae4b93f0587dc130e512e95a1f4794e0b0233@changeid/
>
> I find your approach cleaner, so:
>
> Acked-by: Heiko Schocher <hs@denx.de>
>
> @Patrick: Could you test this patch please?
The option from Patrick does look more predictable, but looking at the
old implementation of spi_flash_mtd_number(), that one handled even
cases of parallel-nor and spi-nor (both using the nor%d) present on the
same system. This:
static int spi_flash_mtd_number(void)
{
#ifdef CONFIG_SYS_MAX_FLASH_BANKS
return CONFIG_SYS_MAX_FLASH_BANKS;
#else
return 0;
#endif
}
...
sprintf(sf_mtd_name, "nor%d", spi_flash_mtd_number());
The patch from Patrick does not, it does per-spi-nor-only counting using
the dev_seq() there, which is more predictable, but local to spi-nor.
The MTD core has its own IDR counting, which is used by this patch and
is global to the MTD core. That has two issues, one is that it is
possibly too global and counts both nor%d and nand%d, which means it
would fail on systems with both SPI NOR and regular NAND. The other is
it is likely less predictable (what happens if the SPI NOR and parallel
NOR gets probed in the reverse order ? I know it is unlikely, but it can
happen in the future).
So I think we need a third option which I would suggest be based on the
patch from Patrick, but which would take into account the other nor%d
devices like parallel NOR flashes.
That would likely have to happen in the mtd core using some modified IDR
counting. I think you might need to modify it such that you would pass
in the prefix of the device (like 'nor') and then do per-prefix
counting, with the special case that parallel NORs are counted first.
Also, that would also have to consider probing of multiple SPI NORs in
either order (say you have two, if you do 'sf probe 0:0 ; sf probe 0:1'
vs. 'sf probe 0:1 ; sf probe 0:0'), and make sure they get enumerated
with the same nor%d value either way, that might be where the dev_seq()
would come in.
next prev parent reply other threads:[~2021-09-15 6:03 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-14 23:06 [PATCH] mtd: spi-nor: Fix SF MTDIDS when registering multiple MTDs with DM enabled Marek Vasut
2021-09-15 4:23 ` Heiko Schocher
2021-09-15 5:59 ` Marek Vasut [this message]
2021-09-15 8:53 ` Patrice CHOTARD
2021-09-15 8:57 ` Marek Vasut
2021-09-15 9:17 ` Patrice CHOTARD
2021-09-15 9:21 ` Marek Vasut
2021-09-15 12:00 ` Patrick DELAUNAY
2021-09-15 12:10 ` Marek Vasut
2021-09-16 7:50 ` Patrick DELAUNAY
2021-09-16 14:06 ` Patrick DELAUNAY
2021-09-16 15:16 ` Marek Behún
2021-09-16 17:40 ` Marek Vasut
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=fa7f2e07-30c3-dc7b-64ce-7d971c56d2cc@denx.de \
--to=marex@denx.de \
--cc=hs@denx.de \
--cc=jagan@amarulasolutions.com \
--cc=marek.behun@nic.cz \
--cc=miquel.raynal@bootlin.com \
--cc=pali@kernel.org \
--cc=patrice.chotard@foss.st.com \
--cc=patrick.delaunay@st.com \
--cc=priyanka.jain@nxp.com \
--cc=sjg@chromium.org \
--cc=u-boot@lists.denx.de \
/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.