Change in coreboot[master]: sb/intel/bd82x6x: Clean up MBP item ID handling

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45263 )

Change subject: sb/intel/bd82x6x: Clean up MBP item ID handling
......................................................................

sb/intel/bd82x6x: Clean up MBP item ID handling

Since this ID is only used to concatenate two values, it doesn't matter
how it is defined as long as the definition is the same everywhere.

Update the `MBP_MAKE_IDENT` macro to match what Lynx Point does. Also,
improve some log messages to print the components of the item ID.

Change-Id: I908a6973ab64b1162ec5b835c7c4ffba671d4f05
Signed-off-by: Angel Pons 
---
M src/southbridge/intel/bd82x6x/me.h
M src/southbridge/intel/bd82x6x/me_8.x.c
2 files changed, 7 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/45263/1

diff --git a/src/southbridge/intel/bd82x6x/me.h b/src/southbridge/intel/bd82x6x/me.h
index dc1fa48..29089a9 100644
--- a/src/southbridge/intel/bd82x6x/me.h
+++ b/src/southbridge/intel/bd82x6x/me.h
@@ -284,7 +284,7 @@
 /* HWA Items */
 #define MBP_HWA_REQUEST_ITEM	1

-#define MBP_MAKE_IDENT(appid, item) (((item) << 8) + (appid))
+#define MBP_MAKE_IDENT(appid, item) (((appid) << 8) | (item))

#define MBP_IDENT(appid, item) MBP_MAKE_IDENT(MBP_APPID_##appid, MBP_##appid##_##item##_ITEM)

diff --git a/src/southbridge/intel/bd82x6x/me_8.x.c b/src/southbridge/intel/bd82x6x/me_8.x.c
index 6d0a82d..45e7efe 100644
--- a/src/southbridge/intel/bd82x6x/me_8.x.c
+++ b/src/southbridge/intel/bd82x6x/me_8.x.c
@@ -239,7 +239,6 @@
    mbp_header mbp_hdr;
    mbp_item_header	mbp_item_hdr;
    u32 me2host_pending;
-	u32 mbp_item_id;
    struct mei_csr host;

me2host_pending = me_to_host_words_pending();
@@ -266,7 +265,6 @@
    while (mbp_hdr.num_entries--) {
    	u32 *copy_addr;
    	u32 copy_size, buffer_room;
-		void *p;

if (!me2host_pending) {
    		printk(BIOS_ERR, "ME: no mbp data %d entries to go!\n",
@@ -285,8 +283,6 @@

me2host_pending -= mbp_item_hdr.length;

-		mbp_item_id = MBP_MAKE_IDENT(mbp_item_hdr.app_id, (u32)mbp_item_hdr.item_id);
-
    	copy_size = mbp_item_hdr.length - 1;

#define SET_UP_COPY(field) { copy_addr = (u32 *)&mbp_data->field;	     \
@@ -294,10 +290,10 @@
    		break;					             \
    	}

-		p = &mbp_item_hdr;
-		printk(BIOS_INFO, "ME: MBP item header %8.8x\n", *((u32*)p));
+		printk(BIOS_INFO, "ME: MBP item: app id = %u, item id = %u, length = %u\n",
+		       mbp_item_hdr.app_id, mbp_item_hdr.item_id, mbp_item_hdr.length);

-		switch (mbp_item_id) {
+		switch (MBP_MAKE_IDENT(mbp_item_hdr.app_id, mbp_item_hdr.item_id)) {
    	case MBP_IDENT(KERNEL, FW_VER):
    		SET_UP_COPY(fw_version_name);

@@ -325,17 +321,15 @@
    		SET_UP_COPY(mfsintegrity);

default:
-			printk(BIOS_ERR, "ME: unknown mbp item id 0x%x! Skipping\n",
-			       mbp_item_id);
+			printk(BIOS_WARN, "ME: Unknown MBP item! Skipping...\n");
    		while (copy_size--)
    			read_cb();
    		continue;
    	}

if (buffer_room != copy_size) {
-			printk(BIOS_ERR, "ME: buffer room %d != %d copy size"
-			       " for item  0x%x!!!\n",
-			       buffer_room, copy_size, mbp_item_id);
+			printk(BIOS_ERR, "ME: buffer room %d != %d copy size!!!\n",
+			       buffer_room, copy_size);
    		return -1;
    	}
    	while (copy_size--)

[Angel Pons (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph,

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

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

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

Change subject: sb/intel/bd82x6x: Clean up MBP item ID handling
......................................................................

sb/intel/bd82x6x: Clean up MBP item ID handling

Since this ID is only used to concatenate two values, it doesn't matter
how it is defined as long as the definition is the same everywhere.

Update the `MBP_MAKE_IDENT` macro to match what Lynx Point does. Also,
improve some log messages to print the components of the item ID.

Change-Id: I908a6973ab64b1162ec5b835c7c4ffba671d4f05
Signed-off-by: Angel Pons 
---
M src/southbridge/intel/bd82x6x/me.h
M src/southbridge/intel/bd82x6x/me_8.x.c
2 files changed, 7 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/45263/2

[Angel Pons (Code Review)]

Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/45263 )

Change subject: sb/intel/bd82x6x: Clean up MBP item ID handling
......................................................................

Abandoned

Rotten