From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ECFA1C433EF for ; Wed, 15 Sep 2021 08:53:46 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2D84460527 for ; Wed, 15 Sep 2021 08:53:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 2D84460527 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=foss.st.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3EBCC80796; Wed, 15 Sep 2021 10:53:43 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=foss.st.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=foss.st.com header.i=@foss.st.com header.b="1NYFbSMK"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 487BA829C6; Wed, 15 Sep 2021 10:53:40 +0200 (CEST) Received: from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com [185.132.182.106]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 4CC5180796 for ; Wed, 15 Sep 2021 10:53:35 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=foss.st.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=prvs=3892492693=patrice.chotard@foss.st.com Received: from pps.filterd (m0241204.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 18F866lL001164; Wed, 15 Sep 2021 10:53:30 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foss.st.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=selector1; bh=EJ1JHeBUZphOY1vzSzyZ1TIGv2VZOp0Kg4co4X4wOjo=; b=1NYFbSMKM2/Kwl/AZpP9uDMOKLbmZVU5ARS3ugKRPq/tRij/scRUOz3VIpsHiwbCMLmE O6mGMtm7a45e27g7ASPpxwkYhRw2IYgJ8PoLY8edreyINKp/a+Ubg41FyufqtxPI1sK/ vvX6x6auSTTA9uOU7dxTvjgoTWxdUZNPw+S0shqf28RNpgvpEZqkh5EQPqECpqUtTD28 e9/Fgvxyw3/Es5AKV+sioz+is9Ob1hV3ifP9WyljLV/O/TX9+xr2CM6IkyEGhkHGwdEH grVBlHZZqsQbPEo4n9SEIln+T6O+6Xen1/kuAUG/97NE2VBRahRjrCH/hqSQzh7mCaj/ iw== Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com with ESMTP id 3b3d198art-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 15 Sep 2021 10:53:30 +0200 Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 3D87610002A; Wed, 15 Sep 2021 10:53:30 +0200 (CEST) Received: from Webmail-eu.st.com (sfhdag2node2.st.com [10.75.127.5]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id D58B121B50D; Wed, 15 Sep 2021 10:53:28 +0200 (CEST) Received: from lmecxl0573.lme.st.com (10.75.127.44) by SFHDAG2NODE2.st.com (10.75.127.5) with Microsoft SMTP Server (TLS) id 15.0.1497.18; Wed, 15 Sep 2021 10:53:28 +0200 Subject: Re: [PATCH] mtd: spi-nor: Fix SF MTDIDS when registering multiple MTDs with DM enabled To: Marek Vasut , , CC: Jagan Teki , =?UTF-8?Q?Marek_Beh=c3=ban?= , Miquel Raynal , =?UTF-8?Q?Pali_Roh=c3=a1r?= , Patrick Delaunay , Priyanka Jain , Simon Glass References: <20210914230622.245747-1-marex@denx.de> <58930899-7190-9c36-6377-cc0c41d1800a@denx.de> From: Patrice CHOTARD Message-ID: <26ec7fa5-f114-9ac6-5956-670cec4d09a1@foss.st.com> Date: Wed, 15 Sep 2021 10:53:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.75.127.44] X-ClientProxiedBy: SFHDAG2NODE2.st.com (10.75.127.5) To SFHDAG2NODE2.st.com (10.75.127.5) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.182.1,Aquarius:18.0.790,Hydra:6.0.391,FMLib:17.0.607.475 definitions=2021-09-15_01,2021-09-14_01,2020-04-07_01 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi All On 9/15/21 7:59 AM, Marek Vasut wrote: > 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 >>> Cc: Heiko Schocher >>> Cc: Jagan Teki >>> Cc: Marek Behún >>> Cc: Miquel Raynal >>> Cc: Pali Rohár >>> Cc: Patrice Chotard >>> Cc: Patrick Delaunay >>> Cc: Priyanka Jain >>> Cc: Simon Glass >>> --- >>>   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 >> >> @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. I just got a try with this patch and unfortunately, it doesn't solve the issue. I tested it on STM32MP1-ev1 board which embedds 1 NAND and 2 SPI-NORs. Here is the output of mtd list command: U-Boot 2021.10-rc4-00001-g1923b5a21d (Sep 15 2021 - 10:48:48 +0200) CPU: STM32MP157AAA Rev.B Model: STMicroelectronics STM32MP157C eval daughter on eval mother Board: stm32mp1 in basic mode (st,stm32mp157c-ev1) Board: MB1263 Var1.0 Rev.C-01 DRAM: 1 GiB Clocks: - MPU : 650 MHz - MCU : 208.878 MHz - AXI : 266.500 MHz - PER : 24 MHz - DDR : 533 MHz WDT: Started with servicing (32s timeout) NAND: 1024 MiB MMC: STM32 SD/MMC: 0, STM32 SD/MMC: 1 Loading Environment from MMC... *** Warning - bad CRC, using default environment In: serial Out: serial Err: serial Net: eth0: ethernet@5800a000 Hit any key to stop autoboot: 0 STM32MP> mtd list SF: Detected mx66l51235l with page size 256 Bytes, erase size 64 KiB, total 64 MiB SF: Detected nor1 with page size 256 Bytes, erase size 64 KiB, total 64 MiB List of MTD devices: * nand0 - type: NAND flash - block size: 0x40000 bytes - min I/O: 0x1000 bytes - OOB size: 224 bytes - OOB available: 118 bytes - ECC strength: 8 bits - ECC step size: 512 bytes - bitflip threshold: 6 bits - 0x000000000000-0x000040000000 : "nand0" * nor2 - device: mx66l51235l@0 - parent: spi@58003000 - driver: jedec_spi_nor - path: /soc/spi@58003000/mx66l51235l@0 - type: NOR flash - block size: 0x10000 bytes - min I/O: 0x1 bytes - 0x000000000000-0x000004000000 : "nor2" * nor2 - device: mx66l51235l@1 - parent: spi@58003000 - driver: jedec_spi_nor - path: /soc/spi@58003000/mx66l51235l@1 - type: NOR flash - block size: 0x10000 bytes - min I/O: 0x1 bytes - 0x000000000000-0x000004000000 : "nor2" Patrice