Message ID | 20230510110835.26115-7-AVKrasnov@sberdevices.ru |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp3544823vqo; Wed, 10 May 2023 04:28:06 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4cQQbelYDZYXUPf0acTbdsOBMvmD38LhCdtNoGOTNb6Tgk4wQuZzhizulM8HcZNsJksDL/ X-Received: by 2002:a17:903:2cb:b0:1ac:73e9:de5 with SMTP id s11-20020a17090302cb00b001ac73e90de5mr12776847plk.46.1683718086008; Wed, 10 May 2023 04:28:06 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683718085; cv=none; d=google.com; s=arc-20160816; b=WYeyTtyEMLep86H02QEDSk7Xk4XwdO9tkfNIH6arOr1knb6OFeTALYlVZBS5209H4d 33rOZOGOStyZrVmdJlT076BCZqHdw4SKvBWcQKRqOfxzkZ3V+fl3V636i++/muU4M56p dofnIGqIGuxZZd6FNjrN5NsK3F49D/7yqjsL4w3UdLS2hIS9S2AFTI8meGorfzCEbEhe UAFNXShzHroxZa5V60g+FJ19SpXV5SLw+64esoZGzof0jtZSdE8S3btHkx4vje6JHfFW jn+c1fCAGnDvoLK8cFbNP6DHxZyRv+Bwf8dq67uSSFobIW6RxWuubdl3xW8iltHGqpgB dQ0g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=wpnQqYLiMeJcmuuKybbquqxEgUyT5kRAw1QOCUfaxQQ=; b=rfVitQlnotthspcjtvjhgUQ/uer5xjU+nrG6sNTQT7vt9cT2dZc7P86ooNFXDt8oFQ xeN0mveIGKQJfy6KrbS09UkoS01HtxYACWTIIEr09NXxoBZrGZ6wVoe+xH2fBari4cww h+j/Q8Ux5Y5D86rnHCdWxhSj0O3SgBCecOUPO78M2jsw49eEftojHWSxZP3IbOxnig4g t4C4WsGPuXCbB0TxQUgRAKkocT/P+tE7tKmA5tMdO8+TrXlPTuSZ1JIen0ytiQqGHVRj 8827OdZnttY/K/6DuT+KEISIKb50wXpWYvKllbjbqZdJgQgUW60KSKtxbYIZc92nk5D6 iOgA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sberdevices.ru header.s=mail header.b="DeN/8RDV"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=sberdevices.ru Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y9-20020a17090322c900b001ab23391d6asi4115609plg.590.2023.05.10.04.27.53; Wed, 10 May 2023 04:28:05 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@sberdevices.ru header.s=mail header.b="DeN/8RDV"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=sberdevices.ru Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236791AbjEJLOZ (ORCPT <rfc822;jeantsuru.cumc.mandola@gmail.com> + 99 others); Wed, 10 May 2023 07:14:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39944 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236792AbjEJLOH (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 10 May 2023 07:14:07 -0400 Received: from mx.sberdevices.ru (mx.sberdevices.ru [45.89.227.171]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 324BC2106 for <linux-kernel@vger.kernel.org>; Wed, 10 May 2023 04:13:56 -0700 (PDT) Received: from s-lin-edge02.sberdevices.ru (localhost [127.0.0.1]) by mx.sberdevices.ru (Postfix) with ESMTP id 121B25FD27; Wed, 10 May 2023 14:13:54 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sberdevices.ru; s=mail; t=1683717234; bh=wpnQqYLiMeJcmuuKybbquqxEgUyT5kRAw1QOCUfaxQQ=; h=From:To:Subject:Date:Message-ID:MIME-Version:Content-Type; b=DeN/8RDVdwKnRQM6hFMX9b456raaqf05mrqn7EAbZJuZ5ZEjutNZSOn4Zsjh1qYNE jlK1ZGFVg1dMDcBPbo1Y33YF+NgCYEb51gnGFFsUDRjZs9jedT+75tJOY1gjPP5aEo DAHNp45k+TYuCBkikcyqRJ+heJZxmGgLy+y2V62SqRtc7bzyxJ2FmIHSodxMq2rfaP yVJiJxbwEgVqb9n7BmPFBDgUmveJTWHylZYJTpL+fBnTAF5+yEG5wytGyAVtzcDCSV rc3OF9ijrJHyqnOGcxhxqI/nWeQwEHXsXyBE9jmqBz2Hxhmim0LkH4r941b1lPBphK qFrbsBVRBEerA== Received: from S-MS-EXCH01.sberdevices.ru (S-MS-EXCH01.sberdevices.ru [172.16.1.4]) by mx.sberdevices.ru (Postfix) with ESMTP; Wed, 10 May 2023 14:13:54 +0300 (MSK) From: Arseniy Krasnov <AVKrasnov@sberdevices.ru> To: Liang Yang <liang.yang@amlogic.com>, Miquel Raynal <miquel.raynal@bootlin.com>, Richard Weinberger <richard@nod.at>, Vignesh Raghavendra <vigneshr@ti.com>, Neil Armstrong <neil.armstrong@linaro.org>, Kevin Hilman <khilman@baylibre.com>, Jerome Brunet <jbrunet@baylibre.com>, Martin Blumenstingl <martin.blumenstingl@googlemail.com>, Jianxin Pan <jianxin.pan@amlogic.com>, Yixun Lan <yixun.lan@amlogic.com> CC: <oxffffaa@gmail.com>, <kernel@sberdevices.ru>, Arseniy Krasnov <AVKrasnov@sberdevices.ru>, <linux-mtd@lists.infradead.org>, <linux-arm-kernel@lists.infradead.org>, <linux-amlogic@lists.infradead.org>, <linux-kernel@vger.kernel.org> Subject: [PATCH v3 6/6] mtd: rawnand: meson: rename node for chip select Date: Wed, 10 May 2023 14:08:34 +0300 Message-ID: <20230510110835.26115-7-AVKrasnov@sberdevices.ru> X-Mailer: git-send-email 2.35.0 In-Reply-To: <20230510110835.26115-1-AVKrasnov@sberdevices.ru> References: <20230510110835.26115-1-AVKrasnov@sberdevices.ru> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [172.16.1.6] X-ClientProxiedBy: S-MS-EXCH01.sberdevices.ru (172.16.1.4) To S-MS-EXCH01.sberdevices.ru (172.16.1.4) X-KSMG-Rule-ID: 4 X-KSMG-Message-Action: clean X-KSMG-AntiSpam-Status: not scanned, disabled by settings X-KSMG-AntiSpam-Interceptor-Info: not scanned X-KSMG-AntiPhishing: not scanned, disabled by settings X-KSMG-AntiVirus: Kaspersky Secure Mail Gateway, version 1.1.2.30, bases: 2023/05/10 09:03:00 #21252424 X-KSMG-AntiVirus-Status: Clean, skipped X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1765506375869248554?= X-GMAIL-MSGID: =?utf-8?q?1765506375869248554?= |
Series |
refactoring and fix for Meson NAND
|
|
Commit Message
Arseniy Krasnov
May 10, 2023, 11:08 a.m. UTC
This renames node with values for chip select from "reg" to "cs". It is
needed because when OTP access is enabled on the attached storage, MTD
subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
tries to use "reg" node in its own manner, supposes that it has another
layout. All of this leads to device initialization failure.
Example:
[...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
[...] mtd mtd0: Failed to register OTP NVMEM device
[...] meson-nand ffe07800.nfc: failed to register MTD device: -22
[...] meson-nand ffe07800.nfc: failed to init NAND chips
[...] meson-nand: probe of ffe07800.nfc failed with error -22
Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
---
drivers/mtd/nand/raw/meson_nand.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
Hello Arseniy, On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov <AVKrasnov@sberdevices.ru> wrote: > > This renames node with values for chip select from "reg" to "cs". It is > needed because when OTP access is enabled on the attached storage, MTD > subsystem registers this storage in the NVMEM subsystem. NVMEM in turn > tries to use "reg" node in its own manner, supposes that it has another > layout. All of this leads to device initialization failure. In general: if we change the device-tree interface (in this case: replacing a "reg" with a "cs" property) the dt-bindings have to be updated as well. Documentation/devicetree/bindings/mtd/nand-controller.yaml and Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show that the chip select of a NAND chip is specified with a "reg" property. Also the code has to be backwards compatible with old .dtbs. > Example: > > [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/... > [...] mtd mtd0: Failed to register OTP NVMEM device > [...] meson-nand ffe07800.nfc: failed to register MTD device: -22 > [...] meson-nand ffe07800.nfc: failed to init NAND chips > [...] meson-nand: probe of ffe07800.nfc failed with error -22 This is odd - can you please share your definition of the &nfc node? &nfc { nand_chip0: nand@0 { reg = <0>; }; }; This should result in nand_set_flash_node() being called with &nand_chip0 (if it's called with &nfc then something is buggy in our driver). If there's no child nodes within &nand_chip0 then why would the MTD-to-NVMEM code think that it has to parse something? If you do have child nodes and those are partitions, then make sure that the structure is correct (see the extra "partitions" node inside which all partitions are nested): &nand_chip0 { partitions { compatible = "fixed-partitions"; #address-cells = <1>; #size-cells = <1>; partition@0 { label = "u-boot"; reg = <0x0000000 0x4000>; read-only; }; }; }; Best regards,, Martin
Hi Martin & Arseniy, martin.blumenstingl@googlemail.com wrote on Wed, 10 May 2023 22:40:37 +0200: > Hello Arseniy, > > On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov > <AVKrasnov@sberdevices.ru> wrote: > > > > This renames node with values for chip select from "reg" to "cs". It is > > needed because when OTP access is enabled on the attached storage, MTD > > subsystem registers this storage in the NVMEM subsystem. NVMEM in turn > > tries to use "reg" node in its own manner, supposes that it has another > > layout. All of this leads to device initialization failure. > In general: if we change the device-tree interface (in this case: > replacing a "reg" with a "cs" property) the dt-bindings have to be > updated as well. True, and I would add, bindings should not be broken. > Documentation/devicetree/bindings/mtd/nand-controller.yaml and > Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show > that the chip select of a NAND chip is specified with a "reg" > property. All NAND controller binding expect the chip-select to be in the 'reg' property, very much like a spi device would use reg to store the cs as well: the reg property tells you how you address the device. I also fully agree with Martin's comments below. Changing reg is likely a wrong approach :) > Also the code has to be backwards compatible with old .dtbs. > > > Example: > > > > [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/... > > [...] mtd mtd0: Failed to register OTP NVMEM device > > [...] meson-nand ffe07800.nfc: failed to register MTD device: -22 > > [...] meson-nand ffe07800.nfc: failed to init NAND chips > > [...] meson-nand: probe of ffe07800.nfc failed with error -22 > This is odd - can you please share your definition of the &nfc node? > > &nfc { > nand_chip0: nand@0 { > reg = <0>; > }; > }; > > This should result in nand_set_flash_node() being called with > &nand_chip0 (if it's called with &nfc then something is buggy in our > driver). > If there's no child nodes within &nand_chip0 then why would the > MTD-to-NVMEM code think that it has to parse something? > If you do have child nodes and those are partitions, then make sure > that the structure is correct (see the extra "partitions" node inside > which all partitions are nested): > &nand_chip0 { > partitions { > compatible = "fixed-partitions"; > #address-cells = <1>; > #size-cells = <1>; > > partition@0 { > label = "u-boot"; > reg = <0x0000000 0x4000>; > read-only; > }; > }; > }; > > > Best regards,, > Martin Thanks, Miquèl
On 10.05.2023 23:53, Miquel Raynal wrote: Hello Martin, Miquel > Hi Martin & Arseniy, > > martin.blumenstingl@googlemail.com wrote on Wed, 10 May 2023 22:40:37 > +0200: > >> Hello Arseniy, >> >> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov >> <AVKrasnov@sberdevices.ru> wrote: >>> >>> This renames node with values for chip select from "reg" to "cs". It is >>> needed because when OTP access is enabled on the attached storage, MTD >>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn >>> tries to use "reg" node in its own manner, supposes that it has another >>> layout. All of this leads to device initialization failure. >> In general: if we change the device-tree interface (in this case: >> replacing a "reg" with a "cs" property) the dt-bindings have to be >> updated as well. > > True, and I would add, bindings should not be broken. I see, that's true. That is bad way to change bindings. > >> Documentation/devicetree/bindings/mtd/nand-controller.yaml and >> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show >> that the chip select of a NAND chip is specified with a "reg" >> property. > > All NAND controller binding expect the chip-select to be in the > 'reg' property, very much like a spi device would use reg to store the > cs as well: the reg property tells you how you address the device. > > I also fully agree with Martin's comments below. Changing reg is likely > a wrong approach :) > >> Also the code has to be backwards compatible with old .dtbs. >> >>> Example: >>> >>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/... >>> [...] mtd mtd0: Failed to register OTP NVMEM device >>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22 >>> [...] meson-nand ffe07800.nfc: failed to init NAND chips >>> [...] meson-nand: probe of ffe07800.nfc failed with error -22 >> This is odd - can you please share your definition of the &nfc node? Sure, here it is: mtd_nand: nfc@7800 { compatible = "amlogic,meson-axg-nfc"; ... nand@0 { reg = <0>; }; } I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ? >> >> &nfc { >> nand_chip0: nand@0 { >> reg = <0>; >> }; >> }; >> >> This should result in nand_set_flash_node() being called with >> &nand_chip0 (if it's called with &nfc then something is buggy in our >> driver). >> If there's no child nodes within &nand_chip0 then why would the >> MTD-to-NVMEM code think that it has to parse something? >> If you do have child nodes and those are partitions, then make sure >> that the structure is correct (see the extra "partitions" node inside >> which all partitions are nested): >> &nand_chip0 { >> partitions { >> compatible = "fixed-partitions"; >> #address-cells = <1>; >> #size-cells = <1>; >> >> partition@0 { >> label = "u-boot"; >> reg = <0x0000000 0x4000>; >> read-only; >> }; >> }; >> }; No, partition nodes are disabled in this case. Thanks, Arseniy >> >> >> Best regards,, >> Martin > > > Thanks, > Miquèl
Hi Arseniy, avkrasnov@sberdevices.ru wrote on Thu, 11 May 2023 11:59:07 +0300: > On 10.05.2023 23:53, Miquel Raynal wrote: > > Hello Martin, Miquel > > > Hi Martin & Arseniy, > > > > martin.blumenstingl@googlemail.com wrote on Wed, 10 May 2023 22:40:37 > > +0200: > > > >> Hello Arseniy, > >> > >> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov > >> <AVKrasnov@sberdevices.ru> wrote: > >>> > >>> This renames node with values for chip select from "reg" to "cs". It is > >>> needed because when OTP access is enabled on the attached storage, MTD > >>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn > >>> tries to use "reg" node in its own manner, supposes that it has another > >>> layout. All of this leads to device initialization failure. > >> In general: if we change the device-tree interface (in this case: > >> replacing a "reg" with a "cs" property) the dt-bindings have to be > >> updated as well. > > > > True, and I would add, bindings should not be broken. > > I see, that's true. That is bad way to change bindings. > > > > >> Documentation/devicetree/bindings/mtd/nand-controller.yaml and > >> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show > >> that the chip select of a NAND chip is specified with a "reg" > >> property. > > > > All NAND controller binding expect the chip-select to be in the > > 'reg' property, very much like a spi device would use reg to store the > > cs as well: the reg property tells you how you address the device. > > > > I also fully agree with Martin's comments below. Changing reg is likely > > a wrong approach :) > > > >> Also the code has to be backwards compatible with old .dtbs. > >> > >>> Example: > >>> > >>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/... > >>> [...] mtd mtd0: Failed to register OTP NVMEM device > >>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22 > >>> [...] meson-nand ffe07800.nfc: failed to init NAND chips > >>> [...] meson-nand: probe of ffe07800.nfc failed with error -22 > >> This is odd - can you please share your definition of the &nfc node? > > Sure, here it is: > > mtd_nand: nfc@7800 { > compatible = "amlogic,meson-axg-nfc"; > ... > nand@0 { > reg = <0>; > }; > } > > I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose > that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called > with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such > situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ? We recently had issues with nvmem parsing, but I believe a mainline kernel should now be perfectly working on this regard. What version of the Linux kernel are you using? Thanks, Miquèl
On 11.05.2023 12:12, Miquel Raynal wrote: > Hi Arseniy, > > avkrasnov@sberdevices.ru wrote on Thu, 11 May 2023 11:59:07 +0300: > >> On 10.05.2023 23:53, Miquel Raynal wrote: >> >> Hello Martin, Miquel >> >>> Hi Martin & Arseniy, >>> >>> martin.blumenstingl@googlemail.com wrote on Wed, 10 May 2023 22:40:37 >>> +0200: >>> >>>> Hello Arseniy, >>>> >>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov >>>> <AVKrasnov@sberdevices.ru> wrote: >>>>> >>>>> This renames node with values for chip select from "reg" to "cs". It is >>>>> needed because when OTP access is enabled on the attached storage, MTD >>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn >>>>> tries to use "reg" node in its own manner, supposes that it has another >>>>> layout. All of this leads to device initialization failure. >>>> In general: if we change the device-tree interface (in this case: >>>> replacing a "reg" with a "cs" property) the dt-bindings have to be >>>> updated as well. >>> >>> True, and I would add, bindings should not be broken. >> >> I see, that's true. That is bad way to change bindings. >> >>> >>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and >>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show >>>> that the chip select of a NAND chip is specified with a "reg" >>>> property. >>> >>> All NAND controller binding expect the chip-select to be in the >>> 'reg' property, very much like a spi device would use reg to store the >>> cs as well: the reg property tells you how you address the device. >>> >>> I also fully agree with Martin's comments below. Changing reg is likely >>> a wrong approach :) >>> >>>> Also the code has to be backwards compatible with old .dtbs. >>>> >>>>> Example: >>>>> >>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/... >>>>> [...] mtd mtd0: Failed to register OTP NVMEM device >>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22 >>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips >>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22 >>>> This is odd - can you please share your definition of the &nfc node? >> >> Sure, here it is: >> >> mtd_nand: nfc@7800 { >> compatible = "amlogic,meson-axg-nfc"; >> ... >> nand@0 { >> reg = <0>; >> }; >> } >> >> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose >> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called >> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such >> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ? > > We recently had issues with nvmem parsing, but I believe a mainline > kernel should now be perfectly working on this regard. What version of > the Linux kernel are you using? My current version is: VERSION = 6 PATCHLEVEL = 2 SUBLEVEL = 0 EXTRAVERSION = -rc8 Fix was in drivers/nvmem/* ? Thanks, Arseniy > > Thanks, > Miquèl
On 11.05.2023 12:17, Arseniy Krasnov wrote: > > > On 11.05.2023 12:12, Miquel Raynal wrote: >> Hi Arseniy, >> >> avkrasnov@sberdevices.ru wrote on Thu, 11 May 2023 11:59:07 +0300: >> >>> On 10.05.2023 23:53, Miquel Raynal wrote: >>> >>> Hello Martin, Miquel >>> >>>> Hi Martin & Arseniy, >>>> >>>> martin.blumenstingl@googlemail.com wrote on Wed, 10 May 2023 22:40:37 >>>> +0200: >>>> >>>>> Hello Arseniy, >>>>> >>>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov >>>>> <AVKrasnov@sberdevices.ru> wrote: >>>>>> >>>>>> This renames node with values for chip select from "reg" to "cs". It is >>>>>> needed because when OTP access is enabled on the attached storage, MTD >>>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn >>>>>> tries to use "reg" node in its own manner, supposes that it has another >>>>>> layout. All of this leads to device initialization failure. >>>>> In general: if we change the device-tree interface (in this case: >>>>> replacing a "reg" with a "cs" property) the dt-bindings have to be >>>>> updated as well. >>>> >>>> True, and I would add, bindings should not be broken. >>> >>> I see, that's true. That is bad way to change bindings. >>> >>>> >>>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and >>>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show >>>>> that the chip select of a NAND chip is specified with a "reg" >>>>> property. >>>> >>>> All NAND controller binding expect the chip-select to be in the >>>> 'reg' property, very much like a spi device would use reg to store the >>>> cs as well: the reg property tells you how you address the device. >>>> >>>> I also fully agree with Martin's comments below. Changing reg is likely >>>> a wrong approach :) >>>> >>>>> Also the code has to be backwards compatible with old .dtbs. >>>>> >>>>>> Example: >>>>>> >>>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/... >>>>>> [...] mtd mtd0: Failed to register OTP NVMEM device >>>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22 >>>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips >>>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22 >>>>> This is odd - can you please share your definition of the &nfc node? >>> >>> Sure, here it is: >>> >>> mtd_nand: nfc@7800 { >>> compatible = "amlogic,meson-axg-nfc"; >>> ... >>> nand@0 { >>> reg = <0>; >>> }; >>> } >>> >>> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose >>> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called >>> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such >>> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ? >> >> We recently had issues with nvmem parsing, but I believe a mainline >> kernel should now be perfectly working on this regard. What version of >> the Linux kernel are you using? > > My current version is: > > VERSION = 6 > PATCHLEVEL = 2 > SUBLEVEL = 0 > EXTRAVERSION = -rc8 > > Fix was in drivers/nvmem/* ? > > Thanks, Arseniy Upd: I resolved problem in the following way: nand@0 { reg = <0>;//chip select otp@0 { #address-cells = <2>; #size-cells = <0>; compatible = "user-otp"; reg = <A B>; }; otp@1 { #address-cells = <2>; #size-cells = <0>; compatible = "factory-otp"; reg = <C D>; }; }; Now nvmem subsystem parses 'otp@0' and 'otp@1' and error is gone. 'compatible' values are the same as in drivers/mtd/mtdcore.c:mtd_otp_nvmem_add(). 'reg' in 'nand@0' is used as chip select as supposed. I think, this patch should be abandoned in the next version. Thanks, Arseniy > >> >> Thanks, >> Miquèl
Hi Arseniy, avkrasnov@sberdevices.ru wrote on Thu, 11 May 2023 13:16:59 +0300: > On 11.05.2023 12:17, Arseniy Krasnov wrote: > > > > > > On 11.05.2023 12:12, Miquel Raynal wrote: > >> Hi Arseniy, > >> > >> avkrasnov@sberdevices.ru wrote on Thu, 11 May 2023 11:59:07 +0300: > >> > >>> On 10.05.2023 23:53, Miquel Raynal wrote: > >>> > >>> Hello Martin, Miquel > >>> > >>>> Hi Martin & Arseniy, > >>>> > >>>> martin.blumenstingl@googlemail.com wrote on Wed, 10 May 2023 22:40:37 > >>>> +0200: > >>>> > >>>>> Hello Arseniy, > >>>>> > >>>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov > >>>>> <AVKrasnov@sberdevices.ru> wrote: > >>>>>> > >>>>>> This renames node with values for chip select from "reg" to "cs". It is > >>>>>> needed because when OTP access is enabled on the attached storage, MTD > >>>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn > >>>>>> tries to use "reg" node in its own manner, supposes that it has another > >>>>>> layout. All of this leads to device initialization failure. > >>>>> In general: if we change the device-tree interface (in this case: > >>>>> replacing a "reg" with a "cs" property) the dt-bindings have to be > >>>>> updated as well. > >>>> > >>>> True, and I would add, bindings should not be broken. > >>> > >>> I see, that's true. That is bad way to change bindings. > >>> > >>>> > >>>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and > >>>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show > >>>>> that the chip select of a NAND chip is specified with a "reg" > >>>>> property. > >>>> > >>>> All NAND controller binding expect the chip-select to be in the > >>>> 'reg' property, very much like a spi device would use reg to store the > >>>> cs as well: the reg property tells you how you address the device. > >>>> > >>>> I also fully agree with Martin's comments below. Changing reg is likely > >>>> a wrong approach :) > >>>> > >>>>> Also the code has to be backwards compatible with old .dtbs. > >>>>> > >>>>>> Example: > >>>>>> > >>>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/... > >>>>>> [...] mtd mtd0: Failed to register OTP NVMEM device > >>>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22 > >>>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips > >>>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22 > >>>>> This is odd - can you please share your definition of the &nfc node? > >>> > >>> Sure, here it is: > >>> > >>> mtd_nand: nfc@7800 { > >>> compatible = "amlogic,meson-axg-nfc"; > >>> ... > >>> nand@0 { > >>> reg = <0>; > >>> }; > >>> } > >>> > >>> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose > >>> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called > >>> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such > >>> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ? > >> > >> We recently had issues with nvmem parsing, but I believe a mainline > >> kernel should now be perfectly working on this regard. What version of > >> the Linux kernel are you using? > > > > My current version is: > > > > VERSION = 6 > > PATCHLEVEL = 2 > > SUBLEVEL = 0 > > EXTRAVERSION = -rc8 > > > > Fix was in drivers/nvmem/* ? > > > > Thanks, Arseniy > > Upd: I resolved problem in the following way: > > nand@0 { > reg = <0>;//chip select > partitions { compatible = ... > otp@0 { > #address-cells = <2>; > #size-cells = <0>; #address/size-cells is not needed here > compatible = "user-otp"; > reg = <A B>; > }; > otp@1 { > #address-cells = <2>; > #size-cells = <0>; Ditto > compatible = "factory-otp"; > reg = <C D>; > }; }; > }; > > Now nvmem subsystem parses 'otp@0' and 'otp@1' and error is gone. 'compatible' values are > the same as in drivers/mtd/mtdcore.c:mtd_otp_nvmem_add(). 'reg' in 'nand@0' is used as > chip select as supposed. I don't fully get it. The parsing on the nvmem side should not fail if there is no subpartition/otp-region defined. Can you confirm an empty NAND device node works? Because your last e-mail suggested the opposite. > > I think, this patch should be abandoned in the next version. > > Thanks, Arseniy > > > > >> > >> Thanks, > >> Miquèl Thanks, Miquèl
On 11.05.2023 15:11, Miquel Raynal wrote: > Hi Arseniy, > > avkrasnov@sberdevices.ru wrote on Thu, 11 May 2023 13:16:59 +0300: > >> On 11.05.2023 12:17, Arseniy Krasnov wrote: >>> >>> >>> On 11.05.2023 12:12, Miquel Raynal wrote: >>>> Hi Arseniy, >>>> >>>> avkrasnov@sberdevices.ru wrote on Thu, 11 May 2023 11:59:07 +0300: >>>> >>>>> On 10.05.2023 23:53, Miquel Raynal wrote: >>>>> >>>>> Hello Martin, Miquel >>>>> >>>>>> Hi Martin & Arseniy, >>>>>> >>>>>> martin.blumenstingl@googlemail.com wrote on Wed, 10 May 2023 22:40:37 >>>>>> +0200: >>>>>> >>>>>>> Hello Arseniy, >>>>>>> >>>>>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov >>>>>>> <AVKrasnov@sberdevices.ru> wrote: >>>>>>>> >>>>>>>> This renames node with values for chip select from "reg" to "cs". It is >>>>>>>> needed because when OTP access is enabled on the attached storage, MTD >>>>>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn >>>>>>>> tries to use "reg" node in its own manner, supposes that it has another >>>>>>>> layout. All of this leads to device initialization failure. >>>>>>> In general: if we change the device-tree interface (in this case: >>>>>>> replacing a "reg" with a "cs" property) the dt-bindings have to be >>>>>>> updated as well. >>>>>> >>>>>> True, and I would add, bindings should not be broken. >>>>> >>>>> I see, that's true. That is bad way to change bindings. >>>>> >>>>>> >>>>>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and >>>>>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show >>>>>>> that the chip select of a NAND chip is specified with a "reg" >>>>>>> property. >>>>>> >>>>>> All NAND controller binding expect the chip-select to be in the >>>>>> 'reg' property, very much like a spi device would use reg to store the >>>>>> cs as well: the reg property tells you how you address the device. >>>>>> >>>>>> I also fully agree with Martin's comments below. Changing reg is likely >>>>>> a wrong approach :) >>>>>> >>>>>>> Also the code has to be backwards compatible with old .dtbs. >>>>>>> >>>>>>>> Example: >>>>>>>> >>>>>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/... >>>>>>>> [...] mtd mtd0: Failed to register OTP NVMEM device >>>>>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22 >>>>>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips >>>>>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22 >>>>>>> This is odd - can you please share your definition of the &nfc node? >>>>> >>>>> Sure, here it is: >>>>> >>>>> mtd_nand: nfc@7800 { >>>>> compatible = "amlogic,meson-axg-nfc"; >>>>> ... >>>>> nand@0 { >>>>> reg = <0>; >>>>> }; >>>>> } >>>>> >>>>> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose >>>>> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called >>>>> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such >>>>> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ? >>>> >>>> We recently had issues with nvmem parsing, but I believe a mainline >>>> kernel should now be perfectly working on this regard. What version of >>>> the Linux kernel are you using? >>> >>> My current version is: >>> >>> VERSION = 6 >>> PATCHLEVEL = 2 >>> SUBLEVEL = 0 >>> EXTRAVERSION = -rc8 >>> >>> Fix was in drivers/nvmem/* ? >>> >>> Thanks, Arseniy >> >> Upd: I resolved problem in the following way: >> >> nand@0 { >> reg = <0>;//chip select >> > partitions { > compatible = ... > >> otp@0 { >> #address-cells = <2>; >> #size-cells = <0>; > > #address/size-cells is not needed here > >> compatible = "user-otp"; >> reg = <A B>; >> }; >> otp@1 { >> #address-cells = <2>; >> #size-cells = <0>; > > Ditto > >> compatible = "factory-otp"; >> reg = <C D>; >> }; > > }; > >> }; >> >> Now nvmem subsystem parses 'otp@0' and 'otp@1' and error is gone. 'compatible' values are >> the same as in drivers/mtd/mtdcore.c:mtd_otp_nvmem_add(). 'reg' in 'nand@0' is used as >> chip select as supposed. > > I don't fully get it. The parsing on the nvmem side should not fail if > there is no subpartition/otp-region defined. Can you confirm an empty > NAND device node works? Because your last e-mail suggested the opposite. Ok, so i'll describe what happens in my case. Let's NAND node be like this (IIUC this is considered as empty NAND device): mtd_nand: nfc@7800 { compatible = "amlogic,meson-axg-nfc"; ... nand@0 { reg = <0>; }; } I see, that 1) 'mtd_otp_nvmem_add()' calls 'mtd_otp_nvmem_register()' twice for two types of OTP memory "user-otp" and "factory-otp". Let's take a look only on "user-otp". 2) 'mtd_otp_nvmem_register()' tries to lookup for node in 'nand@0' which is compatible with "user-otp" and then passes found (or not found, e.g. NULL) node to 'nvmem_register()'. 3) 'nvmem_register()' uses this node iterating over its childs and searching value "reg" in each child. If "user-otp" node is not found in 2), 'nvmem_register()' uses node 'nfc@7800' also looking for "reg" value in each of its child. In this case it found "reg" in 'nand@0' and fails. Now, if i add "compatible = "user-otp";" to 'nand@0', in step 2) search will be successful, and "reg" value will be used from this new node (or we remove "reg" from it - nothing happens as You wrote). So, problem is that nvmem tries to parse node with invalid "reg" value. Also I see, that 'nvmem_register()' is called earlier in 'mtd_nvmem_add()', but with no effect. I think, that it is not related with enabled OTP feature. Thanks, Arseniy > >> >> I think, this patch should be abandoned in the next version. >> >> Thanks, Arseniy >> >>> >>>> >>>> Thanks, >>>> Miquèl > > > Thanks, > Miquèl
Hi Arseniy, I'm adding Rafał & Michael: any idea what could be wrong? The behavior below does not look expected at all, but I thought we (= Rafał, mainly) already sorted this out? > >>>>>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov > >>>>>>> <AVKrasnov@sberdevices.ru> wrote: > >>>>>>>> > >>>>>>>> This renames node with values for chip select from "reg" to "cs". It is > >>>>>>>> needed because when OTP access is enabled on the attached storage, MTD > >>>>>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn > >>>>>>>> tries to use "reg" node in its own manner, supposes that it has another > >>>>>>>> layout. All of this leads to device initialization failure. > >>>>>>> In general: if we change the device-tree interface (in this case: > >>>>>>> replacing a "reg" with a "cs" property) the dt-bindings have to be > >>>>>>> updated as well. > >>>>>> > >>>>>> True, and I would add, bindings should not be broken. > >>>>> > >>>>> I see, that's true. That is bad way to change bindings. > >>>>> > >>>>>> > >>>>>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and > >>>>>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show > >>>>>>> that the chip select of a NAND chip is specified with a "reg" > >>>>>>> property. > >>>>>> > >>>>>> All NAND controller binding expect the chip-select to be in the > >>>>>> 'reg' property, very much like a spi device would use reg to store the > >>>>>> cs as well: the reg property tells you how you address the device. > >>>>>> > >>>>>> I also fully agree with Martin's comments below. Changing reg is likely > >>>>>> a wrong approach :) > >>>>>> > >>>>>>> Also the code has to be backwards compatible with old .dtbs. > >>>>>>> > >>>>>>>> Example: > >>>>>>>> > >>>>>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/... > >>>>>>>> [...] mtd mtd0: Failed to register OTP NVMEM device > >>>>>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22 > >>>>>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips > >>>>>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22 > >>>>>>> This is odd - can you please share your definition of the &nfc node? > >>>>> > >>>>> Sure, here it is: > >>>>> > >>>>> mtd_nand: nfc@7800 { > >>>>> compatible = "amlogic,meson-axg-nfc"; > >>>>> ... > >>>>> nand@0 { > >>>>> reg = <0>; > >>>>> }; > >>>>> } > >>>>> > >>>>> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose > >>>>> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called > >>>>> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such > >>>>> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ? > >>>> > >>>> We recently had issues with nvmem parsing, but I believe a mainline > >>>> kernel should now be perfectly working on this regard. What version of > >>>> the Linux kernel are you using? > >>> > >>> My current version is: > >>> > >>> VERSION = 6 > >>> PATCHLEVEL = 2 > >>> SUBLEVEL = 0 > >>> EXTRAVERSION = -rc8 > >>> > >>> Fix was in drivers/nvmem/* ? > >>> > >>> Thanks, Arseniy > >> > >> Upd: I resolved problem in the following way: > >> > >> nand@0 { > >> reg = <0>;//chip select > >> > > partitions { > > compatible = ... > > > >> otp@0 { > >> #address-cells = <2>; > >> #size-cells = <0>; > > > > #address/size-cells is not needed here > > > >> compatible = "user-otp"; > >> reg = <A B>; > >> }; > >> otp@1 { > >> #address-cells = <2>; > >> #size-cells = <0>; > > > > Ditto > > > >> compatible = "factory-otp"; > >> reg = <C D>; > >> }; > > > > }; > > > >> }; > >> > >> Now nvmem subsystem parses 'otp@0' and 'otp@1' and error is gone. 'compatible' values are > >> the same as in drivers/mtd/mtdcore.c:mtd_otp_nvmem_add(). 'reg' in 'nand@0' is used as > >> chip select as supposed. > > > > I don't fully get it. The parsing on the nvmem side should not fail if > > there is no subpartition/otp-region defined. Can you confirm an empty > > NAND device node works? Because your last e-mail suggested the opposite. > > Ok, so i'll describe what happens in my case. Let's NAND node be like this (IIUC this is > considered as empty NAND device): > > mtd_nand: nfc@7800 { > compatible = "amlogic,meson-axg-nfc"; > ... > nand@0 { > reg = <0>; > }; > } > > I see, that > > 1) 'mtd_otp_nvmem_add()' calls 'mtd_otp_nvmem_register()' twice for two types of > OTP memory "user-otp" and "factory-otp". Let's take a look only on "user-otp". > 2) 'mtd_otp_nvmem_register()' tries to lookup for node in 'nand@0' which is compatible with > "user-otp" and then passes found (or not found, e.g. NULL) node to 'nvmem_register()'. > 3) 'nvmem_register()' uses this node iterating over its childs and searching value "reg" in > each child. If "user-otp" node is not found in 2), 'nvmem_register()' uses node 'nfc@7800' > also looking for "reg" value in each of its child. In this case it found "reg" in 'nand@0' > and fails. > > Now, if i add "compatible = "user-otp";" to 'nand@0', in step 2) search will be successful, > and "reg" value will be used from this new node (or we remove "reg" from it - nothing happens > as You wrote). So, problem is that nvmem tries to parse node with invalid "reg" value. > > Also I see, that 'nvmem_register()' is called earlier in 'mtd_nvmem_add()', but with no effect. > I think, that it is not related with enabled OTP feature. > > Thanks, Arseniy
On 12.05.2023 17:49, Miquel Raynal wrote: > Hi Arseniy, > > I'm adding Rafał & Michael: any idea what could be wrong? The behavior > below does not look expected at all, but I thought we (= Rafał, mainly) > already sorted this out? > Hi Miquel, thanks for help! just to clarify an expected behaviour: if i have the following layout in the device tree mtd_nand: nfc@7800 { compatible = "amlogic,meson-axg-nfc"; ... nand@0 { reg = <0>; }; } node used by 'nvmem_add_cells_from_of()' must be NULL? or 'nand@0'? I guess, that in above dts I have node 'nfc@7800' in use, because 'mtd_otp_nvmem_register()' uses the following device before passing 'config' to 'nvmem_register()': /* OTP nvmem will be registered on the physical device */ config.dev = mtd->dev.parent; 'mtd->dev.parent' is 'nfc@7800'. May be 'mtd_otp_nvmem_register()' must initialize 'no_of_node' field of 'config' like in 'mtd_nvmem_add()' ? This field is documented as: * @no_of_node: Device should not use the parent's of_node even if it's !NULL. In this case node passed to 'nvmem_add_cells_from_of()' will be NULL. Thanks, Arseniy >>>>>>>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov >>>>>>>>> <AVKrasnov@sberdevices.ru> wrote: >>>>>>>>>> >>>>>>>>>> This renames node with values for chip select from "reg" to "cs". It is >>>>>>>>>> needed because when OTP access is enabled on the attached storage, MTD >>>>>>>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn >>>>>>>>>> tries to use "reg" node in its own manner, supposes that it has another >>>>>>>>>> layout. All of this leads to device initialization failure. >>>>>>>>> In general: if we change the device-tree interface (in this case: >>>>>>>>> replacing a "reg" with a "cs" property) the dt-bindings have to be >>>>>>>>> updated as well. >>>>>>>> >>>>>>>> True, and I would add, bindings should not be broken. >>>>>>> >>>>>>> I see, that's true. That is bad way to change bindings. >>>>>>> >>>>>>>> >>>>>>>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and >>>>>>>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show >>>>>>>>> that the chip select of a NAND chip is specified with a "reg" >>>>>>>>> property. >>>>>>>> >>>>>>>> All NAND controller binding expect the chip-select to be in the >>>>>>>> 'reg' property, very much like a spi device would use reg to store the >>>>>>>> cs as well: the reg property tells you how you address the device. >>>>>>>> >>>>>>>> I also fully agree with Martin's comments below. Changing reg is likely >>>>>>>> a wrong approach :) >>>>>>>> >>>>>>>>> Also the code has to be backwards compatible with old .dtbs. >>>>>>>>> >>>>>>>>>> Example: >>>>>>>>>> >>>>>>>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/... >>>>>>>>>> [...] mtd mtd0: Failed to register OTP NVMEM device >>>>>>>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22 >>>>>>>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips >>>>>>>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22 >>>>>>>>> This is odd - can you please share your definition of the &nfc node? >>>>>>> >>>>>>> Sure, here it is: >>>>>>> >>>>>>> mtd_nand: nfc@7800 { >>>>>>> compatible = "amlogic,meson-axg-nfc"; >>>>>>> ... >>>>>>> nand@0 { >>>>>>> reg = <0>; >>>>>>> }; >>>>>>> } >>>>>>> >>>>>>> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose >>>>>>> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called >>>>>>> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such >>>>>>> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ? >>>>>> >>>>>> We recently had issues with nvmem parsing, but I believe a mainline >>>>>> kernel should now be perfectly working on this regard. What version of >>>>>> the Linux kernel are you using? >>>>> >>>>> My current version is: >>>>> >>>>> VERSION = 6 >>>>> PATCHLEVEL = 2 >>>>> SUBLEVEL = 0 >>>>> EXTRAVERSION = -rc8 >>>>> >>>>> Fix was in drivers/nvmem/* ? >>>>> >>>>> Thanks, Arseniy >>>> >>>> Upd: I resolved problem in the following way: >>>> >>>> nand@0 { >>>> reg = <0>;//chip select >>>> >>> partitions { >>> compatible = ... >>> >>>> otp@0 { >>>> #address-cells = <2>; >>>> #size-cells = <0>; >>> >>> #address/size-cells is not needed here >>> >>>> compatible = "user-otp"; >>>> reg = <A B>; >>>> }; >>>> otp@1 { >>>> #address-cells = <2>; >>>> #size-cells = <0>; >>> >>> Ditto >>> >>>> compatible = "factory-otp"; >>>> reg = <C D>; >>>> }; >>> >>> }; >>> >>>> }; >>>> >>>> Now nvmem subsystem parses 'otp@0' and 'otp@1' and error is gone. 'compatible' values are >>>> the same as in drivers/mtd/mtdcore.c:mtd_otp_nvmem_add(). 'reg' in 'nand@0' is used as >>>> chip select as supposed. >>> >>> I don't fully get it. The parsing on the nvmem side should not fail if >>> there is no subpartition/otp-region defined. Can you confirm an empty >>> NAND device node works? Because your last e-mail suggested the opposite. >> >> Ok, so i'll describe what happens in my case. Let's NAND node be like this (IIUC this is >> considered as empty NAND device): >> >> mtd_nand: nfc@7800 { >> compatible = "amlogic,meson-axg-nfc"; >> ... >> nand@0 { >> reg = <0>; >> }; >> } >> >> I see, that >> >> 1) 'mtd_otp_nvmem_add()' calls 'mtd_otp_nvmem_register()' twice for two types of >> OTP memory "user-otp" and "factory-otp". Let's take a look only on "user-otp". >> 2) 'mtd_otp_nvmem_register()' tries to lookup for node in 'nand@0' which is compatible with >> "user-otp" and then passes found (or not found, e.g. NULL) node to 'nvmem_register()'. >> 3) 'nvmem_register()' uses this node iterating over its childs and searching value "reg" in >> each child. If "user-otp" node is not found in 2), 'nvmem_register()' uses node 'nfc@7800' >> also looking for "reg" value in each of its child. In this case it found "reg" in 'nand@0' >> and fails. >> >> Now, if i add "compatible = "user-otp";" to 'nand@0', in step 2) search will be successful, >> and "reg" value will be used from this new node (or we remove "reg" from it - nothing happens >> as You wrote). So, problem is that nvmem tries to parse node with invalid "reg" value. >> >> Also I see, that 'nvmem_register()' is called earlier in 'mtd_nvmem_add()', but with no effect. >> I think, that it is not related with enabled OTP feature. >> >> Thanks, Arseniy
diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c index bc99a098f3ca..28371919a6c5 100644 --- a/drivers/mtd/nand/raw/meson_nand.c +++ b/drivers/mtd/nand/raw/meson_nand.c @@ -1419,7 +1419,7 @@ meson_nfc_nand_chip_init(struct device *dev, int ret, i; u32 tmp, nsels; - nsels = of_property_count_elems_of_size(np, "reg", sizeof(u32)); + nsels = of_property_count_elems_of_size(np, "cs", sizeof(u32)); if (!nsels || nsels > MAX_CE_NUM) { dev_err(dev, "invalid register property size\n"); return -EINVAL; @@ -1433,7 +1433,7 @@ meson_nfc_nand_chip_init(struct device *dev, meson_chip->nsels = nsels; for (i = 0; i < nsels; i++) { - ret = of_property_read_u32_index(np, "reg", i, &tmp); + ret = of_property_read_u32_index(np, "cs", i, &tmp); if (ret) { dev_err(dev, "could not retrieve register property: %d\n", ret);