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

[Srinivas Kandagatla]

Hi Boris/Bartosz,

On 21/08/18 06:44, Boris Brezillon wrote:
> > > 4/ Add a ->of_xlate() hook that would be called if present by the
> > >     framework instead of using the default parsing we have right now.
> > That is a bit cleaner, but I don't think it would be worse the
> > complexity.
> But it's way more flexible than putting everything in the nvmem
> framework. BTW, did you notice that nvmem-cells parsing does not work
> with flashes bigger than 4GB, because the framework assumes
> #address-cells and #size-cells are always 1. That's probably something
> we'll have to fix for the MTD case.

I have hacked up some thing on these lines to add a custom match 
function for nvmem provider and it looks like it can work for mtd case.

This addresses concern #1 "to ignore of_node from dev pointer passed to 
nvmem_config" also provides way to do some sanity checks on nvmem cell node.
In this patch I have just added a simple mtd_nvmem_match() example which 
will be always true, however we can add checks here to see if the np is 
actually a nvmem-cells node or something on those lines to enforce the 
bindings. Please fix and remove this from nvmem-core patch incase you 
plan to use/test this.

We still have one open issue of supporting #address-cells and 
#size-cells in nvmem, which I can look at if you are happy with this 
approach!

----------------------------------->cut<---------------------------------
Author: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
Date:   Tue Aug 21 10:07:24 2018 +0100

    nvmem: core: add custom match function support

    Some nvmem providers might not have a simple DT layout, nvmem cells
    could be part of the unpartioned space or with-in partition or
    even in sub partition of the provider.

    Current matching function is expecting that the provider should be
    immediate parent of the cell, which might not be true for the above
    cases. So allow a custom match function for such devices which can
    validate and match the cell as per the provider specific bindings.

    Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index a57302eaceb5..33541b18ac30 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -502,6 +502,19 @@ static int mtd_nvmem_reg_read(void *priv, unsigned 
int offset,
        return retlen == bytes ? 0 : -EIO;
 }

+static int mtd_nvmem_match(void *priv, struct device *dev,
+                          struct device_node *np)
+{
+       struct mtd_info *mtd = priv;
+
+       /*
+        * Add more checks to make sure device node is inline with
+        * binding if required
+        */
+
+       return &mtd->dev == dev->parent;
+}
+
 static int mtd_nvmem_add(struct mtd_info *mtd)
 {
        struct nvmem_config config = { };
@@ -516,6 +529,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
        config.read_only = true;
        config.root_only = true;
        config.priv = mtd;
+       config.match = mtd_nvmem_match;

        mtd->nvmem = devm_nvmem_register(&mtd->dev, &config);
        if (IS_ERR(mtd->nvmem)) {
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 3a8bf832243d..32bc4e70522c 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -41,6 +41,7 @@ struct nvmem_device {
        struct device           *base_dev;
        nvmem_reg_read_t        reg_read;
        nvmem_reg_write_t       reg_write;
+       nvmem_match_t           match;
        void *priv;
 };

@@ -265,6 +266,11 @@ static struct bus_type nvmem_bus_type = {

 static int of_nvmem_match(struct device *dev, void *nvmem_np)
 {
+       struct nvmem_device *nvmem = to_nvmem_device(dev);
+
+       if (nvmem->match)
+               return nvmem->match(nvmem->priv, dev, nvmem_np);
+
        return dev->of_node == nvmem_np;
 }

@@ -482,7 +488,9 @@ struct nvmem_device *nvmem_register(const struct 
nvmem_config *config)
        nvmem->priv = config->priv;
        nvmem->reg_read = config->reg_read;
        nvmem->reg_write = config->reg_write;
-       nvmem->dev.of_node = config->dev->of_node;
+
+       if (!config->match)
+               nvmem->dev.of_node = config->dev->of_node;

        if (config->id == -1 && config->name) {
                dev_set_name(&nvmem->dev, "%s", config->name);
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 24def6ad09bb..b29059bb406e 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -14,6 +14,7 @@

 #include <linux/err.h>
 #include <linux/errno.h>
+#include <linux/of.h>

 struct nvmem_device;
 struct nvmem_cell_info;
@@ -21,6 +22,9 @@ typedef int (*nvmem_reg_read_t)(void *priv, unsigned 
int offset,
                                void *val, size_t bytes);
 typedef int (*nvmem_reg_write_t)(void *priv, unsigned int offset,
                                 void *val, size_t bytes);
+typedef int (*nvmem_match_t)(void *priv, struct device *dev,
+                            struct device_node *np);
+

 /**
  * struct nvmem_config - NVMEM device configuration
@@ -58,6 +62,7 @@ struct nvmem_config {
        bool                    root_only;
        nvmem_reg_read_t        reg_read;
        nvmem_reg_write_t       reg_write;
+       nvmem_match_t           match;
        int     size;
        int     word_size;
        int     stride;

----------------------------------->cut<---------------------------------

thanks,
srini