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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E8909C07E9D for ; Fri, 23 Sep 2022 17:29:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:References:In-Reply-To:Subject:Cc:To:From :Date:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=LvTX7bN+nkmCKHofvzY7638arPf1G79o9IB8i+ifA1w=; b=Jp88fDNQN0keuwy749cSH1rNTY Euz4dptWOfNtGo2QZA5dXg2nhmX2IOpnT5p2uyY60dCzaY6JK9gPLba/E5W1qIO4lDes4lHGvfHlh zr1kGj+Xsdh1la4G8hSHlg1XW5qXizskpAoD4YONAG1wVuNR4u6KIDFp719o6CRc3ZSww+XanMfwg abCRs7+WS4LFuE+8jB7vKERFNL1NQ53mM3h3mTo04XFVaXYkHLPCUx+ON/Kf3WLBLlSOX40p4TeL+ cSAewaHEnKDAJJokVcvmr3l9y0j6zHmx5HuigtQBajAzULzNfCPyQG7ti018xLT5dUF5YCSQQqWqI TxfRxStA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1obmTw-0058QM-EO; Fri, 23 Sep 2022 17:28:56 +0000 Received: from 0001.3ffe.de ([2a01:4f8:c0c:9d57::1] helo=mail.3ffe.de) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1obmTs-0058O6-Hf; Fri, 23 Sep 2022 17:28:54 +0000 Received: from 3ffe.de (0001.3ffe.de [IPv6:2a01:4f8:c0c:9d57::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.3ffe.de (Postfix) with ESMTPSA id 050A41250; Fri, 23 Sep 2022 19:28:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=walle.cc; s=mail2022082101; t=1663954126; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=eHn7dUEakYs6EqU0H1lIKA8WzZRAHG8KE3zOYOsxZq8=; b=Ov+iQUNVotABs01Sc1gLX+z8jfuca90pkWpVb7jbIBKu8YCorG8EVj19b//AeaE0Dq5bEz /Kaz6B5fyMmwdDOL0Xsj+w2mpOEup4KmYkEbPBwgBg7H5UsBM6e6ihgwlUj7vF6ZcqWS3e MY2776cd5nKhhXQALckyeWMWS77QDonO6o1Ootf55Zll7rwfH2Ot+StdKHH/VbUnlR3VGa yS3KbCTIy7zP2BCweyeY1973HEe0XILvuyebFTG3DMEGkiF1VI6c75Qo97xQ5kp2HQwtRW 1QXBRrBPWzmVjI0oGZVOErPfWbZ1HsAM0iTL6WpVVE2L9LoJzToH+xMJKKvzIg== MIME-Version: 1.0 Date: Fri, 23 Sep 2022 19:28:45 +0200 From: Michael Walle To: Miquel Raynal Cc: Richard Weinberger , Vignesh Raghavendra , Rob Herring , Krzysztof Kozlowski , Srinivas Kandagatla , Shawn Guo , Li Yang , =?UTF-8?Q?Rafa=C5=82_Mi=C5=82ecki?= , Frank Rowand , linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Ahmad Fatoum , Philipp Zabel , Robert Marko , Thomas Petazzoni Subject: Re: [PATCH v2 00/20] nvmem: core: introduce NVMEM layouts In-Reply-To: <20220923174759.299e504e@xps-13> References: <20220901221857.2600340-1-michael@walle.cc> <20220923174759.299e504e@xps-13> User-Agent: Roundcube Webmail/1.4.13 Message-ID: <0c0522a2c7e4b2401955acf5bcd4eb8e@walle.cc> X-Sender: michael@walle.cc X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220923_102852_812803_0CE4E01A X-CRM114-Status: GOOD ( 42.19 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, Am 2022-09-23 17:47, schrieb Miquel Raynal: > I have a few additional questions regarding the bindings. > > michael@walle.cc wrote on Fri, 2 Sep 2022 00:18:37 +0200: > >> This is now the third attempt to fetch the MAC addresses from the VPD >> for the Kontron sl28 boards. Previous discussions can be found here: >> https://lore.kernel.org/lkml/20211228142549.1275412-1-michael@walle.cc/ >> >> >> NVMEM cells are typically added by board code or by the devicetree. >> But >> as the cells get more complex, there is (valid) push back from the >> devicetree maintainers to not put that handling in the devicetree. >> >> Therefore, introduce NVMEM layouts. They operate on the NVMEM device >> and >> can add cells during runtime. That way it is possible to add more >> complex >> cells than it is possible right now with the offset/length/bits >> description in the device tree. For example, you can have post >> processing >> for individual cells (think of endian swapping, or ethernet offset >> handling). >> >> The imx-ocotp driver is the only user of the global post processing >> hook, >> convert it to nvmem layouts and drop the global post pocessing hook. >> Please >> note, that this change is only compile-time tested. >> >> You can also have cells which have no static offset, like the >> ones in an u-boot environment. The last patches will convert the >> current >> u-boot environment driver to a NVMEM layout and lifting the >> restriction >> that it only works with mtd devices. But as it will change the >> required >> compatible strings, it is marked as RFC for now. It also needs to have >> its device tree schema update which is left out here. These two >> patches >> are not expected to be applied, but rather to show another example of >> how to use the layouts. >> >> For now, the layouts are selected by a specific compatible string in a >> device tree. E.g. the VPD on the kontron sl28 do (within a SPI flash >> node): >> compatible = "kontron,sl28-vpd", "user-otp"; >> or if you'd use the u-boot environment (within an MTD patition): >> compatible = "u-boot,env", "nvmem"; >> >> The "user-otp" (or "nvmem") will lead to a NVMEM device, the >> "kontron,sl28-vpd" (or "u-boot,env") will then apply the specific >> layout >> on top of the NVMEM device. > > So if I understand correctly, there should be: > - one DT node defining the storage medium eeprom/mtd/whatever, > - another DT node defining the nvmem device with two compatibles, the > "nvmem" (or "user-otp") and the layout. > Is this correct? Actually I was a bit surprised because generally > speaking, DT maintainers (rightfully) do not want to describe how we > use devices, the nvmem abstraction looks like a Linux thing when on top > of mtd devices for instance, so I just wanted to confirm this point. What do you mean by two nodes? Two separate ones or one being a subnode of the other? There is only one (storage) node and nvmem cells as subnodes. The two compatibles aren't strictly needed. But it will simplify the drivers in linux greatly. Otherwise the storage driver would need to know for which compatibles it needs to register a nvmem device. E.g. MTD devices determine that by the "nvmem" compatible. The OTP driver will probe by "{user,factory}-otp". If you'd only have one compatible, the storage driver would need a list of all the layouts so it can register a nvmem device. But also from a device tree POV this makes sense IMHO because the second compatible is a more specific one. With only the more generic compatible you just get a nvmem device without any cells - or only the cells described in the device tree. Regarding "describe how the devices are used": then there shouldn't be nvmem (cell) bindings at all, because you are actually describing how you are using the nvmem provider. So IMHO having for example the compatible "kontron,sl28-vpd" is actually fits more than having a nvmem provider compatible with cells subnodes. > Then, as we have an nvmem device described in the DT, why not just > pointing at the nvmem device from the cell consumer, rather than still > having the need to define all the cells that the nvmem device will > produce in the DT? See also https://lore.kernel.org/linux-devicetree/4bf16e18-1591-8bc9-7c46-649391de3761@linaro.org/ > Maybe an example to show what I mean. Here is the current way: > > nvmem_provider: nvmem-provider { > properties; > > mycell: my_cell { > [properties;] > }; > }; > > And we point to a cell with: > > nvmem-cells = <&mycell>; > > But, as for the tlv tables, there are many cells that will be produced, > and the driver may anyway just ask for the cell name (eg. performing a > lookup of the "mac-address" cell name), so why bothering to describe > all > the cells in the DT, like: > > nvmem-cells-providers = <&nvmem_provider>; > > What do you think? Ok, you even go one step further and even removing the argument of the phandle and you are proposing to use the nvmem-cell-name, right? That might work with simple cells created by a layout. But what if there are two consumers with different names for the same cell? Consumer bindings might already be present, e.g. the ethernet bindings will use "mac-address". What if there is another binding which want to use that cell but doesn't name it "mac-address"? IMHO to reference a nvmem cell you shouldn't rely on the consumer. Also keep in mind, that the number of arguments is fixed and given by the "#.*-cells" property found on the target node. Therefore, that won't work if you have cells where one has an argument and another don't. > Maybe for the mac addresses this is a bit limiting as, in practice, we > often have base mac addresses available and using: > > nvmem-cells = <&mycell INDEX>; > > allows to dynamically create many different mac addresses, but I wonder > if the approach would be interesting for other cell types. Just an open > question. So how would your idea work with that? Maybe we could support both? But again, I'm not sure if it is a good idea to mix the consumer with the provider. -michael _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel