Change in coreboot[master]: soc/amd/picasso: Allow mainboard to provide pci ddi descriptors

[Raul Rangel (Code Review)]

Hello Aaron Durbin,

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

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

to review the following change.

Change subject: soc/amd/picasso: Allow mainboard to provide pci ddi descriptors
......................................................................

soc/amd/picasso: Allow mainboard to provide pci ddi descriptors

Mainboards must provide their DDI descriptors.

BUG=b:153502861

Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Change-Id: Ib3f115711e74d0e6eb5b063b3dccb36b265779af
Signed-off-by: Raul E Rangel 
---
M src/soc/amd/picasso/Makefile.inc
A src/soc/amd/picasso/fsp_params.c
A src/soc/amd/picasso/include/soc/platform_descriptors.h
3 files changed, 72 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/40875/1

diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc
index d7cf9c0..55338a2 100644
--- a/src/soc/amd/picasso/Makefile.inc
+++ b/src/soc/amd/picasso/Makefile.inc
@@ -58,6 +58,7 @@
 ramstage-y += finalize.c
 ramstage-y += soc_util.c
 ramstage-y += psp.c
+ramstage-y += fsp_params.c

all-y += reset.c

diff --git a/src/soc/amd/picasso/fsp_params.c b/src/soc/amd/picasso/fsp_params.c
new file mode 100644
index 0000000..0dbda09
--- /dev/null
+++ b/src/soc/amd/picasso/fsp_params.c
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+#include <device/pci.h>
+#include <soc/pci_devs.h>
+#include <soc/platform_descriptors.h>
+#include <fsp/api.h>
+#include "chip.h"
+
+static void fill_pcie_descriptors(FSP_S_CONFIG *scfg,
+			const picasso_fsp_pcie_descriptor *descs, size_t num)
+{
+	size_t i;
+	picasso_fsp_pcie_descriptor *fsp_pcie;
+
+	/* FIXME: this violates C rules. */
+	fsp_pcie = (picasso_fsp_pcie_descriptor *)(scfg->dxio_descriptor0);
+
+	for (i = 0; i < num; i++) {
+		fsp_pcie[i] = descs[i];
+	}
+}
+
+static void fill_ddi_descriptors(FSP_S_CONFIG *scfg,
+			const picasso_fsp_ddi_descriptor *descs, size_t num)
+{
+	size_t i;
+	picasso_fsp_ddi_descriptor *fsp_ddi;
+
+	/* FIXME: this violates C rules. */
+	fsp_ddi = (picasso_fsp_ddi_descriptor *)&(scfg->ddi_descriptor0);
+
+	for (i = 0; i < num; i++) {
+		fsp_ddi[i] = descs[i];
+	}
+}
+static void fsp_fill_pcie_ddi_descriptors(FSP_S_CONFIG *scfg)
+{
+	const picasso_fsp_pcie_descriptor *fsp_pcie;
+	const picasso_fsp_ddi_descriptor *fsp_ddi;
+	size_t num_pcie;
+	size_t num_ddi;
+
+	mainboard_get_pcie_ddi_descriptors(&fsp_pcie, &num_pcie,
+						&fsp_ddi, &num_ddi);
+	fill_pcie_descriptors(scfg, fsp_pcie, num_pcie);
+	fill_ddi_descriptors(scfg, fsp_ddi, num_ddi);
+}
+
+void platform_fsp_silicon_init_params_cb(FSPS_UPD *supd)
+{
+	FSP_S_CONFIG *scfg = &supd->FspsConfig;
+
+	fsp_fill_pcie_ddi_descriptors(scfg);
+}
diff --git a/src/soc/amd/picasso/include/soc/platform_descriptors.h b/src/soc/amd/picasso/include/soc/platform_descriptors.h
new file mode 100644
index 0000000..bc67550
--- /dev/null
+++ b/src/soc/amd/picasso/include/soc/platform_descriptors.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+#ifndef __PICASSO_PLATFORM_DESCRIPTORS_H__
+#define __PICASSO_PLATFORM_DESCRIPTORS_H__
+
+#include <types.h>
+#include <platform_descriptors.h>
+#include <FspsUpd.h>
+
+/* Mainboard callback to obtain PCIe and DDI descriptors. */
+void mainboard_get_pcie_ddi_descriptors(
+		const picasso_fsp_pcie_descriptor **pcie_descs, size_t *pcie_num,
+		const picasso_fsp_ddi_descriptor **ddi_descs, size_t *ddi_num);
+
+#endif /* __PICASSO_PLATFORM_DESCRIPTORS_H__ */

[build bot (Jenkins) (Code Review)]

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

Change subject: soc/amd/picasso: Allow mainboard to provide pci ddi descriptors
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40875/1/src/soc/amd/picasso/fsp_par... 
File src/soc/amd/picasso/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/40875/1/src/soc/amd/picasso/fsp_par... 
PS1, Line 19: 	for (i = 0; i < num; i++) {
braces {} are not necessary for single statement blocks

https://review.coreboot.org/c/coreboot/+/40875/1/src/soc/amd/picasso/fsp_par... 
PS1, Line 33: 	for (i = 0; i < num; i++) {
braces {} are not necessary for single statement blocks

[Aaron Durbin (Code Review)]

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

Change subject: soc/amd/picasso: Allow mainboard to provide pci ddi descriptors
......................................................................

Patch Set 1: Code-Review+2

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40875 )

Change subject: soc/amd/picasso: Allow mainboard to provide pci ddi descriptors
......................................................................

Patch Set 1: Code-Review+2

[Patrick Georgi (Code Review)]

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

Change subject: soc/amd/picasso: Allow mainboard to provide pci ddi descriptors
......................................................................

soc/amd/picasso: Allow mainboard to provide pci ddi descriptors

Mainboards must provide their DDI descriptors.

BUG=b:153502861

Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+...
Change-Id: Ib3f115711e74d0e6eb5b063b3dccb36b265779af
Signed-off-by: Raul E Rangel

Reviewed-on: https://review.coreboot.org/c/coreboot/+/40875
Reviewed-by: Aaron Durbin 
Reviewed-by: Furquan Shaikh 
Tested-by: build bot (Jenkins) 
---
M src/soc/amd/picasso/Makefile.inc
A src/soc/amd/picasso/fsp_params.c
A src/soc/amd/picasso/include/soc/platform_descriptors.h
3 files changed, 72 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Aaron Durbin: Looks good to me, approved
  Furquan Shaikh: Looks good to me, approved

diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc
index 4790ecb..c7b6fb8 100644
--- a/src/soc/amd/picasso/Makefile.inc
+++ b/src/soc/amd/picasso/Makefile.inc
@@ -58,6 +58,7 @@
 ramstage-y += finalize.c
 ramstage-y += soc_util.c
 ramstage-y += psp.c
+ramstage-y += fsp_params.c

all-y += reset.c

diff --git a/src/soc/amd/picasso/fsp_params.c b/src/soc/amd/picasso/fsp_params.c
new file mode 100644
index 0000000..0dbda09
--- /dev/null
+++ b/src/soc/amd/picasso/fsp_params.c
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+#include <device/pci.h>
+#include <soc/pci_devs.h>
+#include <soc/platform_descriptors.h>
+#include <fsp/api.h>
+#include "chip.h"
+
+static void fill_pcie_descriptors(FSP_S_CONFIG *scfg,
+			const picasso_fsp_pcie_descriptor *descs, size_t num)
+{
+	size_t i;
+	picasso_fsp_pcie_descriptor *fsp_pcie;
+
+	/* FIXME: this violates C rules. */
+	fsp_pcie = (picasso_fsp_pcie_descriptor *)(scfg->dxio_descriptor0);
+
+	for (i = 0; i < num; i++) {
+		fsp_pcie[i] = descs[i];
+	}
+}
+
+static void fill_ddi_descriptors(FSP_S_CONFIG *scfg,
+			const picasso_fsp_ddi_descriptor *descs, size_t num)
+{
+	size_t i;
+	picasso_fsp_ddi_descriptor *fsp_ddi;
+
+	/* FIXME: this violates C rules. */
+	fsp_ddi = (picasso_fsp_ddi_descriptor *)&(scfg->ddi_descriptor0);
+
+	for (i = 0; i < num; i++) {
+		fsp_ddi[i] = descs[i];
+	}
+}
+static void fsp_fill_pcie_ddi_descriptors(FSP_S_CONFIG *scfg)
+{
+	const picasso_fsp_pcie_descriptor *fsp_pcie;
+	const picasso_fsp_ddi_descriptor *fsp_ddi;
+	size_t num_pcie;
+	size_t num_ddi;
+
+	mainboard_get_pcie_ddi_descriptors(&fsp_pcie, &num_pcie,
+						&fsp_ddi, &num_ddi);
+	fill_pcie_descriptors(scfg, fsp_pcie, num_pcie);
+	fill_ddi_descriptors(scfg, fsp_ddi, num_ddi);
+}
+
+void platform_fsp_silicon_init_params_cb(FSPS_UPD *supd)
+{
+	FSP_S_CONFIG *scfg = &supd->FspsConfig;
+
+	fsp_fill_pcie_ddi_descriptors(scfg);
+}
diff --git a/src/soc/amd/picasso/include/soc/platform_descriptors.h b/src/soc/amd/picasso/include/soc/platform_descriptors.h
new file mode 100644
index 0000000..bc67550
--- /dev/null
+++ b/src/soc/amd/picasso/include/soc/platform_descriptors.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+#ifndef __PICASSO_PLATFORM_DESCRIPTORS_H__
+#define __PICASSO_PLATFORM_DESCRIPTORS_H__
+
+#include <types.h>
+#include <platform_descriptors.h>
+#include <FspsUpd.h>
+
+/* Mainboard callback to obtain PCIe and DDI descriptors. */
+void mainboard_get_pcie_ddi_descriptors(
+		const picasso_fsp_pcie_descriptor **pcie_descs, size_t *pcie_num,
+		const picasso_fsp_ddi_descriptor **ddi_descs, size_t *ddi_num);
+
+#endif /* __PICASSO_PLATFORM_DESCRIPTORS_H__ */