On Tue, 21 Aug 2018 10:38:13 +0100
Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> wrote:
> 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;
>
That might work if nvmem cells are defined directly under the mtdnode.
If we go for this approach, I'd recommend replacing this ->match() hook
by ->is_nvmem_cell() and pass it the cell node instead of the nvmem
node, because what we're really after here is knowing which subnode is
an nvmem cell and which subnode is not.
