linux-kernel - Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 21 Aug 2018 11:11:58 +0100
From:   Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:     Boris Brezillon <boris.brezillon@...tlin.com>
Cc:     Alban <albeu@...e.fr>, Bartosz Golaszewski <brgl@...ev.pl>,
        Jonathan Corbet <corbet@....net>, Sekhar Nori <nsekhar@...com>,
        Kevin Hilman <khilman@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        David Woodhouse <dwmw2@...radead.org>,
        Brian Norris <computersforpeace@...il.com>,
        Marek Vasut <marek.vasut@...il.com>,
        Richard Weinberger <richard@....at>,
        Grygorii Strashko <grygorii.strashko@...com>,
        "David S . Miller" <davem@...emloft.net>,
        Naren <naren.kernel@...il.com>,
        Mauro Carvalho Chehab <mchehab+samsung@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Lukas Wunner <lukas@...ner.de>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>,
        Sven Van Asbroeck <svendev@...x.com>,
        Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh@...nel.org>,
        David Lechner <david@...hnology.com>,
        Andrew Lunn <andrew@...n.ch>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-i2c@...r.kernel.org, linux-mtd@...ts.infradead.org,
        linux-omap@...r.kernel.org, netdev@...r.kernel.org,
        Bartosz Golaszewski <bgolaszewski@...libre.com>
Subject: Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the
 nvmem API



On 21/08/18 10:56, Boris Brezillon wrote:
> On Tue, 21 Aug 2018 10:50:07 +0100
> Srinivas Kandagatla<srinivas.kandagatla@...aro.org>  wrote:
> 
>> On 20/08/18 19:20, Boris Brezillon wrote:
>>> On Mon, 20 Aug 2018 11:43:34 +0100
>>> Srinivas Kandagatla<srinivas.kandagatla@...aro.org>  wrote:
>>>    
>>>> Overall am still not able to clear visualize on how MTD bindings with
>>>> nvmem cells would look in both partition and un-partition usecases?
>>>> An example DT would be nice here!!
>>> Something along those lines:
>>>    
>> This looks good to me.
>>> 	mtdnode {
>>> 		nvmem-cells {
>>> 			#address-cells = <1>;
>>> 			#size-cells = <1>;
>>>
>>> 			cell@0 {
>>> 				reg = <0x0 0x14>;
>>> 			};
>>> 		};
>>>
>>> 		partitions {
>>> 			compatible = "fixed-partitions";
>>> 			#address-cells = <1>;
>>> 			#size-cells = <1>;
>>>
>>> 			partition@0 {
>>> 				reg = <0x0 0x20000>;
>>>
>>> 				nvmem-cells {
>>> 					#address-cells = <1>;
>>> 					#size-cells = <1>;
>>>
>>> 					cell@0 {
>>> 						reg = <0x0 0x10>;
>>> 					};
>>> 				};
>>> 			};
>>> 		};
>>> 	}; >
>> Just curious...Is there a reason why we can't do it like this?:
>> Is this because of issue of #address-cells and #size-cells Or mtd
>> bindings always prefer subnodes?
>>
>> 	mtdnode {
>> 		reg = <0x0123000 0x40000>;
>> 		#address-cells = <1>;
>> 		#size-cells = <1>;
>> 		cell@0 {
>> 			compatible = "nvmem-cell";
>> 			reg = <0x0 0x14>;
>> 		};
>>
>> 		partitions {
>> 			compatible = "fixed-partitions";
>> 			#address-cells = <1>;
>> 			#size-cells = <1>;
>>
>> 			partition@0 {
>> 				reg = <0x0 0x20000>;
>> 				cell@0 {
>> 					compatible = "nvmem-cell";
>> 					reg = <0x0 0x10>;
>> 				};
>> 			};
>> 		};
>> 	};
> It's because partitions were initially directly defined under the mtd
> node, so, if you have an old DT you might have something like:
> 
> 	mtdnode {
> 		reg = <0x0123000 0x40000>;
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 
> 		partition@0 {
> 			reg = <0x0 0x20000>;
> 			...
> 		};
> 		...
> 	};
> 
> If we use such a DT with this patch applied, the NVMEM framework will
> consider MTD partitions as nvmem cells, which is not what we want.
Yep, I agree.
TBH, I wanted to add compatible string to nvmem-cell at some point in 
time and it seems more natural update too. One of the reason we 
discussed this in the past was parsers. Looks like mtd can make use of this.

We should be able to add this as an optional flag in nvmem_config to 
enforce this check in case providers wanted to.

Do you think that would help mtd nvmem case?
Also I felt like nvmem-cells subnode seems to be a bit heavy!

thanks,
srini