linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Cc: Kamel Bouhara <kamel.bouhara@bootlin.com>,
	ceggers@arri.de, kernel@pengutronix.de,
	Ahmad Fatoum <a.fatoum@pengutronix.de>,
	Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v4] nvmem: core: skip child nodes not matching binding
Date: Mon, 18 Jan 2021 14:24:11 +0100	[thread overview]
Message-ID: <20210118132411.6268-1-a.fatoum@pengutronix.de> (raw)

The nvmem cell binding applies to all eeprom child nodes matching
"^.*@[0-9a-f]+$" without taking a compatible into account.

Linux drivers, like at24, are even more extensive and assume
_all_ at24 eeprom child nodes to be nvmem cells since e888d445ac33
("nvmem: resolve cells from DT at registration time").

Since df5f3b6f5357 ("dt-bindings: nvmem: stm32: new property for
data access"), the additionalProperties: True means it's Ok to have
other properties as long as they don't match "^.*@[0-9a-f]+$".

The barebox bootloader extends the MTD partitions binding to
EEPROM and can fix up following device tree node:

  &eeprom {
    partitions {
      compatible = "fixed-partitions";
    };
  };

This is allowed binding-wise, but drivers using nvmem_register()
like at24 will fail to parse because the function expects all child
nodes to have a reg property present. This results in the whole
EEPROM driver probe failing despite the device tree being correct.

Fix this by skipping nodes lacking a reg property instead of
returning an error. This effectively makes the drivers adhere
to the binding because all nodes with a unit address must have
a reg property and vice versa.

Fixes: e888d445ac33 ("nvmem: resolve cells from DT at registration time").
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

---
Previous Discussion: https://lore.kernel.org/lkml/20200428111829.2215-1-a.fatoum@pengutronix.de/
v1:
  - Ignore all nodes with a unit address (i.e. contain @)
v1 -> v2:
  - use ->full_name instead of ->name as to not break existing correct
    cells (Christian)
v2 -> v3:
  - use optional compatible property to weed out nodes instead of name
  - extend binding documentation (Srini)
v3 -> v4:
  - drop optional nvmem-cell compatible because it's redundant (Rob)
  - Make driver adhere to binding instead of changing binding

As review feedback on v3, Rob suggested moving nvmem cells into a
separate MTD partition. This sound good for people who want MTD
partitions and nvmem to coexist, but my problem described above where
MTD partitions are already fixed up top level into an EEPROM node isn't
solved by this. Revisiting the issue, I think the correct way forward is
along the lines of v1 & v2, where the driver is fixed to actually adhere
to the existing binding. Srini didn't like string matching for @ in driver
code, so I now check for presence of reg instead. They are equivalent
per device tree specification. From v0.3:

  "The unit-address must match the first address specified in the
  reg property of the node. If the node has no reg property, the
  @unit-address must be omitted"...

Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 drivers/nvmem/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 177f5bf27c6d..f114df55f403 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -682,7 +682,9 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
 
 	for_each_child_of_node(parent, child) {
 		addr = of_get_property(child, "reg", &len);
-		if (!addr || (len < 2 * sizeof(u32))) {
+		if (!addr)
+			continue;
+		if (len < 2 * sizeof(u32)) {
 			dev_err(dev, "nvmem: invalid reg on %pOF\n", child);
 			return -EINVAL;
 		}
-- 
2.30.0


             reply	other threads:[~2021-01-18 13:26 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-18 13:24 Ahmad Fatoum [this message]
2021-01-22 15:27 ` [PATCH v4] nvmem: core: skip child nodes not matching binding Srinivas Kandagatla

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210118132411.6268-1-a.fatoum@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=bgolaszewski@baylibre.com \
    --cc=ceggers@arri.de \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=kamel.bouhara@bootlin.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=srinivas.kandagatla@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).