Re: [PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()
On Mon, Mar 01, 2021 at 03:03:11PM +0100, Marek Behun wrote: > On Thu, 25 Feb 2021 15:39:09 -0500 > Tom Rini wrote: > > > Well, past migration deadline doesn't mean it can cause CI to fail. I'm > > just now throwing things out that are 2 years past the deadline, and > > we'll probably be doing that for a few releases. > > Tom, I would like to send v4 without the CONFIG_DM #ifdefs. > > tqma6s_wru4_mmc_defconfig is the only defconfig that defines CONFIG_DM=n > > Since I do not have this board, I cannot work on converting it to DM. > > I can either > - add patch removing this board from U-Boot to this patch series > - send patch removing this board as separate from this series and send > this series only after the board is removed > - stick if #ifdefs > - try to use IS_ENABLED(CONFIG_DM), but this would need some extra > patches into header files because of undefined references (we really > don't want this) > - something different > > How should I proceed? You can say your series depends on: https://patchwork.ozlabs.org/project/uboot/patch/20210221010634.21310-42-tr...@konsulko.com/ which drops tqma6s_wru4_mmc_defconfig and you should submit a PR against https://github.com/u-boot/u-boot/ which will trigger a CI run in Azure so you can see what else fails to build, or if that really is the only board that would fail to build, in this case. Thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()
On Thu, 25 Feb 2021 15:39:09 -0500 Tom Rini wrote: > Well, past migration deadline doesn't mean it can cause CI to fail. I'm > just now throwing things out that are 2 years past the deadline, and > we'll probably be doing that for a few releases. Tom, I would like to send v4 without the CONFIG_DM #ifdefs. tqma6s_wru4_mmc_defconfig is the only defconfig that defines CONFIG_DM=n Since I do not have this board, I cannot work on converting it to DM. I can either - add patch removing this board from U-Boot to this patch series - send patch removing this board as separate from this series and send this series only after the board is removed - stick if #ifdefs - try to use IS_ENABLED(CONFIG_DM), but this would need some extra patches into header files because of undefined references (we really don't want this) - something different How should I proceed? Marek
Re: [PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()
On Thu, Feb 25, 2021 at 09:35:13PM +0100, Marek Behun wrote: > On Thu, 25 Feb 2021 15:28:56 -0500 > Tom Rini wrote: > > > On Thu, Feb 25, 2021 at 09:07:40PM +0100, Marek Behun wrote: > > > On Thu, 25 Feb 2021 14:31:42 -0500 > > > Simon Glass wrote: > > > > > > > We should not need CONFIG_DM here...it should be enabled for all > > > > boards. You can always disable MTD for a board if not, or send a > > > > removable patch. > > > > > > > > If for some reason you do, please use if (IS_ENABLED() so that 'dev' > > > > can always be declared. > > > > > > Simon, it still isn't enabled for all boards. For example > > > tqma6s_wru4_mmc_defconfig does not compile with this. I actually wrote > > > this into commit message: > > > > > > Although CONFIG_DM is compulsory since v2020.01, there are still some > > > boards (for example tqma6s_wru4_mmc_defconfig) that don't enable it. > > > > > > But if breaking such boards is not a problem anymore, I will gladly > > > just remove the ifdefs :) Should I? > > > > So, I'm working hard at dropping boards that are well past migration > > deadlines. That specific one fails DM_MMC more importantly, and I will > > be dropping it if it's not migrated, after v2021.04 is out. What else > > fails? If you rebase your series on my > > WIP/remove-non-AHCI_LIBATA-drivers (as it has the most board removals in > > it), what fails to build if your series is just DM only? > > > > I haven't tried building all boards, just quickly found a defconfig > with disabled CONFIG_DM :) > With removing the CONFIG_DM ifdefs this series shouldn't break anything > that is already past migration deadline. Well, past migration deadline doesn't mean it can cause CI to fail. I'm just now throwing things out that are 2 years past the deadline, and we'll probably be doing that for a few releases. -- Tom signature.asc Description: PGP signature
Re: [PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()
On Thu, 25 Feb 2021 15:28:56 -0500 Tom Rini wrote: > On Thu, Feb 25, 2021 at 09:07:40PM +0100, Marek Behun wrote: > > On Thu, 25 Feb 2021 14:31:42 -0500 > > Simon Glass wrote: > > > > > We should not need CONFIG_DM here...it should be enabled for all > > > boards. You can always disable MTD for a board if not, or send a > > > removable patch. > > > > > > If for some reason you do, please use if (IS_ENABLED() so that 'dev' > > > can always be declared. > > > > Simon, it still isn't enabled for all boards. For example > > tqma6s_wru4_mmc_defconfig does not compile with this. I actually wrote > > this into commit message: > > > > Although CONFIG_DM is compulsory since v2020.01, there are still some > > boards (for example tqma6s_wru4_mmc_defconfig) that don't enable it. > > > > But if breaking such boards is not a problem anymore, I will gladly > > just remove the ifdefs :) Should I? > > So, I'm working hard at dropping boards that are well past migration > deadlines. That specific one fails DM_MMC more importantly, and I will > be dropping it if it's not migrated, after v2021.04 is out. What else > fails? If you rebase your series on my > WIP/remove-non-AHCI_LIBATA-drivers (as it has the most board removals in > it), what fails to build if your series is just DM only? > I haven't tried building all boards, just quickly found a defconfig with disabled CONFIG_DM :) With removing the CONFIG_DM ifdefs this series shouldn't break anything that is already past migration deadline. Marek
Re: [PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()
On Thu, Feb 25, 2021 at 09:07:40PM +0100, Marek Behun wrote: > On Thu, 25 Feb 2021 14:31:42 -0500 > Simon Glass wrote: > > > We should not need CONFIG_DM here...it should be enabled for all > > boards. You can always disable MTD for a board if not, or send a > > removable patch. > > > > If for some reason you do, please use if (IS_ENABLED() so that 'dev' > > can always be declared. > > Simon, it still isn't enabled for all boards. For example > tqma6s_wru4_mmc_defconfig does not compile with this. I actually wrote > this into commit message: > > Although CONFIG_DM is compulsory since v2020.01, there are still some > boards (for example tqma6s_wru4_mmc_defconfig) that don't enable it. > > But if breaking such boards is not a problem anymore, I will gladly > just remove the ifdefs :) Should I? So, I'm working hard at dropping boards that are well past migration deadlines. That specific one fails DM_MMC more importantly, and I will be dropping it if it's not migrated, after v2021.04 is out. What else fails? If you rebase your series on my WIP/remove-non-AHCI_LIBATA-drivers (as it has the most board removals in it), what fails to build if your series is just DM only? -- Tom signature.asc Description: PGP signature
Re: [PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()
On Thu, 25 Feb 2021 14:31:42 -0500 Simon Glass wrote: > We should not need CONFIG_DM here...it should be enabled for all > boards. You can always disable MTD for a board if not, or send a > removable patch. > > If for some reason you do, please use if (IS_ENABLED() so that 'dev' > can always be declared. Simon, it still isn't enabled for all boards. For example tqma6s_wru4_mmc_defconfig does not compile with this. I actually wrote this into commit message: Although CONFIG_DM is compulsory since v2020.01, there are still some boards (for example tqma6s_wru4_mmc_defconfig) that don't enable it. But if breaking such boards is not a problem anymore, I will gladly just remove the ifdefs :) Should I? Marek
Re: [PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()
Hi Marek,
On Thu, 25 Feb 2021 at 09:14, Marek Behún wrote:
>
> The get_mtd_device_nm() function (code imported from Linux) simply
> iterates all registered MTD devices and compares the given name with
> all MTDs' names.
>
> With SPI_FLASH_MTD enabled U-Boot registers a SPI-NOR as a MTD device
> with name identical to the SPI flash chip name (from SPI ID table). Thus
> for a board with multiple same SPI-NORs it registers multiple MTDs, but
> all with the same name (such as "s25fl164k"). We do not want to change
> this behaviour, since such a change could break existing boot scripts,
> which can rely on a hardcoded name.
>
> In order to allow somehow to uniqely select a MTD device, change
> get_mtd_device_nm() function as such:
> - if first character of name is '/', interpret it as OF path
> - otherwise compare the name with MTDs name and MTDs device name.
>
> In the following example a board has two "s25fl164k" SPI-NORs. They both
> have name "s25fl164k", thus cannot be uniquely selected via this name.
> With this change, the user can select the second SPI-NOR either with
> "spi-nor@1" or "/soc/spi@10600/spi-nor@1".
>
> Example:
> => mtd list
> List of MTD devices:
> * s25fl164k
> - device: spi-nor@0
> - parent: spi@10600
> - driver: jedec_spi_nor
> - path: /soc/spi@10600/spi-nor@0
> - type: NOR flash
> - block size: 0x1000 bytes
> - min I/O: 0x1 bytes
> - 0x-0x0080 : "s25fl164k"
> * s25fl164k
> - device: spi-nor@1
> - parent: spi@10600
> - driver: jedec_spi_nor
> - path: /soc/spi@10600/spi-nor@1
> - type: NOR flash
> - block size: 0x1000 bytes
> - min I/O: 0x1 bytes
> - 0x-0x0080 : "s25fl164k"
>
> This change adds code that depends on CONFIG_DM. Although CONFIG_DM
> is compulsory since v2020.01, there are still some boards (for example
> tqma6s_wru4_mmc_defconfig) that don't enable it. Therefore the code
> guards this parts by #ifdefs.
>
> Signed-off-by: Marek Behún
> Cc: Jagan Teki
> Cc: Priyanka Jain
> Cc: Simon Glass
> Cc: Heiko Schocher
> Cc: Jagan Teki
> Cc: Patrick Delaunay
> Cc: Patrice CHOTARD
> Cc: Miquel Raynal
> ---
> drivers/mtd/mtdcore.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 0d1f94c6cb..7c894b2e41 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -780,13 +780,42 @@ struct mtd_info *get_mtd_device_nm(const char *name)
> {
> int err = -ENODEV;
> struct mtd_info *mtd = NULL, *other;
> +#ifdef CONFIG_DM
> + struct udevice *dev = NULL;
> +#endif
> +
> +#ifdef CONFIG_DM
We should not need CONFIG_DM here...it should be enabled for all
boards. You can always disable MTD for a board if not, or send a
removable patch.
If for some reason you do, please use if (IS_ENABLED() so that 'dev'
can always be declared.
> + /*
> +* If the first character of mtd name is '/', interpret it as OF path.
> +* Otherwise try comparing by mtd->name and mtd->dev->name.
> +*/
> + if (*name == '/') {
> + err = device_get_global_by_ofnode(ofnode_path(name), );
> + if (err)
> + return ERR_PTR(err);
> + }
> +#endif
>
> mutex_lock(_table_mutex);
>
> mtd_for_each_device(other) {
> +#ifdef CONFIG_DM
> + if ((dev && !mtd_is_partition(other) && other->dev == dev) ||
> + !strcmp(name, other->name) ||
> + (!mtd_is_partition(other) && other->dev &&
> +!strcmp(name, other->dev->name))) {
> +#else
> if (!strcmp(name, other->name)) {
> +#endif
> +#ifdef __UBOOT__
> + if (mtd)
> + printf("\nWarning: MTD name \"%s\" is not
> unique!\n\n",
> + name);
> + mtd = other;
> +#else /* !__UBOOT__ */
> mtd = other;
> break;
> +#endif /* !__UBOOT__ */
> }
> }
>
> --
> 2.26.2
>
Regards,
Simon
