All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Actually implement nvmem support for mtd
@ 2020-09-19 22:30 ` Ansuel Smith
  0 siblings, 0 replies; 18+ messages in thread
From: Ansuel Smith @ 2020-09-19 22:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Ansuel Smith, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, David S. Miller, Jakub Kicinski, Andrew Lunn,
	Heiner Kallweit, Russell King, Frank Rowand, Boris Brezillon,
	linux-mtd, devicetree, linux-kernel, netdev

The mtd support for the nvmem api has been stalled from 2018 with a patch
half pushed hoping that a scheme is found for the mtd name later. This
pathset try to address this and add a very needed feature for the
mac-address.

My solution to the already discussed problem here [1] is to keep it simple.
A mtd subpartition can be set as a nvmem-provider with a specific tag and
each direct subnode is treat by the nvmem api as a nvmem-cell and gets
correctly registred. The mtd driver will treat a direct subnode of the
subpartition as a legacy subpartition of the subpartition using the
'fixed-partition' parser. To fix this every nvmem-cell has to have the
'nvmem-cell' tag and the fixed-partition parser will skip these node if 
this tag is detected. Simple as that. The subpartition with the tag 
'nvmem-provider' will then be set by the config to not skip the of
registration in the config and the nvmem-cells are correctly registred
and can be used to set mac-address of the devices on the system.

The last 2 patch try to address a problem in the embedded devices (mostly
routers) that have the mac-address saved in a dedicated partition and is
a ""master"" mac-address. Each device increment or decrement the extracted
mac-address based on the device number. The increment function is
implemented in the of_net function to get the mac-address that every net
driver should allready use if they don't have a trusty mac-address source.
(One example of this implementation are many ath10k device that gets the
mac-address from the art mtd partition assigned to the network driver and
increments it 1 for the wifi 2.4ghz and 2 for the wifi 5ghz).

I really hope my mtd nvmem implementation doesn't conflicts with others
and can be used, since this would remove many patch used to get mac-address
and other nvmem data.

[1] https://lore.kernel.org/patchwork/patch/765435/

Changes:
v2:
* Fix compile error (missing mtd_node in mtdcore)

Ansuel Smith (4):
  mtd: Add nvmem support for mtd nvmem-providers
  dt-bindings: mtd: partition: Document use of nvmem-provider
  of_net: add mac-address-increment support
  dt-bindings: net: Document use of mac-address-increment

 .../devicetree/bindings/mtd/partition.txt     | 59 +++++++++++++++++++
 .../bindings/net/ethernet-controller.yaml     | 19 ++++++
 drivers/mtd/mtdcore.c                         |  3 +-
 drivers/mtd/parsers/ofpart.c                  |  8 +++
 drivers/of/of_net.c                           | 53 +++++++++++++----
 5 files changed, 129 insertions(+), 13 deletions(-)

-- 
2.27.0


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

* [PATCH v2 0/4] Actually implement nvmem support for mtd
@ 2020-09-19 22:30 ` Ansuel Smith
  0 siblings, 0 replies; 18+ messages in thread
From: Ansuel Smith @ 2020-09-19 22:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Andrew Lunn, Vignesh Raghavendra, Boris Brezillon,
	Richard Weinberger, Russell King, Ansuel Smith, devicetree,
	Rob Herring, linux-mtd, netdev, Jakub Kicinski, Frank Rowand,
	David S. Miller, linux-kernel, Heiner Kallweit

The mtd support for the nvmem api has been stalled from 2018 with a patch
half pushed hoping that a scheme is found for the mtd name later. This
pathset try to address this and add a very needed feature for the
mac-address.

My solution to the already discussed problem here [1] is to keep it simple.
A mtd subpartition can be set as a nvmem-provider with a specific tag and
each direct subnode is treat by the nvmem api as a nvmem-cell and gets
correctly registred. The mtd driver will treat a direct subnode of the
subpartition as a legacy subpartition of the subpartition using the
'fixed-partition' parser. To fix this every nvmem-cell has to have the
'nvmem-cell' tag and the fixed-partition parser will skip these node if 
this tag is detected. Simple as that. The subpartition with the tag 
'nvmem-provider' will then be set by the config to not skip the of
registration in the config and the nvmem-cells are correctly registred
and can be used to set mac-address of the devices on the system.

The last 2 patch try to address a problem in the embedded devices (mostly
routers) that have the mac-address saved in a dedicated partition and is
a ""master"" mac-address. Each device increment or decrement the extracted
mac-address based on the device number. The increment function is
implemented in the of_net function to get the mac-address that every net
driver should allready use if they don't have a trusty mac-address source.
(One example of this implementation are many ath10k device that gets the
mac-address from the art mtd partition assigned to the network driver and
increments it 1 for the wifi 2.4ghz and 2 for the wifi 5ghz).

I really hope my mtd nvmem implementation doesn't conflicts with others
and can be used, since this would remove many patch used to get mac-address
and other nvmem data.

[1] https://lore.kernel.org/patchwork/patch/765435/

Changes:
v2:
* Fix compile error (missing mtd_node in mtdcore)

Ansuel Smith (4):
  mtd: Add nvmem support for mtd nvmem-providers
  dt-bindings: mtd: partition: Document use of nvmem-provider
  of_net: add mac-address-increment support
  dt-bindings: net: Document use of mac-address-increment

 .../devicetree/bindings/mtd/partition.txt     | 59 +++++++++++++++++++
 .../bindings/net/ethernet-controller.yaml     | 19 ++++++
 drivers/mtd/mtdcore.c                         |  3 +-
 drivers/mtd/parsers/ofpart.c                  |  8 +++
 drivers/of/of_net.c                           | 53 +++++++++++++----
 5 files changed, 129 insertions(+), 13 deletions(-)

-- 
2.27.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 1/4] mtd: Add nvmem support for mtd nvmem-providers
  2020-09-19 22:30 ` Ansuel Smith
@ 2020-09-19 22:30   ` Ansuel Smith
  -1 siblings, 0 replies; 18+ messages in thread
From: Ansuel Smith @ 2020-09-19 22:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Ansuel Smith, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, David S. Miller, Jakub Kicinski, Andrew Lunn,
	Heiner Kallweit, Russell King, Frank Rowand, Boris Brezillon,
	linux-mtd, devicetree, linux-kernel, netdev

Introduce 2 new bindings for the mtd structure.
Mtd partitions can be set as 'nvmem-provider' and any subpartition defined
with the tag 'nvmem-cell' are skipped by the 'fixed-partitions' parser
and registred as a nvmem cell by the nvmem api.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/mtd/mtdcore.c        | 3 ++-
 drivers/mtd/parsers/ofpart.c | 8 ++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 7d930569a7df..ba5236db8318 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -551,6 +551,7 @@ static int mtd_nvmem_reg_read(void *priv, unsigned int offset,
 
 static int mtd_nvmem_add(struct mtd_info *mtd)
 {
+	struct device_node *mtd_node = mtd_get_of_node(mtd);
 	struct nvmem_config config = {};
 
 	config.id = -1;
@@ -563,7 +564,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
 	config.stride = 1;
 	config.read_only = true;
 	config.root_only = true;
-	config.no_of_node = true;
+	config.no_of_node = !of_property_read_bool(mtd_node, "nvmem-provider");
 	config.priv = mtd;
 
 	mtd->nvmem = nvmem_register(&config);
diff --git a/drivers/mtd/parsers/ofpart.c b/drivers/mtd/parsers/ofpart.c
index daf507c123e6..442e039214bc 100644
--- a/drivers/mtd/parsers/ofpart.c
+++ b/drivers/mtd/parsers/ofpart.c
@@ -61,6 +61,10 @@ static int parse_fixed_partitions(struct mtd_info *master,
 		if (!dedicated && node_has_compatible(pp))
 			continue;
 
+		/* skip adding if a nvmem-cell is detected */
+		if (of_property_read_bool(pp, "nvmem-cell"))
+			continue;
+
 		nr_parts++;
 	}
 
@@ -80,6 +84,10 @@ static int parse_fixed_partitions(struct mtd_info *master,
 		if (!dedicated && node_has_compatible(pp))
 			continue;
 
+		/* skip adding if a nvmem-cell is detected */
+		if (of_property_read_bool(pp, "nvmem-cell"))
+			continue;
+
 		reg = of_get_property(pp, "reg", &len);
 		if (!reg) {
 			if (dedicated) {
-- 
2.27.0


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

* [PATCH v2 1/4] mtd: Add nvmem support for mtd nvmem-providers
@ 2020-09-19 22:30   ` Ansuel Smith
  0 siblings, 0 replies; 18+ messages in thread
From: Ansuel Smith @ 2020-09-19 22:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Andrew Lunn, Vignesh Raghavendra, Boris Brezillon,
	Richard Weinberger, Russell King, Ansuel Smith, devicetree,
	Rob Herring, linux-mtd, netdev, Jakub Kicinski, Frank Rowand,
	David S. Miller, linux-kernel, Heiner Kallweit

Introduce 2 new bindings for the mtd structure.
Mtd partitions can be set as 'nvmem-provider' and any subpartition defined
with the tag 'nvmem-cell' are skipped by the 'fixed-partitions' parser
and registred as a nvmem cell by the nvmem api.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/mtd/mtdcore.c        | 3 ++-
 drivers/mtd/parsers/ofpart.c | 8 ++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 7d930569a7df..ba5236db8318 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -551,6 +551,7 @@ static int mtd_nvmem_reg_read(void *priv, unsigned int offset,
 
 static int mtd_nvmem_add(struct mtd_info *mtd)
 {
+	struct device_node *mtd_node = mtd_get_of_node(mtd);
 	struct nvmem_config config = {};
 
 	config.id = -1;
@@ -563,7 +564,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
 	config.stride = 1;
 	config.read_only = true;
 	config.root_only = true;
-	config.no_of_node = true;
+	config.no_of_node = !of_property_read_bool(mtd_node, "nvmem-provider");
 	config.priv = mtd;
 
 	mtd->nvmem = nvmem_register(&config);
diff --git a/drivers/mtd/parsers/ofpart.c b/drivers/mtd/parsers/ofpart.c
index daf507c123e6..442e039214bc 100644
--- a/drivers/mtd/parsers/ofpart.c
+++ b/drivers/mtd/parsers/ofpart.c
@@ -61,6 +61,10 @@ static int parse_fixed_partitions(struct mtd_info *master,
 		if (!dedicated && node_has_compatible(pp))
 			continue;
 
+		/* skip adding if a nvmem-cell is detected */
+		if (of_property_read_bool(pp, "nvmem-cell"))
+			continue;
+
 		nr_parts++;
 	}
 
@@ -80,6 +84,10 @@ static int parse_fixed_partitions(struct mtd_info *master,
 		if (!dedicated && node_has_compatible(pp))
 			continue;
 
+		/* skip adding if a nvmem-cell is detected */
+		if (of_property_read_bool(pp, "nvmem-cell"))
+			continue;
+
 		reg = of_get_property(pp, "reg", &len);
 		if (!reg) {
 			if (dedicated) {
-- 
2.27.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 2/4] dt-bindings: mtd: partition: Document use of nvmem-provider
  2020-09-19 22:30 ` Ansuel Smith
@ 2020-09-19 22:30   ` Ansuel Smith
  -1 siblings, 0 replies; 18+ messages in thread
From: Ansuel Smith @ 2020-09-19 22:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Ansuel Smith, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, David S. Miller, Jakub Kicinski, Andrew Lunn,
	Heiner Kallweit, Russell King, Frank Rowand, Boris Brezillon,
	linux-mtd, devicetree, linux-kernel, netdev

Document the use of this 2 new bindings, nvmem-provider and nvmem-cell,
used to describe the nvmem cell that the subpartition provide to the
nvmem api and the system. Nvmem cell are direct subnode of the
subpartition and are skipped by the 'fixed-partitions' parser if they
contain the 'nvmem-cell' tag. The subpartition must have the
'nvmem-provider' tag or the subpartition will not register the cell to
the nvmem api.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../devicetree/bindings/mtd/partition.txt     | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 4a39698221a2..66d3a3f0a021 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -64,6 +64,16 @@ Optional properties:
 - slc-mode: This parameter, if present, allows one to emulate SLC mode on a
   partition attached to an MLC NAND thus making this partition immune to
   paired-pages corruptions
+- nvmem-provider : Optionally a subpartition can be set as a nvmem-provider. This can
+  be very useful if some data like the mac-address is stored in a special partition
+  at a specific offset. Subpartition that describe nvmem-cell must have set the
+  'nvmem-cell' of they will be treated as a subpartition and not skipped and registred
+  as nvmem cells. In this specific case '#address-cells' and '#size-cells' must be
+  provided.
+- nvmem-cell : A direct subnode of a subpartition can be described as a nvmem-cell and
+  skipped by the fixed-partition parser and registred as a nvmem-cell of the registred
+  nvmem subpartition IF it does contain the 'nvmem-provider tag. If the subpartition
+  lacks of such tag the subnode will be skipped and the nvmem api won't register them.
 
 Examples:
 
@@ -158,3 +168,52 @@ flash@3 {
 		};
 	};
 };
+
+flash@0 {
+	partitions {
+		compatible = "fixed-partitions";
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		partition@0 {
+			label = "u-boot";
+			reg = <0x0000000 0x100000>;
+			read-only;
+		};
+
+		art: art@1200000 {
+			label = "art";
+			reg = <0x1200000 0x0140000>;
+			read-only;
+			nvmem-provider;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			macaddr_gmac1: macaddr_gmac1@0 {
+				nvmem-cell;
+				reg = <0x0 0x6>;
+			};
+
+			macaddr_gmac2: macaddr_gmac2@6 {
+				nvmem-cell;
+				reg = <0x6 0x6>;
+			};
+
+			macaddr_wifi: macaddr_wifi@6 {
+				nvmem-cell;
+				reg = <0x6 0x6>;
+			};
+
+			pre_cal_24g: pre_cal_24g@1000 {
+				nvmem-cell;
+				reg = <0x1000 0x2f20>;
+			};
+
+			pre_cal_5g: pre_cal_5g@5000{
+				nvmem-cell;
+				reg = <0x5000 0x2f20>;
+			};
+		};
+	};
+};
\ No newline at end of file
-- 
2.27.0


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

* [PATCH v2 2/4] dt-bindings: mtd: partition: Document use of nvmem-provider
@ 2020-09-19 22:30   ` Ansuel Smith
  0 siblings, 0 replies; 18+ messages in thread
From: Ansuel Smith @ 2020-09-19 22:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Andrew Lunn, Vignesh Raghavendra, Boris Brezillon,
	Richard Weinberger, Russell King, Ansuel Smith, devicetree,
	Rob Herring, linux-mtd, netdev, Jakub Kicinski, Frank Rowand,
	David S. Miller, linux-kernel, Heiner Kallweit

Document the use of this 2 new bindings, nvmem-provider and nvmem-cell,
used to describe the nvmem cell that the subpartition provide to the
nvmem api and the system. Nvmem cell are direct subnode of the
subpartition and are skipped by the 'fixed-partitions' parser if they
contain the 'nvmem-cell' tag. The subpartition must have the
'nvmem-provider' tag or the subpartition will not register the cell to
the nvmem api.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../devicetree/bindings/mtd/partition.txt     | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 4a39698221a2..66d3a3f0a021 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -64,6 +64,16 @@ Optional properties:
 - slc-mode: This parameter, if present, allows one to emulate SLC mode on a
   partition attached to an MLC NAND thus making this partition immune to
   paired-pages corruptions
+- nvmem-provider : Optionally a subpartition can be set as a nvmem-provider. This can
+  be very useful if some data like the mac-address is stored in a special partition
+  at a specific offset. Subpartition that describe nvmem-cell must have set the
+  'nvmem-cell' of they will be treated as a subpartition and not skipped and registred
+  as nvmem cells. In this specific case '#address-cells' and '#size-cells' must be
+  provided.
+- nvmem-cell : A direct subnode of a subpartition can be described as a nvmem-cell and
+  skipped by the fixed-partition parser and registred as a nvmem-cell of the registred
+  nvmem subpartition IF it does contain the 'nvmem-provider tag. If the subpartition
+  lacks of such tag the subnode will be skipped and the nvmem api won't register them.
 
 Examples:
 
@@ -158,3 +168,52 @@ flash@3 {
 		};
 	};
 };
+
+flash@0 {
+	partitions {
+		compatible = "fixed-partitions";
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		partition@0 {
+			label = "u-boot";
+			reg = <0x0000000 0x100000>;
+			read-only;
+		};
+
+		art: art@1200000 {
+			label = "art";
+			reg = <0x1200000 0x0140000>;
+			read-only;
+			nvmem-provider;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			macaddr_gmac1: macaddr_gmac1@0 {
+				nvmem-cell;
+				reg = <0x0 0x6>;
+			};
+
+			macaddr_gmac2: macaddr_gmac2@6 {
+				nvmem-cell;
+				reg = <0x6 0x6>;
+			};
+
+			macaddr_wifi: macaddr_wifi@6 {
+				nvmem-cell;
+				reg = <0x6 0x6>;
+			};
+
+			pre_cal_24g: pre_cal_24g@1000 {
+				nvmem-cell;
+				reg = <0x1000 0x2f20>;
+			};
+
+			pre_cal_5g: pre_cal_5g@5000{
+				nvmem-cell;
+				reg = <0x5000 0x2f20>;
+			};
+		};
+	};
+};
\ No newline at end of file
-- 
2.27.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 3/4] of_net: add mac-address-increment support
  2020-09-19 22:30 ` Ansuel Smith
@ 2020-09-19 22:30   ` Ansuel Smith
  -1 siblings, 0 replies; 18+ messages in thread
From: Ansuel Smith @ 2020-09-19 22:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Ansuel Smith, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, David S. Miller, Jakub Kicinski, Andrew Lunn,
	Heiner Kallweit, Russell King, Frank Rowand, Boris Brezillon,
	linux-mtd, devicetree, linux-kernel, netdev

Lots of embedded devices use the mac-address of other interface
extracted from nvmem cells and increments it by one or two. Add two
bindings to integrate this and directly use the right mac-address for
the interface. Some example are some routers that use the gmac
mac-address stored in the art partition and increments it by one for the
wifi. mac-address-increment-byte bindings is used to tell what byte of
the mac-address has to be increased (if not defined the last byte is
increased) and mac-address-increment tells how much the byte decided
early has to be increased.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/of/of_net.c | 53 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index 6e411821583e..171f5ea6f371 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -45,7 +45,7 @@ int of_get_phy_mode(struct device_node *np, phy_interface_t *interface)
 }
 EXPORT_SYMBOL_GPL(of_get_phy_mode);
 
-static const void *of_get_mac_addr(struct device_node *np, const char *name)
+static void *of_get_mac_addr(struct device_node *np, const char *name)
 {
 	struct property *pp = of_find_property(np, name, NULL);
 
@@ -54,26 +54,31 @@ static const void *of_get_mac_addr(struct device_node *np, const char *name)
 	return NULL;
 }
 
-static const void *of_get_mac_addr_nvmem(struct device_node *np)
+static void *of_get_mac_addr_nvmem(struct device_node *np, int *err)
 {
 	int ret;
 	const void *mac;
 	u8 nvmem_mac[ETH_ALEN];
 	struct platform_device *pdev = of_find_device_by_node(np);
 
-	if (!pdev)
-		return ERR_PTR(-ENODEV);
+	if (!pdev) {
+		*err = -ENODEV;
+		return NULL;
+	}
 
 	ret = nvmem_get_mac_address(&pdev->dev, &nvmem_mac);
 	if (ret) {
 		put_device(&pdev->dev);
-		return ERR_PTR(ret);
+		*err = ret;
+		return NULL;
 	}
 
 	mac = devm_kmemdup(&pdev->dev, nvmem_mac, ETH_ALEN, GFP_KERNEL);
 	put_device(&pdev->dev);
-	if (!mac)
-		return ERR_PTR(-ENOMEM);
+	if (!mac) {
+		*err = -ENOMEM;
+		return NULL;
+	}
 
 	return mac;
 }
@@ -98,24 +103,48 @@ static const void *of_get_mac_addr_nvmem(struct device_node *np)
  * this case, the real MAC is in 'local-mac-address', and 'mac-address' exists
  * but is all zeros.
  *
+ * DT can tell the system to increment the mac-address after is extracted by
+ * using:
+ * - mac-address-increment-byte to decide what byte to increase
+ *   (if not defined is increased the last byte)
+ * - mac-address-increment to decide how much to increase
+ *
  * Return: Will be a valid pointer on success and ERR_PTR in case of error.
 */
 const void *of_get_mac_address(struct device_node *np)
 {
-	const void *addr;
+	u32 inc_idx, mac_inc;
+	int ret = 0;
+	u8 *addr;
+
+	/* Check first if the increment byte is present and valid.
+	 * If not set assume to increment the last byte if found.
+	 */
+	if (of_property_read_u32(np, "mac-address-increment-byte", &inc_idx))
+		inc_idx = 5;
+	if (inc_idx > 5)
+		return ERR_PTR(-EINVAL);
 
 	addr = of_get_mac_addr(np, "mac-address");
 	if (addr)
-		return addr;
+		goto found;
 
 	addr = of_get_mac_addr(np, "local-mac-address");
 	if (addr)
-		return addr;
+		goto found;
 
 	addr = of_get_mac_addr(np, "address");
 	if (addr)
-		return addr;
+		goto found;
+
+	addr = of_get_mac_addr_nvmem(np, &ret);
+	if (ret)
+		return ERR_PTR(ret);
+
+found:
+	if (!of_property_read_u32(np, "mac-address-increment", &mac_inc))
+		addr[inc_idx] += mac_inc;
 
-	return of_get_mac_addr_nvmem(np);
+	return addr;
 }
 EXPORT_SYMBOL(of_get_mac_address);
-- 
2.27.0


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

* [PATCH v2 3/4] of_net: add mac-address-increment support
@ 2020-09-19 22:30   ` Ansuel Smith
  0 siblings, 0 replies; 18+ messages in thread
From: Ansuel Smith @ 2020-09-19 22:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Andrew Lunn, Vignesh Raghavendra, Boris Brezillon,
	Richard Weinberger, Russell King, Ansuel Smith, devicetree,
	Rob Herring, linux-mtd, netdev, Jakub Kicinski, Frank Rowand,
	David S. Miller, linux-kernel, Heiner Kallweit

Lots of embedded devices use the mac-address of other interface
extracted from nvmem cells and increments it by one or two. Add two
bindings to integrate this and directly use the right mac-address for
the interface. Some example are some routers that use the gmac
mac-address stored in the art partition and increments it by one for the
wifi. mac-address-increment-byte bindings is used to tell what byte of
the mac-address has to be increased (if not defined the last byte is
increased) and mac-address-increment tells how much the byte decided
early has to be increased.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/of/of_net.c | 53 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index 6e411821583e..171f5ea6f371 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -45,7 +45,7 @@ int of_get_phy_mode(struct device_node *np, phy_interface_t *interface)
 }
 EXPORT_SYMBOL_GPL(of_get_phy_mode);
 
-static const void *of_get_mac_addr(struct device_node *np, const char *name)
+static void *of_get_mac_addr(struct device_node *np, const char *name)
 {
 	struct property *pp = of_find_property(np, name, NULL);
 
@@ -54,26 +54,31 @@ static const void *of_get_mac_addr(struct device_node *np, const char *name)
 	return NULL;
 }
 
-static const void *of_get_mac_addr_nvmem(struct device_node *np)
+static void *of_get_mac_addr_nvmem(struct device_node *np, int *err)
 {
 	int ret;
 	const void *mac;
 	u8 nvmem_mac[ETH_ALEN];
 	struct platform_device *pdev = of_find_device_by_node(np);
 
-	if (!pdev)
-		return ERR_PTR(-ENODEV);
+	if (!pdev) {
+		*err = -ENODEV;
+		return NULL;
+	}
 
 	ret = nvmem_get_mac_address(&pdev->dev, &nvmem_mac);
 	if (ret) {
 		put_device(&pdev->dev);
-		return ERR_PTR(ret);
+		*err = ret;
+		return NULL;
 	}
 
 	mac = devm_kmemdup(&pdev->dev, nvmem_mac, ETH_ALEN, GFP_KERNEL);
 	put_device(&pdev->dev);
-	if (!mac)
-		return ERR_PTR(-ENOMEM);
+	if (!mac) {
+		*err = -ENOMEM;
+		return NULL;
+	}
 
 	return mac;
 }
@@ -98,24 +103,48 @@ static const void *of_get_mac_addr_nvmem(struct device_node *np)
  * this case, the real MAC is in 'local-mac-address', and 'mac-address' exists
  * but is all zeros.
  *
+ * DT can tell the system to increment the mac-address after is extracted by
+ * using:
+ * - mac-address-increment-byte to decide what byte to increase
+ *   (if not defined is increased the last byte)
+ * - mac-address-increment to decide how much to increase
+ *
  * Return: Will be a valid pointer on success and ERR_PTR in case of error.
 */
 const void *of_get_mac_address(struct device_node *np)
 {
-	const void *addr;
+	u32 inc_idx, mac_inc;
+	int ret = 0;
+	u8 *addr;
+
+	/* Check first if the increment byte is present and valid.
+	 * If not set assume to increment the last byte if found.
+	 */
+	if (of_property_read_u32(np, "mac-address-increment-byte", &inc_idx))
+		inc_idx = 5;
+	if (inc_idx > 5)
+		return ERR_PTR(-EINVAL);
 
 	addr = of_get_mac_addr(np, "mac-address");
 	if (addr)
-		return addr;
+		goto found;
 
 	addr = of_get_mac_addr(np, "local-mac-address");
 	if (addr)
-		return addr;
+		goto found;
 
 	addr = of_get_mac_addr(np, "address");
 	if (addr)
-		return addr;
+		goto found;
+
+	addr = of_get_mac_addr_nvmem(np, &ret);
+	if (ret)
+		return ERR_PTR(ret);
+
+found:
+	if (!of_property_read_u32(np, "mac-address-increment", &mac_inc))
+		addr[inc_idx] += mac_inc;
 
-	return of_get_mac_addr_nvmem(np);
+	return addr;
 }
 EXPORT_SYMBOL(of_get_mac_address);
-- 
2.27.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
  2020-09-19 22:30 ` Ansuel Smith
@ 2020-09-19 22:30   ` Ansuel Smith
  -1 siblings, 0 replies; 18+ messages in thread
From: Ansuel Smith @ 2020-09-19 22:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Ansuel Smith, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, David S. Miller, Jakub Kicinski, Andrew Lunn,
	Heiner Kallweit, Russell King, Frank Rowand, Boris Brezillon,
	linux-mtd, devicetree, linux-kernel, netdev

Two new bindings are now supported by the of_net driver to increase (or
decrease) a mac-address. This can be very useful in case where the
system extract the mac-address for the device from a dedicated partition
and have a generic mac-address that needs to be incremented based on the
device number.
- mac-address-increment-byte is used to tell what byte must be
  incremented (if not set the last byte is increased)
- mac-address-increment is used to tell how much to increment of the
  extracted mac-address decided byte.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../bindings/net/ethernet-controller.yaml     | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index fa2baca8c726..43f2f21faf41 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -32,6 +32,25 @@ properties:
       - minItems: 6
         maxItems: 6
 
+  mac-address-increment:
+    description:
+      The MAC address can optionally be increased (or decreased using
+      negative values) from the original value readed (from a nvmem cell
+      for example). This can be used if the mac is readed from a dedicated
+      partition and must be increased based on the number of device
+      present in the system.
+    minimum: -255
+    maximum: 255
+
+  mac-address-increment-byte:
+    description:
+      If 'mac-address-increment' is defined, this will tell what byte of
+      the mac-address will be increased. If 'mac-address-increment' is
+      not defined, this option will do nothing.
+    default: 5
+    minimum: 0
+    maximum: 5
+
   max-frame-size:
     $ref: /schemas/types.yaml#definitions/uint32
     description:
-- 
2.27.0


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

* [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
@ 2020-09-19 22:30   ` Ansuel Smith
  0 siblings, 0 replies; 18+ messages in thread
From: Ansuel Smith @ 2020-09-19 22:30 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Andrew Lunn, Vignesh Raghavendra, Boris Brezillon,
	Richard Weinberger, Russell King, Ansuel Smith, devicetree,
	Rob Herring, linux-mtd, netdev, Jakub Kicinski, Frank Rowand,
	David S. Miller, linux-kernel, Heiner Kallweit

Two new bindings are now supported by the of_net driver to increase (or
decrease) a mac-address. This can be very useful in case where the
system extract the mac-address for the device from a dedicated partition
and have a generic mac-address that needs to be incremented based on the
device number.
- mac-address-increment-byte is used to tell what byte must be
  incremented (if not set the last byte is increased)
- mac-address-increment is used to tell how much to increment of the
  extracted mac-address decided byte.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../bindings/net/ethernet-controller.yaml     | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index fa2baca8c726..43f2f21faf41 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -32,6 +32,25 @@ properties:
       - minItems: 6
         maxItems: 6
 
+  mac-address-increment:
+    description:
+      The MAC address can optionally be increased (or decreased using
+      negative values) from the original value readed (from a nvmem cell
+      for example). This can be used if the mac is readed from a dedicated
+      partition and must be increased based on the number of device
+      present in the system.
+    minimum: -255
+    maximum: 255
+
+  mac-address-increment-byte:
+    description:
+      If 'mac-address-increment' is defined, this will tell what byte of
+      the mac-address will be increased. If 'mac-address-increment' is
+      not defined, this option will do nothing.
+    default: 5
+    minimum: 0
+    maximum: 5
+
   max-frame-size:
     $ref: /schemas/types.yaml#definitions/uint32
     description:
-- 
2.27.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
  2020-09-19 22:30   ` Ansuel Smith
@ 2020-09-20  0:31     ` Andrew Lunn
  -1 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2020-09-20  0:31 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, David S. Miller, Jakub Kicinski, Heiner Kallweit,
	Russell King, Frank Rowand, Boris Brezillon, linux-mtd,
	devicetree, linux-kernel, netdev

> +  mac-address-increment:
> +    description:
> +      The MAC address can optionally be increased (or decreased using
> +      negative values) from the original value readed (from a nvmem cell

Read is irregular, there is no readed, just read.

> +      for example). This can be used if the mac is readed from a dedicated
> +      partition and must be increased based on the number of device
> +      present in the system.

You should probably add there is no underflow/overflow to other bytes
of the MAC address. 00:01:02:03:04:ff + 1 == 00:01:02:03:04:00.

> +    minimum: -255
> +    maximum: 255
> +
> +  mac-address-increment-byte:
> +    description:
> +      If 'mac-address-increment' is defined, this will tell what byte of
> +      the mac-address will be increased. If 'mac-address-increment' is
> +      not defined, this option will do nothing.
> +    default: 5
> +    minimum: 0
> +    maximum: 5

Is there a real need for this? A value of 0 seems like a bad idea,
since a unicast address could easily become a multicast address, which
is not valid for an interface address. It also does not seem like a
good idea to allow the OUI to be changed. So i think only bytes 3-5
should be allowed, but even then, i don't think this is needed, unless
you do have a clear use case.

    Andrew

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

* Re: [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
@ 2020-09-20  0:31     ` Andrew Lunn
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2020-09-20  0:31 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: devicetree, Vignesh Raghavendra, Boris Brezillon,
	Richard Weinberger, Russell King, linux-kernel, Rob Herring,
	linux-mtd, Miquel Raynal, Jakub Kicinski, netdev, Frank Rowand,
	David S. Miller, Heiner Kallweit

> +  mac-address-increment:
> +    description:
> +      The MAC address can optionally be increased (or decreased using
> +      negative values) from the original value readed (from a nvmem cell

Read is irregular, there is no readed, just read.

> +      for example). This can be used if the mac is readed from a dedicated
> +      partition and must be increased based on the number of device
> +      present in the system.

You should probably add there is no underflow/overflow to other bytes
of the MAC address. 00:01:02:03:04:ff + 1 == 00:01:02:03:04:00.

> +    minimum: -255
> +    maximum: 255
> +
> +  mac-address-increment-byte:
> +    description:
> +      If 'mac-address-increment' is defined, this will tell what byte of
> +      the mac-address will be increased. If 'mac-address-increment' is
> +      not defined, this option will do nothing.
> +    default: 5
> +    minimum: 0
> +    maximum: 5

Is there a real need for this? A value of 0 seems like a bad idea,
since a unicast address could easily become a multicast address, which
is not valid for an interface address. It also does not seem like a
good idea to allow the OUI to be changed. So i think only bytes 3-5
should be allowed, but even then, i don't think this is needed, unless
you do have a clear use case.

    Andrew

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* R: [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
  2020-09-20  0:31     ` Andrew Lunn
@ 2020-09-20  0:39       ` ansuelsmth
  -1 siblings, 0 replies; 18+ messages in thread
From: ansuelsmth @ 2020-09-20  0:39 UTC (permalink / raw)
  To: 'Andrew Lunn'
  Cc: 'Miquel Raynal', 'Richard Weinberger',
	'Vignesh Raghavendra', 'Rob Herring',
	'David S. Miller', 'Jakub Kicinski',
	'Heiner Kallweit', 'Russell King',
	'Frank Rowand', 'Boris Brezillon',
	linux-mtd, devicetree, linux-kernel, netdev



> -----Messaggio originale-----
> Da: Andrew Lunn <andrew@lunn.ch>
> Inviato: domenica 20 settembre 2020 02:31
> A: Ansuel Smith <ansuelsmth@gmail.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>; Richard Weinberger
> <richard@nod.at>; Vignesh Raghavendra <vigneshr@ti.com>; Rob Herring
> <robh+dt@kernel.org>; David S. Miller <davem@davemloft.net>; Jakub
> Kicinski <kuba@kernel.org>; Heiner Kallweit <hkallweit1@gmail.com>;
> Russell King <linux@armlinux.org.uk>; Frank Rowand
> <frowand.list@gmail.com>; Boris Brezillon <bbrezillon@kernel.org>; linux-
> mtd@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Oggetto: Re: [PATCH v2 4/4] dt-bindings: net: Document use of mac-
> address-increment
> 
> > +  mac-address-increment:
> > +    description:
> > +      The MAC address can optionally be increased (or decreased using
> > +      negative values) from the original value readed (from a nvmem
cell
> 
> Read is irregular, there is no readed, just read.
> 
> > +      for example). This can be used if the mac is readed from a
dedicated
> > +      partition and must be increased based on the number of device
> > +      present in the system.
> 
> You should probably add there is no underflow/overflow to other bytes
> of the MAC address. 00:01:02:03:04:ff + 1 == 00:01:02:03:04:00.
> 
> > +    minimum: -255
> > +    maximum: 255
> > +
> > +  mac-address-increment-byte:
> > +    description:
> > +      If 'mac-address-increment' is defined, this will tell what byte
of
> > +      the mac-address will be increased. If 'mac-address-increment' is
> > +      not defined, this option will do nothing.
> > +    default: 5
> > +    minimum: 0
> > +    maximum: 5
> 
> Is there a real need for this? A value of 0 seems like a bad idea,
> since a unicast address could easily become a multicast address, which
> is not valid for an interface address. It also does not seem like a
> good idea to allow the OUI to be changed. So i think only bytes 3-5
> should be allowed, but even then, i don't think this is needed, unless
> you do have a clear use case.
> 
>     Andrew

Honestly the mac-address-increment-byte is added to give user some control
but I
don't really have a use case for it. Should I limit it to 3 or just remove
the function?
Will address the other 2 comment in v3.
Thx for review.


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

* R: [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
@ 2020-09-20  0:39       ` ansuelsmth
  0 siblings, 0 replies; 18+ messages in thread
From: ansuelsmth @ 2020-09-20  0:39 UTC (permalink / raw)
  To: 'Andrew Lunn'
  Cc: devicetree, 'Vignesh Raghavendra',
	'Boris Brezillon', 'Richard Weinberger',
	'Russell King', linux-kernel, 'Rob Herring',
	linux-mtd, 'Miquel Raynal', 'Jakub Kicinski',
	netdev, 'Frank Rowand', 'David S. Miller',
	'Heiner Kallweit'



> -----Messaggio originale-----
> Da: Andrew Lunn <andrew@lunn.ch>
> Inviato: domenica 20 settembre 2020 02:31
> A: Ansuel Smith <ansuelsmth@gmail.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>; Richard Weinberger
> <richard@nod.at>; Vignesh Raghavendra <vigneshr@ti.com>; Rob Herring
> <robh+dt@kernel.org>; David S. Miller <davem@davemloft.net>; Jakub
> Kicinski <kuba@kernel.org>; Heiner Kallweit <hkallweit1@gmail.com>;
> Russell King <linux@armlinux.org.uk>; Frank Rowand
> <frowand.list@gmail.com>; Boris Brezillon <bbrezillon@kernel.org>; linux-
> mtd@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Oggetto: Re: [PATCH v2 4/4] dt-bindings: net: Document use of mac-
> address-increment
> 
> > +  mac-address-increment:
> > +    description:
> > +      The MAC address can optionally be increased (or decreased using
> > +      negative values) from the original value readed (from a nvmem
cell
> 
> Read is irregular, there is no readed, just read.
> 
> > +      for example). This can be used if the mac is readed from a
dedicated
> > +      partition and must be increased based on the number of device
> > +      present in the system.
> 
> You should probably add there is no underflow/overflow to other bytes
> of the MAC address. 00:01:02:03:04:ff + 1 == 00:01:02:03:04:00.
> 
> > +    minimum: -255
> > +    maximum: 255
> > +
> > +  mac-address-increment-byte:
> > +    description:
> > +      If 'mac-address-increment' is defined, this will tell what byte
of
> > +      the mac-address will be increased. If 'mac-address-increment' is
> > +      not defined, this option will do nothing.
> > +    default: 5
> > +    minimum: 0
> > +    maximum: 5
> 
> Is there a real need for this? A value of 0 seems like a bad idea,
> since a unicast address could easily become a multicast address, which
> is not valid for an interface address. It also does not seem like a
> good idea to allow the OUI to be changed. So i think only bytes 3-5
> should be allowed, but even then, i don't think this is needed, unless
> you do have a clear use case.
> 
>     Andrew

Honestly the mac-address-increment-byte is added to give user some control
but I
don't really have a use case for it. Should I limit it to 3 or just remove
the function?
Will address the other 2 comment in v3.
Thx for review.


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: R: [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
  2020-09-20  0:39       ` ansuelsmth
@ 2020-09-20  1:22         ` Andrew Lunn
  -1 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2020-09-20  1:22 UTC (permalink / raw)
  To: ansuelsmth
  Cc: 'Miquel Raynal', 'Richard Weinberger',
	'Vignesh Raghavendra', 'Rob Herring',
	'David S. Miller', 'Jakub Kicinski',
	'Heiner Kallweit', 'Russell King',
	'Frank Rowand', 'Boris Brezillon',
	linux-mtd, devicetree, linux-kernel, netdev

On Sun, Sep 20, 2020 at 02:39:39AM +0200, ansuelsmth@gmail.com wrote:
> 
> 
> > -----Messaggio originale-----
> > Da: Andrew Lunn <andrew@lunn.ch>
> > Inviato: domenica 20 settembre 2020 02:31
> > A: Ansuel Smith <ansuelsmth@gmail.com>
> > Cc: Miquel Raynal <miquel.raynal@bootlin.com>; Richard Weinberger
> > <richard@nod.at>; Vignesh Raghavendra <vigneshr@ti.com>; Rob Herring
> > <robh+dt@kernel.org>; David S. Miller <davem@davemloft.net>; Jakub
> > Kicinski <kuba@kernel.org>; Heiner Kallweit <hkallweit1@gmail.com>;
> > Russell King <linux@armlinux.org.uk>; Frank Rowand
> > <frowand.list@gmail.com>; Boris Brezillon <bbrezillon@kernel.org>; linux-
> > mtd@lists.infradead.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Oggetto: Re: [PATCH v2 4/4] dt-bindings: net: Document use of mac-
> > address-increment
> > 
> > > +  mac-address-increment:
> > > +    description:
> > > +      The MAC address can optionally be increased (or decreased using
> > > +      negative values) from the original value readed (from a nvmem
> cell
> > 
> > Read is irregular, there is no readed, just read.
> > 
> > > +      for example). This can be used if the mac is readed from a
> dedicated
> > > +      partition and must be increased based on the number of device
> > > +      present in the system.
> > 
> > You should probably add there is no underflow/overflow to other bytes
> > of the MAC address. 00:01:02:03:04:ff + 1 == 00:01:02:03:04:00.
> > 
> > > +    minimum: -255
> > > +    maximum: 255
> > > +
> > > +  mac-address-increment-byte:
> > > +    description:
> > > +      If 'mac-address-increment' is defined, this will tell what byte
> of
> > > +      the mac-address will be increased. If 'mac-address-increment' is
> > > +      not defined, this option will do nothing.
> > > +    default: 5
> > > +    minimum: 0
> > > +    maximum: 5
> > 
> > Is there a real need for this? A value of 0 seems like a bad idea,
> > since a unicast address could easily become a multicast address, which
> > is not valid for an interface address. It also does not seem like a
> > good idea to allow the OUI to be changed. So i think only bytes 3-5
> > should be allowed, but even then, i don't think this is needed, unless
> > you do have a clear use case.
> > 
> >     Andrew
> 
> Honestly the mac-address-increment-byte is added to give user some control
> but I
> don't really have a use case for it. Should I limit it to 3 or just remove
> the function?

If you have no use case, just remove it and document that last byte
will be incremented. I somebody does need it, it can be added in a
backwards compatible way.

     Andrew

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

* Re: R: [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
@ 2020-09-20  1:22         ` Andrew Lunn
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2020-09-20  1:22 UTC (permalink / raw)
  To: ansuelsmth
  Cc: devicetree, 'Vignesh Raghavendra',
	'Boris Brezillon', 'Richard Weinberger',
	'Russell King', linux-kernel, 'Rob Herring',
	linux-mtd, 'Miquel Raynal', 'Jakub Kicinski',
	netdev, 'Frank Rowand', 'David S. Miller',
	'Heiner Kallweit'

On Sun, Sep 20, 2020 at 02:39:39AM +0200, ansuelsmth@gmail.com wrote:
> 
> 
> > -----Messaggio originale-----
> > Da: Andrew Lunn <andrew@lunn.ch>
> > Inviato: domenica 20 settembre 2020 02:31
> > A: Ansuel Smith <ansuelsmth@gmail.com>
> > Cc: Miquel Raynal <miquel.raynal@bootlin.com>; Richard Weinberger
> > <richard@nod.at>; Vignesh Raghavendra <vigneshr@ti.com>; Rob Herring
> > <robh+dt@kernel.org>; David S. Miller <davem@davemloft.net>; Jakub
> > Kicinski <kuba@kernel.org>; Heiner Kallweit <hkallweit1@gmail.com>;
> > Russell King <linux@armlinux.org.uk>; Frank Rowand
> > <frowand.list@gmail.com>; Boris Brezillon <bbrezillon@kernel.org>; linux-
> > mtd@lists.infradead.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Oggetto: Re: [PATCH v2 4/4] dt-bindings: net: Document use of mac-
> > address-increment
> > 
> > > +  mac-address-increment:
> > > +    description:
> > > +      The MAC address can optionally be increased (or decreased using
> > > +      negative values) from the original value readed (from a nvmem
> cell
> > 
> > Read is irregular, there is no readed, just read.
> > 
> > > +      for example). This can be used if the mac is readed from a
> dedicated
> > > +      partition and must be increased based on the number of device
> > > +      present in the system.
> > 
> > You should probably add there is no underflow/overflow to other bytes
> > of the MAC address. 00:01:02:03:04:ff + 1 == 00:01:02:03:04:00.
> > 
> > > +    minimum: -255
> > > +    maximum: 255
> > > +
> > > +  mac-address-increment-byte:
> > > +    description:
> > > +      If 'mac-address-increment' is defined, this will tell what byte
> of
> > > +      the mac-address will be increased. If 'mac-address-increment' is
> > > +      not defined, this option will do nothing.
> > > +    default: 5
> > > +    minimum: 0
> > > +    maximum: 5
> > 
> > Is there a real need for this? A value of 0 seems like a bad idea,
> > since a unicast address could easily become a multicast address, which
> > is not valid for an interface address. It also does not seem like a
> > good idea to allow the OUI to be changed. So i think only bytes 3-5
> > should be allowed, but even then, i don't think this is needed, unless
> > you do have a clear use case.
> > 
> >     Andrew
> 
> Honestly the mac-address-increment-byte is added to give user some control
> but I
> don't really have a use case for it. Should I limit it to 3 or just remove
> the function?

If you have no use case, just remove it and document that last byte
will be incremented. I somebody does need it, it can be added in a
backwards compatible way.

     Andrew

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* RE: R: [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
  2020-09-20  1:22         ` Andrew Lunn
@ 2020-09-20  9:45           ` ansuelsmth
  -1 siblings, 0 replies; 18+ messages in thread
From: ansuelsmth @ 2020-09-20  9:45 UTC (permalink / raw)
  To: 'Andrew Lunn'
  Cc: 'Miquel Raynal', 'Richard Weinberger',
	'Vignesh Raghavendra', 'Rob Herring',
	'David S. Miller', 'Jakub Kicinski',
	'Heiner Kallweit', 'Russell King',
	'Frank Rowand', 'Boris Brezillon',
	linux-mtd, devicetree, linux-kernel, netdev

> On Sun, Sep 20, 2020 at 02:39:39AM +0200, ansuelsmth@gmail.com
> wrote:
> >
> >
> > > -----Messaggio originale-----
> > > Da: Andrew Lunn <andrew@lunn.ch>
> > > Inviato: domenica 20 settembre 2020 02:31
> > > A: Ansuel Smith <ansuelsmth@gmail.com>
> > > Cc: Miquel Raynal <miquel.raynal@bootlin.com>; Richard Weinberger
> > > <richard@nod.at>; Vignesh Raghavendra <vigneshr@ti.com>; Rob
> Herring
> > > <robh+dt@kernel.org>; David S. Miller <davem@davemloft.net>; Jakub
> > > Kicinski <kuba@kernel.org>; Heiner Kallweit <hkallweit1@gmail.com>;
> > > Russell King <linux@armlinux.org.uk>; Frank Rowand
> > > <frowand.list@gmail.com>; Boris Brezillon <bbrezillon@kernel.org>;
> linux-
> > > mtd@lists.infradead.org; devicetree@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; netdev@vger.kernel.org
> > > Oggetto: Re: [PATCH v2 4/4] dt-bindings: net: Document use of mac-
> > > address-increment
> > >
> > > > +  mac-address-increment:
> > > > +    description:
> > > > +      The MAC address can optionally be increased (or decreased
using
> > > > +      negative values) from the original value readed (from a nvmem
> > cell
> > >
> > > Read is irregular, there is no readed, just read.
> > >
> > > > +      for example). This can be used if the mac is readed from a
> > dedicated
> > > > +      partition and must be increased based on the number of device
> > > > +      present in the system.
> > >
> > > You should probably add there is no underflow/overflow to other bytes
> > > of the MAC address. 00:01:02:03:04:ff + 1 == 00:01:02:03:04:00.
> > >
> > > > +    minimum: -255
> > > > +    maximum: 255
> > > > +
> > > > +  mac-address-increment-byte:
> > > > +    description:
> > > > +      If 'mac-address-increment' is defined, this will tell what
byte
> > of
> > > > +      the mac-address will be increased. If 'mac-address-increment'
is
> > > > +      not defined, this option will do nothing.
> > > > +    default: 5
> > > > +    minimum: 0
> > > > +    maximum: 5
> > >
> > > Is there a real need for this? A value of 0 seems like a bad idea,
> > > since a unicast address could easily become a multicast address, which
> > > is not valid for an interface address. It also does not seem like a
> > > good idea to allow the OUI to be changed. So i think only bytes 3-5
> > > should be allowed, but even then, i don't think this is needed, unless
> > > you do have a clear use case.
> > >
> > >     Andrew
> >
> > Honestly the mac-address-increment-byte is added to give user some
> control
> > but I
> > don't really have a use case for it. Should I limit it to 3 or just
remove
> > the function?
> 
> If you have no use case, just remove it and document that last byte
> will be incremented. I somebody does need it, it can be added in a
> backwards compatible way.
> 
>      Andrew

I just rechecked mac-address-increment-byte and we have one device that
use it and would benefits from this.  I will change the Documentation to
min 3 and leave it.


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

* RE: R: [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
@ 2020-09-20  9:45           ` ansuelsmth
  0 siblings, 0 replies; 18+ messages in thread
From: ansuelsmth @ 2020-09-20  9:45 UTC (permalink / raw)
  To: 'Andrew Lunn'
  Cc: devicetree, 'Vignesh Raghavendra',
	'Boris Brezillon', 'Richard Weinberger',
	'Russell King', linux-kernel, 'Rob Herring',
	linux-mtd, 'Miquel Raynal', 'Jakub Kicinski',
	netdev, 'Frank Rowand', 'David S. Miller',
	'Heiner Kallweit'

> On Sun, Sep 20, 2020 at 02:39:39AM +0200, ansuelsmth@gmail.com
> wrote:
> >
> >
> > > -----Messaggio originale-----
> > > Da: Andrew Lunn <andrew@lunn.ch>
> > > Inviato: domenica 20 settembre 2020 02:31
> > > A: Ansuel Smith <ansuelsmth@gmail.com>
> > > Cc: Miquel Raynal <miquel.raynal@bootlin.com>; Richard Weinberger
> > > <richard@nod.at>; Vignesh Raghavendra <vigneshr@ti.com>; Rob
> Herring
> > > <robh+dt@kernel.org>; David S. Miller <davem@davemloft.net>; Jakub
> > > Kicinski <kuba@kernel.org>; Heiner Kallweit <hkallweit1@gmail.com>;
> > > Russell King <linux@armlinux.org.uk>; Frank Rowand
> > > <frowand.list@gmail.com>; Boris Brezillon <bbrezillon@kernel.org>;
> linux-
> > > mtd@lists.infradead.org; devicetree@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; netdev@vger.kernel.org
> > > Oggetto: Re: [PATCH v2 4/4] dt-bindings: net: Document use of mac-
> > > address-increment
> > >
> > > > +  mac-address-increment:
> > > > +    description:
> > > > +      The MAC address can optionally be increased (or decreased
using
> > > > +      negative values) from the original value readed (from a nvmem
> > cell
> > >
> > > Read is irregular, there is no readed, just read.
> > >
> > > > +      for example). This can be used if the mac is readed from a
> > dedicated
> > > > +      partition and must be increased based on the number of device
> > > > +      present in the system.
> > >
> > > You should probably add there is no underflow/overflow to other bytes
> > > of the MAC address. 00:01:02:03:04:ff + 1 == 00:01:02:03:04:00.
> > >
> > > > +    minimum: -255
> > > > +    maximum: 255
> > > > +
> > > > +  mac-address-increment-byte:
> > > > +    description:
> > > > +      If 'mac-address-increment' is defined, this will tell what
byte
> > of
> > > > +      the mac-address will be increased. If 'mac-address-increment'
is
> > > > +      not defined, this option will do nothing.
> > > > +    default: 5
> > > > +    minimum: 0
> > > > +    maximum: 5
> > >
> > > Is there a real need for this? A value of 0 seems like a bad idea,
> > > since a unicast address could easily become a multicast address, which
> > > is not valid for an interface address. It also does not seem like a
> > > good idea to allow the OUI to be changed. So i think only bytes 3-5
> > > should be allowed, but even then, i don't think this is needed, unless
> > > you do have a clear use case.
> > >
> > >     Andrew
> >
> > Honestly the mac-address-increment-byte is added to give user some
> control
> > but I
> > don't really have a use case for it. Should I limit it to 3 or just
remove
> > the function?
> 
> If you have no use case, just remove it and document that last byte
> will be incremented. I somebody does need it, it can be added in a
> backwards compatible way.
> 
>      Andrew

I just rechecked mac-address-increment-byte and we have one device that
use it and would benefits from this.  I will change the Documentation to
min 3 and leave it.


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

end of thread, other threads:[~2020-09-20  9:46 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-19 22:30 [PATCH v2 0/4] Actually implement nvmem support for mtd Ansuel Smith
2020-09-19 22:30 ` Ansuel Smith
2020-09-19 22:30 ` [PATCH v2 1/4] mtd: Add nvmem support for mtd nvmem-providers Ansuel Smith
2020-09-19 22:30   ` Ansuel Smith
2020-09-19 22:30 ` [PATCH v2 2/4] dt-bindings: mtd: partition: Document use of nvmem-provider Ansuel Smith
2020-09-19 22:30   ` Ansuel Smith
2020-09-19 22:30 ` [PATCH v2 3/4] of_net: add mac-address-increment support Ansuel Smith
2020-09-19 22:30   ` Ansuel Smith
2020-09-19 22:30 ` [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment Ansuel Smith
2020-09-19 22:30   ` Ansuel Smith
2020-09-20  0:31   ` Andrew Lunn
2020-09-20  0:31     ` Andrew Lunn
2020-09-20  0:39     ` R: " ansuelsmth
2020-09-20  0:39       ` ansuelsmth
2020-09-20  1:22       ` Andrew Lunn
2020-09-20  1:22         ` Andrew Lunn
2020-09-20  9:45         ` ansuelsmth
2020-09-20  9:45           ` ansuelsmth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.