Hi Boris,
Boris Brezillon boris.brezillon@bootlin.com wrote on Tue, 30 Oct 2018 23:02:50 +0100:
On Tue, 30 Oct 2018 11:59:13 +0100 Stefan Roese sr@denx.de wrote:
On 30.10.18 11:41, Boris Brezillon wrote:
On Tue, 30 Oct 2018 11:13:37 +0100 Stefan Roese sr@denx.de wrote:
Hi Boris,
On 30.10.18 11:03, Boris Brezillon wrote:
On Tue, 30 Oct 2018 10:51:51 +0100 Stefan Roese sr@denx.de wrote:
Calling "mtdparts" currently fails when its called before any other mtd command (or ubi command) has been called. The MTD devices are not probed at this point and therefore it fails e.g. with this message:
=> mtdparts Device spi-nand0 not found!
IIRC, we decided that mtdparts should not call mtd_probe_devices() to encourage people to stop using it.
I see. But I don't quite get how this missing call (and reslting error message) would encourage people to stop using it.
You're right, this message does not encourage people to stop using mtdparts on existing setups (mtdparts should work just fine on any MTD devices except SPI NANDs) but it does discourage them from using it on spi-nand devices since it returns an error.
IMHO, that's more confusing than discouraging.
This patch adds a call to mtd_probe_devices() to mtdparts_init() to solve this issue. This also fixes a problem when calling "ubi part" as first flash storage related command.
Hm, this one is unexpected. Miquel, any idea why this happens. Do we need to enable a specific option if we want mtd_probe_devices() to be called in the ubi part path?
Please note that "ubi part part-foo" does still work. It only prints this error message before attaching the MTD partition. The error is printed because of this call-chain:
ubi_part() -> ubi_detach() -> mtdparts_init()
So again, mtdparts_init() is called without the MTD devices being probed.
I guess we forgot to remove this mtdparts_init() call from the detach path. I think it's no longer needed since we now call mtd_probe_devices() in ubi_part(), and mtd_probe_devices() will take care of creating MTD partitions based on the mtdparts= and mtdids= variables.
A quick test reveals that this removal does not remove the error message. Instead the command does not work anymore at all:
=> ubi part nand Partition nand not found!
Before (and without my patch) its this:
=> ubi part nand Device spi-nand0 not found! Error initializing mtdparts! ubi0: attaching mtd2 ubi0: scanning is finished ubi0: attached mtd2 (name "nand", size 128 MiB)
I think I found what's missing in mtd_probe_devices(): we don't use the default mtdparts and mtdids when those env vars are NULL (see what's done in mtdparts_init() to handle this case [1]).
[1]https://elixir.bootlin.com/u-boot/v2018.11-rc3/source/cmd/mtdparts.c#L1763
Isn't the right solution to always define these env variables when they are needed? Defining such default behavior with a Kconfig entry is, from my opinion, a lot of noise for such an useless feature...
Thanks, Miquèl