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 23E37C433F5 for ; Wed, 15 Sep 2021 12:10:42 +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 64B086135F for ; Wed, 15 Sep 2021 12:10:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 64B086135F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=denx.de 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 C68A082BC4; Wed, 15 Sep 2021 14:10:39 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1631707840; bh=ZUhwE0XLnqTmC7IWXDSMxklze6A3LqMtcVIy5a/jmsQ=; h=Subject:To:Cc:References:From:Date:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=A6RJ1JfvgWuION2A31F6Jng2HS7BKxeZgnmbqcmbYfH5/M8Et0D1MyEgHRl7KrvMM 3GMpPCLDAT+/nmjEpFA+yjsB9y8cCnDfvA7hYEnFU+G6COSPlPxZHRqsMd1MO+b2a/ SB1Ly3HukSFh3TfKcQhZj86LEbaHo+UNugQkPJtMTjlt4pK7S86cxDBBZSG7IlrnPL 7eH6H5o60FzYiSv4r4dT6D200zNjBR7XPfPAlkRlm4UzvOdMzgRnEX7d1v/amE4MxA 6UwQ5EoARlbeSzzdu9tjrGIFneFhbY1Tvcr7M7R9HTzo7ZBul7maKR2bUBDKzMtZs+ s5uYBViOe7zRw== Received: from [IPv6:::1] (p578adb1c.dip0.t-ipconnect.de [87.138.219.28]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 83B0680202; Wed, 15 Sep 2021 14:10:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1631707837; bh=ZUhwE0XLnqTmC7IWXDSMxklze6A3LqMtcVIy5a/jmsQ=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=Uhdz+x5lqmmADncCqT6PyvsQ83KSaG57CbLUFT330WuHvGo3heTu3vsMQwHYLoyMo s7AbCuHoGl+MGk1NxgWZnRxvoIAqaGQxSEKyTW8x9vdZ85P7AHod4iO7/Io5bAx87x 72E9/jCSd7cSZKP8khnYT5+WPPrl+JQqrd4xcXAysiz3zQY28f2qO9xFR7OficsyWH 4pUIAhyuXdUzjM2LJbCRrM0dqvQxXNsBxPTmAuo73+IEmE/JALY5atcWt5R++7mbIZ 9ch9+aY+sxc/GmgWtb/6qlJEQ4lDIJ4XiBuZhPcD4yyLH62EgQURMVz50Sou/dvM3a BEqSty8GKCZEg== Subject: Re: [PATCH] mtd: spi-nor: Fix SF MTDIDS when registering multiple MTDs with DM enabled To: Patrick DELAUNAY , Patrice CHOTARD , hs@denx.de, u-boot@lists.denx.de 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> <26ec7fa5-f114-9ac6-5956-670cec4d09a1@foss.st.com> <0d04af89-c568-dbe3-b06d-ce120b8fb0ca@foss.st.com> <453fcd12-2305-b56a-721b-3e2abbd6f261@foss.st.com> From: Marek Vasut Message-ID: Date: Wed, 15 Sep 2021 14:10:35 +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: <453fcd12-2305-b56a-721b-3e2abbd6f261@foss.st.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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 On 9/15/21 2:00 PM, Patrick DELAUNAY wrote: Hi, >>> On 9/15/21 10:57 AM, Marek Vasut wrote: >>>> On 9/15/21 10:53 AM, Patrice CHOTARD wrote: >>>>> 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" >>>> >>>> Right , that's what I was afraid of. The patch from Patrick would >>>> fail if there are multiple disparate NOR technologies (parallel NOR >>>> and SPI NOR). So we need a third option. >>> >>> And also HyperFlash, which today, are seen as "nor". >> >> Dang ... that too. I have a system with HF and SPI NOR which I could >> use for testing I _think_. Do you expect Patrick to send out a V2 of >> his patch with this stuff above addressed ? > > > Hi, > > > I will change the name to "spi-nor%d" in V2 (alligned with "spi-nand") > > to avoid conflict with other "nor" device. No, that is not an option, because that will break existing devices, grep for MTDIDS.*nor in configs/ . > I need to check if that solve my issue. [...]