Messages in this thread | | | Subject | Re: [PATCH v4] nvmem: core: skip child nodes not matching binding | From | Srinivas Kandagatla <> | Date | Fri, 22 Jan 2021 15:27:52 +0000 |
| |
On 18/01/2021 13:24, Ahmad Fatoum wrote: > 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(-)
Applied thanks,
--srini
> > 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; > } >
| |