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]
Message-ID: <20180821133136.1fada1b6@bbrezillon>
Date:   Tue, 21 Aug 2018 13:31:36 +0200
From:   Boris Brezillon <boris.brezillon@...tlin.com>
To:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
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 Tue, 21 Aug 2018 10:38:13 +0100
Srinivas Kandagatla <srinivas.kandagatla@...aro.org> 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@...aro.org>
> 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@...aro.org>
> 
> 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.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ