lkml.org 
[lkml]   [2021]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4] nvmem: core: skip child nodes not matching binding
From
Date


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;
> }
>

\
 
 \ /
  Last update: 2021-01-22 16:30    [W:0.326 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site