Change in coreboot[master]: soc/intel/cannonlake: Add chip config for SATA strength

[Jamie Chen (Code Review)]

Jamie Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38432 )

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

soc/intel/cannonlake: Add chip config for SATA strength

Add config to chip.h for tuning SATA gen3 strength.

Change-Id: I4dcd23834fa3c01c1d88697a7bb8cf361709b62e
Signed-off-by: Jamie Chen 
---
M src/soc/intel/cannonlake/chip.h
M src/soc/intel/cannonlake/romstage/fsp_params.c
2 files changed, 30 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/38432/1

diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h
index 07a67cd..54d1d70 100644
--- a/src/soc/intel/cannonlake/chip.h
+++ b/src/soc/intel/cannonlake/chip.h
@@ -390,6 +390,14 @@
    /* SATA Power Optimizer */
    uint8_t satapwroptimize;

+	/* SATA Gen3 Strength */
+	uint8_t PchSataHsioRxGen3EqBoostMagEnable[8];
+	uint8_t PchSataHsioRxGen3EqBoostMag[8];
+	uint8_t PchSataHsioTxGen3DownscaleAmpEnable[8];
+	uint8_t PchSataHsioTxGen3DownscaleAmp[8];
+	uint8_t PchSataHsioTxGen3DeEmphEnable[8];
+	uint8_t PchSataHsioTxGen3DeEmph[8];
+
    /* Enable or disable eDP device */
    uint8_t DdiPortEdp;

diff --git a/src/soc/intel/cannonlake/romstage/fsp_params.c b/src/soc/intel/cannonlake/romstage/fsp_params.c
index 5c74d4a..8b7ac0e 100644
--- a/src/soc/intel/cannonlake/romstage/fsp_params.c
+++ b/src/soc/intel/cannonlake/romstage/fsp_params.c
@@ -101,6 +101,28 @@
    dev = pcidev_path_on_root(SA_DEVFN_IPU);
    if (dev)
    	m_cfg->SaIpuEnable = dev->enabled;
+
+	/* SATA Gen3 strength */
+	for (i = 0; i < 8; i++) {
+		if (config->PchSataHsioRxGen3EqBoostMagEnable[i]) {
+			m_cfg->PchSataHsioRxGen3EqBoostMagEnable[i] =
+				config->PchSataHsioRxGen3EqBoostMagEnable[i];
+			m_cfg->PchSataHsioRxGen3EqBoostMag[i] =
+				config->PchSataHsioRxGen3EqBoostMag[i];
+		}
+		if (config->PchSataHsioTxGen3DownscaleAmpEnable[i]) {
+			m_cfg->PchSataHsioTxGen3DownscaleAmpEnable[i] =
+				config->PchSataHsioTxGen3DownscaleAmpEnable[i];
+			m_cfg->PchSataHsioTxGen3DownscaleAmp[i] =
+				config->PchSataHsioTxGen3DownscaleAmp[i];
+		}
+		if (config->PchSataHsioTxGen3DeEmphEnable[i]) {
+			m_cfg->PchSataHsioTxGen3DeEmphEnable[i] =
+				config->PchSataHsioTxGen3DeEmphEnable[i];
+			m_cfg->PchSataHsioTxGen3DeEmph[i] =
+				config->PchSataHsioTxGen3DeEmph[i];
+		}
+	}
 }

void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version)

[Paul Menzel (Code Review)]

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

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

Patch Set 1: Code-Review+1

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38432 )

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

Patch Set 1: Code-Review+1

(2 comments)

https://review.coreboot.org/c/coreboot/+/38432/1/src/soc/intel/cannonlake/ch... 
File src/soc/intel/cannonlake/chip.h:

https://review.coreboot.org/c/coreboot/+/38432/1/src/soc/intel/cannonlake/ch... 
PS1, Line 394: 8
I suggest using a #define FOO_MAX 8 here.

https://review.coreboot.org/c/coreboot/+/38432/1/src/soc/intel/cannonlake/ro... 
File src/soc/intel/cannonlake/romstage/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/38432/1/src/soc/intel/cannonlake/ro... 
PS1, Line 106: 8
and changing the bound to FOO_MAX here.

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38432 )

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38432/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38432/1//COMMIT_MSG@9 
PS1, Line 9: Add config to chip.h for tuning SATA gen3 strength.
Is there a bug for this so when it winds up in our tree it is visible to us?

[Jamie Chen (Code Review)]

Hello Patrick Rudolph, Edward O'Callaghan, Tim Wawrzynczak, Paul Menzel, build bot (Jenkins),

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

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

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

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

soc/intel/cannonlake: Add chip config for SATA strength

Add config to chip.h for tuning SATA gen3 strength.

BUG=b:147351936
BRANCH=none
TEST=build successful in puff

Change-Id: I4dcd23834fa3c01c1d88697a7bb8cf361709b62e
Signed-off-by: Jamie Chen 
---
M src/soc/intel/cannonlake/chip.h
M src/soc/intel/cannonlake/romstage/fsp_params.c
2 files changed, 31 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/38432/2

[Jamie Chen (Code Review)]

Jamie Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38432 )

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/c/coreboot/+/38432/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38432/1//COMMIT_MSG@9 
PS1, Line 9: Add config to chip.h for tuning SATA gen3 strength.
> Is there a bug for this so when it winds up in our tree it is visible to us?

Done

https://review.coreboot.org/c/coreboot/+/38432/1/src/soc/intel/cannonlake/ch... 
File src/soc/intel/cannonlake/chip.h:

https://review.coreboot.org/c/coreboot/+/38432/1/src/soc/intel/cannonlake/ch... 
PS1, Line 394: 8
> I suggest using a #define FOO_MAX 8 here.

Done

https://review.coreboot.org/c/coreboot/+/38432/1/src/soc/intel/cannonlake/ro... 
File src/soc/intel/cannonlake/romstage/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/38432/1/src/soc/intel/cannonlake/ro... 
PS1, Line 106: 8
> and changing the bound to FOO_MAX here.

Done

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38432 )

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

Patch Set 2: Code-Review+2

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38432 )

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38432/2/src/soc/intel/cannonlake/ch... 
File src/soc/intel/cannonlake/chip.h:

https://review.coreboot.org/c/coreboot/+/38432/2/src/soc/intel/cannonlake/ch... 
PS2, Line 395: 	uint8_t PchSataHsioRxGen3EqBoostMagEnable[SOC_INTEL_CML_SATA_DEV_MAX];
             : 	uint8_t PchSataHsioRxGen3EqBoostMag[SOC_INTEL_CML_SATA_DEV_MAX];
             : 	uint8_t PchSataHsioTxGen3DownscaleAmpEnable[SOC_INTEL_CML_SATA_DEV_MAX];
             : 	uint8_t PchSataHsioTxGen3DownscaleAmp[SOC_INTEL_CML_SATA_DEV_MAX];
             : 	uint8_t PchSataHsioTxGen3DeEmphEnable[SOC_INTEL_CML_SATA_DEV_MAX];
             : 	uint8_t PchSataHsioTxGen3DeEmph[SOC_INTEL_CML_SATA_DEV_MAX];
Just a suggestion to make a struct out of this, then all the settings for one SATA device are together when they're in the devicetree.

[Jamie Chen (Code Review)]

Hello Patrick Rudolph, Edward O'Callaghan, Tim Chen, Tim Wawrzynczak, Paul Menzel, build bot (Jenkins),

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

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

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

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

soc/intel/cannonlake: Add chip config for SATA strength

Add config to chip.h for tuning SATA gen3 strength.

BUG=b:147351936
BRANCH=none
TEST=build successful in puff

Change-Id: I4dcd23834fa3c01c1d88697a7bb8cf361709b62e
Signed-off-by: Jamie Chen 
---
M src/soc/intel/cannonlake/chip.h
A src/soc/intel/cannonlake/include/soc/sata.h
M src/soc/intel/cannonlake/romstage/fsp_params.c
3 files changed, 59 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/38432/4

[Jamie Chen (Code Review)]

Jamie Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38432 )

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38432/2/src/soc/intel/cannonlake/ch... 
File src/soc/intel/cannonlake/chip.h:

https://review.coreboot.org/c/coreboot/+/38432/2/src/soc/intel/cannonlake/ch... 
PS2, Line 395: 	uint8_t PchSataHsioRxGen3EqBoostMagEnable[SOC_INTEL_CML_SATA_DEV_MAX];
             : 	uint8_t PchSataHsioRxGen3EqBoostMag[SOC_INTEL_CML_SATA_DEV_MAX];
             : 	uint8_t PchSataHsioTxGen3DownscaleAmpEnable[SOC_INTEL_CML_SATA_DEV_MAX];
             : 	uint8_t PchSataHsioTxGen3DownscaleAmp[SOC_INTEL_CML_SATA_DEV_MAX];
             : 	uint8_t PchSataHsioTxGen3DeEmphEnable[SOC_INTEL_CML_SATA_DEV_MAX];
             : 	uint8_t PchSataHsioTxGen3DeEmph[SOC_INTEL_CML_SATA_DEV_MAX];
> Just a suggestion to make a struct out of this, then all the settings for one SATA device are togeth […]

Done

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38432 )

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

Patch Set 4: Code-Review+2

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38432 )

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

Patch Set 5: Code-Review+2

[Patrick Georgi (Code Review)]

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

Change subject: soc/intel/cannonlake: Add chip config for SATA strength
......................................................................

soc/intel/cannonlake: Add chip config for SATA strength

Add config to chip.h for tuning SATA gen3 strength.

BUG=b:147351936
BRANCH=none
TEST=build successful in puff

Change-Id: I4dcd23834fa3c01c1d88697a7bb8cf361709b62e
Signed-off-by: Jamie Chen 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38432
Reviewed-by: Edward O'Callaghan 
Reviewed-by: Tim Wawrzynczak 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/cannonlake/chip.h
A src/soc/intel/cannonlake/include/soc/sata.h
M src/soc/intel/cannonlake/romstage/fsp_params.c
3 files changed, 59 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Tim Wawrzynczak: Looks good to me, approved
  Edward O'Callaghan: Looks good to me, approved

diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h
index 07a67cd..0712146 100644
--- a/src/soc/intel/cannonlake/chip.h
+++ b/src/soc/intel/cannonlake/chip.h
@@ -29,6 +29,7 @@
 #include <soc/pci_devs.h>
 #include <soc/pm.h>
 #include <soc/pmc.h>
+#include <soc/sata.h>
 #include <soc/serialio.h>
 #include <soc/usb.h>
 #include <soc/vr_config.h>
@@ -39,6 +40,7 @@
 #endif

#define SOC_INTEL_CML_UART_DEV_MAX 3
+#define SOC_INTEL_CML_SATA_DEV_MAX 8

struct soc_intel_cannonlake_config {

@@ -390,6 +392,9 @@
    /* SATA Power Optimizer */
    uint8_t satapwroptimize;

+	/* SATA Gen3 Strength */
+	struct sata_port_config sata_port[SOC_INTEL_CML_SATA_DEV_MAX];
+
    /* Enable or disable eDP device */
    uint8_t DdiPortEdp;

diff --git a/src/soc/intel/cannonlake/include/soc/sata.h b/src/soc/intel/cannonlake/include/soc/sata.h
new file mode 100644
index 0000000..17802c3
--- /dev/null
+++ b/src/soc/intel/cannonlake/include/soc/sata.h
@@ -0,0 +1,32 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2020 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 _SOC_SATA_H_
+#define _SOC_SATA_H_
+
+#include <stdint.h>
+
+/* SATA Gen3 strength */
+struct sata_port_config {
+	uint8_t RxGen3EqBoostMagEnable;
+	uint8_t RxGen3EqBoostMag;
+	uint8_t TxGen3DownscaleAmpEnable;
+	uint8_t TxGen3DownscaleAmp;
+	uint8_t TxGen3DeEmphEnable;
+	uint8_t TxGen3DeEmph;
+};
+
+#endif
diff --git a/src/soc/intel/cannonlake/romstage/fsp_params.c b/src/soc/intel/cannonlake/romstage/fsp_params.c
index 5c74d4a..3c5be30 100644
--- a/src/soc/intel/cannonlake/romstage/fsp_params.c
+++ b/src/soc/intel/cannonlake/romstage/fsp_params.c
@@ -101,6 +101,28 @@
    dev = pcidev_path_on_root(SA_DEVFN_IPU);
    if (dev)
    	m_cfg->SaIpuEnable = dev->enabled;
+
+	/* SATA Gen3 strength */
+	for (i = 0; i < SOC_INTEL_CML_SATA_DEV_MAX; i++) {
+		if (config->sata_port[i].RxGen3EqBoostMagEnable) {
+			m_cfg->PchSataHsioRxGen3EqBoostMagEnable[i] =
+				config->sata_port[i].RxGen3EqBoostMagEnable;
+			m_cfg->PchSataHsioRxGen3EqBoostMag[i] =
+				config->sata_port[i].RxGen3EqBoostMag;
+		}
+		if (config->sata_port[i].TxGen3DownscaleAmpEnable) {
+			m_cfg->PchSataHsioTxGen3DownscaleAmpEnable[i] =
+				config->sata_port[i].TxGen3DownscaleAmpEnable;
+			m_cfg->PchSataHsioTxGen3DownscaleAmp[i] =
+				config->sata_port[i].TxGen3DownscaleAmp;
+		}
+		if (config->sata_port[i].TxGen3DeEmphEnable) {
+			m_cfg->PchSataHsioTxGen3DeEmphEnable[i] =
+				config->sata_port[i].TxGen3DeEmphEnable;
+			m_cfg->PchSataHsioTxGen3DeEmph[i] =
+				config->sata_port[i].TxGen3DeEmph;
+		}
+	}
 }

void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version)