u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: "Marek Behún" <marek.behun@nic.cz>
Cc: "Patrick Delaunay" <patrick.delaunay@foss.st.com>,
	"Tom Rini" <trini@konsulko.com>,
	u-boot@lists.denx.de, "Pali Rohár" <pali@kernel.org>,
	"Jagan Teki" <jagan@amarulasolutions.com>,
	"Christophe KERELLO" <christophe.kerello@foss.st.com>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Priyanka Jain" <priyanka.jain@nxp.com>,
	"Patrice Chotard" <patrice.chotard@foss.st.com>,
	"Heiko Schocher" <hs@denx.de>, "Simon Glass" <sjg@chromium.org>,
	"Vignesh R" <vigneshr@ti.com>,
	"U-Boot STM32" <uboot-stm32@st-md-mailman.stormreply.com>
Subject: Re: [PATCH v4 0/2] mtd: spi: nor: force mtd name to "nor%d"
Date: Wed, 22 Sep 2021 21:24:24 +0200	[thread overview]
Message-ID: <56df80f7-aa1d-3cff-5b29-16fdafcf7bcf@denx.de> (raw)
In-Reply-To: <20210922210536.6c9c2f9e@thinkpad>

On 9/22/21 9:05 PM, Marek Behún wrote:

Hi,

[...]

>>> I find this either not complete or not needed:
>>>
>>> - either you need mtd names to be of this format so that old MTDPARTS
>>>     config definitions do not need to be changed, i.e. something like
>>>       CONFIG_MTDPARTS_DEFAULT="nor0:1M(u-boot),0x1000@0xfff000(env)"
>>>     does not work currently, and you want to make it work.
>>>
>>>     I find your solution here incomplete because MTDPARTS can also be
>>>     used to be passed to Linux as mtdparts parameter, but there is no
>>>     guarantee that the "norN" numbering you are creating in U-Boot will
>>>     be the same as the one in kernel.
>>>
>>> - or it is not needed, because you can remove MTDPARTS definition from
>>>     the board config entirely and move the information into device tree.
>>>     In fact this was the main idea behind making the series
>>>       Support SPI NORs and OF partitions in `mtd list`
>>>     The SPI-NOR MTDs after this series can have conflicting names,
>>>     because you can still choose between them via OF path with the `mtd`
>>>     command.
>>>
>>>     Tom and I were of the opinion that MTDPARTS should be deprecated and
>>>     removed in favor of OF. Marek Vasut says that this is not possible
>>>     for every board, and so needs to stay.
>>>
>>> BTW, I find it a little weird for Marek to defend old API which should
>>> be converted to DT, when in discussion about DM USB / Nokia N900
>>> USB TTY console [1] he was defending the opinion that we should be
>>> heading to DT in U-Boot.
>>>
>>> [1]
>>> https://patchwork.ozlabs.org/project/uboot/patch/20210618145724.2558-1-pali@kernel.org/
>>
>> That USB discussion is completely unrelated to the problem here, the USB
>> discussion is about internal (i.e. not user facing) conversion to DM/DT.
>> The user-facing ABI does not change there. Also, that discussion was
>> about patching USB stack to permit new non-DM/DT operation, not fixing
>> existing one.
> 
> This is not only about the user ABI (altough now I agree that you are
> correct there, see below). What I meant is this:
>    Should we push for converting to device-tree even if for some boards
>    it is not possible, and would mean removing them?

The N900 could however be converted to DT as far as I can tell, there 
was a solution which didn't end up patching the USB core with legacy stuff.

>    Because you are saying that MTDPARTS cannot be converted to DT for
>    some boards.
> 
>    But N900 also cannot be reasonably converted because of space
>    issues, as far as I understood. Yes, it has gigabytes of eMMC storage,
>    and it was proposed to put SPL in MTD and U-Boot proper into eMMC on
>    VFAT/ext4, but this simply cannot be done reasonably, because:
>    - it would break Linux userspace (existing OS upgrade system would
>      have to be rewritten and backwords compatibility would be broken)

Not really, there was also the suggestion to use falcon boot and have 
SPL boot Linux directly (i.e. use SPL instead of U-Boot), while only put 
U-Boot into eMMC and boot it if needed and/or as a fallback.

That way you wouldn't break the existing updaters, because you install 
that SPL instead of current U-Boot, and new one could be added to 
install the extra u-boot binary. And that solves your space issue, 
forever no less.

>    - it would make bootstrapping (booting newer version of U-Boot) while
>      developing U-Boot a pain in the ass or maybe even impossible

Booting U-Boot from U-Boot is unsupported as far as I can tell. It may 
or may not work.

>    - I beleive there was some other reason Pali mentioned, but I cannot
>      remember anymore
> 
>> This problem here is user facing ABI, the mtdparts/mtdids. That user
>> facing ABI got broken. Boards which do depend on it, even those
>> currently in tree, are broken. Not all boards can update their
>> environment, so some backward compatibility of the user facing ABI
>> should be in place, even though it might not be to the degree Linux
>> kernel does so. So far, it seems most of the U-Boot command line
>> interface has managed to retain backward compatibility, I don't see why
>> this here should be handled any differently.
> 
> OK, I get that the if `mtd nor0` was working before, it should work also
> now. But the conversion from MTDPARTS to device tree could be probably
> done for lots of these, see below.

Based on the comment from Tom, I think we are talking about two 
different things here. I am NOT talking about passing mtdparts to Linux 
at all, there using DT is clear.

I am talking about using nor%d in MTDIDS in U-Boot UBI code to look up 
from which device to attach UBI in U-Boot.

>> Note that there are not just a few boards that are broken, but hundreds.
>> I believe that itself justifies a fix, instead of just throwing all
>> those hundreds of boards overboard.
>>
>> u-boot$ git grep -l CONFIG_MTDIDS configs | wc -l
>> 203
> 
> Only 96 of those also grep the substring "nor". But okay, that is still
> a lot. The question is how many of them could be rewritten to DT:
> 
>    for cfg in $(git grep -l 'CONFIG_MTDIDS.*nor[0-9]' configs); do
>      fgrep CONFIG_DEFAULT_DEVICE_TREE "$cfg"
>    done | wc -l
> 
> 92 of those 96 have CONFIG_DEFAULT_DEVICE_TREE defined.
> 
> Of these, 65 contain CONFIG_DM_SPI_FLASH=y, so at least these 65 could
> be converted. Of the rest 27, how many could also be converted to DM?
> How may use non-DM drivers?

See my remark above, are we both talking about the same thing -- mtd%d 
usage in U-Boot MTD code to look up MTD device for UBI in U-Boot ?

  parent reply	other threads:[~2021-09-22 19:37 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-22 16:29 [PATCH v4 0/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
2021-09-22 16:29 ` [PATCH v4 1/2] mtd: cfi_flash: use cfi_flash_num_flash_banks only when supported Patrick Delaunay
2021-09-28 18:45   ` Tom Rini
2021-09-22 16:29 ` [PATCH v4 2/2] mtd: spi: nor: force mtd name to "nor%d" Patrick Delaunay
2021-09-28 18:45   ` Tom Rini
2021-09-22 17:29 ` [PATCH v4 0/2] " Marek Behún
2021-09-22 18:24   ` Marek Vasut
2021-09-22 18:42     ` Tom Rini
2021-09-22 19:08       ` Marek Behún
2021-09-22 19:12       ` Marek Vasut
2021-09-22 19:05     ` Marek Behún
2021-09-22 19:23       ` Tom Rini
2021-09-22 19:39         ` Marek Vasut
2021-09-22 19:24       ` Marek Vasut [this message]
2021-09-22 19:41         ` Tom Rini
2021-09-22 19:42         ` Tom Rini
2021-09-22 19:46         ` Tom Rini
2021-09-22 19:56           ` Marek Vasut
2021-09-22 20:00             ` Tom Rini
2021-09-23  1:32               ` Marek Vasut
2021-09-23  9:04                 ` Patrick DELAUNAY
2021-09-24 18:22                   ` Tom Rini
2021-09-24 19:25                     ` Marek Behún
2021-09-24 20:09                       ` Marek Vasut
2021-09-25  0:12                         ` Marek Behún
2021-09-25  3:06                           ` 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=56df80f7-aa1d-3cff-5b29-16fdafcf7bcf@denx.de \
    --to=marex@denx.de \
    --cc=christophe.kerello@foss.st.com \
    --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@foss.st.com \
    --cc=priyanka.jain@nxp.com \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=uboot-stm32@st-md-mailman.stormreply.com \
    --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;
as well as URLs for NNTP newsgroup(s).