On 16/04/2019 13:05, Petr Štetiar wrote:
From: John Crispin <john@xxxxxxxxxxx> Many embedded devices have information such as MAC addresses stored inside MTD devices. This patch allows us to add a property inside a node describing a network interface. The new property points at a MTD partition with an offset where the MAC address can be found. This patch has originated in OpenWrt some time ago, so in order to consider usefulness of this patch, here are some real-world numbers which hopefully speak for themselves: * mtd-mac-address used 497 times in 357 device tree files * mtd-mac-address-increment used 74 times in 58 device tree files * mtd-mac-address-increment-byte used 1 time in 1 device tree file Signed-off-by: John Crispin <john@xxxxxxxxxxx> Signed-off-by: Felix Fietkau <nbd@xxxxxxxx> [cleanup of the patch for upstream submission] Signed-off-by: Petr Štetiar <ynezz@xxxxxxx> ---
[snip]
+static const void *of_get_mac_address_mtd(struct device_node *np) +{ +#ifdef CONFIG_MTD + void *addr; + size_t retlen; + int size, ret; + u8 mac[ETH_ALEN]; + phandle phandle; + const char *part; + const __be32 *list; + struct mtd_info *mtd; + struct property *prop; + u32 mac_inc = 0; + u32 inc_idx = ETH_ALEN-1; + struct device_node *mtd_np = NULL;
Reverse christmas tree would look a bit nicer here.
+ + list = of_get_property(np, "mtd-mac-address", &size); + if (!list || (size != (2 * sizeof(*list)))) + return NULL; + + phandle = be32_to_cpup(list++); + if (phandle) + mtd_np = of_find_node_by_phandle(phandle); + + if (!mtd_np) + return NULL; + + part = of_get_property(mtd_np, "label", NULL); + if (!part) + part = mtd_np->name; + + mtd = get_mtd_device_nm(part); + if (IS_ERR(mtd)) + return NULL; + + ret = mtd_read(mtd, be32_to_cpup(list), ETH_ALEN, &retlen, mac); + put_mtd_device(mtd); + + of_property_read_u32(np, "mtd-mac-address-increment-byte", &inc_idx);
of_property_read_u8() would probably be good here since this can't be bigger than 5 anyway.
+ if (inc_idx > ETH_ALEN-1) + return NULL; > + if (!of_property_read_u32(np, "mtd-mac-address-increment", &mac_inc)) + mac[inc_idx] += mac_inc;
If I use a number greater than and included 128; this will cause a roll over, should this be range checked? Similarly, using of_property_read_u8() might be a better fit?
Other than those, LGTM -- Florian