Change in coreboot[master]: sb/nvidia/{ck804,mcp55}: Use new device references for MAC EEPROM

[Nico Huber (Code Review)]

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35489 )

Change subject: sb/nvidia/{ck804,mcp55}: Use new device references for MAC EEPROM
......................................................................

sb/nvidia/{ck804,mcp55}: Use new device references for MAC EEPROM

Instead of using fragile dev_find_slot_on_smbus(), link the devices
in the devicetree directly.

msi/ms9652 referenced a non-existing device (commented), so remove
the reference entirely.

Change-Id: I050e681142c5b19f97fec865e887eda157ee830b
Signed-off-by: Nico Huber 
---
M src/mainboard/msi/ms9652_fam10/devicetree.cb
M src/mainboard/supermicro/h8dmr_fam10/devicetree.cb
M src/mainboard/supermicro/h8qme_fam10/devicetree.cb
M src/mainboard/tyan/s2912_fam10/devicetree.cb
M src/southbridge/nvidia/ck804/chip.h
M src/southbridge/nvidia/ck804/nic.c
M src/southbridge/nvidia/mcp55/chip.h
M src/southbridge/nvidia/mcp55/nic.c
8 files changed, 56 insertions(+), 77 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/35489/1

diff --git a/src/mainboard/msi/ms9652_fam10/devicetree.cb b/src/mainboard/msi/ms9652_fam10/devicetree.cb
index c6dae6d..1815870 100644
--- a/src/mainboard/msi/ms9652_fam10/devicetree.cb
+++ b/src/mainboard/msi/ms9652_fam10/devicetree.cb
@@ -149,9 +149,6 @@
    				register "ide0_enable" = "1"
    				register "sata0_enable" = "1"
    				register "sata1_enable" = "1"
-					# 1: SMBus under 2e.8, 2: SM0 3: SM1
-					register "mac_eeprom_smbus" = "3"
-					register "mac_eeprom_addr" = "0x51"
    			end
    		end
    		device pci 18.0 on end	# HT 1.0
diff --git a/src/mainboard/supermicro/h8dmr_fam10/devicetree.cb b/src/mainboard/supermicro/h8dmr_fam10/devicetree.cb
index ee88c31..89c579e 100644
--- a/src/mainboard/supermicro/h8dmr_fam10/devicetree.cb
+++ b/src/mainboard/supermicro/h8dmr_fam10/devicetree.cb
@@ -103,7 +103,7 @@
    					#	 device i2c 55 on end
    					# end
    					chip drivers/generic/generic	# MAC EEPROM
-							device i2c 51 on end
+							device i2c 51 alias mac on end
    					end
    				end
    				device pci 2.0 on end	# USB 1.1
@@ -133,9 +133,7 @@
    				register "ide0_enable" = "1"
    				register "sata0_enable" = "1"
    				register "sata1_enable" = "1"
-					# 1: SMBus under 2e.8, 2: SM0 3: SM1
-					register "mac_eeprom_smbus" = "3"
-					register "mac_eeprom_addr" = "0x51"
+					use mac as mac_eeprom
    			end
    		end
    		device pci 18.1 on end
diff --git a/src/mainboard/supermicro/h8qme_fam10/devicetree.cb b/src/mainboard/supermicro/h8qme_fam10/devicetree.cb
index 317d643..de78ce2 100644
--- a/src/mainboard/supermicro/h8qme_fam10/devicetree.cb
+++ b/src/mainboard/supermicro/h8qme_fam10/devicetree.cb
@@ -56,11 +56,8 @@
    				end
    				device pci 1.1 on end
    				device pci 1.1 on			# SM 1
-						# PCI device SMBus address will
-						# depend on addon PCI device, do
-						# we need to scan_smbus_bus?
    					chip drivers/generic/generic	# MAC EEPROM
-							device i2c 51 on end
+							device i2c 51 alias mac on end
    					end
    				end
    				device pci 2.0 on end	# USB 1.1
@@ -84,9 +81,7 @@
    				register "ide0_enable" = "1"
    				register "sata0_enable" = "1"
    				register "sata1_enable" = "1"
-					# 1: SMBus under 2e.8, 2: SM0 3: SM1
-					register "mac_eeprom_smbus" = "3"
-					register "mac_eeprom_addr" = "0x51"
+					use mac as mac_eeprom
    			end
    		end
    		device pci 18.1 on end
diff --git a/src/mainboard/tyan/s2912_fam10/devicetree.cb b/src/mainboard/tyan/s2912_fam10/devicetree.cb
index e49e16b..4a4118e 100644
--- a/src/mainboard/tyan/s2912_fam10/devicetree.cb
+++ b/src/mainboard/tyan/s2912_fam10/devicetree.cb
@@ -103,7 +103,7 @@
    					#	 device i2c 55 on end
    					# end
    					chip drivers/generic/generic	# MAC EEPROM
-							device i2c 51 on end
+							device i2c 51 alias mac on end
    					end
    				end
    				device pci 2.0 on end	# USB 1.1
@@ -127,9 +127,7 @@
    				register "ide0_enable" = "1"
    				register "sata0_enable" = "1"
    				register "sata1_enable" = "1"
-					# 1: SMBus under 2e.8, 2: SM0 3: SM1
-					register "mac_eeprom_smbus" = "3"
-					register "mac_eeprom_addr" = "0x51"
+					use mac as mac_eeprom
    			end
    		end
    		device pci 18.1 on end
diff --git a/src/southbridge/nvidia/ck804/chip.h b/src/southbridge/nvidia/ck804/chip.h
index b1676ce..16fdbde 100644
--- a/src/southbridge/nvidia/ck804/chip.h
+++ b/src/southbridge/nvidia/ck804/chip.h
@@ -17,14 +17,15 @@
 #ifndef SOUTHBRIDGE_NVIDIA_CK804_CHIP_H
 #define SOUTHBRIDGE_NVIDIA_CK804_CHIP_H

+#include <device/device.h>
+
 struct southbridge_nvidia_ck804_config {
    unsigned int usb1_hc_reset : 1;
    unsigned int ide0_enable : 1;
    unsigned int ide1_enable : 1;
    unsigned int sata0_enable : 1;
    unsigned int sata1_enable : 1;
-	unsigned int mac_eeprom_smbus;
-	unsigned int mac_eeprom_addr;
+	DEVTREE_CONST struct device *mac_eeprom;
 };

extern struct pci_operations ck804_pci_ops;
diff --git a/src/southbridge/nvidia/ck804/nic.c b/src/southbridge/nvidia/ck804/nic.c
index 5dbb6b4..188dc23 100644
--- a/src/southbridge/nvidia/ck804/nic.c
+++ b/src/southbridge/nvidia/ck804/nic.c
@@ -47,37 +47,31 @@

conf = dev->chip_info;

-	if (conf->mac_eeprom_smbus != 0) {
+	if (conf->mac_eeprom) {
    	/* Read MAC address from EEPROM at first. */
-		struct device *dev_eeprom;
-		dev_eeprom = dev_find_slot_on_smbus(conf->mac_eeprom_smbus,
-					   conf->mac_eeprom_addr);
-
-		if (dev_eeprom) {
-			/* If that is valid we will use that. */
-			unsigned char dat[6];
-			int i, status;
-			for (i = 0; i < 6; i++) {
-				status = smbus_read_byte(dev_eeprom, i);
-				if (status < 0)
-					break;
-				dat[i] = status & 0xff;
+		/* If that is valid we will use that. */
+		unsigned char dat[6];
+		int i, status;
+		for (i = 0; i < 6; i++) {
+			status = smbus_read_byte(conf->mac_eeprom, i);
+			if (status < 0)
+				break;
+			dat[i] = status & 0xff;
+		}
+		if (status >= 0) {
+			mac_l = 0;
+			for (i = 3; i >= 0; i--) {
+				mac_l <<= 8;
+				mac_l += dat[i];
    		}
-			if (status >= 0) {
-				mac_l = 0;
-				for (i = 3; i >= 0; i--) {
-					mac_l <<= 8;
-					mac_l += dat[i];
+			if (mac_l != 0xffffffff) {
+				mac_l += nic_index;
+				mac_h = 0;
+				for (i = 5; i >= 4; i--) {
+					mac_h <<= 8;
+					mac_h += dat[i];
    			}
-				if (mac_l != 0xffffffff) {
-					mac_l += nic_index;
-					mac_h = 0;
-					for (i = 5; i >= 4; i--) {
-						mac_h <<= 8;
-						mac_h += dat[i];
-					}
-					eeprom_valid = 1;
-				}
+				eeprom_valid = 1;
    		}
    	}
    }
diff --git a/src/southbridge/nvidia/mcp55/chip.h b/src/southbridge/nvidia/mcp55/chip.h
index 4bc8428..36eea22 100644
--- a/src/southbridge/nvidia/mcp55/chip.h
+++ b/src/southbridge/nvidia/mcp55/chip.h
@@ -18,14 +18,15 @@
 #ifndef SOUTHBRIDGE_NVIDIA_MCP55_CHIP_H
 #define SOUTHBRIDGE_NVIDIA_MCP55_CHIP_H

+#include <device/device.h>
+
 struct southbridge_nvidia_mcp55_config
 {
    unsigned int ide0_enable : 1;
    unsigned int ide1_enable : 1;
    unsigned int sata0_enable : 1;
    unsigned int sata1_enable : 1;
-	unsigned int mac_eeprom_smbus;
-	unsigned int mac_eeprom_addr;
+	DEVTREE_CONST struct device *mac_eeprom;
 };

#endif
diff --git a/src/southbridge/nvidia/mcp55/nic.c b/src/southbridge/nvidia/mcp55/nic.c
index af4df44..b562503 100644
--- a/src/southbridge/nvidia/mcp55/nic.c
+++ b/src/southbridge/nvidia/mcp55/nic.c
@@ -124,36 +124,31 @@

conf = dev->chip_info;

-	if (conf->mac_eeprom_smbus != 0) {
+	if (conf->mac_eeprom) {
 //	read MAC address from EEPROM at first
-		struct device *dev_eeprom;
-		dev_eeprom = dev_find_slot_on_smbus(conf->mac_eeprom_smbus, conf->mac_eeprom_addr);
-
-		if (dev_eeprom) {
-		//	if that is valid we will use that
-			unsigned char dat[6];
-			int status;
-			int i;
-			for (i=0;i<6;i++) {
-				status = smbus_read_byte(dev_eeprom, i);
-				if (status < 0) break;
-				dat[i] = status & 0xff;
+	//	if that is valid we will use that
+		unsigned char dat[6];
+		int status;
+		int i;
+		for (i=0;i<6;i++) {
+			status = smbus_read_byte(conf->mac_eeprom, i);
+			if (status < 0) break;
+			dat[i] = status & 0xff;
+		}
+		if (status >= 0) {
+			mac_l = 0;
+			for (i=3;i>=0;i--) {
+				mac_l <<= 8;
+				mac_l += dat[i];
    		}
-			if (status >= 0) {
-				mac_l = 0;
-				for (i=3;i>=0;i--) {
-					mac_l <<= 8;
-					mac_l += dat[i];
+			if (mac_l != 0xffffffff) {
+				mac_l += nic_index;
+				mac_h = 0;
+				for (i=5;i>=4;i--) {
+					mac_h <<= 8;
+					mac_h += dat[i];
    			}
-				if (mac_l != 0xffffffff) {
-					mac_l += nic_index;
-					mac_h = 0;
-					for (i=5;i>=4;i--) {
-						mac_h <<= 8;
-						mac_h += dat[i];
-					}
-					eeprom_valid = 1;
-				}
+				eeprom_valid = 1;
    		}
    	}
    }

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35489 )

Change subject: sb/nvidia/{ck804,mcp55}: Use new device references for MAC EEPROM
......................................................................

Patch Set 1: Code-Review+1

(3 comments)

Looks good

https://review.coreboot.org/c/coreboot/+/35489/1/src/mainboard/supermicro/h8... 
File src/mainboard/supermicro/h8dmr_fam10/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/35489/1/src/mainboard/supermicro/h8... 
PS1, Line 106: mac
"mac" alone is pretty short and confusing... What about "mac_dev"? I can't really find a name that I like a lot, so if nothing better comes up, it's not a big deal

https://review.coreboot.org/c/coreboot/+/35489/1/src/mainboard/supermicro/h8... 
PS1, Line 136: use mac as mac_eeprom
Other than the "mac" name issue, WONDERFUL! I LOVE IT!

https://review.coreboot.org/c/coreboot/+/35489/1/src/southbridge/nvidia/mcp5... 
File src/southbridge/nvidia/mcp55/nic.c:

PS1: 
Eww... The code style... IT HURTS!

But I don't need to look at these files, so I don't care in the slightest.

[Nico Huber (Code Review)]

Nico Huber has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35489 )

Change subject: sb/nvidia/{ck804,mcp55}: Use new device references for MAC EEPROM
......................................................................

Abandoned

Code dropped from master.