Change in ...coreboot[master]: soc/intel/apl: enable lpc at bootblock

[Thomas Heijligen (Code Review)]

Hello Nico Huber,

I'd like you to do a code review. Please visit

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

to review the following change.

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

soc/intel/apl: enable lpc at bootblock

Serial console can be attached to lpc bus. Enable it for early debug

Change-Id: Ib3d09e4eb23655825b7603f49f0bad31116ac6e1
Signed-off-by: Nico Huber 
Signed-off-by: Thomas Heijligen 
---
M src/soc/intel/apollolake/bootblock/bootblock.c
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/lpc.c
M src/soc/intel/common/block/lpc/lpc_lib.c
4 files changed, 21 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/31371/1

diff --git a/src/soc/intel/apollolake/bootblock/bootblock.c b/src/soc/intel/apollolake/bootblock/bootblock.c
index cf3e839..e0217ef 100644
--- a/src/soc/intel/apollolake/bootblock/bootblock.c
+++ b/src/soc/intel/apollolake/bootblock/bootblock.c
@@ -20,6 +20,7 @@
 #include <device/pci.h>
 #include <intelblocks/cpulib.h>
 #include <intelblocks/fast_spi.h>
+#include <intelblocks/lpc_lib.h>
 #include <intelblocks/p2sb.h>
 #include <intelblocks/pcr.h>
 #include <intelblocks/rtc.h>
@@ -120,4 +121,6 @@
    	paging_set_default_pat();
    	paging_enable_for_car("pdpt", "pt");
    }
+
+	pch_enable_lpc();
 }
diff --git a/src/soc/intel/apollolake/chip.h b/src/soc/intel/apollolake/chip.h
index b8f9f8c..2f25a3b 100644
--- a/src/soc/intel/apollolake/chip.h
+++ b/src/soc/intel/apollolake/chip.h
@@ -44,6 +44,9 @@
    /* Common structure containing soc config data required by common code*/
    struct soc_intel_common_config common_soc_config;

+	/* Generic i/o decode ranges routed to LPC */
+	uint32_t lpc_gen_dec[4];
+
    /*
     * Mapping from PCIe root port to CLKREQ input on the SOC. The SOC has
     * four CLKREQ inputs, but six root ports. Root ports without an
diff --git a/src/soc/intel/apollolake/lpc.c b/src/soc/intel/apollolake/lpc.c
index a20b82a..ffa4cf5 100644
--- a/src/soc/intel/apollolake/lpc.c
+++ b/src/soc/intel/apollolake/lpc.c
@@ -42,6 +42,18 @@
    return apl_lpc_fixed_mmio_ranges;
 }

+void soc_get_gen_io_dec_range(const struct device *dev, uint32_t *gen_io_dec)
+{
+	const struct soc_intel_apollolake_config *const config = dev->chip_info;
+
+	if (!config) {
+		memset(gen_io_dec, 0x00, sizeof(config->lpc_gen_dec));
+		return;
+	}
+
+	memcpy(gen_io_dec, config->lpc_gen_dec, sizeof(config->lpc_gen_dec));
+}
+
 static const struct pad_config lpc_gpios[] = {
 #if IS_ENABLED(CONFIG_SOC_INTEL_GLK)
 #if !IS_ENABLED(CONFIG_SOC_ESPI)
diff --git a/src/soc/intel/common/block/lpc/lpc_lib.c b/src/soc/intel/common/block/lpc/lpc_lib.c
index fb50b74..d584d2c 100644
--- a/src/soc/intel/common/block/lpc/lpc_lib.c
+++ b/src/soc/intel/common/block/lpc/lpc_lib.c
@@ -24,6 +24,9 @@
 #include <lib.h>
 #include "lpc_def.h"
 #include <soc/pci_devs.h>
+#include <stdint.h>
+
+__weak void soc_setup_dmi_pcr_io_dec(uint32_t *gen_io_dec) {}

uint16_t lpc_enable_fixed_io_ranges(uint16_t io_enables)
 {

[Nico Huber (Code Review)]

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

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

Patch Set 2:

Looking through the history for reviewers, I realized that
this was a feature years ago. Don't know why it was removed.

[Patrick Rudolph (Code Review)]

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

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

Patch Set 2: Code-Review+2

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/31371/2/src/soc/intel/apollolake/chip.h 
File src/soc/intel/apollolake/chip.h:

https://review.coreboot.org/#/c/31371/2/src/soc/intel/apollolake/chip.h@48 
PS2, Line 48: 4
shouldn't this be based on the expected size? i.e. LPC_NUM_GENERIC_IO_RANGES ?

[Nico Huber (Code Review)]

Nico Huber has uploaded a new patch set (#3) to the change originally created by Thomas Heijligen. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

soc/intel/apl: enable lpc at bootblock

Serial console can be attached to lpc bus. Enable it for early debug

Change-Id: Ib3d09e4eb23655825b7603f49f0bad31116ac6e1
Signed-off-by: Nico Huber 
Signed-off-by: Thomas Heijligen 
---
M src/soc/intel/apollolake/bootblock/bootblock.c
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/lpc.c
M src/soc/intel/common/block/lpc/lpc_lib.c
4 files changed, 21 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/31371/3

[Nico Huber (Code Review)]

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

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/31371/2/src/soc/intel/apollolake/chip.h 
File src/soc/intel/apollolake/chip.h:

https://review.coreboot.org/#/c/31371/2/src/soc/intel/apollolake/chip.h@48 
PS2, Line 48: 4
> shouldn't this be based on the expected size? i.e. […]

Thanks, Done

Change in coreboot[master]: [WIP] mAL10

[Felix Singer (Code Review)]

Felix Singer has uploaded a new patch set (#4) to the change originally created by Thomas Heijligen. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: [WIP] mAL10
......................................................................

[WIP] mAL10

Change-Id: Ib3d09e4eb23655825b7603f49f0bad31116ac6e1
Signed-off-by: Felix Singer 
---
M src/lib/bootblock.c
A src/mainboard/kontron/mal10/Kconfig
A src/mainboard/kontron/mal10/Kconfig.name
A src/mainboard/kontron/mal10/Makefile.inc
A src/mainboard/kontron/mal10/board_info.txt
A src/mainboard/kontron/mal10/bootblock.c
A src/mainboard/kontron/mal10/default.fmd
A src/mainboard/kontron/mal10/devicetree.cb
A src/mainboard/kontron/mal10/dsdt.asl
A src/mainboard/kontron/mal10/early_gpio.h
A src/mainboard/kontron/mal10/gma-mainboard.ads
A src/mainboard/kontron/mal10/gpio.h
A src/mainboard/kontron/mal10/mainboard.c
A src/mainboard/kontron/mal10/romstage.c
M src/soc/intel/apollolake/Makefile.inc
M src/soc/intel/apollolake/bootblock/bootblock.c
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/lpc.c
M src/soc/intel/common/block/lpc/lpc_lib.c
19 files changed, 433 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/31371/4

Change in coreboot[master]: [WIP] mAL10

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: [WIP] mAL10
......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/31371/4/src/mainboard/kontron/mal10... 
File src/mainboard/kontron/mal10/gpio.h:

https://review.coreboot.org/c/coreboot/+/31371/4/src/mainboard/kontron/mal10... 
PS4, Line 71: 	PAD_CFG_NF_IOSSTATE_IOSTERM(GPIO_172, DN_20K, DEEP, NF1, HIZCRx1, DISPUPD), /* SDCARD_CLK */
line over 96 characters

https://review.coreboot.org/c/coreboot/+/31371/4/src/mainboard/kontron/mal10... 
File src/mainboard/kontron/mal10/romstage.c:

https://review.coreboot.org/c/coreboot/+/31371/4/src/mainboard/kontron/mal10... 
PS4, Line 7: void mainboard_memory_init_params(FSPM_UPD *const mupd)
need consistent spacing around '*' (ctx:WxV)

Change in coreboot[master]: mb/kontron/mAL10: Add new mainboard

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: mb/kontron/mAL10: Add new mainboard
......................................................................

Patch Set 6:

(1 comment)

We should split the patch up into soc vs mainboard patches.

https://review.coreboot.org/c/coreboot/+/31371/6/src/lib/bootblock.c 
File src/lib/bootblock.c:

https://review.coreboot.org/c/coreboot/+/31371/6/src/lib/bootblock.c@11 
PS6, Line 11: #include <reset.h>
Why is this in here?

Change in coreboot[master]: mb/kontron/mAL10: Add new mainboard

[Felix Singer (Code Review)]

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: mb/kontron/mAL10: Add new mainboard
......................................................................

Patch Set 6:
> We should split the patch up into soc vs mainboard patches.

Sorry for the mess. That's not intended.

Looks like we mixed up some change-ids somehow. I will undo this.

Change in coreboot[master]: soc/intel/apl: enable lpc at bootblock

[Felix Singer (Code Review)]

Felix Singer has uploaded a new patch set (#7) to the change originally created by Thomas Heijligen. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

soc/intel/apl: enable lpc at bootblock

Serial console can be attached to lpc bus. Enable it for early debug

Change-Id: Ib3d09e4eb23655825b7603f49f0bad31116ac6e1
Signed-off-by: Nico Huber 
Signed-off-by: Thomas Heijligen 
---
M src/soc/intel/apollolake/bootblock/bootblock.c
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/lpc.c
M src/soc/intel/common/block/lpc/lpc_lib.c
4 files changed, 21 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/31371/7

Change in coreboot[master]: soc/intel/apl: enable lpc at bootblock

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

Patch Set 7: Code-Review+1

(2 comments)

https://review.coreboot.org/c/coreboot/+/31371/7//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/31371/7//COMMIT_MSG@9 
PS7, Line 9: Serial console can be attached to lpc bus. Enable it for early debug
Please add a dot/period at the end.

https://review.coreboot.org/c/coreboot/+/31371/7/src/soc/intel/apollolake/lp... 
File src/soc/intel/apollolake/lpc.c:

https://review.coreboot.org/c/coreboot/+/31371/7/src/soc/intel/apollolake/lp... 
PS7, Line 50: 0x00
0x0?

Change in coreboot[master]: soc/intel/apl: enable lpc at bootblock

[Nico Huber (Code Review)]

Nico Huber has uploaded a new patch set (#9) to the change originally created by Thomas Heijligen. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

soc/intel/apl: enable lpc at bootblock

Serial console can be attached to lpc bus. Enable it for early debug.

Change-Id: Ib3d09e4eb23655825b7603f49f0bad31116ac6e1
Signed-off-by: Nico Huber 
Signed-off-by: Thomas Heijligen 
---
M src/soc/intel/apollolake/bootblock/bootblock.c
M src/soc/intel/apollolake/chip.h
M src/soc/intel/apollolake/lpc.c
M src/soc/intel/common/block/lpc/lpc_lib.c
4 files changed, 20 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/31371/9

Change in coreboot[master]: soc/intel/apl: enable lpc at bootblock

[Nico Huber (Code Review)]

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

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

Patch Set 9:

(3 comments)

https://review.coreboot.org/c/coreboot/+/31371/7//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/31371/7//COMMIT_MSG@9 
PS7, Line 9: Serial console can be attached to lpc bus. Enable it for early debug
> Please add a dot/period at the end.

Done

https://review.coreboot.org/c/coreboot/+/31371/6/src/lib/bootblock.c 
File src/lib/bootblock.c:

https://review.coreboot.org/c/coreboot/+/31371/6/src/lib/bootblock.c@11 
PS6, Line 11: #include <reset.h>
> Why is this in here?

Bad push, fixed.

https://review.coreboot.org/c/coreboot/+/31371/7/src/soc/intel/apollolake/lp... 
File src/soc/intel/apollolake/lpc.c:

https://review.coreboot.org/c/coreboot/+/31371/7/src/soc/intel/apollolake/lp... 
PS7, Line 50: 0x00
> 0x0?

`0x00` is how I remind myself that we write bytes. I would understand `0`
but why `0x0`? It just looks like a weird emoticon.

Change in coreboot[master]: soc/intel/apl: enable lpc at bootblock

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

Patch Set 9:

(1 comment)

https://review.coreboot.org/c/coreboot/+/31371/7/src/soc/intel/apollolake/lp... 
File src/soc/intel/apollolake/lpc.c:

https://review.coreboot.org/c/coreboot/+/31371/7/src/soc/intel/apollolake/lp... 
PS7, Line 50: 0x00
> `0x00` is how I remind myself that we write bytes. I would understand `0` […]

Ack

Change in coreboot[master]: soc/intel/apl: enable lpc at bootblock

[Nico Huber (Code Review)]

Attention is currently required from: Thomas Heijligen.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

Patch Set 9:

(1 comment)

Patchset:

PS9: 
Looks like this can be abandoned.

Change in coreboot[master]: soc/intel/apl: enable lpc at bootblock

[Thomas Heijligen (Code Review)]

Thomas Heijligen has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31371 )

Change subject: soc/intel/apl: enable lpc at bootblock
......................................................................

Abandoned