Change in coreboot[master]: mainboard/intel/common: Add some common board and memory APIs
Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37707 )
Change subject: mainboard/intel/common: Add some common board and memory APIs ......................................................................
mainboard/intel/common: Add some common board and memory APIs
Add support for below APIs in mainboard intel/common:
1. get_board_id: Returns board ID from EC. 2. get_spd_index: Returns SPD index from board ID.
Change-Id: I40c9454b583d6bc0e3034f3400beb822124ba8f4 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- A src/mainboard/intel/common/Makefile.inc A src/mainboard/intel/common/board/Makefile.inc A src/mainboard/intel/common/board/board_id.c A src/mainboard/intel/common/include/intel_mb/board_id.h A src/mainboard/intel/common/include/intel_mb/spd.h A src/mainboard/intel/common/memory/Makefile.inc A src/mainboard/intel/common/memory/spd.c 7 files changed, 143 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/37707/1
diff --git a/src/mainboard/intel/common/Makefile.inc b/src/mainboard/intel/common/Makefile.inc new file mode 100644 index 0000000..54bba3f --- /dev/null +++ b/src/mainboard/intel/common/Makefile.inc @@ -0,0 +1,8 @@ +ifeq ($(CONFIG_MAINBOARD_INTEL_COMMON),y) + +subdirs-y += board/ +subdirs-y += memory/ + +CPPFLAGS_common += -I$(src)/mainboard/intel/common/include/ + +endif diff --git a/src/mainboard/intel/common/board/Makefile.inc b/src/mainboard/intel/common/board/Makefile.inc new file mode 100644 index 0000000..4781dfb --- /dev/null +++ b/src/mainboard/intel/common/board/Makefile.inc @@ -0,0 +1,3 @@ +romstage-y += board_id.c +ramstage-y += board_id.c + diff --git a/src/mainboard/intel/common/board/board_id.c b/src/mainboard/intel/common/board/board_id.c new file mode 100644 index 0000000..19897c2 --- /dev/null +++ b/src/mainboard/intel/common/board/board_id.c @@ -0,0 +1,52 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <boardid.h> +#include <ec/acpi/ec.h> +#include <ec/google/chromeec/ec.h> +#include <intel_mb/board_id.h> +#include <stdint.h> + +static int get_board_id_via_ext_ec(void) +{ + uint32_t id = BOARD_ID_INIT; + + if (google_chromeec_get_board_version(&id)) + id = BOARD_ID_UNKNOWN; + + return id; +} + +/* Get Board ID via EC I/O port write/read */ +int get_board_id(void) +{ + MAYBE_STATIC_NONZERO int id = -1; + + if (id < 0) { + if (CONFIG(EC_GOOGLE_CHROMEEC)) + id = get_board_id_via_ext_ec(); + else{ + uint8_t buffer[2]; + uint8_t index; + if (send_ec_command(EC_FAB_ID_CMD) == 0) { + for (index = 0; index < sizeof(buffer); index++) + buffer[index] = recv_ec_data(); + id = (buffer[0] << 8) | buffer[1]; + } + } + } + + return id; +} diff --git a/src/mainboard/intel/common/include/intel_mb/board_id.h b/src/mainboard/intel/common/include/intel_mb/board_id.h new file mode 100644 index 0000000..9aac527 --- /dev/null +++ b/src/mainboard/intel/common/include/intel_mb/board_id.h @@ -0,0 +1,30 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _MAINBOARD_COMMON_BOARD_ID_H_ +#define _MAINBOARD_COMMON_BOARD_ID_H_ + +#include <stdint.h> + +/* Board/FAB ID Command */ +#define EC_FAB_ID_CMD 0x0D + +/* + * Returns board information (board id[15:8] and + * Fab info[7:0]) on success and < 0 on error + */ +int get_board_id(void); + +#endif /* _MAINBOARD_COMMON_BOARD_ID_H_ */ diff --git a/src/mainboard/intel/common/include/intel_mb/spd.h b/src/mainboard/intel/common/include/intel_mb/spd.h new file mode 100644 index 0000000..3f834b8 --- /dev/null +++ b/src/mainboard/intel/common/include/intel_mb/spd.h @@ -0,0 +1,24 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _MAINBOARD_COMMON_SPD_H_ +#define _MAINBOARD_COMMON_SPD_H_ + +#include <stdint.h> + +/* Returns SPD index from board ID */ +uint8_t get_spd_index(void); + +#endif /* _MAINBOARD_COMMON_SPD_H_ */ diff --git a/src/mainboard/intel/common/memory/Makefile.inc b/src/mainboard/intel/common/memory/Makefile.inc new file mode 100644 index 0000000..c0e5f53 --- /dev/null +++ b/src/mainboard/intel/common/memory/Makefile.inc @@ -0,0 +1,2 @@ +romstage-y += spd.c + diff --git a/src/mainboard/intel/common/memory/spd.c b/src/mainboard/intel/common/memory/spd.c new file mode 100644 index 0000000..5da5bf4 --- /dev/null +++ b/src/mainboard/intel/common/memory/spd.c @@ -0,0 +1,24 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <intel_mb/board_id.h> +#include <intel_mb/spd.h> +#include <stdint.h> + +uint8_t get_spd_index(void) +{ + return get_board_id() & 0x7; + +}
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37707
to look at the new patch set (#3).
Change subject: mainboard/intel/common: Add some common board and memory APIs ......................................................................
mainboard/intel/common: Add some common board and memory APIs
Add support for below APIs in mainboard intel/common:
1. get_board_id: Returns board ID from EC. 2. get_spd_index: Returns SPD index from board ID.
Change-Id: I40c9454b583d6bc0e3034f3400beb822124ba8f4 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- A src/mainboard/intel/common/Makefile.inc A src/mainboard/intel/common/board/Makefile.inc A src/mainboard/intel/common/board/board_id.c A src/mainboard/intel/common/include/intel_mb/board_id.h A src/mainboard/intel/common/include/intel_mb/spd.h A src/mainboard/intel/common/memory/Makefile.inc A src/mainboard/intel/common/memory/spd.c 7 files changed, 141 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/37707/3
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37707 )
Change subject: mainboard/intel/common: Add some common board and memory APIs ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... File src/mainboard/intel/common/board/board_id.c:
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... PS4, Line 24: uint32_t why use a variable of different signature from the return type?
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... PS4, Line 38: if (CONFIG(EC_GOOGLE_CHROMEEC)) : id = get_board_id_via_ext_ec(); : else use brackets on if condition too.
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... PS4, Line 40: { space before.
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... PS4, Line 45: buffer[index] = recv_ec_data(); id |= recv_ec_data() << (8 * (1 - index)); after clearing id?
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... File src/mainboard/intel/common/memory/spd.c:
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... PS4, Line 23: remove newline.
Hello Subrata Banik, Wonkyu Kim, Maulik V Vaghela, Ravishankar Sarawadi, Rizwan Qureshi, V Sowmya, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37707
to look at the new patch set (#5).
Change subject: mainboard/intel/common: Add some common board and memory APIs ......................................................................
mainboard/intel/common: Add some common board and memory APIs
Add support for below APIs in mainboard intel/common:
1. get_board_id: Returns board ID from EC. 2. get_spd_index: Returns SPD index from board ID.
Change-Id: I40c9454b583d6bc0e3034f3400beb822124ba8f4 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- A src/mainboard/intel/common/Makefile.inc A src/mainboard/intel/common/board/Makefile.inc A src/mainboard/intel/common/board/board_id.c A src/mainboard/intel/common/include/intel_mb/board_id.h A src/mainboard/intel/common/include/intel_mb/spd.h A src/mainboard/intel/common/memory/Makefile.inc A src/mainboard/intel/common/memory/spd.c 7 files changed, 139 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/37707/5
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37707 )
Change subject: mainboard/intel/common: Add some common board and memory APIs ......................................................................
Patch Set 5:
(5 comments)
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... File src/mainboard/intel/common/board/board_id.c:
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... PS4, Line 24: uint32_t
why use a variable of different signature from the return type?
Done
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... PS4, Line 38: if (CONFIG(EC_GOOGLE_CHROMEEC)) : id = get_board_id_via_ext_ec(); : else
use brackets on if condition too.
Done
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... PS4, Line 40: {
space before.
Done
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... PS4, Line 45: buffer[index] = recv_ec_data();
id |= recv_ec_data() << (8 * (1 - index)); after clearing id?
Current one looks cleaner, can we keep as is?
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... File src/mainboard/intel/common/memory/spd.c:
https://review.coreboot.org/c/coreboot/+/37707/4/src/mainboard/intel/common/... PS4, Line 23:
remove newline.
Done
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/37707?usp=email )
Change subject: mainboard/intel/common: Add some common board and memory APIs ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Aamir Bohra (Code Review) -
Arthur Heymans (Code Review)
-
Martin L Roth (Code Review)