FIRMWARE: bcm47xx_nvram: Replace mac address parsing

PATCH In review Andy Shevchenko 3 participants 4 messages Dec 21, 2017 – Mar 8, 2018

Subsystems (4)

Networking Network drivers MIPS Broadcom nvram driver

lore

Share & tools

lore

Andy Shevchenko

author

2017-12-21 14:41 UTC #0 lore raw

PATCH v2 1/1 FIRMWARE: bcm47xx_nvram: Replace mac address parsing

Replace sscanf() with mac_pton().

Signed-off-by Andy Shevchenko <[email protected]>

---
- use negative condition to be consistent with the rest code

 drivers/firmware/broadcom/Kconfig         |  1 +
 drivers/firmware/broadcom/bcm47xx_sprom.c | 18 +++---------------
 2 files changed, 4 insertions(+), 15 deletions(-)

drivers/firmware/broadcom/Kconfig +1 -0

index 5e29f83e7b39..c041dcb7ea52 100644
@@ -13,6 +13,7 @@ config BCM47XX_NVRAM
 config BCM47XX_SPROM
 	bool "Broadcom SPROM driver"
 	depends on BCM47XX_NVRAM
+	select GENERIC_NET_UTILS
 	help
 	  Broadcom devices store configuration data in SPROM. Accessing it is
 	  specific to the bus host type, e.g. PCI(e) devices have it mapped in

drivers/firmware/broadcom/bcm47xx_sprom.c +3 ~~-15~~

index 62aa3cf09b4d..4787f86c8ac1 100644
@@ -137,20 +137,6 @@ static void nvram_read_leddc(const char *prefix, const char *name,
 	*leddc_off_time = (val >> 16) & 0xff;
 }
 
-static void bcm47xx_nvram_parse_macaddr(char *buf, u8 macaddr[6])
-{
-	if (strchr(buf, ':'))
-		sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", &macaddr[0],
-			&macaddr[1], &macaddr[2], &macaddr[3], &macaddr[4],
-			&macaddr[5]);
-	else if (strchr(buf, '-'))
-		sscanf(buf, "%hhx-%hhx-%hhx-%hhx-%hhx-%hhx", &macaddr[0],
-			&macaddr[1], &macaddr[2], &macaddr[3], &macaddr[4],
-			&macaddr[5]);
-	else
-		pr_warn("Can not parse mac address: %s\n", buf);
-}
-
 static void nvram_read_macaddr(const char *prefix, const char *name,
 			       u8 val[6], bool fallback)
 {

@@ -161,7 +147,9 @@ static void nvram_read_macaddr(const char *prefix, const char *name,
 	if (err < 0)
 		return;
 
-	bcm47xx_nvram_parse_macaddr(buf, val);
+	strreplace(buf, '-', ':');
+	if (!mac_pton(buf, val))
+		pr_warn("Can not parse mac address: %s\n", buf);
 }
 
 static void nvram_read_alpha2(const char *prefix, const char *name,

Signature · 1 line

2.15.1

Hauke Mehrtens

Acked-by on v2 1/1

2017-12-21 16:42 UTC #1 lore raw

Quoted 3 lines · Andy Shevchenko

Replace sscanf() with mac_pton().
Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Hauke Mehrtens <[email protected]>

The patch looks good, but I haven't tested them on my devices.

Quoted 55 lines

---
- use negative condition to be consistent with the rest code

 drivers/firmware/broadcom/Kconfig         |  1 +
 drivers/firmware/broadcom/bcm47xx_sprom.c | 18 +++---------------
 2 files changed, 4 insertions(+), 15 deletions(-)

drivers/firmware/broadcom/Kconfig +1 -0

index 5e29f83e7b39..c041dcb7ea52 100644
@@ -13,6 +13,7 @@ config BCM47XX_NVRAM
 config BCM47XX_SPROM
 	bool "Broadcom SPROM driver"
 	depends on BCM47XX_NVRAM
+	select GENERIC_NET_UTILS
 	help
 	  Broadcom devices store configuration data in SPROM. Accessing it is
 	  specific to the bus host type, e.g. PCI(e) devices have it mapped in

drivers/firmware/broadcom/bcm47xx_sprom.c +3 ~~-15~~

index 62aa3cf09b4d..4787f86c8ac1 100644
@@ -137,20 +137,6 @@ static void nvram_read_leddc(const char *prefix, const char *name,
 	*leddc_off_time = (val >> 16) & 0xff;
 }
 
-static void bcm47xx_nvram_parse_macaddr(char *buf, u8 macaddr[6])
-{
-	if (strchr(buf, ':'))
-		sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", &macaddr[0],
-			&macaddr[1], &macaddr[2], &macaddr[3], &macaddr[4],
-			&macaddr[5]);
-	else if (strchr(buf, '-'))
-		sscanf(buf, "%hhx-%hhx-%hhx-%hhx-%hhx-%hhx", &macaddr[0],
-			&macaddr[1], &macaddr[2], &macaddr[3], &macaddr[4],
-			&macaddr[5]);
-	else
-		pr_warn("Can not parse mac address: %s\n", buf);
-}
-
 static void nvram_read_macaddr(const char *prefix, const char *name,
 			       u8 val[6], bool fallback)
 {

@@ -161,7 +147,9 @@ static void nvram_read_macaddr(const char *prefix, const char *name,
 	if (err < 0)
 		return;
 
-	bcm47xx_nvram_parse_macaddr(buf, val);
+	strreplace(buf, '-', ':');
+	if (!mac_pton(buf, val))
+		pr_warn("Can not parse mac address: %s\n", buf);
 }
 
 static void nvram_read_alpha2(const char *prefix, const char *name,

Andy Shevchenko

author

2018-01-18 13:41 UTC #2 lore raw

Quoted 9 lines · Hauke Mehrtens

Quoted 3 lines · Andy Shevchenko
Replace sscanf() with mac_pton().
Signed-off-by Andy Shevchenko <[email protected]>
Acked-by Hauke Mehrtens <[email protected]>
The patch looks good, but I haven't tested them on my devices.

Any news on this, anyone?

Quoted 66 lines

---
- use negative condition to be consistent with the rest code

 drivers/firmware/broadcom/Kconfig         |  1 +
 drivers/firmware/broadcom/bcm47xx_sprom.c | 18 +++---------------
 2 files changed, 4 insertions(+), 15 deletions(-)

drivers/firmware/broadcom/Kconfig +1 -0

index 5e29f83e7b39..c041dcb7ea52 100644
@@ -13,6 +13,7 @@ config BCM47XX_NVRAM
 config BCM47XX_SPROM
 	bool "Broadcom SPROM driver"
 	depends on BCM47XX_NVRAM
+	select GENERIC_NET_UTILS
 	help
 	  Broadcom devices store configuration data in SPROM.

Accessing it is
specific to the bus host type, e.g. PCI(e) devices have
it mapped in

drivers/firmware/broadcom/bcm47xx_sprom.c +0 -8

index 62aa3cf09b4d..4787f86c8ac1 100644
@@ -137,20 +137,6 @@ static void nvram_read_leddc(const char
 *prefix, const char *name,
 	*leddc_off_time = (val >> 16) & 0xff;
 }
 
-static void bcm47xx_nvram_parse_macaddr(char *buf, u8 macaddr[6])
-{
-	if (strchr(buf, ':'))
-		sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
 &macaddr[0],
-			&macaddr[1], &macaddr[2], &macaddr[3],
 &macaddr[4],
-			&macaddr[5]);
-	else if (strchr(buf, '-'))
-		sscanf(buf, "%hhx-%hhx-%hhx-%hhx-%hhx-%hhx",

&macaddr[0],

drivers/firmware/broadcom/bcm47xx_sprom.c +0 -1

-			&macaddr[1], &macaddr[2], &macaddr[3],

&macaddr[4],

drivers/firmware/broadcom/bcm47xx_sprom.c +0 -5

-			&macaddr[5]);
-	else
-		pr_warn("Can not parse mac address: %s\n", buf);
-}
-
 static void nvram_read_macaddr(const char *prefix, const char

*name,

drivers/firmware/broadcom/bcm47xx_sprom.c +3 -1

 			       u8 val[6], bool fallback)
 {

@@ -161,7 +147,9 @@ static void nvram_read_macaddr(const char
 *prefix, const char *name,
 	if (err < 0)
 		return;
 
-	bcm47xx_nvram_parse_macaddr(buf, val);
+	strreplace(buf, '-', ':');
+	if (!mac_pton(buf, val))
+		pr_warn("Can not parse mac address: %s\n", buf);
 }

static void nvram_read_alpha2(const char *prefix, const char *name,

Signature · 2 lines

Andy Shevchenko <[email protected]>
Intel Finland Oy

James Hogan

2018-03-08 17:37 UTC #3 lore raw

Quoted 5 lines · Andy Shevchenko

Quoted 4 lines · Hauke Mehrtens
Quoted 2 lines · Andy Shevchenko
Replace sscanf() with mac_pton().

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
Signed-off-by Andy Shevchenko <[email protected]>
Acked-by Hauke Mehrtens <[email protected]>
The patch looks good, but I haven't tested them on my devices.
Any news on this, anyone?

Changes to this usually seem to go through the MIPS tree, so I've applied for 4.17.

Thanks
James