Change in coreboot[master]: src: Include <smbios.h> when appropriate

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Include <smbios.h> when appropriate
......................................................................

src: Include <smbios.h> when appropriate

Change-Id: I12345a5b6c9ce94ca9f8b555154b2278a8ff97bf
Signed-off-by: Elyes HAOUAS 
---
M src/include/device/device.h
M src/soc/intel/braswell/chip.h
M src/soc/intel/cannonlake/chip.h
M src/soc/intel/skylake/chip.h
4 files changed, 1 insertion(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/39816/1

diff --git a/src/include/device/device.h b/src/include/device/device.h
index 333ac5d..bbe861e 100644
--- a/src/include/device/device.h
+++ b/src/include/device/device.h
@@ -5,6 +5,7 @@
 #include <device/resource.h>
 #include <device/path.h>
 #include <device/pci_type.h>
+#include <smbios.h>
 #include <types.h>

struct device;
@@ -29,7 +30,6 @@

struct bus;

-struct smbios_type11;
 struct acpi_rsdp;

struct device_operations {
diff --git a/src/soc/intel/braswell/chip.h b/src/soc/intel/braswell/chip.h
index 026e491..767ecde 100644
--- a/src/soc/intel/braswell/chip.h
+++ b/src/soc/intel/braswell/chip.h
@@ -24,7 +24,6 @@
 #include <fsp/util.h>
 #include <intelblocks/lpc_lib.h>
 #include <soc/pci_devs.h>
-#include <smbios.h>

#define SVID_CONFIG1		1
 #define SVID_CONFIG3		3
diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h
index 7794fd4..5f58f08 100644
--- a/src/soc/intel/cannonlake/chip.h
+++ b/src/soc/intel/cannonlake/chip.h
@@ -20,7 +20,6 @@
 #include <intelblocks/gpio.h>
 #include <intelblocks/gspi.h>
 #include <intelblocks/lpc_lib.h>
-#include <smbios.h>
 #include <stdint.h>
 #include <soc/gpio.h>
 #include <soc/pch.h>
diff --git a/src/soc/intel/skylake/chip.h b/src/soc/intel/skylake/chip.h
index 178ab03..880c1d6 100644
--- a/src/soc/intel/skylake/chip.h
+++ b/src/soc/intel/skylake/chip.h
@@ -31,7 +31,6 @@
 #include <soc/serialio.h>
 #include <soc/usb.h>
 #include <soc/vr_config.h>
-#include <smbios.h>

#define MAX_PEG_PORTS	3

Change in coreboot[master]: src/*/chip: Include <smbios.h> when appropriate

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39816

to look at the new patch set (#2).

Change subject: src/*/chip: Include <smbios.h> when appropriate
......................................................................

src/*/chip: Include <smbios.h> when appropriate

Change-Id: I12345a5b6c9ce94ca9f8b555154b2278a8ff97bf
Signed-off-by: Elyes HAOUAS 
---
M src/include/device/device.h
M src/soc/intel/braswell/chip.h
M src/soc/intel/cannonlake/chip.h
M src/soc/intel/skylake/chip.h
4 files changed, 1 insertion(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/39816/2

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39816

to look at the new patch set (#3).

Change subject: src: Include <smbios.h> when appropriate
......................................................................

src: Include <smbios.h> when appropriate

Change-Id: I12345a5b6c9ce94ca9f8b555154b2278a8ff97bf
Signed-off-by: Elyes HAOUAS 
---
M src/drivers/i2c/at24rf08c/at24rf08c.c
M src/drivers/intel/gma/intel_ddi.c
M src/include/device/device.h
M src/include/dimm_info_util.h
M src/mainboard/google/volteer/mainboard.c
M src/mainboard/packardbell/ms2290/mainboard.c
M src/soc/intel/braswell/chip.h
M src/soc/intel/cannonlake/chip.h
M src/soc/intel/skylake/chip.h
9 files changed, 1 insertion(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/39816/3

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39816

to look at the new patch set (#4).

Change subject: src: Include <smbios.h> when appropriate
......................................................................

src: Include <smbios.h> when appropriate

Change-Id: I12345a5b6c9ce94ca9f8b555154b2278a8ff97bf
Signed-off-by: Elyes HAOUAS 
---
M src/drivers/i2c/at24rf08c/at24rf08c.c
M src/drivers/intel/gma/intel_ddi.c
M src/include/device/device.h
M src/mainboard/google/volteer/mainboard.c
M src/mainboard/packardbell/ms2290/mainboard.c
M src/soc/intel/braswell/chip.h
M src/soc/intel/cannonlake/chip.h
M src/soc/intel/skylake/chip.h
8 files changed, 1 insertion(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/39816/4

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Include <smbios.h> when appropriate
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39816/4/src/soc/intel/braswell/chip... 
File src/soc/intel/braswell/chip.h:

https://review.coreboot.org/c/coreboot/+/39816/4/src/soc/intel/braswell/chip... 
PS4, Line 27: smbios
That's actually consumed by the devicetree.c
As it doesn't generate a build error there must be another smbios.h included somewhere else

[Angel Pons (Code Review)]

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

Change subject: src: Include <smbios.h> when appropriate
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39816/4/src/soc/intel/braswell/chip... 
File src/soc/intel/braswell/chip.h:

https://review.coreboot.org/c/coreboot/+/39816/4/src/soc/intel/braswell/chip... 
PS4, Line 27: smbios
> That's actually consumed by the devicetree.c […]

Indirect inclusion, which is pretty annoying

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Include <smbios.h> when appropriate
......................................................................

Patch Set 4:

(1 comment)

Thank you.

https://review.coreboot.org/c/coreboot/+/39816/4/src/soc/intel/braswell/chip... 
File src/soc/intel/braswell/chip.h:

https://review.coreboot.org/c/coreboot/+/39816/4/src/soc/intel/braswell/chip... 
PS4, Line 27: smbios
> Indirect inclusion, which is pretty annoying

I think that smbios is missing here: "src/include/device/device.h"
it will add it to "static.c" and will be used by "devicetree.cb"

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Include <smbios.h> when appropriate
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39816/4/src/soc/intel/braswell/chip... 
File src/soc/intel/braswell/chip.h:

https://review.coreboot.org/c/coreboot/+/39816/4/src/soc/intel/braswell/chip... 
PS4, Line 27: smbios
> I think that smbios is missing here: "src/include/device/device.h" […]

Done

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Include <smbios.h> when appropriate
......................................................................

Patch Set 5:

(1 comment)

All the removals look good.

https://review.coreboot.org/c/coreboot/+/39816/4/src/include/device/device.h 
File src/include/device/device.h:

PS4: 
This change is questionable, because the declaration of `struct smbios_type11`
here shows that it was not included intentionally.

Change in coreboot[master]: src: Remove unused 'include <smbios.h>'

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39816

to look at the new patch set (#6).

Change subject: src: Remove unused 'include <smbios.h>'
......................................................................

src: Remove unused 'include <smbios.h>'

Change-Id: I12345a5b6c9ce94ca9f8b555154b2278a8ff97bf
Signed-off-by: Elyes HAOUAS 
---
M src/drivers/i2c/at24rf08c/at24rf08c.c
M src/drivers/intel/gma/intel_ddi.c
M src/mainboard/google/volteer/mainboard.c
M src/mainboard/packardbell/ms2290/mainboard.c
4 files changed, 0 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/39816/6

Change in coreboot[master]: src: Remove unused 'include <smbios.h>'

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Remove unused 'include <smbios.h>'
......................................................................

Patch Set 6: Code-Review+2

Why did you drop the `chip.h` changes?

Change in coreboot[master]: src: Remove unused 'include <smbios.h>'

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Remove unused 'include <smbios.h>'
......................................................................

Patch Set 6:
> Patch Set 6: Code-Review+2
>
> Why did you drop the `chip.h` changes?

devicetree.cb needs indirect include of smbios.h in chip.h

Change in coreboot[master]: src: Remove unused 'include <smbios.h>'

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Remove unused 'include <smbios.h>'
......................................................................

Patch Set 6:
> > Why did you drop the `chip.h` changes?
>
> devicetree.cb needs indirect include of smbios.h in chip.h

Why???

Change in coreboot[master]: src: Remove unused 'include <smbios.h>'

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Remove unused 'include <smbios.h>'
......................................................................

Patch Set 6:
> > > Why did you drop the `chip.h` changes?
> >
> > devicetree.cb needs indirect include of smbios.h in chip.h
>
> Why???

Also, according to Jenkins that's not true:
> Patch Set 1: Verified+1

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Nico Huber, Angel Pons, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39816

to look at the new patch set (#7).

Change subject: src: Include <smbios.h> when appropriate
......................................................................

src: Include <smbios.h> when appropriate

Change-Id: I12345a5b6c9ce94ca9f8b555154b2278a8ff97bf
Signed-off-by: Elyes HAOUAS 
---
M src/drivers/i2c/at24rf08c/at24rf08c.c
M src/drivers/intel/gma/intel_ddi.c
M src/include/device/device.h
M src/mainboard/google/volteer/mainboard.c
M src/mainboard/packardbell/ms2290/mainboard.c
M src/soc/intel/braswell/chip.h
M src/soc/intel/cannonlake/chip.h
M src/soc/intel/skylake/chip.h
8 files changed, 1 insertion(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/39816/7

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Include <smbios.h> when appropriate
......................................................................

Patch Set 6:
> Patch Set 6:
> > > > Why did you drop the `chip.h` changes?
> > >
> > > devicetree.cb needs indirect include of smbios.h in chip.h
> >
> > Why???
>
> Also, according to Jenkins that's not true:
> > Patch Set 1: Verified+1

done
thx

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Include <smbios.h> when appropriate
......................................................................

Patch Set 7:

Patch set 7 seems to be the same as 5. Is that intended?

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Include <smbios.h> when appropriate
......................................................................

Patch Set 7:
> Patch Set 7:
>
> Patch set 7 seems to be the same as 5. Is that intended?

yes, step back to 5

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Include <smbios.h> when appropriate
......................................................................

Patch Set 7:
> > Patch set 7 seems to be the same as 5. Is that intended?
>
> yes, step back to 5

No sure where we misunderstand each other. The `chip.h` changes seem
reasonable and I'm willing to merge them. The `device.h` change is
unrelated and questionable, I can't give +2 on it. You can wait for
another reviewer, though, who knows better what is "appropriate".

[Angel Pons (Code Review)]

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

Change subject: src: Include <smbios.h> when appropriate
......................................................................

Patch Set 7: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/39816/7/src/include/device/device.h 
File src/include/device/device.h:

PS7: 
Please do this on a separate change

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Include <smbios.h> when appropriate
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39816/7/src/include/device/device.h 
File src/include/device/device.h:

PS7:
> Please do this on a separate change

I will, but changes on device/device.h must be merged before removing smbios.h from the other files ....

Change in coreboot[master]: src: Remove not used 'include <smbios.h>'

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Nico Huber, Angel Pons, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39816

to look at the new patch set (#9).

Change subject: src: Remove not used 'include <smbios.h>'
......................................................................

src: Remove not used 'include <smbios.h>'

Change-Id: I12345a5b6c9ce94ca9f8b555154b2278a8ff97bf
Signed-off-by: Elyes HAOUAS 
---
M src/drivers/i2c/at24rf08c/at24rf08c.c
M src/drivers/intel/gma/intel_ddi.c
M src/mainboard/google/volteer/mainboard.c
M src/mainboard/packardbell/ms2290/mainboard.c
M src/soc/intel/braswell/chip.h
M src/soc/intel/cannonlake/chip.h
M src/soc/intel/skylake/chip.h
7 files changed, 0 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/39816/9

Change in coreboot[master]: src: Remove not used 'include <smbios.h>'

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Remove not used 'include <smbios.h>'
......................................................................

Patch Set 9: Code-Review+2

Change in coreboot[master]: src: Remove not used 'include <smbios.h>'

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Remove not used 'include <smbios.h>'
......................................................................

Patch Set 9:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39816/4/src/include/device/device.h 
File src/include/device/device.h:

PS4:
> This change is questionable, because the declaration of `struct smbios_type11` […]

Done

Change in coreboot[master]: src: Remove not used 'include <smbios.h>'

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Remove not used 'include <smbios.h>'
......................................................................

Patch Set 9:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39816/7/src/include/device/device.h 
File src/include/device/device.h:

PS7:
> I will, but changes on device/device.h must be merged before removing smbios. […]

Ok, that's where the confusion started. I missed that patch set 1 already
changed `device/device.h`. So we never saw the errors if it isn't changed,
and hence nobody could fix/understand them. I'll rebase the current version
on master to see the errors, please bear with me.

Change in coreboot[master]: src: Remove not used 'include <smbios.h>'

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Remove not used 'include <smbios.h>'
......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39816/7/src/include/device/device.h 
File src/include/device/device.h:

PS7:
> Ok, that's where the confusion started. I missed that patch set 1 already […]

Nico, please freel free :))

Change in coreboot[master]: src: Remove not used 'include <smbios.h>'

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Remove not used 'include <smbios.h>'
......................................................................

Patch Set 11: Code-Review+2

Change in coreboot[master]: src: Remove not used 'include <smbios.h>'

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39816 )

Change subject: src: Remove not used 'include <smbios.h>'
......................................................................

src: Remove not used 'include <smbios.h>'

Change-Id: I12345a5b6c9ce94ca9f8b555154b2278a8ff97bf
Signed-off-by: Elyes HAOUAS 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39816
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Georgi 
Reviewed-by: Nico Huber 
Reviewed-by: Angel Pons 
---
M src/drivers/i2c/at24rf08c/at24rf08c.c
M src/drivers/intel/gma/intel_ddi.c
M src/mainboard/google/volteer/mainboard.c
M src/mainboard/packardbell/ms2290/mainboard.c
M src/soc/intel/braswell/chip.h
M src/soc/intel/cannonlake/chip.h
M src/soc/intel/skylake/chip.h
7 files changed, 0 insertions(+), 7 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved
  Nico Huber: Looks good to me, approved
  Angel Pons: Looks good to me, but someone else must approve

diff --git a/src/drivers/i2c/at24rf08c/at24rf08c.c b/src/drivers/i2c/at24rf08c/at24rf08c.c
index 3511502..3890638 100644
--- a/src/drivers/i2c/at24rf08c/at24rf08c.c
+++ b/src/drivers/i2c/at24rf08c/at24rf08c.c
@@ -4,7 +4,6 @@
 #include <types.h>
 #include <device/device.h>
 #include <device/smbus.h>
-#include <smbios.h>
 #include <console/console.h>

static void at24rf08c_init(struct device *dev)
diff --git a/src/drivers/intel/gma/intel_ddi.c b/src/drivers/intel/gma/intel_ddi.c
index 3a8fe50..59d3010 100644
--- a/src/drivers/intel/gma/intel_ddi.c
+++ b/src/drivers/intel/gma/intel_ddi.c
@@ -31,7 +31,6 @@
 #include <device/pci_def.h>
 #include <console/console.h>
 #include <arch/acpi.h>
-#include <smbios.h>
 #include <device/pci.h>
 #include <ec/google/chromeec/ec.h>
 #include <cpu/x86/tsc.h>
diff --git a/src/mainboard/google/volteer/mainboard.c b/src/mainboard/google/volteer/mainboard.c
index 4e9843c..0db43d1 100644
--- a/src/mainboard/google/volteer/mainboard.c
+++ b/src/mainboard/google/volteer/mainboard.c
@@ -12,7 +12,6 @@
 #include <ec/ec.h>
 #include <ec/google/chromeec/ec.h>
 #include <soc/gpio.h>
-#include <smbios.h>
 #include <vendorcode/google/chromeos/chromeos.h>
 #include <variant/gpio.h>

diff --git a/src/mainboard/packardbell/ms2290/mainboard.c b/src/mainboard/packardbell/ms2290/mainboard.c
index 8d4bb8e..184401a 100644
--- a/src/mainboard/packardbell/ms2290/mainboard.c
+++ b/src/mainboard/packardbell/ms2290/mainboard.c
@@ -9,7 +9,6 @@
 #include <drivers/intel/gma/int15.h>
 #include <pc80/keyboard.h>
 #include <cpu/x86/lapic.h>
-#include <smbios.h>

static void mainboard_enable(struct device *dev)
 {
diff --git a/src/soc/intel/braswell/chip.h b/src/soc/intel/braswell/chip.h
index 76eb50e..04c017e 100644
--- a/src/soc/intel/braswell/chip.h
+++ b/src/soc/intel/braswell/chip.h
@@ -14,7 +14,6 @@
 #include <fsp/util.h>
 #include <intelblocks/lpc_lib.h>
 #include <soc/pci_devs.h>
-#include <smbios.h>

#define SVID_CONFIG1		1
 #define SVID_CONFIG3		3
diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h
index 578473b..b74291e 100644
--- a/src/soc/intel/cannonlake/chip.h
+++ b/src/soc/intel/cannonlake/chip.h
@@ -9,7 +9,6 @@
 #include <intelblocks/gpio.h>
 #include <intelblocks/gspi.h>
 #include <intelblocks/lpc_lib.h>
-#include <smbios.h>
 #include <stdint.h>
 #include <soc/gpio.h>
 #include <soc/pch.h>
diff --git a/src/soc/intel/skylake/chip.h b/src/soc/intel/skylake/chip.h
index 1b9cc4a..892f262 100644
--- a/src/soc/intel/skylake/chip.h
+++ b/src/soc/intel/skylake/chip.h
@@ -21,7 +21,6 @@
 #include <soc/serialio.h>
 #include <soc/usb.h>
 #include <soc/vr_config.h>
-#include <smbios.h>

#define MAX_PEG_PORTS	3