Change in ...coreboot[master]: SMBIOS: Add new MEMORY_{TYPE,ECHNOLOGY,OPERATING} macros

[HAOUAS Elyes (Code Review)]

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

Change subject: SMBIOS: Add new MEMORY_{TYPE,ECHNOLOGY,OPERATING} macros
......................................................................

SMBIOS: Add new MEMORY_{TYPE,ECHNOLOGY,OPERATING} macros

Change-Id: I4e466614d0a9e8c89f298594a5785af775b22a95
Signed-off-by: Elyes HAOUAS 
---
M src/include/smbios.h
1 file changed, 15 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31423/1

diff --git a/src/include/smbios.h b/src/include/smbios.h
index 7e24907..463b64e 100644
--- a/src/include/smbios.h
+++ b/src/include/smbios.h
@@ -83,6 +83,21 @@
 #define MEMORY_TYPE_DETAIL_NON_VOLATILE		(1 << 12)
 #define MEMORY_TYPE_DETAIL_REGISTERED		(1 << 13)
 #define MEMORY_TYPE_DETAIL_UNBUFFERED		(1 << 14)
+#define MEMORY_TYPE_DETAIL_LRDIMM		(1 << 15)
+
+#define MEMORY_TECHNOLOGY_OTHER				0x01
+#define MEMORY_TECHNOLOGY_UNKNOWN			0x02
+#define MEMORY_TECHNOLOGY_DRAM				0x03
+#define MEMORY_TECHNOLOGY_NVDIMM_N			0x04
+#define MEMORY_TECHNOLOGY_NVDIMM_F			0x05
+#define MEMORY_TECHNOLOGY_NVDIMM_P			0x06
+#define MEMORY_TECHNOLOGY_INTEL_PERSISTENT_MEMORY	0x07
+
+#define MEMORY_OPERATING_MODE_CAP_OTHER				(1 << 1)
+#define MEMORY_OPERATING_MODE_CAP_UNKNOWN			(1 << 2)
+#define MEMORY_OPERATING_MODE_CAP_VOLATILE			(1 << 3)
+#define MEMORY_OPERATING_MODE_CAP_BYTE_ACCESS_PERSISTENT	(1 << 4)
+#define MEMORY_OPERATING_MODE_CAP_BLOCK_ACCESS_PERSISTENT	(1 << 5)

typedef enum {
    MEMORY_BUS_WIDTH_8 = 0,

Change in ...coreboot[master]: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros

[HAOUAS Elyes (Code Review)]

Hello Kyösti Mälkki, build bot (Jenkins),

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

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

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

Change subject: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros
......................................................................

SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros

Change-Id: I4e466614d0a9e8c89f298594a5785af775b22a95
Signed-off-by: Elyes HAOUAS 
---
M src/include/smbios.h
1 file changed, 15 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31423/2

Change in ...coreboot[master]: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31423 )

Change subject: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/#/c/31423/2/src/include/smbios.h 
File src/include/smbios.h:

https://review.coreboot.org/#/c/31423/2/src/include/smbios.h@94 
PS2, Line 94: MEMORY
Why double memory? You start with the word memory, you don't need to finish with the word memory too(other defined fields don't have it).

https://review.coreboot.org/#/c/31423/2/src/include/smbios.h@97 
PS2, Line 97: MODE_CAP
MODE_CAP? Why not simply MODE? Or are you indicating it's a capability, and that more then 1 bit can be set at the same time?

Change in ...coreboot[master]: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros

[HAOUAS Elyes (Code Review)]

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

Change subject: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/31423/2/src/include/smbios.h 
File src/include/smbios.h:

https://review.coreboot.org/#/c/31423/2/src/include/smbios.h@97 
PS2, Line 97: MODE_CAP
> MODE_CAP? Why not simply MODE? Or are you indicating it's a capability, and that more then 1 bit can […]

that does indicate supported modes, so it can be more than 1 bit.
If I remove "CAP", it can be confused with the current configured operating mode.

Change in ...coreboot[master]: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros

[HAOUAS Elyes (Code Review)]

Hello Kyösti Mälkki, Richard Spiegel, build bot (Jenkins),

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

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

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

Change subject: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros
......................................................................

SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros

Change-Id: I4e466614d0a9e8c89f298594a5785af775b22a95
Signed-off-by: Elyes HAOUAS 
---
M src/include/smbios.h
1 file changed, 15 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31423/3

Change in ...coreboot[master]: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros

[HAOUAS Elyes (Code Review)]

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

Change subject: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/31423/3/src/include/smbios.h 
File src/include/smbios.h:

https://review.coreboot.org/#/c/31423/3/src/include/smbios.h@95 
PS3, Line 95: 
should I add a comment here? for exmple :
/* Supported operating mode(s) */

Change in ...coreboot[master]: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31423 )

Change subject: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros
......................................................................

Patch Set 3: Code-Review+2

(1 comment)

https://review.coreboot.org/#/c/31423/3/src/include/smbios.h 
File src/include/smbios.h:

https://review.coreboot.org/#/c/31423/3/src/include/smbios.h@95 
PS3, Line 95:
> should I add a comment here? for exmple : […]

No, whoever use this will have the spec and know what it is. It just caught me off guard.

Change in ...coreboot[master]: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31423 )

Change subject: SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros
......................................................................

SMBIOS: Add new MEMORY_{TYPE,TECHNOLOGY,OPERATING} macros

Change-Id: I4e466614d0a9e8c89f298594a5785af775b22a95
Signed-off-by: Elyes HAOUAS 
Reviewed-on: https://review.coreboot.org/c/31423
Tested-by: build bot (Jenkins) 
Reviewed-by: Richard Spiegel 
---
M src/include/smbios.h
1 file changed, 15 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Richard Spiegel: Looks good to me, approved

diff --git a/src/include/smbios.h b/src/include/smbios.h
index fd4deb7..34a3e5b 100644
--- a/src/include/smbios.h
+++ b/src/include/smbios.h
@@ -83,6 +83,21 @@
 #define MEMORY_TYPE_DETAIL_NON_VOLATILE		(1 << 12)
 #define MEMORY_TYPE_DETAIL_REGISTERED		(1 << 13)
 #define MEMORY_TYPE_DETAIL_UNBUFFERED		(1 << 14)
+#define MEMORY_TYPE_DETAIL_LRDIMM		(1 << 15)
+
+#define MEMORY_TECHNOLOGY_OTHER			0x01
+#define MEMORY_TECHNOLOGY_UNKNOWN		0x02
+#define MEMORY_TECHNOLOGY_DRAM			0x03
+#define MEMORY_TECHNOLOGY_NVDIMM_N		0x04
+#define MEMORY_TECHNOLOGY_NVDIMM_F		0x05
+#define MEMORY_TECHNOLOGY_NVDIMM_P		0x06
+#define MEMORY_TECHNOLOGY_INTEL_PERSISTENT	0x07
+
+#define MEMORY_OPERATING_MODE_CAP_OTHER				(1 << 1)
+#define MEMORY_OPERATING_MODE_CAP_UNKNOWN			(1 << 2)
+#define MEMORY_OPERATING_MODE_CAP_VOLATILE			(1 << 3)
+#define MEMORY_OPERATING_MODE_CAP_BYTE_ACCESS_PERSISTENT	(1 << 4)
+#define MEMORY_OPERATING_MODE_CAP_BLOCK_ACCESS_PERSISTENT	(1 << 5)

typedef enum {
    MEMORY_BUS_WIDTH_8 = 0,