[PATCH v1 00/11] prepare Protonic board code for mainline

[PATCH v1 00/11] prepare Protonic board code for mainline

[Oleksij Rempel]

This patch series is a preparation and actual Protonic board code.
Following changes was made in the main code:
- register DT root node as device to make it possible to loade board
  code as usual driver
- provide helpers to read ethernet mac from nvmem cell register in a
  devicetree.
- provide barebox-serial driver which should be a nvmem cell consumer and
  serial-number DT property provider.
- provide gpio helper to read gpio based board revision.

Oleksij Rempel (11):
  ARM: dts: imx6q-prti6q: fix PHY register
  of: base: register DT root as device
  ARM: embest-riotboard: port board file to the driver model
  net: port nvmem_get_mac_address() from linux kernel
  of: of_net: sync of_get_mac_address() with latest kernel version
  net: fec_imx: use of_get_mac_address() to get mac address out of DT
  of: of_device_get_match_compatible() helper
  gpiolib: add gpio_array_to_id helper to get ID out of GPIO array
  ARM: protonic-imx6: port protonic specific board code
  of: add barebox-serial driver
  ARM: dts: imx6q-prti6q: add barebox.serial node

 arch/arm/boards/embest-riotboard/board.c |  18 +-
 arch/arm/boards/protonic-imx6/Makefile   |   1 +
 arch/arm/boards/protonic-imx6/board.c    | 598 +++++++++++++++++++++++
 arch/arm/dts/imx6q-prti6q.dts            |   9 +-
 drivers/gpio/gpiolib.c                   |  28 ++
 drivers/net/fec_imx.c                    |   9 +
 drivers/of/Makefile                      |   2 +-
 drivers/of/barebox_serial.c              | 111 +++++
 drivers/of/base.c                        |   5 +
 drivers/of/device.c                      |  12 +
 drivers/of/of_net.c                      |  63 ++-
 include/gpio.h                           |   5 +
 include/net.h                            |   4 +
 include/of_device.h                      |   6 +
 net/eth.c                                |  38 ++
 15 files changed, 889 insertions(+), 20 deletions(-)
 create mode 100644 arch/arm/boards/protonic-imx6/board.c
 create mode 100644 drivers/of/barebox_serial.c

-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 01/11] ARM: dts: imx6q-prti6q: fix PHY register

[Oleksij Rempel]

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 arch/arm/dts/imx6q-prti6q.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/dts/imx6q-prti6q.dts b/arch/arm/dts/imx6q-prti6q.dts
index f0469d5f62..76bb4d53d3 100644
--- a/arch/arm/dts/imx6q-prti6q.dts
+++ b/arch/arm/dts/imx6q-prti6q.dts
@@ -225,8 +225,8 @@
 		#size-cells = <0>;
 
 		/* Microchip KSZ9031RNX PHY */
-		rgmii_phy: ethernet-phy@0 {
-			reg = <0>;
+		rgmii_phy: ethernet-phy@4 {
+			reg = <4>;
 			interrupts-extended = <&gpio1 28 IRQ_TYPE_LEVEL_LOW>;
 			reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;
 			reset-assert-us = <10000>;
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 02/11] of: base: register DT root as device

[Oleksij Rempel]

A usual board file contains at least one of_machine_is_compatible().
Some of the have a rather long list with complicated version logic.

To avoid own implementation for driver management, register the root node
of device tree as platform device. So, the main platform bus can attach
proper board driver. After this patch a typical board.c file can reuse
existing driver infrastructure.

After this patch, you will be able to see all registered board drivers
with drvinfo as fallow:
...
board-embest-riot
board-protonic-imx6
    machine.of
...

With devinfo, you'll be able to get some board specific information,
if this is implemented:
barebox@Protonic PRTI6Q board:/ devinfo machine.of
Driver: board-protonic-imx6
Bus: platform
Parameters:
  boardid: 0 (type: uint32)
  boardrev: 1 (type: uint32)

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/of/base.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 4c633bcd49..c48bf7a7fb 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2133,6 +2133,7 @@ static void of_probe_memory(void)
 int of_probe(void)
 {
 	struct device_node *firmware;
+	struct device_d *dev;
 
 	if(!root_node)
 		return -ENODEV;
@@ -2149,6 +2150,10 @@ int of_probe(void)
 	if (firmware)
 		of_platform_populate(firmware, NULL, NULL);
 
+	dev = of_platform_device_create(root_node, NULL);
+	if (dev)
+		dev_set_name(dev, "%s.of", "machine");
+
 	of_clk_init(root_node, NULL);
 	of_platform_populate(root_node, of_default_bus_match_table, NULL);
 
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 03/11] ARM: embest-riotboard: port board file to the driver model

[Oleksij Rempel]

This patch can be used as example for the new board-driver
functionality.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 arch/arm/boards/embest-riotboard/board.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boards/embest-riotboard/board.c b/arch/arm/boards/embest-riotboard/board.c
index 2e0cc9f0ab..746ecf0562 100644
--- a/arch/arm/boards/embest-riotboard/board.c
+++ b/arch/arm/boards/embest-riotboard/board.c
@@ -51,11 +51,8 @@ static int ar8035_phy_fixup(struct phy_device *dev)
 	return 0;
 }
 
-static int riotboard_device_init(void)
+static int riotboard_probe(struct device_d *dev)
 {
-	if (!of_machine_is_compatible("riot,imx6s-riotboard"))
-		return 0;
-
 	phy_register_fixup_for_uid(0x004dd072, 0xffffffef, ar8035_phy_fixup);
 
 	imx6_bbu_internal_mmc_register_handler("emmc", "/dev/mmc3.barebox",
@@ -65,4 +62,15 @@ static int riotboard_device_init(void)
 
 	return 0;
 }
-device_initcall(riotboard_device_init);
+
+static const struct of_device_id riotboard_of_match[] = {
+	{ .compatible = "riot,imx6s-riotboard" },
+	{},
+};
+
+static struct driver_d riotboard_driver = {
+	.name = "board-embest-riot",
+	.probe = riotboard_probe,
+	.of_compatible = DRV_OF_COMPAT(riotboard_of_match),
+};
+device_platform_driver(riotboard_driver);
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 04/11] net: port nvmem_get_mac_address() from linux kernel

[Oleksij Rempel]

Port nvmem help to read mac-address from nvmem cell linked to the
ethernet node. This function was ported from kernel 5.8-rc1.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 include/net.h |  4 ++++
 net/eth.c     | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/include/net.h b/include/net.h
index 54db8a179a..a5bfe9e7f8 100644
--- a/include/net.h
+++ b/include/net.h
@@ -97,9 +97,13 @@ static inline void of_eth_register_ethaddr(struct device_node *node,
 		const char *ethaddr)
 {
 }
+static inline int nvmem_get_mac_address(struct device_d *dev, void *addrbuf)
+{
+}
 #else
 void eth_register_ethaddr(int ethid, const char *ethaddr);
 void of_eth_register_ethaddr(struct device_node *node, const char *ethaddr);
+int nvmem_get_mac_address(struct device_d *dev, void *addrbuf);
 #endif
 /*
  *	Ethernet header
diff --git a/net/eth.c b/net/eth.c
index e3d0d06efe..b33ef5fe1f 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -29,6 +29,7 @@
 #include <globalvar.h>
 #include <environment.h>
 #include <linux/ctype.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/stat.h>
 
 static uint64_t last_link_check;
@@ -152,6 +153,43 @@ void of_eth_register_ethaddr(struct device_node *node, const char *ethaddr)
 	list_add_tail(&addr->list, &ethaddr_list);
 }
 
+/**
+ * Obtain the MAC address from an nvmem cell named 'mac-address' associated
+ * with given device.
+ *
+ * @dev:	Device with which the mac-address cell is associated.
+ * @addrbuf:	Buffer to which the MAC address will be copied on success.
+ *
+ * Returns 0 on success or a negative error number on failure.
+ */
+int nvmem_get_mac_address(struct device_d *dev, void *addrbuf)
+{
+	struct nvmem_cell *cell;
+	const void *mac;
+	size_t len;
+
+	cell = nvmem_cell_get(dev, "mac-address");
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	mac = nvmem_cell_read(cell, &len);
+	nvmem_cell_put(cell);
+
+	if (IS_ERR(mac))
+		return PTR_ERR(mac);
+
+	if (len != ETH_ALEN || !is_valid_ether_addr(mac)) {
+		kfree(mac);
+		return -EINVAL;
+	}
+
+	memcpy(addrbuf, mac, ETH_ALEN);
+	kfree(mac);
+
+	return 0;
+}
+EXPORT_SYMBOL(nvmem_get_mac_address);
+
 struct eth_device *eth_get_byname(const char *ethname)
 {
 	struct eth_device *edev;
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 05/11] of: of_net: sync of_get_mac_address() with latest kernel version

[Oleksij Rempel]

Sync of_get_mac_address() with kernel 5.8-rc1 do add nvmem cell support

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/of/of_net.c | 63 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 12 deletions(-)

diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index cee4597195..946c4a4352 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -67,12 +67,45 @@ int of_get_phy_mode(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_get_phy_mode);
 
+static const void *of_get_mac_addr(struct device_node *np, const char *name)
+{
+	struct property *pp = of_find_property(np, name, NULL);
+
+	if (pp && pp->length == ETH_ALEN && is_valid_ether_addr(pp->value))
+		return pp->value;
+	return NULL;
+}
+
+static const void *of_get_mac_addr_nvmem(struct device_node *np)
+{
+	int ret;
+	const void *mac;
+	u8 nvmem_mac[ETH_ALEN];
+	struct device_d *dev = of_find_device_by_node(np);
+
+	if (!dev)
+		return ERR_PTR(-ENODEV);
+
+	ret = nvmem_get_mac_address(dev, &nvmem_mac);
+	if (ret) {
+		return ERR_PTR(ret);
+	}
+
+	mac = kmemdup(nvmem_mac, ETH_ALEN, GFP_KERNEL);
+	if (!mac)
+		return ERR_PTR(-ENOMEM);
+
+	return mac;
+}
+
 /**
  * Search the device tree for the best MAC address to use.  'mac-address' is
  * checked first, because that is supposed to contain to "most recent" MAC
  * address. If that isn't set, then 'local-mac-address' is checked next,
- * because that is the default address.  If that isn't set, then the obsolete
- * 'address' is checked, just in case we're using an old device tree.
+ * because that is the default address. If that isn't set, then the obsolete
+ * 'address' is checked, just in case we're using an old device tree. If any
+ * of the above isn't set, then try to get MAC address from nvmem cell named
+ * 'mac-address'.
  *
  * Note that the 'address' property is supposed to contain a virtual address of
  * the register set, but some DTS files have redefined that property to be the
@@ -84,19 +117,25 @@ EXPORT_SYMBOL_GPL(of_get_phy_mode);
  * addresses.  Some older U-Boots only initialized 'local-mac-address'.  In
  * this case, the real MAC is in 'local-mac-address', and 'mac-address' exists
  * but is all zeros.
+ *
+ * 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 *p;
-	int len, i;
-	const char *str[] = { "mac-address", "local-mac-address", "address" };
-
-	for (i = 0; i < ARRAY_SIZE(str); i++) {
-		p = of_get_property(np, str[i], &len);
-		if (p && (len == 6) && is_valid_ether_addr(p))
-			return p;
-	}
+	const void *addr;
 
-	return NULL;
+	addr = of_get_mac_addr(np, "mac-address");
+	if (addr)
+		return addr;
+
+	addr = of_get_mac_addr(np, "local-mac-address");
+	if (addr)
+		return addr;
+
+	addr = of_get_mac_addr(np, "address");
+	if (addr)
+		return addr;
+
+	return of_get_mac_addr_nvmem(np);
 }
 EXPORT_SYMBOL(of_get_mac_address);
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 06/11] net: fec_imx: use of_get_mac_address() to get mac address out of DT

[Oleksij Rempel]

A mac address can be provided by the DT over nvmem cell. Start to use
of_get_mac_address() since it will be able to read nvmem cell for us.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/fec_imx.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/fec_imx.c b/drivers/net/fec_imx.c
index 0c2d600d12..578a85c28b 100644
--- a/drivers/net/fec_imx.c
+++ b/drivers/net/fec_imx.c
@@ -628,6 +628,7 @@ static void fec_free_receive_packets(struct fec_priv *fec, int count, int size)
 static int fec_probe_dt(struct device_d *dev, struct fec_priv *fec)
 {
 	struct device_node *mdiobus;
+	const char *mac_addr;
 	int ret;
 
 	ret = of_get_phy_mode(dev->device_node);
@@ -640,6 +641,14 @@ static int fec_probe_dt(struct device_d *dev, struct fec_priv *fec)
 	if (mdiobus)
 		fec->miibus.dev.device_node = mdiobus;
 
+	mac_addr = of_get_mac_address(dev->device_node);
+	if (IS_ERR(mac_addr))
+		return PTR_ERR(mac_addr);
+
+	of_eth_register_ethaddr(dev->device_node, mac_addr);
+
+	kfree(mac_addr);
+
 	return 0;
 }
 #else
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v1 06/11] net: fec_imx: use of_get_mac_address() to get mac address out of DT

[Sascha Hauer]

On Thu, Jul 23, 2020 at 12:33:21PM +0200, Oleksij Rempel wrote:
> A mac address can be provided by the DT over nvmem cell. Start to use
> of_get_mac_address() since it will be able to read nvmem cell for us.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/fec_imx.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/fec_imx.c b/drivers/net/fec_imx.c
> index 0c2d600d12..578a85c28b 100644
> --- a/drivers/net/fec_imx.c
> +++ b/drivers/net/fec_imx.c
> @@ -628,6 +628,7 @@ static void fec_free_receive_packets(struct fec_priv *fec, int count, int size)
>  static int fec_probe_dt(struct device_d *dev, struct fec_priv *fec)
>  {
>  	struct device_node *mdiobus;
> +	const char *mac_addr;
>  	int ret;
>  
>  	ret = of_get_phy_mode(dev->device_node);
> @@ -640,6 +641,14 @@ static int fec_probe_dt(struct device_d *dev, struct fec_priv *fec)
>  	if (mdiobus)
>  		fec->miibus.dev.device_node = mdiobus;
>  
> +	mac_addr = of_get_mac_address(dev->device_node);
> +	if (IS_ERR(mac_addr))
> +		return PTR_ERR(mac_addr);

A MAC address *can* be provided by the DT, but it's not mandatory, no?
Shouldn't you return successfully here when no MAC adress is found?

Sascha

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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v1 06/11] net: fec_imx: use of_get_mac_address() to get mac address out of DT

[Oleksij Rempel]

[-- Attachment #1.1: Type: text/plain, Size: 1716 bytes --]

On Mon, Aug 03, 2020 at 10:13:14PM +0200, Sascha Hauer wrote:
> On Thu, Jul 23, 2020 at 12:33:21PM +0200, Oleksij Rempel wrote:
> > A mac address can be provided by the DT over nvmem cell. Start to use
> > of_get_mac_address() since it will be able to read nvmem cell for us.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  drivers/net/fec_imx.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/net/fec_imx.c b/drivers/net/fec_imx.c
> > index 0c2d600d12..578a85c28b 100644
> > --- a/drivers/net/fec_imx.c
> > +++ b/drivers/net/fec_imx.c
> > @@ -628,6 +628,7 @@ static void fec_free_receive_packets(struct fec_priv *fec, int count, int size)
> >  static int fec_probe_dt(struct device_d *dev, struct fec_priv *fec)
> >  {
> >  	struct device_node *mdiobus;
> > +	const char *mac_addr;
> >  	int ret;
> >  
> >  	ret = of_get_phy_mode(dev->device_node);
> > @@ -640,6 +641,14 @@ static int fec_probe_dt(struct device_d *dev, struct fec_priv *fec)
> >  	if (mdiobus)
> >  		fec->miibus.dev.device_node = mdiobus;
> >  
> > +	mac_addr = of_get_mac_address(dev->device_node);
> > +	if (IS_ERR(mac_addr))
> > +		return PTR_ERR(mac_addr);
> 
> A MAC address *can* be provided by the DT, but it's not mandatory, no?
> Shouldn't you return successfully here when no MAC adress is found?

ok, fixed.

Regards,
Oleksij
-- 
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 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 07/11] of: of_device_get_match_compatible() helper

[Oleksij Rempel]

Some times we need to know, against which compatible did the driver
was registered. So, instead of coding it in the driver, add generic
helper for all drivers.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/of/device.c | 12 ++++++++++++
 include/of_device.h |  6 ++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 67a67bd565..b3f522e1fa 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -31,3 +31,15 @@ const void *of_device_get_match_data(const struct device_d *dev)
 	return match->data;
 }
 EXPORT_SYMBOL(of_device_get_match_data);
+
+const char *of_device_get_match_compatible(const struct device_d *dev)
+{
+	const struct of_device_id *match;
+
+	match = of_match_device(dev->driver->of_compatible, dev);
+	if (!match)
+		return NULL;
+
+	return match->compatible;
+}
+EXPORT_SYMBOL(of_device_get_match_compatible);
diff --git a/include/of_device.h b/include/of_device.h
index 244f5fcbbb..cef6d5b5cc 100644
--- a/include/of_device.h
+++ b/include/of_device.h
@@ -22,6 +22,7 @@ static inline int of_driver_match_device(struct device_d *dev,
 }
 
 extern const void *of_device_get_match_data(const struct device_d *dev);
+extern const char *of_device_get_match_compatible(const struct device_d *dev);
 
 #else /* CONFIG_OFTREE */
 
@@ -36,6 +37,11 @@ static inline const void *of_device_get_match_data(const struct device_d *dev)
 	return NULL;
 }
 
+static inline const char *of_device_get_match_compatible(const struct device_d *dev)
+{
+	return NULL;
+}
+
 static inline const struct of_device_id *__of_match_device(
 		const struct of_device_id *matches, const struct device_d *dev)
 {
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 08/11] gpiolib: add gpio_array_to_id helper to get ID out of GPIO array

[Oleksij Rempel]

Some boards provide a board version and/or ID coded by pull-up/down
resistors connected to the gpio pins (or pins which can be multiplexed
at some point as gpio). In this case every one implements own gpio id
reader function.

To avoid it, provide the common helper function to extract a value out
of provided gpio array.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 28 ++++++++++++++++++++++++++++
 include/gpio.h         |  5 +++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 27674af54c..2e9150d2ea 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -381,6 +381,34 @@ void gpio_free_array(const struct gpio *array, size_t num)
 }
 EXPORT_SYMBOL_GPL(gpio_free_array);
 
+int gpio_array_to_id(const struct gpio *array, size_t num, u32 *val)
+{
+	u32 id = 0;
+	int ret, i;
+
+	if (num > 32)
+		return -EOVERFLOW;
+
+	ret = gpio_request_array(array, num);
+	if (ret)
+		return ret;
+
+	/* Wait untill logic level will be stable */
+	udelay(5);
+	for (i = 0; i < num; i++) {
+		ret = gpio_is_active(array[i].gpio);
+		if (ret < 0)
+			return ret;
+
+		id |= ret << i;
+	}
+
+	*val = id;
+
+	return 0;
+}
+EXPORT_SYMBOL(gpio_array_to_id);
+
 static int gpiochip_find_base(int start, int ngpio)
 {
 	int i;
diff --git a/include/gpio.h b/include/gpio.h
index 98c5b93ba2..81beb47309 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -145,6 +145,10 @@ static inline void gpio_free_array(const struct gpio *array, size_t num)
 	/* GPIO can never have been requested */
 	WARN_ON(1);
 }
+static inline int gpio_array_to_id(const struct gpio *array, size_t num, u32 *val)
+{
+	return -EINVAL;
+}
 #else
 int gpio_request(unsigned gpio, const char *label);
 int gpio_find_by_name(const char *name);
@@ -153,6 +157,7 @@ void gpio_free(unsigned gpio);
 int gpio_request_one(unsigned gpio, unsigned long flags, const char *label);
 int gpio_request_array(const struct gpio *array, size_t num);
 void gpio_free_array(const struct gpio *array, size_t num);
+int gpio_array_to_id(const struct gpio *array, size_t num, u32 *val);
 #endif
 
 struct gpio_chip;
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v1 08/11] gpiolib: add gpio_array_to_id helper to get ID out of GPIO array

[Sascha Hauer]

Hi Oleksij,

On Thu, Jul 23, 2020 at 12:33:23PM +0200, Oleksij Rempel wrote:
> Some boards provide a board version and/or ID coded by pull-up/down
> resistors connected to the gpio pins (or pins which can be multiplexed
> at some point as gpio). In this case every one implements own gpio id
> reader function.
> 
> To avoid it, provide the common helper function to extract a value out
> of provided gpio array.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/gpio/gpiolib.c | 28 ++++++++++++++++++++++++++++
>  include/gpio.h         |  5 +++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 27674af54c..2e9150d2ea 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -381,6 +381,34 @@ void gpio_free_array(const struct gpio *array, size_t num)
>  }
>  EXPORT_SYMBOL_GPL(gpio_free_array);
>  
> +int gpio_array_to_id(const struct gpio *array, size_t num, u32 *val)
> +{
> +	u32 id = 0;
> +	int ret, i;
> +
> +	if (num > 32)
> +		return -EOVERFLOW;
> +
> +	ret = gpio_request_array(array, num);
> +	if (ret)
> +		return ret;

This function expects an unrequested array and returns with the array
requested. This is at least inconsistent.

> +
> +	/* Wait untill logic level will be stable */

s/untill/until/

> +	udelay(5);

This delay looks rather arbitrary. Is it really necessary?

> +	for (i = 0; i < num; i++) {
> +		ret = gpio_is_active(array[i].gpio);
> +		if (ret < 0)
> +			return ret;
> +
> +		id |= ret << i;

'ret' is signed. Does this work when i == 31 ?

Sascha


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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v1 08/11] gpiolib: add gpio_array_to_id helper to get ID out of GPIO array

[Oleksij Rempel]

[-- Attachment #1.1: Type: text/plain, Size: 2267 bytes --]

Hi Sascha,

On Mon, Aug 03, 2020 at 10:22:22PM +0200, Sascha Hauer wrote:
> Hi Oleksij,
> 
> On Thu, Jul 23, 2020 at 12:33:23PM +0200, Oleksij Rempel wrote:
> > Some boards provide a board version and/or ID coded by pull-up/down
> > resistors connected to the gpio pins (or pins which can be multiplexed
> > at some point as gpio). In this case every one implements own gpio id
> > reader function.
> > 
> > To avoid it, provide the common helper function to extract a value out
> > of provided gpio array.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  drivers/gpio/gpiolib.c | 28 ++++++++++++++++++++++++++++
> >  include/gpio.h         |  5 +++++
> >  2 files changed, 33 insertions(+)
> > 
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index 27674af54c..2e9150d2ea 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -381,6 +381,34 @@ void gpio_free_array(const struct gpio *array, size_t num)
> >  }
> >  EXPORT_SYMBOL_GPL(gpio_free_array);
> >  
> > +int gpio_array_to_id(const struct gpio *array, size_t num, u32 *val)
> > +{
> > +	u32 id = 0;
> > +	int ret, i;
> > +
> > +	if (num > 32)
> > +		return -EOVERFLOW;
> > +
> > +	ret = gpio_request_array(array, num);
> > +	if (ret)
> > +		return ret;
> 
> This function expects an unrequested array and returns with the array
> requested. This is at least inconsistent.

fixed

> > +
> > +	/* Wait untill logic level will be stable */
> 
> s/untill/until/

fixed

> > +	udelay(5);
> 
> This delay looks rather arbitrary. Is it really necessary?

The vendor reported that otherwise will not always read proper value.

> > +	for (i = 0; i < num; i++) {
> > +		ret = gpio_is_active(array[i].gpio);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		id |= ret << i;
> 
> 'ret' is signed. Does this work when i == 31 ?

yes, it works

Regards,
Oleksij

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 09/11] ARM: protonic-imx6: port protonic specific board code

[Oleksij Rempel]

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 arch/arm/boards/protonic-imx6/Makefile |   1 +
 arch/arm/boards/protonic-imx6/board.c  | 598 +++++++++++++++++++++++++
 2 files changed, 599 insertions(+)
 create mode 100644 arch/arm/boards/protonic-imx6/board.c

diff --git a/arch/arm/boards/protonic-imx6/Makefile b/arch/arm/boards/protonic-imx6/Makefile
index b08c4a93ca..01c7a259e9 100644
--- a/arch/arm/boards/protonic-imx6/Makefile
+++ b/arch/arm/boards/protonic-imx6/Makefile
@@ -1 +1,2 @@
+obj-y += board.o
 lwl-y += lowlevel.o
diff --git a/arch/arm/boards/protonic-imx6/board.c b/arch/arm/boards/protonic-imx6/board.c
new file mode 100644
index 0000000000..7f501ad28b
--- /dev/null
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -0,0 +1,598 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (c) 2020 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+// Copyright (C) 2014 Protonic Holland
+// Copyright (C) 2012 Steffen Trumtrar, Pengutronix
+
+#include <common.h>
+#include <gpio.h>
+#include <mach/imx6.h>
+#include <of_device.h>
+
+#define GPIO_HW_REV_ID  {\
+	{IMX_GPIO_NR(2, 8), GPIOF_DIR_IN | GPIOF_ACTIVE_LOW, "rev_id0"}, \
+	{IMX_GPIO_NR(2, 9), GPIOF_DIR_IN | GPIOF_ACTIVE_LOW, "rev_id1"}, \
+	{IMX_GPIO_NR(2, 10), GPIOF_DIR_IN | GPIOF_ACTIVE_LOW, "rev_id2"} \
+}
+
+#define GPIO_HW_TYPE_ID  {\
+	{IMX_GPIO_NR(2, 11), GPIOF_DIR_IN | GPIOF_ACTIVE_LOW, "hw_id0"}, \
+	{IMX_GPIO_NR(2, 12), GPIOF_DIR_IN | GPIOF_ACTIVE_LOW, "hw_id1"}, \
+	{IMX_GPIO_NR(2, 13), GPIOF_DIR_IN | GPIOF_ACTIVE_LOW, "hw_id2"}, \
+	{IMX_GPIO_NR(2, 14), GPIOF_DIR_IN | GPIOF_ACTIVE_LOW, "hw_id3"}, \
+	{IMX_GPIO_NR(2, 15), GPIOF_DIR_IN | GPIOF_ACTIVE_LOW, "hw_id4"} \
+}
+
+enum {
+	HW_TYPE_PRTI6Q = 0,
+	HW_TYPE_PRTWD2 = 1,
+	HW_TYPE_ALTI6S = 2,
+	HW_TYPE_VICUT1 = 4,
+	HW_TYPE_ALTI6P = 6,
+	HW_TYPE_PRTMVT = 8,
+	HW_TYPE_PRTI6G = 10,
+	HW_TYPE_PRTRVT = 12,
+	HW_TYPE_VICUT2 = 16,
+	HW_TYPE_PLYM2M = 20,
+	HW_TYPE_PRTVT7 = 22,
+	HW_TYPE_LANMCU = 23,
+	HW_TYPE_PLYBAS = 24,
+	HW_TYPE_VICTGO = 28,
+};
+
+struct prt_imx6_priv;
+struct prt_machine_data {
+	unsigned int hw_id;
+	unsigned int hw_rev;
+	unsigned int i2c_addr;
+	unsigned int i2c_adapter;
+	int (*init)(struct prt_imx6_priv *priv);
+};
+
+struct prt_imx6_priv {
+	struct device_d *dev;
+	const struct prt_machine_data *dcfg;
+	unsigned int hw_id;
+	unsigned int hw_rev;
+};
+
+static int prt_imx6_nvmem_link_serial(struct prt_imx6_priv *priv,
+				      struct device_node *root,
+				      struct device_node *nvmem_node)
+{
+	struct device_node *ser_node;
+	phandle nvmem_handle;
+	int ret;
+
+	/* Ignore non-barebox DTs */
+	if (root != of_get_root_node())
+		return 0;
+
+	ser_node = of_find_compatible_node(root, NULL, "barebox,serial");
+	if (!ser_node) {
+		dev_err(priv->dev, "Cant find the serial node\n");
+		return -ENODEV;
+	}
+
+	nvmem_handle = of_node_create_phandle(nvmem_node);
+
+	ret = of_property_write_u32(ser_node, "nvmem-cells", nvmem_handle);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int prt_imx6_nvmem_link_fec(struct prt_imx6_priv *priv,
+				   struct device_node *root,
+				   struct device_node *nvmem_node)
+{
+	struct device_node *fec_node;
+	phandle nvmem_handle;
+	int ret;
+
+	fec_node = of_find_node_by_alias(root, "ethernet0");
+	if (!fec_node) {
+		dev_err(priv->dev, "Cant find the fec node\n");
+		return -ENODEV;
+	}
+
+	nvmem_handle = of_node_create_phandle(nvmem_node);
+
+	ret = of_property_write_u32(fec_node, "nvmem-cells", nvmem_handle);
+	if (ret)
+		return ret;
+
+	ret = of_property_write_string(fec_node, "nvmem-cell-names",
+				       "mac-address");
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static struct device_node *
+prt_imx6_create_nvmem_cells(struct prt_imx6_priv *priv,
+			    struct device_node *nvmem_node,
+			    const char *node_name, size_t offset, size_t size)
+{
+	struct device_node *nvcell_node;
+	int na, ns, len = 0;
+	int ret;
+	u8 *tmp;
+
+	nvcell_node = of_create_node(nvmem_node, node_name);
+	if (!nvcell_node) {
+		dev_err(priv->dev, "Failed to create %s cell\n", node_name);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	na = of_n_addr_cells(nvcell_node);
+	ns = of_n_size_cells(nvcell_node);
+	tmp = xzalloc((na + ns) * 4);
+
+	of_write_number(tmp + len, offset, na);
+	len += na * 4;
+	of_write_number(tmp + len, size, ns);
+	len += ns * 4;
+
+	ret = of_set_property(nvcell_node, "reg", tmp, len, 1);
+	kfree(tmp);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return nvcell_node;
+}
+
+static int prt_imx6_rfid_fixup(struct device_node *root, void *data)
+{
+	struct prt_imx6_priv *priv = data;
+	const struct prt_machine_data *dcfg = priv->dcfg;
+	struct device_node *node, *i2c_node, *tmp_node;
+	char *eeprom_node_name, *alias;
+	int na, ns, len = 0;
+	int ret;
+	u8 *tmp;
+
+	if (!root) {
+		dev_err(priv->dev, "Unable to find the root node\n");
+		return -ENODEV;
+	}
+
+	alias = basprintf("i2c%d", dcfg->i2c_adapter);
+	if (!alias) {
+		ret = -ENOMEM;
+		goto exit_error;
+	}
+
+	i2c_node = of_find_node_by_alias(root, alias);
+	if (!i2c_node) {
+		dev_err(priv->dev, "Unsupported i2c adapter\n");
+		ret = -ENODEV;
+		goto free_alias;
+	}
+
+	eeprom_node_name = basprintf("/eeprom@%x", dcfg->i2c_addr);
+	if (!eeprom_node_name) {
+		ret = -ENOMEM;
+		goto free_alias;
+	}
+
+	node = of_create_node(i2c_node, eeprom_node_name);
+	if (!node) {
+		dev_err(priv->dev, "Filed to create node %s\n",
+			eeprom_node_name);
+		ret = -ENOMEM;
+		goto free_eeprom;
+	}
+
+	ret = of_property_write_string(node, "compatible", "atmel,24c256");
+	if (ret)
+		goto free_eeprom;
+
+	na = of_n_addr_cells(node);
+	ns = of_n_size_cells(node);
+	tmp = xzalloc((na + ns) * 4);
+
+	of_write_number(tmp + len, dcfg->i2c_addr, na);
+	len += na * 4;
+	of_write_number(tmp + len, 0, ns);
+	len += ns * 4;
+
+	ret = of_set_property(node, "reg", tmp, len, 1);
+	kfree(tmp);
+	if (ret)
+		goto free_eeprom;
+
+	ret = of_property_write_u32(node, "#size-cells", 1);
+	if (ret)
+		goto free_eeprom;
+
+	ret = of_property_write_u32(node, "#address-cells", 1);
+	if (ret)
+		goto free_eeprom;
+
+	tmp_node = prt_imx6_create_nvmem_cells(priv, node, "/mac-address@0",
+					       0x6000, 6);
+	if (IS_ERR(tmp_node)) {
+		ret = PTR_ERR(tmp_node);
+		goto free_eeprom;
+	}
+
+	ret = prt_imx6_nvmem_link_fec(priv, root, tmp_node);
+	if (ret)
+		goto free_eeprom;
+
+	tmp_node = prt_imx6_create_nvmem_cells(priv, node, "/serial@6",
+					       0x6006, 10);
+	if (IS_ERR(tmp_node)) {
+		ret = PTR_ERR(tmp_node);
+		goto free_eeprom;
+	}
+
+	ret = prt_imx6_nvmem_link_serial(priv, root, tmp_node);
+	if (ret)
+		goto free_eeprom;
+
+	return 0;
+free_eeprom:
+	kfree(eeprom_node_name);
+free_alias:
+	kfree(alias);
+exit_error:
+	dev_err(priv->dev, "Failed to apply fixup\n");
+	return ret;
+}
+
+static int prt_imx6_init_victgo(struct prt_imx6_priv *priv)
+{
+	int ret = 0;
+
+	/* Bit 1 of HW-REV is pulled low by 2k2, but must be high on some
+	 * revisions
+	 */
+	if (priv->hw_rev & 2) {
+		ret = gpio_direction_output(IMX_GPIO_NR(2, 9), 1);
+		if (ret)
+			dev_err(priv->dev, "Filed to set gpio up\n");
+	}
+
+	return ret;
+}
+
+static const struct prt_machine_data prt_imx6_cfg_alti6p[] = {
+	{
+		.hw_id = HW_TYPE_ALTI6P,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_victgo[] = {
+	{
+		.hw_id = HW_TYPE_VICTGO,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+		.init = prt_imx6_init_victgo,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_vicut1[] = {
+	{
+		.hw_id = HW_TYPE_VICUT1,
+		.hw_rev = 0,
+		.i2c_addr = 0x50,
+		.i2c_adapter = 1,
+	}, {
+		.hw_id = HW_TYPE_VICUT1,
+		.hw_rev = 1,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = HW_TYPE_VICUT2,
+		.hw_rev = 1,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_vicut1q[] = {
+	{
+		.hw_id = HW_TYPE_VICUT1,
+		.hw_rev = 0,
+		.i2c_addr = 0x50,
+		.i2c_adapter = 1,
+	}, {
+		.hw_id = HW_TYPE_VICUT1,
+		.hw_rev = 1,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = HW_TYPE_VICUT2,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = HW_TYPE_VICUT2,
+		.hw_rev = 1,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_vicutp[] = {
+	{
+		.hw_id = HW_TYPE_VICUT2,
+		.hw_rev = 1,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_lanmcu[] = {
+	{
+		.hw_id = HW_TYPE_LANMCU,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_plybas[] = {
+	{
+		.hw_id = HW_TYPE_PLYBAS,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_plym2m[] = {
+	{
+		.hw_id = HW_TYPE_PLYM2M,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_prti6g[] = {
+	{
+		.hw_id = HW_TYPE_PRTI6G,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_prti6q[] = {
+	{
+		.hw_id = HW_TYPE_PRTI6Q,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 2,
+	}, {
+		.hw_id = HW_TYPE_PRTI6Q,
+		.hw_rev = 1,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_prtmvt[] = {
+	{
+		.hw_id = HW_TYPE_PRTMVT,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_prtrvt[] = {
+	{
+		.hw_id = HW_TYPE_PRTRVT,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_prtvt7[] = {
+	{
+		.hw_id = HW_TYPE_PRTVT7,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_prtwd2[] = {
+	{
+		.hw_id = HW_TYPE_PRTWD2,
+		.hw_rev = 0,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0,
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static const struct prt_machine_data prt_imx6_cfg_prtwd3[] = {
+	{
+		.hw_id = HW_TYPE_PRTWD2,
+		.hw_rev = 2,
+		.i2c_addr = 0x51,
+		.i2c_adapter = 0
+	}, {
+		.hw_id = UINT_MAX
+	},
+};
+
+static int prt_imx6_get_id(struct prt_imx6_priv *priv)
+{
+	struct gpio gpios_type[] = GPIO_HW_TYPE_ID;
+	struct gpio gpios_rev[] = GPIO_HW_REV_ID;
+	int ret;
+
+	ret = gpio_array_to_id(gpios_type, ARRAY_SIZE(gpios_type), &priv->hw_id);
+	if (ret)
+		goto exit_get_id;
+
+	ret = gpio_array_to_id(gpios_rev, ARRAY_SIZE(gpios_rev), &priv->hw_rev);
+	if (ret)
+		goto exit_get_id;
+
+	return 0;
+exit_get_id:
+	dev_err(priv->dev, "Failed to read gpio ID\n");
+	return ret;
+}
+
+static int prt_imx6_get_dcfg(struct prt_imx6_priv *priv)
+{
+	const struct prt_machine_data *dcfg, *found = NULL;
+	int ret;
+
+	dcfg = of_device_get_match_data(priv->dev);
+	if (!dcfg) {
+		ret = -EINVAL;
+		goto exit_get_dcfg;
+	}
+
+	for (; dcfg->hw_id != UINT_MAX; dcfg++) {
+		if (dcfg->hw_id != priv->hw_id)
+			continue;
+		if (dcfg->hw_rev > priv->hw_rev)
+			break;
+		found = dcfg;
+	}
+
+	if (!found) {
+		ret = -ENODEV;
+		goto exit_get_dcfg;
+	}
+
+	priv->dcfg = found;
+
+	return 0;
+exit_get_dcfg:
+	dev_err(priv->dev, "Failed to get dcfg\n");
+	return ret;
+}
+
+static int prt_imx6_probe(struct device_d *dev)
+{
+	struct prt_imx6_priv *priv;
+	struct param_d *p;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto exit_probe;
+	}
+
+	priv->dev = dev;
+
+	pr_info("Detected machine type: %s\n",
+		of_device_get_match_compatible(priv->dev));
+
+	ret = prt_imx6_get_id(priv);
+	if (ret)
+		goto free_priv;
+
+	pr_info("  HW type:     %d\n", priv->hw_id);
+	pr_info("  HW revision: %d\n", priv->hw_rev);
+
+	ret = prt_imx6_get_dcfg(priv);
+	if (ret)
+		goto free_priv;
+
+	if (priv->dcfg->init) {
+		ret = priv->dcfg->init(priv);
+		if (ret)
+			goto free_priv;
+	}
+
+	p = dev_add_param_uint32_ro(dev, "boardrev", &priv->hw_rev, "%u");
+	if (IS_ERR(p)) {
+		ret = PTR_ERR(p);
+		goto free_priv;
+	}
+
+	p = dev_add_param_uint32_ro(dev, "boardid", &priv->hw_id, "%u");
+	if (IS_ERR(p)) {
+		ret = PTR_ERR(p);
+		goto free_priv;
+	}
+
+	ret = prt_imx6_rfid_fixup(of_get_root_node(), priv);
+	if (ret)
+		goto free_priv;
+
+	ret = of_register_fixup(prt_imx6_rfid_fixup, priv);
+	if (ret) {
+		dev_err(dev, "Failed to register fixup\n");
+		goto free_priv;
+	}
+
+	return 0;
+free_priv:
+	kfree(priv);
+exit_probe:
+	dev_err(dev, "Probe filed\n");
+	return ret;
+}
+
+static const struct of_device_id prt_imx6_of_match[] = {
+	{ .compatible = "alt,alti6p", .data = &prt_imx6_cfg_alti6p },
+	{ .compatible = "kvg,victgo", .data = &prt_imx6_cfg_victgo },
+	{ .compatible = "kvg,vicut1", .data = &prt_imx6_cfg_vicut1 },
+	{ .compatible = "kvg,vicut1q", .data = &prt_imx6_cfg_vicut1q },
+	{ .compatible = "kvg,vicutp", .data = &prt_imx6_cfg_vicutp },
+	{ .compatible = "lan,lanmcu", .data = &prt_imx6_cfg_lanmcu },
+	{ .compatible = "ply,plybas", .data = &prt_imx6_cfg_plybas },
+	{ .compatible = "ply,plym2m", .data = &prt_imx6_cfg_plym2m },
+	{ .compatible = "prt,prti6g", .data = &prt_imx6_cfg_prti6g },
+	{ .compatible = "prt,prti6q", .data = &prt_imx6_cfg_prti6q },
+	{ .compatible = "prt,prtmvt", .data = &prt_imx6_cfg_prtmvt },
+	{ .compatible = "prt,prtrvt", .data = &prt_imx6_cfg_prtrvt },
+	{ .compatible = "prt,prtvt7", .data = &prt_imx6_cfg_prtvt7 },
+	{ .compatible = "prt,prtwd2", .data = &prt_imx6_cfg_prtwd2 },
+	{ .compatible = "prt,prtwd3", .data = &prt_imx6_cfg_prtwd3 },
+	{ /* sentinel */ },
+};
+
+static struct driver_d prt_imx6_board_driver = {
+	.name = "board-protonic-imx6",
+	.probe = prt_imx6_probe,
+	.of_compatible = DRV_OF_COMPAT(prt_imx6_of_match),
+};
+postcore_platform_driver(prt_imx6_board_driver);
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v1 09/11] ARM: protonic-imx6: port protonic specific board code

[Sascha Hauer]

On Thu, Jul 23, 2020 at 12:33:24PM +0200, Oleksij Rempel wrote:
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  arch/arm/boards/protonic-imx6/Makefile |   1 +
>  arch/arm/boards/protonic-imx6/board.c  | 598 +++++++++++++++++++++++++
>  2 files changed, 599 insertions(+)
>  create mode 100644 arch/arm/boards/protonic-imx6/board.c
> 
> +static int prt_imx6_nvmem_link_fec(struct prt_imx6_priv *priv,
> +				   struct device_node *root,
> +				   struct device_node *nvmem_node)
> +{
> +	struct device_node *fec_node;
> +	phandle nvmem_handle;
> +	int ret;
> +
> +	fec_node = of_find_node_by_alias(root, "ethernet0");
> +	if (!fec_node) {
> +		dev_err(priv->dev, "Cant find the fec node\n");
> +		return -ENODEV;
> +	}
> +
> +	nvmem_handle = of_node_create_phandle(nvmem_node);
> +
> +	ret = of_property_write_u32(fec_node, "nvmem-cells", nvmem_handle);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_write_string(fec_node, "nvmem-cell-names",
> +				       "mac-address");
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}

barebox will already write the "mac-address" property to the fec node.
Isn't this enough?

> +
> +static struct device_node *
> +prt_imx6_create_nvmem_cells(struct prt_imx6_priv *priv,
> +			    struct device_node *nvmem_node,
> +			    const char *node_name, size_t offset, size_t size)
> +{
> +	struct device_node *nvcell_node;
> +	int na, ns, len = 0;
> +	int ret;
> +	u8 *tmp;
> +
> +	nvcell_node = of_create_node(nvmem_node, node_name);
> +	if (!nvcell_node) {
> +		dev_err(priv->dev, "Failed to create %s cell\n", node_name);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	na = of_n_addr_cells(nvcell_node);
> +	ns = of_n_size_cells(nvcell_node);
> +	tmp = xzalloc((na + ns) * 4);
> +
> +	of_write_number(tmp + len, offset, na);
> +	len += na * 4;
> +	of_write_number(tmp + len, size, ns);
> +	len += ns * 4;
> +
> +	ret = of_set_property(nvcell_node, "reg", tmp, len, 1);
> +	kfree(tmp);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return nvcell_node;
> +}
> +
> +static int prt_imx6_rfid_fixup(struct device_node *root, void *data)
> +{
> +	struct prt_imx6_priv *priv = data;
> +	const struct prt_machine_data *dcfg = priv->dcfg;
> +	struct device_node *node, *i2c_node, *tmp_node;
> +	char *eeprom_node_name, *alias;
> +	int na, ns, len = 0;
> +	int ret;
> +	u8 *tmp;
> +
> +	if (!root) {
> +		dev_err(priv->dev, "Unable to find the root node\n");
> +		return -ENODEV;
> +	}
> +
> +	alias = basprintf("i2c%d", dcfg->i2c_adapter);
> +	if (!alias) {
> +		ret = -ENOMEM;
> +		goto exit_error;
> +	}
> +
> +	i2c_node = of_find_node_by_alias(root, alias);
> +	if (!i2c_node) {
> +		dev_err(priv->dev, "Unsupported i2c adapter\n");
> +		ret = -ENODEV;
> +		goto free_alias;
> +	}
> +
> +	eeprom_node_name = basprintf("/eeprom@%x", dcfg->i2c_addr);
> +	if (!eeprom_node_name) {
> +		ret = -ENOMEM;
> +		goto free_alias;
> +	}
> +
> +	node = of_create_node(i2c_node, eeprom_node_name);
> +	if (!node) {
> +		dev_err(priv->dev, "Filed to create node %s\n",
> +			eeprom_node_name);

s/filed/failed/, multiple times in this patch.

> +		ret = -ENOMEM;
> +		goto free_eeprom;
> +	}
> +
> +	ret = of_property_write_string(node, "compatible", "atmel,24c256");
> +	if (ret)
> +		goto free_eeprom;
> +
> +	na = of_n_addr_cells(node);
> +	ns = of_n_size_cells(node);
> +	tmp = xzalloc((na + ns) * 4);
> +
> +	of_write_number(tmp + len, dcfg->i2c_addr, na);
> +	len += na * 4;
> +	of_write_number(tmp + len, 0, ns);
> +	len += ns * 4;
> +
> +	ret = of_set_property(node, "reg", tmp, len, 1);
> +	kfree(tmp);
> +	if (ret)
> +		goto free_eeprom;
> +
> +	ret = of_property_write_u32(node, "#size-cells", 1);
> +	if (ret)
> +		goto free_eeprom;
> +
> +	ret = of_property_write_u32(node, "#address-cells", 1);
> +	if (ret)
> +		goto free_eeprom;
> +
> +	tmp_node = prt_imx6_create_nvmem_cells(priv, node, "/mac-address@0",
> +					       0x6000, 6);
> +	if (IS_ERR(tmp_node)) {
> +		ret = PTR_ERR(tmp_node);
> +		goto free_eeprom;
> +	}
> +
> +	ret = prt_imx6_nvmem_link_fec(priv, root, tmp_node);
> +	if (ret)
> +		goto free_eeprom;
> +
> +	tmp_node = prt_imx6_create_nvmem_cells(priv, node, "/serial@6",
> +					       0x6006, 10);
> +	if (IS_ERR(tmp_node)) {
> +		ret = PTR_ERR(tmp_node);
> +		goto free_eeprom;
> +	}
> +
> +	ret = prt_imx6_nvmem_link_serial(priv, root, tmp_node);
> +	if (ret)
> +		goto free_eeprom;
> +
> +	return 0;
> +free_eeprom:
> +	kfree(eeprom_node_name);
> +free_alias:
> +	kfree(alias);
> +exit_error:
> +	dev_err(priv->dev, "Failed to apply fixup\n");

It's always nice to print the error code along with such a message.

> +static int prt_imx6_probe(struct device_d *dev)
> +{
> +	struct prt_imx6_priv *priv;
> +	struct param_d *p;
> +	int ret;
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		ret = -ENOMEM;
> +		goto exit_probe;
> +	}
> +
> +	priv->dev = dev;
> +
> +	pr_info("Detected machine type: %s\n",
> +		of_device_get_match_compatible(priv->dev));
> +
> +	ret = prt_imx6_get_id(priv);
> +	if (ret)
> +		goto free_priv;
> +
> +	pr_info("  HW type:     %d\n", priv->hw_id);
> +	pr_info("  HW revision: %d\n", priv->hw_rev);
> +
> +	ret = prt_imx6_get_dcfg(priv);
> +	if (ret)
> +		goto free_priv;
> +
> +	if (priv->dcfg->init) {
> +		ret = priv->dcfg->init(priv);
> +		if (ret)
> +			goto free_priv;
> +	}
> +
> +	p = dev_add_param_uint32_ro(dev, "boardrev", &priv->hw_rev, "%u");
> +	if (IS_ERR(p)) {
> +		ret = PTR_ERR(p);
> +		goto free_priv;
> +	}
> +
> +	p = dev_add_param_uint32_ro(dev, "boardid", &priv->hw_id, "%u");
> +	if (IS_ERR(p)) {
> +		ret = PTR_ERR(p);
> +		goto free_priv;
> +	}
> +
> +	ret = prt_imx6_rfid_fixup(of_get_root_node(), priv);
> +	if (ret)
> +		goto free_priv;
> +
> +	ret = of_register_fixup(prt_imx6_rfid_fixup, priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to register fixup\n");
> +		goto free_priv;
> +	}
> +
> +	return 0;
> +free_priv:
> +	kfree(priv);
> +exit_probe:
> +	dev_err(dev, "Probe filed\n");

The caller should print this error already.

Sascha

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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v1 09/11] ARM: protonic-imx6: port protonic specific board code

[Oleksij Rempel]

[-- Attachment #1.1: Type: text/plain, Size: 7621 bytes --]

On Mon, Aug 03, 2020 at 10:49:23PM +0200, Sascha Hauer wrote:
> On Thu, Jul 23, 2020 at 12:33:24PM +0200, Oleksij Rempel wrote:
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  arch/arm/boards/protonic-imx6/Makefile |   1 +
> >  arch/arm/boards/protonic-imx6/board.c  | 598 +++++++++++++++++++++++++
> >  2 files changed, 599 insertions(+)
> >  create mode 100644 arch/arm/boards/protonic-imx6/board.c
> > 
> > +static int prt_imx6_nvmem_link_fec(struct prt_imx6_priv *priv,
> > +				   struct device_node *root,
> > +				   struct device_node *nvmem_node)
> > +{
> > +	struct device_node *fec_node;
> > +	phandle nvmem_handle;
> > +	int ret;
> > +
> > +	fec_node = of_find_node_by_alias(root, "ethernet0");
> > +	if (!fec_node) {
> > +		dev_err(priv->dev, "Cant find the fec node\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	nvmem_handle = of_node_create_phandle(nvmem_node);
> > +
> > +	ret = of_property_write_u32(fec_node, "nvmem-cells", nvmem_handle);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = of_property_write_string(fec_node, "nvmem-cell-names",
> > +				       "mac-address");
> > +	if (ret)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> 
> barebox will already write the "mac-address" property to the fec node.
> Isn't this enough?

My motivation for doing this is following:
- we can't make sure i2c is already probed at this point
- if we will decide to speed up the boot, it will be more effective to
  let linux read the eeprom
- usually, vendors tend to avoid updating the bootloader in the field.
  So, it is better to provide all needed information the devicetree to
  let the linux make the right choice.
- in long term, even in barebox we will need to work deferred probe. So,
  it is better to prepare the devicetree and let the system do all
  needed work.

> > +
> > +static struct device_node *
> > +prt_imx6_create_nvmem_cells(struct prt_imx6_priv *priv,
> > +			    struct device_node *nvmem_node,
> > +			    const char *node_name, size_t offset, size_t size)
> > +{
> > +	struct device_node *nvcell_node;
> > +	int na, ns, len = 0;
> > +	int ret;
> > +	u8 *tmp;
> > +
> > +	nvcell_node = of_create_node(nvmem_node, node_name);
> > +	if (!nvcell_node) {
> > +		dev_err(priv->dev, "Failed to create %s cell\n", node_name);
> > +		return ERR_PTR(-ENOMEM);
> > +	}
> > +
> > +	na = of_n_addr_cells(nvcell_node);
> > +	ns = of_n_size_cells(nvcell_node);
> > +	tmp = xzalloc((na + ns) * 4);
> > +
> > +	of_write_number(tmp + len, offset, na);
> > +	len += na * 4;
> > +	of_write_number(tmp + len, size, ns);
> > +	len += ns * 4;
> > +
> > +	ret = of_set_property(nvcell_node, "reg", tmp, len, 1);
> > +	kfree(tmp);
> > +	if (ret)
> > +		return ERR_PTR(ret);
> > +
> > +	return nvcell_node;
> > +}
> > +
> > +static int prt_imx6_rfid_fixup(struct device_node *root, void *data)
> > +{
> > +	struct prt_imx6_priv *priv = data;
> > +	const struct prt_machine_data *dcfg = priv->dcfg;
> > +	struct device_node *node, *i2c_node, *tmp_node;
> > +	char *eeprom_node_name, *alias;
> > +	int na, ns, len = 0;
> > +	int ret;
> > +	u8 *tmp;
> > +
> > +	if (!root) {
> > +		dev_err(priv->dev, "Unable to find the root node\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	alias = basprintf("i2c%d", dcfg->i2c_adapter);
> > +	if (!alias) {
> > +		ret = -ENOMEM;
> > +		goto exit_error;
> > +	}
> > +
> > +	i2c_node = of_find_node_by_alias(root, alias);
> > +	if (!i2c_node) {
> > +		dev_err(priv->dev, "Unsupported i2c adapter\n");
> > +		ret = -ENODEV;
> > +		goto free_alias;
> > +	}
> > +
> > +	eeprom_node_name = basprintf("/eeprom@%x", dcfg->i2c_addr);
> > +	if (!eeprom_node_name) {
> > +		ret = -ENOMEM;
> > +		goto free_alias;
> > +	}
> > +
> > +	node = of_create_node(i2c_node, eeprom_node_name);
> > +	if (!node) {
> > +		dev_err(priv->dev, "Filed to create node %s\n",
> > +			eeprom_node_name);
> 
> s/filed/failed/, multiple times in this patch.

fixed

> > +		ret = -ENOMEM;
> > +		goto free_eeprom;
> > +	}
> > +
> > +	ret = of_property_write_string(node, "compatible", "atmel,24c256");
> > +	if (ret)
> > +		goto free_eeprom;
> > +
> > +	na = of_n_addr_cells(node);
> > +	ns = of_n_size_cells(node);
> > +	tmp = xzalloc((na + ns) * 4);
> > +
> > +	of_write_number(tmp + len, dcfg->i2c_addr, na);
> > +	len += na * 4;
> > +	of_write_number(tmp + len, 0, ns);
> > +	len += ns * 4;
> > +
> > +	ret = of_set_property(node, "reg", tmp, len, 1);
> > +	kfree(tmp);
> > +	if (ret)
> > +		goto free_eeprom;
> > +
> > +	ret = of_property_write_u32(node, "#size-cells", 1);
> > +	if (ret)
> > +		goto free_eeprom;
> > +
> > +	ret = of_property_write_u32(node, "#address-cells", 1);
> > +	if (ret)
> > +		goto free_eeprom;
> > +
> > +	tmp_node = prt_imx6_create_nvmem_cells(priv, node, "/mac-address@0",
> > +					       0x6000, 6);
> > +	if (IS_ERR(tmp_node)) {
> > +		ret = PTR_ERR(tmp_node);
> > +		goto free_eeprom;
> > +	}
> > +
> > +	ret = prt_imx6_nvmem_link_fec(priv, root, tmp_node);
> > +	if (ret)
> > +		goto free_eeprom;
> > +
> > +	tmp_node = prt_imx6_create_nvmem_cells(priv, node, "/serial@6",
> > +					       0x6006, 10);
> > +	if (IS_ERR(tmp_node)) {
> > +		ret = PTR_ERR(tmp_node);
> > +		goto free_eeprom;
> > +	}
> > +
> > +	ret = prt_imx6_nvmem_link_serial(priv, root, tmp_node);
> > +	if (ret)
> > +		goto free_eeprom;
> > +
> > +	return 0;
> > +free_eeprom:
> > +	kfree(eeprom_node_name);
> > +free_alias:
> > +	kfree(alias);
> > +exit_error:
> > +	dev_err(priv->dev, "Failed to apply fixup\n");
> 
> It's always nice to print the error code along with such a message.

fixed

> > +static int prt_imx6_probe(struct device_d *dev)
> > +{
> > +	struct prt_imx6_priv *priv;
> > +	struct param_d *p;
> > +	int ret;
> > +
> > +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > +	if (!priv) {
> > +		ret = -ENOMEM;
> > +		goto exit_probe;
> > +	}
> > +
> > +	priv->dev = dev;
> > +
> > +	pr_info("Detected machine type: %s\n",
> > +		of_device_get_match_compatible(priv->dev));
> > +
> > +	ret = prt_imx6_get_id(priv);
> > +	if (ret)
> > +		goto free_priv;
> > +
> > +	pr_info("  HW type:     %d\n", priv->hw_id);
> > +	pr_info("  HW revision: %d\n", priv->hw_rev);
> > +
> > +	ret = prt_imx6_get_dcfg(priv);
> > +	if (ret)
> > +		goto free_priv;
> > +
> > +	if (priv->dcfg->init) {
> > +		ret = priv->dcfg->init(priv);
> > +		if (ret)
> > +			goto free_priv;
> > +	}
> > +
> > +	p = dev_add_param_uint32_ro(dev, "boardrev", &priv->hw_rev, "%u");
> > +	if (IS_ERR(p)) {
> > +		ret = PTR_ERR(p);
> > +		goto free_priv;
> > +	}
> > +
> > +	p = dev_add_param_uint32_ro(dev, "boardid", &priv->hw_id, "%u");
> > +	if (IS_ERR(p)) {
> > +		ret = PTR_ERR(p);
> > +		goto free_priv;
> > +	}
> > +
> > +	ret = prt_imx6_rfid_fixup(of_get_root_node(), priv);
> > +	if (ret)
> > +		goto free_priv;
> > +
> > +	ret = of_register_fixup(prt_imx6_rfid_fixup, priv);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to register fixup\n");
> > +		goto free_priv;
> > +	}
> > +
> > +	return 0;
> > +free_priv:
> > +	kfree(priv);
> > +exit_probe:
> > +	dev_err(dev, "Probe filed\n");

fixed

Regards,
Oleksij

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 10/11] of: add barebox-serial driver

[Oleksij Rempel]

Provide a driver which should act as nvmem consumer
for board serial number information.

To make use of this driver, DTS should contain a serial
node with compatibe "barebox,serial" and nvmem-cell-names
"serial-number":

...
	chosen {
		serial {
			compatible = "barebox,serial";
			nvmem-cell-names = "serial-number";
			nvmem-cells = &some_provider;
		};
	};
...

The driver will read nvmem cell on probe and register
a fixup for the kernel devicetree.

This fixup will create a "/serial-number" property, which is
used by current kernel:
https://elixir.bootlin.com/linux/v5.7/source/arch/arm/kernel/setup.c#L941

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/of/Makefile         |   2 +-
 drivers/of/barebox_serial.c | 111 ++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 drivers/of/barebox_serial.c

diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index b6847752d2..b4a4c36b2a 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -5,5 +5,5 @@ obj-$(CONFIG_OF_PCI) += of_pci.o
 obj-y += partition.o
 obj-y += of_net.o
 obj-$(CONFIG_MTD) += of_mtd.o
-obj-$(CONFIG_OF_BAREBOX_DRIVERS) += barebox.o
+obj-$(CONFIG_OF_BAREBOX_DRIVERS) += barebox.o barebox_serial.o
 obj-$(CONFIG_OF_OVERLAY) += overlay.o resolver.o of_firmware.o
diff --git a/drivers/of/barebox_serial.c b/drivers/of/barebox_serial.c
new file mode 100644
index 0000000000..40efb731d3
--- /dev/null
+++ b/drivers/of/barebox_serial.c
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (c) 2020 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+
+#include <common.h>
+#include <init.h>
+#include <linux/nvmem-consumer.h>
+
+struct bb_ser_priv {
+	struct device_d *dev;
+	char *ser;
+};
+
+static int bb_ser_fixup(struct device_node *root, void *data)
+{
+	struct bb_ser_priv *priv = data;
+	int ret;
+
+	ret = of_property_write_string(root, "serial-number", priv->ser);
+	if (ret)
+		dev_err(priv->dev, "Failed to set /serial-number\n");
+
+	return ret;
+}
+
+static int bb_ser_of_get_nvmem(struct bb_ser_priv *priv)
+{
+	struct nvmem_cell *cell;
+	size_t len;
+	char *ser;
+
+	cell = nvmem_cell_get(priv->dev, "serial-number");
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	ser = nvmem_cell_read(cell, &len);
+	nvmem_cell_put(cell);
+
+	if (IS_ERR(ser))
+		return PTR_ERR(ser);
+
+	/* check if serial number is zero terminated */
+	if (ser[len - 1] != 0x0) {
+		char *tmp;
+
+		tmp = kzalloc(len + 1, GFP_KERNEL);
+		if (!tmp) {
+			kfree(ser);
+			return -ENOMEM;
+		}
+
+		memcpy(tmp, ser, len);
+		tmp[len] = 0x0;
+		kfree(ser);
+		ser = tmp;
+	}
+
+	priv->ser = ser;
+
+	return 0;
+}
+
+static int bb_ser_probe(struct device_d *dev)
+{
+	struct bb_ser_priv *priv;
+	struct param_d *p;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+
+	ret = bb_ser_of_get_nvmem(priv);
+	if (ret) {
+		dev_err(priv->dev, "Failed to read nvmem\n");
+		goto free_priv;
+	}
+
+	p = dev_add_param_string_fixed(dev, "serial-number", priv->ser);
+	if (IS_ERR(p)) {
+		dev_err(priv->dev, "Failed to set param_string\n");
+		ret = PTR_ERR(p);
+		goto free_ser;
+	}
+
+	ret = of_register_fixup(bb_ser_fixup, priv);
+	if (ret) {
+		dev_err(priv->dev, "Failed to register fixup\n");
+		goto free_ser;
+	}
+
+	return 0;
+free_ser:
+	kfree(priv->ser);
+free_priv:
+	kfree(priv);
+	return ret;
+}
+
+static const struct of_device_id bb_ser_of_match[] = {
+	{ .compatible = "barebox,serial", },
+	{ /* sentinel */ },
+};
+
+static struct driver_d bb_ser_driver = {
+	.name = "barebox-serial",
+	.probe = bb_ser_probe,
+	.of_compatible = DRV_OF_COMPAT(bb_ser_of_match),
+};
+device_platform_driver(bb_ser_driver);
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 11/11] ARM: dts: imx6q-prti6q: add barebox.serial node

[Oleksij Rempel]

Add serial node provider

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 arch/arm/dts/imx6q-prti6q.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/dts/imx6q-prti6q.dts b/arch/arm/dts/imx6q-prti6q.dts
index 76bb4d53d3..67e7f63979 100644
--- a/arch/arm/dts/imx6q-prti6q.dts
+++ b/arch/arm/dts/imx6q-prti6q.dts
@@ -18,6 +18,11 @@
 			compatible = "barebox,environment";
 			device-path = &ecspi1, "partname:env";
 		};
+		serial {
+			compatible = "barebox,serial";
+			nvmem-cell-names = "serial-number";
+			/* nvmem-cells will added board code */
+		};
 	};
 
 	memory@10000000 {
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v1 11/11] ARM: dts: imx6q-prti6q: add barebox.serial node

[Sascha Hauer]

On Thu, Jul 23, 2020 at 12:33:26PM +0200, Oleksij Rempel wrote:
> Add serial node provider
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  arch/arm/dts/imx6q-prti6q.dts | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/dts/imx6q-prti6q.dts b/arch/arm/dts/imx6q-prti6q.dts
> index 76bb4d53d3..67e7f63979 100644
> --- a/arch/arm/dts/imx6q-prti6q.dts
> +++ b/arch/arm/dts/imx6q-prti6q.dts
> @@ -18,6 +18,11 @@
>  			compatible = "barebox,environment";
>  			device-path = &ecspi1, "partname:env";
>  		};
> +		serial {
> +			compatible = "barebox,serial";
> +			nvmem-cell-names = "serial-number";
> +			/* nvmem-cells will added board code */

You probably mean "nvmem-cells will be added by board code".

You need board code to fully describe the device which triggers a
freshly written driver which puts the found serial number as
/serial-number into dt. Is this really worth it? I'd just read the
serial number in board code and put it into dt, maybe add some helper
function to set the right property from a given string.

BTW you seem to be lucky that the i2c eeprom driver probes before the
barebox,serial driver, otherwise I think this doesn't work.

Sascha



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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v1 11/11] ARM: dts: imx6q-prti6q: add barebox.serial node

[Oleksij Rempel]

[-- Attachment #1.1: Type: text/plain, Size: 2144 bytes --]

On Mon, Aug 03, 2020 at 11:12:59PM +0200, Sascha Hauer wrote:
> On Thu, Jul 23, 2020 at 12:33:26PM +0200, Oleksij Rempel wrote:
> > Add serial node provider
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  arch/arm/dts/imx6q-prti6q.dts | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/arch/arm/dts/imx6q-prti6q.dts b/arch/arm/dts/imx6q-prti6q.dts
> > index 76bb4d53d3..67e7f63979 100644
> > --- a/arch/arm/dts/imx6q-prti6q.dts
> > +++ b/arch/arm/dts/imx6q-prti6q.dts
> > @@ -18,6 +18,11 @@
> >  			compatible = "barebox,environment";
> >  			device-path = &ecspi1, "partname:env";
> >  		};
> > +		serial {
> > +			compatible = "barebox,serial";
> > +			nvmem-cell-names = "serial-number";
> > +			/* nvmem-cells will added board code */
> 
> You probably mean "nvmem-cells will be added by board code".
> 
> You need board code to fully describe the device which triggers a
> freshly written driver which puts the found serial number as
> /serial-number into dt. Is this really worth it? I'd just read the
> serial number in board code and put it into dt, maybe add some helper
> function to set the right property from a given string.

If eeprom should only be used to read serial-number - yes. But this
RFID-eeprom provides more space and functionality. Why not make it
available for barebox and linux to be able to do more things with it?

> BTW you seem to be lucky that the i2c eeprom driver probes before the
> barebox,serial driver, otherwise I think this doesn't work.

We would have the same issue with the board code. It seems to work just for
this particular barebox version and i2c controller was accidentally probed
before board code.

I think it is time to talk about deferred probe patches made by Lucas ;)

Regards,
Oleksij
-- 
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 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v1 11/11] ARM: dts: imx6q-prti6q: add barebox.serial node

[Sascha Hauer]

On Tue, Aug 04, 2020 at 07:48:29AM +0200, Oleksij Rempel wrote:
> On Mon, Aug 03, 2020 at 11:12:59PM +0200, Sascha Hauer wrote:
> > On Thu, Jul 23, 2020 at 12:33:26PM +0200, Oleksij Rempel wrote:
> > > Add serial node provider
> > > 
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > ---
> > >  arch/arm/dts/imx6q-prti6q.dts | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/arch/arm/dts/imx6q-prti6q.dts b/arch/arm/dts/imx6q-prti6q.dts
> > > index 76bb4d53d3..67e7f63979 100644
> > > --- a/arch/arm/dts/imx6q-prti6q.dts
> > > +++ b/arch/arm/dts/imx6q-prti6q.dts
> > > @@ -18,6 +18,11 @@
> > >  			compatible = "barebox,environment";
> > >  			device-path = &ecspi1, "partname:env";
> > >  		};
> > > +		serial {
> > > +			compatible = "barebox,serial";
> > > +			nvmem-cell-names = "serial-number";
> > > +			/* nvmem-cells will added board code */
> > 
> > You probably mean "nvmem-cells will be added by board code".
> > 
> > You need board code to fully describe the device which triggers a
> > freshly written driver which puts the found serial number as
> > /serial-number into dt. Is this really worth it? I'd just read the
> > serial number in board code and put it into dt, maybe add some helper
> > function to set the right property from a given string.
> 
> If eeprom should only be used to read serial-number - yes. But this
> RFID-eeprom provides more space and functionality. Why not make it
> available for barebox and linux to be able to do more things with it?

Yes, the EEPROM should be available to Linux and barebox, I do not
disagree here. I was referring to the way you extract the serial number
here. You already have the information where the serial number is in
board code. So instead of putting it into the device tree where some
driver picks it up you could use that information directly.

Sascha

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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox