| Message ID | 20170308162001.2b7e2304@avionic-0020 |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi Alban, On Wed, 8 Mar 2017 16:20:01 +0100 Alban <albeu@free.fr> wrote: > On Tue, 7 Mar 2017 22:01:07 +0100 > Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > > > On Tue, 7 Mar 2017 09:26:03 +0100 > > Alban <albeu@free.fr> wrote: > > > > > Config data for drivers, like MAC addresses, is often stored in MTD. > > > Add a binding that define how such data storage can be represented in > > > device tree. > > > > > > Signed-off-by: Alban <albeu@free.fr> > > > --- > > > Changelog: > > > v2: * Added a "Required properties" section with the nvmem-provider > > > property > > > --- > > > .../devicetree/bindings/nvmem/mtd-nvmem.txt | 33 ++++++++++++++++++++++ > > > 1 file changed, 33 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > > > > > > diff --git a/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > > > new file mode 100644 > > > index 0000000..8ed25e6 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/nvmem/mtd-nvmem.txt > > > @@ -0,0 +1,33 @@ > > > += NVMEM in MTD = > > > + > > > +Config data for drivers, like MAC addresses, is often stored in MTD. > > > +This binding define how such data storage can be represented in device tree. > > > + > > > +An MTD can be defined as an NVMEM provider by adding the `nvmem-provider` > > > +property to their node. > > > > If everyone agrees that this is actually needed, then it should > > definitely go in the nvmem binding doc, and we should patch all nvmem > > providers to define this property (even if we keep supporting nodes > > that are not defining it). I'm not fully convinced yet, but I might be > > wrong. > > I really like to hear what the DT people think about this. That was the plan. > > > I also think we should take the "nvmem under flash node without partitions" > > into account now, or at least have a clear plan on how we want to represent > > it. > > > > Something like that? > > Yes, but with the following extras: > > > flash { > nvmem-provider; > > partitions { > > part@X { > > nvmem { > compatible = "nvmem-cells"; > > #address-cells = <1>; > > #size-cells = <1>; > > > > cell@Y { > > }; > > }; > > }; > > }; > > > > nvmem { > compatible = "nvmem-cells"; > > #address-cells = <1>; > > #size-cells = <1>; > > > > cell@X { > > }; > > }; > > }; > > > > Note that patching nvmem core to support the subnode case should be > > pretty easy (see below). > > This shouldn't be needed as nothing would change for the NVMEM devices, > what could be added is a check for the "nvmem-provider" property. > To support the proposed binding we would only need a minor change to > of_nvmem_cell_get(): > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c > index 408b521ee520..6231ea27c9f4 100644 > --- a/drivers/nvmem/core.c > +++ b/drivers/nvmem/core.c > @@ -444,6 +444,10 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) > if (!config->dev) > return ERR_PTR(-EINVAL); > > + if (config->dev->of_node && > + !of_property_read_bool(config->dev->of_node, "nvmem-provider")) > + return ERR_PTR(-ENODEV); > + > nvmem = kzalloc(sizeof(*nvmem), GFP_KERNEL); > if (!nvmem) > return ERR_PTR(-ENOMEM); > @@ -777,6 +781,15 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, > if (!nvmem_np) > return ERR_PTR(-EINVAL); > > + /* handle the new cell binding */ > + if (of_device_is_compatible(nvmem_np, "nvmem-cells")) { > + nvmem_np = of_get_next_parent(cell_np); > + if (!nvmem_np) > + return ERR_PTR(-EINVAL); > + if (!of_property_read_bool(nvmem_np, "nvmem-provider")) > + return ERR_PTR(-ENODEV); > + } > + > nvmem = __nvmem_device_get(nvmem_np, NULL, NULL); > if (IS_ERR(nvmem)) > return ERR_CAST(nvmem); > Yep, works too. Let's wait for a DT review, before taking a decision. Thanks, Boris
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index 408b521ee520..6231ea27c9f4 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -444,6 +444,10 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config) if (!config->dev) return ERR_PTR(-EINVAL); + if (config->dev->of_node && + !of_property_read_bool(config->dev->of_node, "nvmem-provider")) + return ERR_PTR(-ENODEV); + nvmem = kzalloc(sizeof(*nvmem), GFP_KERNEL); if (!nvmem) return ERR_PTR(-ENOMEM); @@ -777,6 +781,15 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, if (!nvmem_np) return ERR_PTR(-EINVAL); + /* handle the new cell binding */ + if (of_device_is_compatible(nvmem_np, "nvmem-cells")) { + nvmem_np = of_get_next_parent(cell_np); + if (!nvmem_np) + return ERR_PTR(-EINVAL); + if (!of_property_read_bool(nvmem_np, "nvmem-provider")) + return ERR_PTR(-ENODEV); + } + nvmem = __nvmem_device_get(nvmem_np, NULL, NULL); if (IS_ERR(nvmem)) return ERR_CAST(nvmem);