linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell"
@ 2020-04-28 11:18 Ahmad Fatoum
  2020-04-28 11:18 ` [PATCH v3 1/2] dt-bindings: " Ahmad Fatoum
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2020-04-28 11:18 UTC (permalink / raw)
  To: linux-kernel; +Cc: kernel, ceggers, Ahmad Fatoum, Rob Herring, devicetree

The nvmem cell binding applies to all objects which match "^.*@[0-9a-f]+$",
without taking a compatible into account. This precludes extension of e.g.
eeprom nodes by any child nodes other than nvmem. Consider following example:

	eeprom@0 {
		reg = <0 64>;
		#address-cells = <1>;
		#size-cells = <1>;

		partitions {
			compatible = "fixed-partitions";
			#address-cells = <1>;
			#size-cells = <1>;
			bits = <64 64 64>; /* to verify it's skipped */

			part@0 {
				reg = <0x00 16>;
			};
		};

		no-cell@10 {
			compatible = "not-nvmem-cell";
			reg = <0x10 4>;
			bits = <64 64 64>; /* to verify it's skipped */
		};

		cell-old@14 {
			reg = <0x14 0x2>;
		};

		cell-new@16 {
			compatible = "nvmem-cell";
			reg = <0x16 4>;
		};
	};

Without this series, the NVMEM driver interprets all direct children of eeprom@0
as NVMEM cells and driver probe fails, because the partitions node lacks a reg
property, e.g.:

  nvmem 0-00000: nvmem: invalid reg on /eeprom@0

Running dtbs_check on the snippet will skip partitions (it doesn't match above
regex), but will flag no-cell@10 and cell-new@16 as invalid.

With this series applied, the driver will skip partitions and no-cell@10,
because they have a compatible but it's not "nvmem-cell".
Both cell-old@14 and cell-new@16 will be interpreted as cells.

Likewise, running dtbs_check on the snippet will skip partitions (compatible
doesn't match and regex doesn't either) and no-cell@10, but accept the other two.

This series resolves an existing clash between this nvmem-cell binding and
the barebox bootloader binding that extends the fixed-partitions MTD
binding to EEPROMs[1]. It's also a building block for getting nvmem cells and
partitions in MTD devices to co-exist in the same device tree node[2].

The changes are backwards-compatible, because, per binding, a compatible
property in a cell node was so far invalid. More details in the commit
messages.

[1]: https://www.mail-archive.com/barebox@lists.infradead.org/msg33944.html
[2]: https://patchwork.ozlabs.org/patch/890741/

v2 -> v3:
  - use optional compatible property to weed out nodes instead of name
  - extend binding documentation (Srini)
v1 -> v2:
  - use ->full_name instead of ->name as to not break existing correct
    cells (Christian)

Cheers,
Ahmad Fatoum (2):
  dt-bindings: nvmem: skip nodes with compatibles other than
    "nvmem-cell"
  nvmem: core: skip nodes with compatibles other than "nvmem-cell"

 Documentation/devicetree/bindings/nvmem/nvmem.yaml | 14 +++++++++++++-
 drivers/nvmem/core.c                               |  4 ++++
 2 files changed, 17 insertions(+), 1 deletion(-)

Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/2] dt-bindings: nvmem: skip nodes with compatibles other than "nvmem-cell"
  2020-04-28 11:18 [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell" Ahmad Fatoum
@ 2020-04-28 11:18 ` Ahmad Fatoum
  2020-05-08  6:56   ` Ahmad Fatoum
  2020-04-28 11:18 ` [PATCH v3 2/2] nvmem: core: " Ahmad Fatoum
  2020-05-12 14:18 ` [PATCH v3 0/2] nvmem: " Rob Herring
  2 siblings, 1 reply; 13+ messages in thread
From: Ahmad Fatoum @ 2020-04-28 11:18 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: kernel, ceggers, Ahmad Fatoum, Rob Herring, devicetree, linux-kernel

For nodes matching the nvmem binding, all child objects matching
"^.*@[0-9a-f]+$" are assumed to be nvmem cells, without taking a
compatible into account.

This precludes:

  - future extension of e.g. eeprom nodes by any child nodes other
    than nvmem cells
  - extending the NVMEM binding to nodes that already have other
    child nodes, e.g., MTD and its partitions

To allow co-existence of nvmem-cells with other nodes, loosen the
binding to consult an optional compatible property for the cells:

  - if a compatible exists, it must be "nvmem-cell"
  - if none exists, it's assumed to be a nvmem cell, like before

As additionalProperties: false was specified for nvmem-cell bindings,
a compatible property was so far invalid. This means no already
compliant device tree should be reinterpreted differently after
this binding adjustment and in that regard, the change is completely
backwards-compatible.

This resolves an existing clash between this nvmem-cell binding and
the barebox bootloader binding that extends the fixed-partitions MTD
binding to EEPROMs.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 Documentation/devicetree/bindings/nvmem/nvmem.yaml | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index 65980224d550..c39f5dd7e1aa 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
@@ -44,9 +44,21 @@ properties:
 
 patternProperties:
   "^.*@[0-9a-f]+$":
-    type: object
+    if:
+      properties:
+        compatible:
+          items:
+            const: nvmem-cell
+    then:
+      $ref: "#/definitions/nvmem-cell"
 
+definitions:
+  nvmem-cell:
     properties:
+      compatible:
+          items:
+            const: nvmem-cell
+
       reg:
         maxItems: 1
         description:
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/2] nvmem: core: skip nodes with compatibles other than "nvmem-cell"
  2020-04-28 11:18 [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell" Ahmad Fatoum
  2020-04-28 11:18 ` [PATCH v3 1/2] dt-bindings: " Ahmad Fatoum
@ 2020-04-28 11:18 ` Ahmad Fatoum
  2020-05-11 11:33   ` Srinivas Kandagatla
  2020-12-30 13:43   ` Kamel Bouhara
  2020-05-12 14:18 ` [PATCH v3 0/2] nvmem: " Rob Herring
  2 siblings, 2 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2020-04-28 11:18 UTC (permalink / raw)
  To: Srinivas Kandagatla; +Cc: kernel, ceggers, Ahmad Fatoum, linux-kernel

The nvmem cell binding used to apply to all objects which match
"^.*@[0-9a-f]+$", without taking a compatible into account, which
precluded extension of EEPROMs by child nodes other than nvmem.

A previous commit changed the binding, so that nvmem cells that
feature a compatible property must have "nvmem-cell" as the value,
otherwise they are skipped.

Adjust the driver to observe the new binding change. This change
does not change behavior for any device tree that was already
compliant with the nvmem binding.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/nvmem/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 05c6ae4b0b97..eb697f5ad07d 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -547,6 +547,10 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
 	parent = dev->of_node;
 
 	for_each_child_of_node(parent, child) {
+		if (of_find_property(child, "compatible", NULL) &&
+		    !of_device_is_compatible(child, "nvmem-cell"))
+			continue;
+
 		addr = of_get_property(child, "reg", &len);
 		if (!addr || (len < 2 * sizeof(u32))) {
 			dev_err(dev, "nvmem: invalid reg on %pOF\n", child);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/2] dt-bindings: nvmem: skip nodes with compatibles other than "nvmem-cell"
  2020-04-28 11:18 ` [PATCH v3 1/2] dt-bindings: " Ahmad Fatoum
@ 2020-05-08  6:56   ` Ahmad Fatoum
  0 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2020-05-08  6:56 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: kernel, ceggers, Rob Herring, devicetree, linux-kernel

Hello Srini,
Hello Rob,

On 4/28/20 1:18 PM, Ahmad Fatoum wrote:
> To allow co-existence of nvmem-cells with other nodes, loosen the
> binding to consult an optional compatible property for the cells:

Gentle ping.

Cheers,
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/2] nvmem: core: skip nodes with compatibles other than "nvmem-cell"
  2020-04-28 11:18 ` [PATCH v3 2/2] nvmem: core: " Ahmad Fatoum
@ 2020-05-11 11:33   ` Srinivas Kandagatla
  2020-12-30 13:43   ` Kamel Bouhara
  1 sibling, 0 replies; 13+ messages in thread
From: Srinivas Kandagatla @ 2020-05-11 11:33 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: kernel, ceggers, linux-kernel



On 28/04/2020 12:18, Ahmad Fatoum wrote:
> The nvmem cell binding used to apply to all objects which match
> "^.*@[0-9a-f]+$", without taking a compatible into account, which
> precluded extension of EEPROMs by child nodes other than nvmem.
> 
> A previous commit changed the binding, so that nvmem cells that
> feature a compatible property must have "nvmem-cell" as the value,
> otherwise they are skipped.
> 
> Adjust the driver to observe the new binding change. This change
> does not change behavior for any device tree that was already
> compliant with the nvmem binding.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Thanks for the patch, this looks good to me, lets wait for Rob to ack 
the bindings!

--srini
> ---
>   drivers/nvmem/core.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 05c6ae4b0b97..eb697f5ad07d 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -547,6 +547,10 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
>   	parent = dev->of_node;
>   
>   	for_each_child_of_node(parent, child) {
> +		if (of_find_property(child, "compatible", NULL) &&
> +		    !of_device_is_compatible(child, "nvmem-cell"))
> +			continue;
> +
>   		addr = of_get_property(child, "reg", &len);
>   		if (!addr || (len < 2 * sizeof(u32))) {
>   			dev_err(dev, "nvmem: invalid reg on %pOF\n", child);
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell"
  2020-04-28 11:18 [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell" Ahmad Fatoum
  2020-04-28 11:18 ` [PATCH v3 1/2] dt-bindings: " Ahmad Fatoum
  2020-04-28 11:18 ` [PATCH v3 2/2] nvmem: core: " Ahmad Fatoum
@ 2020-05-12 14:18 ` Rob Herring
  2020-10-12 15:36   ` Ahmad Fatoum
  2 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2020-05-12 14:18 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: linux-kernel, kernel, ceggers, devicetree

On Tue, Apr 28, 2020 at 01:18:25PM +0200, Ahmad Fatoum wrote:
> The nvmem cell binding applies to all objects which match "^.*@[0-9a-f]+$",
> without taking a compatible into account. This precludes extension of e.g.
> eeprom nodes by any child nodes other than nvmem. Consider following example:
> 
> 	eeprom@0 {
> 		reg = <0 64>;
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 
> 		partitions {
> 			compatible = "fixed-partitions";
> 			#address-cells = <1>;
> 			#size-cells = <1>;
> 			bits = <64 64 64>; /* to verify it's skipped */
> 
> 			part@0 {
> 				reg = <0x00 16>;
> 			};
> 		};
> 
> 		no-cell@10 {
> 			compatible = "not-nvmem-cell";
> 			reg = <0x10 4>;
> 			bits = <64 64 64>; /* to verify it's skipped */
> 		};
> 
> 		cell-old@14 {
> 			reg = <0x14 0x2>;
> 		};
> 
> 		cell-new@16 {
> 			compatible = "nvmem-cell";
> 			reg = <0x16 4>;
> 		};
> 	};
> 
> Without this series, the NVMEM driver interprets all direct children of eeprom@0
> as NVMEM cells and driver probe fails, because the partitions node lacks a reg
> property, e.g.:
> 
>   nvmem 0-00000: nvmem: invalid reg on /eeprom@0
> 
> Running dtbs_check on the snippet will skip partitions (it doesn't match above
> regex), but will flag no-cell@10 and cell-new@16 as invalid.
> 
> With this series applied, the driver will skip partitions and no-cell@10,
> because they have a compatible but it's not "nvmem-cell".

Because you have to support no compatible (forever), there's no point 
adding this compatible.

> Both cell-old@14 and cell-new@16 will be interpreted as cells.
> 
> Likewise, running dtbs_check on the snippet will skip partitions (compatible
> doesn't match and regex doesn't either) and no-cell@10, but accept the other two.
> 
> This series resolves an existing clash between this nvmem-cell binding and
> the barebox bootloader binding that extends the fixed-partitions MTD
> binding to EEPROMs[1]. It's also a building block for getting nvmem cells and
> partitions in MTD devices to co-exist in the same device tree node[2].

This violates having multiple nodes at the same address because you are 
independently overlaying partitions and nvmem cells on same address 
ranges. It also seems seems pretty fragile if you want to update 
partitions.

I think instead, nvmem cells should be contained within a partition. 
The partition should then have a compatible to indicate it contains 
nvmem cells.

Rob

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell"
  2020-05-12 14:18 ` [PATCH v3 0/2] nvmem: " Rob Herring
@ 2020-10-12 15:36   ` Ahmad Fatoum
  2020-11-02 15:23     ` Ahmad Fatoum
  0 siblings, 1 reply; 13+ messages in thread
From: Ahmad Fatoum @ 2020-10-12 15:36 UTC (permalink / raw)
  To: Rob Herring, Srinivas Kandagatla
  Cc: linux-kernel, kernel, ceggers, devicetree

Hello Rob,
Hello Srini,

On 5/12/20 4:18 PM, Rob Herring wrote:
> On Tue, Apr 28, 2020 at 01:18:25PM +0200, Ahmad Fatoum wrote:
>> The nvmem cell binding applies to all objects which match "^.*@[0-9a-f]+$",
>> without taking a compatible into account. This precludes extension of e.g.
>> eeprom nodes by any child nodes other than nvmem. Consider following example:
>>
>> 	eeprom@0 {
>> 		reg = <0 64>;
>> 		#address-cells = <1>;
>> 		#size-cells = <1>;
>>
>> 		partitions {
>> 			compatible = "fixed-partitions";
>> 			#address-cells = <1>;
>> 			#size-cells = <1>;
>> 			bits = <64 64 64>; /* to verify it's skipped */
>>
>> 			part@0 {
>> 				reg = <0x00 16>;
>> 			};
>> 		};
>>
>> 		no-cell@10 {
>> 			compatible = "not-nvmem-cell";
>> 			reg = <0x10 4>;
>> 			bits = <64 64 64>; /* to verify it's skipped */
>> 		};
>>
>> 		cell-old@14 {
>> 			reg = <0x14 0x2>;
>> 		};
>>
>> 		cell-new@16 {
>> 			compatible = "nvmem-cell";
>> 			reg = <0x16 4>;
>> 		};
>> 	};
>>
>> Without this series, the NVMEM driver interprets all direct children of eeprom@0
>> as NVMEM cells and driver probe fails, because the partitions node lacks a reg
>> property, e.g.:
>>
>>   nvmem 0-00000: nvmem: invalid reg on /eeprom@0
>>
>> Running dtbs_check on the snippet will skip partitions (it doesn't match above
>> regex), but will flag no-cell@10 and cell-new@16 as invalid.
>>
>> With this series applied, the driver will skip partitions and no-cell@10,
>> because they have a compatible but it's not "nvmem-cell".
> 
> Because you have to support no compatible (forever), there's no point 
> adding this compatible.

IMO nvmem cells should have had a compatible since the beginning and I figured
better late than never.

>> Both cell-old@14 and cell-new@16 will be interpreted as cells.
>>
>> Likewise, running dtbs_check on the snippet will skip partitions (compatible
>> doesn't match and regex doesn't either) and no-cell@10, but accept the other two.
>>
>> This series resolves an existing clash between this nvmem-cell binding and
>> the barebox bootloader binding that extends the fixed-partitions MTD
>> binding to EEPROMs[1]. It's also a building block for getting nvmem cells and
>> partitions in MTD devices to co-exist in the same device tree node[2].
> 
> This violates having multiple nodes at the same address because you are 
> independently overlaying partitions and nvmem cells on same address 
> ranges. It also seems seems pretty fragile if you want to update 
> partitions.
> 
> I think instead, nvmem cells should be contained within a partition. 
> The partition should then have a compatible to indicate it contains 
> nvmem cells.

I thought I had understood what needs to be done, but now that I finally have time
to do it, I see that this only solves the second issue "extending the NVMEM binding
to nodes that already have other child nodes, e.g., MTD and its partitions".

The first issue: "future extension of e.g. eeprom nodes by any child nodes other than
nvmem cells" isn't solved by having a containing partition.


My issue is that the bootloader fixes up a partitions { compatible = "fixed-partitions"; }
child node into the kernel device tree. The NVMEM core driver tries to parse all eeprom child
nodes as cells and will make the driver probe of the EEPROM fail, because it can't parse that
fixed-partitions node as a nvmem cell.

To allow for co-existence of NVMEM cells and other subnodes, would following patch be
acceptable to you and Srini?

---------------------------------------- 8< --------------------------------------
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
@@ -45,7 +45,15 @@ properties:
 patternProperties:
   "^.*@[0-9a-f]+$":
     type: object
-
+    if:
+      not:
+        properties:
+          compatible:
+    then:
+      $ref: "#/definitions/nvmem-cell"
+
+definitions:
+  nvmem-cell:
     properties:
       reg:
         maxItems: 1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell"
  2020-10-12 15:36   ` Ahmad Fatoum
@ 2020-11-02 15:23     ` Ahmad Fatoum
  2020-11-16 17:04       ` Ahmad Fatoum
  0 siblings, 1 reply; 13+ messages in thread
From: Ahmad Fatoum @ 2020-11-02 15:23 UTC (permalink / raw)
  To: Rob Herring, Srinivas Kandagatla
  Cc: devicetree, ceggers, linux-kernel, kernel

Hello Rob,
Hello Srini,

On 10/12/20 5:36 PM, Ahmad Fatoum wrote:
> On 5/12/20 4:18 PM, Rob Herring wrote:

[snip]

>> I think instead, nvmem cells should be contained within a partition. 
>> The partition should then have a compatible to indicate it contains 
>> nvmem cells.
> 
> I thought I had understood what needs to be done, but now that I finally have time
> to do it, I see that this only solves the second issue "extending the NVMEM binding
> to nodes that already have other child nodes, e.g., MTD and its partitions".
> 
> The first issue: "future extension of e.g. eeprom nodes by any child nodes other than
> nvmem cells" isn't solved by having a containing partition.
> 
> 
> My issue is that the bootloader fixes up a partitions { compatible = "fixed-partitions"; }
> child node into the kernel device tree. The NVMEM core driver tries to parse all eeprom child
> nodes as cells and will make the driver probe of the EEPROM fail, because it can't parse that
> fixed-partitions node as a nvmem cell.
> 
> To allow for co-existence of NVMEM cells and other subnodes, would following patch be
> acceptable to you and Srini?

Gentle ping. Would the patch below be acceptable?

> 
> ---------------------------------------- 8< --------------------------------------
> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> @@ -45,7 +45,15 @@ properties:
>  patternProperties:
>    "^.*@[0-9a-f]+$":
>      type: object
> -
> +    if:
> +      not:
> +        properties:
> +          compatible:
> +    then:
> +      $ref: "#/definitions/nvmem-cell"
> +
> +definitions:
> +  nvmem-cell:
>      properties:
>        reg:
>          maxItems: 1

Cheers,
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell"
  2020-11-02 15:23     ` Ahmad Fatoum
@ 2020-11-16 17:04       ` Ahmad Fatoum
  2020-11-16 17:21         ` Srinivas Kandagatla
  0 siblings, 1 reply; 13+ messages in thread
From: Ahmad Fatoum @ 2020-11-16 17:04 UTC (permalink / raw)
  To: Rob Herring, Srinivas Kandagatla
  Cc: devicetree, ceggers, linux-kernel, kernel

Hello Rob,
Hello Srini,

On 02.11.20 16:23, Ahmad Fatoum wrote:
>>> I think instead, nvmem cells should be contained within a partition. 
>>> The partition should then have a compatible to indicate it contains 
>>> nvmem cells.
>>
>> I thought I had understood what needs to be done, but now that I finally have time
>> to do it, I see that this only solves the second issue "extending the NVMEM binding
>> to nodes that already have other child nodes, e.g., MTD and its partitions".
>>
>> The first issue: "future extension of e.g. eeprom nodes by any child nodes other than
>> nvmem cells" isn't solved by having a containing partition.
>>
>>
>> My issue is that the bootloader fixes up a partitions { compatible = "fixed-partitions"; }
>> child node into the kernel device tree. The NVMEM core driver tries to parse all eeprom child
>> nodes as cells and will make the driver probe of the EEPROM fail, because it can't parse that
>> fixed-partitions node as a nvmem cell.
>>
>> To allow for co-existence of NVMEM cells and other subnodes, would following patch be
>> acceptable to you and Srini?
> 
> Gentle ping. Would the patch below be acceptable?

Did you have time to look at this?

> 
>>
>> ---------------------------------------- 8< --------------------------------------
>> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>> @@ -45,7 +45,15 @@ properties:
>>  patternProperties:
>>    "^.*@[0-9a-f]+$":
>>      type: object
>> -
>> +    if:
>> +      not:
>> +        properties:
>> +          compatible:
>> +    then:
>> +      $ref: "#/definitions/nvmem-cell"
>> +
>> +definitions:
>> +  nvmem-cell:
>>      properties:
>>        reg:
>>          maxItems: 1
> 
> Cheers,
> Ahmad
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell"
  2020-11-16 17:04       ` Ahmad Fatoum
@ 2020-11-16 17:21         ` Srinivas Kandagatla
  2020-11-16 17:28           ` Ahmad Fatoum
  0 siblings, 1 reply; 13+ messages in thread
From: Srinivas Kandagatla @ 2020-11-16 17:21 UTC (permalink / raw)
  To: Ahmad Fatoum, Rob Herring; +Cc: devicetree, ceggers, linux-kernel, kernel



On 16/11/2020 17:04, Ahmad Fatoum wrote:
>>> To allow for co-existence of NVMEM cells and other subnodes, would following patch be
>>> acceptable to you and Srini?
>> Gentle ping. Would the patch below be acceptable?
> Did you have time to look at this?
> 

I did reply back to this thread way back in June saying that

"Thanks for the patch, this looks good to me, lets wait for Rob to ack 
the bindings! "


--srini

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell"
  2020-11-16 17:21         ` Srinivas Kandagatla
@ 2020-11-16 17:28           ` Ahmad Fatoum
  0 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2020-11-16 17:28 UTC (permalink / raw)
  To: Srinivas Kandagatla, Rob Herring
  Cc: devicetree, ceggers, linux-kernel, kernel

Hello Srini,

On 16.11.20 18:21, Srinivas Kandagatla wrote:
> 
> 
> On 16/11/2020 17:04, Ahmad Fatoum wrote:
>>>> To allow for co-existence of NVMEM cells and other subnodes, would following patch be
>>>> acceptable to you and Srini?
>>> Gentle ping. Would the patch below be acceptable?
>> Did you have time to look at this?
>>
> 
> I did reply back to this thread way back in June saying that
> 
> "Thanks for the patch, this looks good to me, lets wait for Rob to ack the bindings! "

Rob replied a day later saying that he would prefer it done otherwise.
What Rob suggested doesn't solve my actual issue, so I suggested another
solution a month ago. I have not yet received feedback on it, so that's
why I pinged:

https://lore.kernel.org/lkml/f03ecee7-c4b6-7a59-7ab8-42c5dfcaffc4@pengutronix.de/

> 
> 
> --srini
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/2] nvmem: core: skip nodes with compatibles other than "nvmem-cell"
  2020-04-28 11:18 ` [PATCH v3 2/2] nvmem: core: " Ahmad Fatoum
  2020-05-11 11:33   ` Srinivas Kandagatla
@ 2020-12-30 13:43   ` Kamel Bouhara
  2021-01-18 13:25     ` Ahmad Fatoum
  1 sibling, 1 reply; 13+ messages in thread
From: Kamel Bouhara @ 2020-12-30 13:43 UTC (permalink / raw)
  To: Ahmad Fatoum, Srinivas Kandagatla; +Cc: kernel, ceggers, linux-kernel

Hi Ahmad,

On 4/28/20 1:18 PM, Ahmad Fatoum wrote:
> The nvmem cell binding used to apply to all objects which match
> "^.*@[0-9a-f]+$", without taking a compatible into account, which
> precluded extension of EEPROMs by child nodes other than nvmem.
> 
> A previous commit changed the binding, so that nvmem cells that
> feature a compatible property must have "nvmem-cell" as the value,
> otherwise they are skipped.
> 
> Adjust the driver to observe the new binding change. This change
> does not change behavior for any device tree that was already
> compliant with the nvmem binding.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Tested-by: Kamel Bouhara <kamel.bouhara@bootlin.com>

> ---
>   drivers/nvmem/core.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 05c6ae4b0b97..eb697f5ad07d 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -547,6 +547,10 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
>   	parent = dev->of_node;
>   
>   	for_each_child_of_node(parent, child) {
> +		if (of_find_property(child, "compatible", NULL) &&
> +		    !of_device_is_compatible(child, "nvmem-cell"))
> +			continue;
> +
>   		addr = of_get_property(child, "reg", &len);
>   		if (!addr || (len < 2 * sizeof(u32))) {
>   			dev_err(dev, "nvmem: invalid reg on %pOF\n", child);
> 

-- 
Kamel Bouhara, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/2] nvmem: core: skip nodes with compatibles other than "nvmem-cell"
  2020-12-30 13:43   ` Kamel Bouhara
@ 2021-01-18 13:25     ` Ahmad Fatoum
  0 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2021-01-18 13:25 UTC (permalink / raw)
  To: Kamel Bouhara, Srinivas Kandagatla; +Cc: kernel, ceggers, linux-kernel

Hello Kamel,

On 30.12.20 14:43, Kamel Bouhara wrote:
> Hi Ahmad,
> 
> On 4/28/20 1:18 PM, Ahmad Fatoum wrote:
>> The nvmem cell binding used to apply to all objects which match
>> "^.*@[0-9a-f]+$", without taking a compatible into account, which
>> precluded extension of EEPROMs by child nodes other than nvmem.
>>
>> A previous commit changed the binding, so that nvmem cells that
>> feature a compatible property must have "nvmem-cell" as the value,
>> otherwise they are skipped.
>>
>> Adjust the driver to observe the new binding change. This change
>> does not change behavior for any device tree that was already
>> compliant with the nvmem binding.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> Tested-by: Kamel Bouhara <kamel.bouhara@bootlin.com>

Thanks for testing. Could you share what use case you have for this?

Rob had objections to this approach:
https://lore.kernel.org/linux-devicetree/20200512141834.GA3023@bogus/

But solving it within a partition doesn't address my original problem:
https://lore.kernel.org/lkml/f03ecee7-c4b6-7a59-7ab8-42c5dfcaffc4@pengutronix.de/

I just sent out a v4, which added you in CC.

Cheers,
Ahmad

> 
>> ---
>>   drivers/nvmem/core.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>> index 05c6ae4b0b97..eb697f5ad07d 100644
>> --- a/drivers/nvmem/core.c
>> +++ b/drivers/nvmem/core.c
>> @@ -547,6 +547,10 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
>>       parent = dev->of_node;
>>         for_each_child_of_node(parent, child) {
>> +        if (of_find_property(child, "compatible", NULL) &&
>> +            !of_device_is_compatible(child, "nvmem-cell"))
>> +            continue;
>> +
>>           addr = of_get_property(child, "reg", &len);
>>           if (!addr || (len < 2 * sizeof(u32))) {
>>               dev_err(dev, "nvmem: invalid reg on %pOF\n", child);
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-01-18 13:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-28 11:18 [PATCH v3 0/2] nvmem: skip nodes with compatibles other than "nvmem-cell" Ahmad Fatoum
2020-04-28 11:18 ` [PATCH v3 1/2] dt-bindings: " Ahmad Fatoum
2020-05-08  6:56   ` Ahmad Fatoum
2020-04-28 11:18 ` [PATCH v3 2/2] nvmem: core: " Ahmad Fatoum
2020-05-11 11:33   ` Srinivas Kandagatla
2020-12-30 13:43   ` Kamel Bouhara
2021-01-18 13:25     ` Ahmad Fatoum
2020-05-12 14:18 ` [PATCH v3 0/2] nvmem: " Rob Herring
2020-10-12 15:36   ` Ahmad Fatoum
2020-11-02 15:23     ` Ahmad Fatoum
2020-11-16 17:04       ` Ahmad Fatoum
2020-11-16 17:21         ` Srinivas Kandagatla
2020-11-16 17:28           ` Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).