Witherspoon: Remove old Witherspoon platform definition

Message ID 20180108055648.24281-1-alistair@popple.id.au
State Superseded
Headers show
Series
  • Witherspoon: Remove old Witherspoon platform definition
Related show

Commit Message

Alistair Popple Jan. 8, 2018, 5:56 a.m.
An old Witherspoon platform definition was added to aid the transition from
versions of Hostboot which didn't have the correct NVLink HDAT information
available and/or planar VPD. These system should now be updated so remove
the possibly incorrect default assumption.

This may disable NVLink on old out-dated systems but it can easily be
restored with the appropriate FW and/or VPD updates. In any case there is a
a 50% chance the existing default behaviour was incorrect as it only
supports 6 GPU systems. Using an incorrect platform definition leads to
undefined behaviour which is more difficult to detect/debug than not
creating the NVLink devices so remove the possibly incorrect default
behaviour.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
 platforms/astbmc/witherspoon.c | 352 -----------------------------------------
 1 file changed, 352 deletions(-)

Comments

Andrew Donnellan Jan. 8, 2018, 6:28 a.m. | #1
On 08/01/18 16:56, Alistair Popple wrote:
> An old Witherspoon platform definition was added to aid the transition from
> versions of Hostboot which didn't have the correct NVLink HDAT information
> available and/or planar VPD. These system should now be updated so remove
> the possibly incorrect default assumption.
> 
> This may disable NVLink on old out-dated systems but it can easily be
> restored with the appropriate FW and/or VPD updates. In any case there is a
> a 50% chance the existing default behaviour was incorrect as it only
> supports 6 GPU systems. Using an incorrect platform definition leads to
> undefined behaviour which is more difficult to detect/debug than not
> creating the NVLink devices so remove the possibly incorrect default
> behaviour.
> 
> Signed-off-by: Alistair Popple <alistair@popple.id.au>

Yay!

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

> ---
>   platforms/astbmc/witherspoon.c | 352 -----------------------------------------
>   1 file changed, 352 deletions(-)
> 
> diff --git a/platforms/astbmc/witherspoon.c b/platforms/astbmc/witherspoon.c
> index b9ed2778..dc6459b9 100644
> --- a/platforms/astbmc/witherspoon.c
> +++ b/platforms/astbmc/witherspoon.c
> @@ -30,309 +30,6 @@
>   
>   #include "astbmc.h"
>   
> -static const struct slot_table_entry witherspoon_slot1[] = {
> -	{
> -		.etype = st_pluggable_slot,
> -		.location = ST_LOC_DEVFN(0,0),
> -		.name = "SLOT0"
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_slot2_shared[] = {
> -	{
> -		.etype = st_pluggable_slot,
> -		.location = ST_LOC_DEVFN(0,0),
> -		.name = "SLOT1"
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_slot3[] = {
> -	{
> -		.etype = st_pluggable_slot,
> -		.location = ST_LOC_DEVFN(0,0),
> -		.name = "SLOT2"
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_slot4[] = {
> -	{
> -		.etype = st_pluggable_slot,
> -		.location = ST_LOC_DEVFN(0,0),
> -		.name = "SLOT3"
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu0[] = {
> -	{
> -		.etype = st_pluggable_slot,
> -		.location = ST_LOC_DEVFN(0x80,0),
> -		.name = "GPU0",
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu1[] = {
> -	{
> -		.etype = st_pluggable_slot,
> -		.location = ST_LOC_DEVFN(0xa0,0),
> -		.name = "GPU1",
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu2[] = {
> -	{
> -		.etype = st_pluggable_slot,
> -		.location = ST_LOC_DEVFN(0xc0,0),
> -		.name = "GPU2",
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu3[] = {
> -	{
> -		.etype = st_pluggable_slot,
> -		.location = ST_LOC_DEVFN(0x60,0),
> -		.name = "GPU3",
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu4[] = {
> -	{
> -		.etype = st_pluggable_slot,
> -		.location = ST_LOC_DEVFN(0x80,0),
> -		.name = "GPU4",
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu5[] = {
> -	{
> -		.etype = st_pluggable_slot,
> -		.location = ST_LOC_DEVFN(0xa0,0),
> -		.name = "GPU5",
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx0_down[] = {
> -	{
> -		.etype = st_builtin_dev,
> -		.location = ST_LOC_DEVFN(0x4a,0),
> -		.children = witherspoon_gpu0,
> -		.name = "GPU0 down",
> -	},
> -	{
> -		.etype = st_builtin_dev,
> -		.location = ST_LOC_DEVFN(0x4b,0),
> -		.children = witherspoon_gpu1,
> -		.name = "GPU1 down",
> -	},
> -	{
> -		.etype = st_builtin_dev,
> -		.location = ST_LOC_DEVFN(0x4c,0),
> -		.children = witherspoon_gpu2,
> -		.name = "GPU2 down",
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx1_down[] = {
> -	{
> -		.etype = st_builtin_dev,
> -		.location = ST_LOC_DEVFN(0x44,0),
> -		.children = witherspoon_gpu3,
> -		.name = "GPU3 down",
> -	},
> -	{
> -		.etype = st_builtin_dev,
> -		.location = ST_LOC_DEVFN(0x45,0),
> -		.children = witherspoon_gpu4,
> -		.name = "GPU4 down",
> -	},
> -	{
> -		.etype = st_builtin_dev,
> -		.location = ST_LOC_DEVFN(0x4d,0),
> -		.children = witherspoon_gpu5,
> -		.name = "GPU5 down",
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx0_up[] = {
> -	{
> -		.etype = st_builtin_dev,
> -		.location = ST_LOC_DEVFN(0x20,0),
> -		.children = witherspoon_plx0_down,
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx1_up[] = {
> -	{
> -		.etype = st_builtin_dev,
> -		.location = ST_LOC_DEVFN(0x20,0),
> -		.children = witherspoon_plx1_down,
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx0_phb[] = {
> -	{
> -		.etype = st_builtin_dev,
> -		.location = ST_LOC_DEVFN(0,0),
> -		.children = witherspoon_plx0_up,
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx1_phb[] = {
> -	{
> -		.etype = st_builtin_dev,
> -		.location = ST_LOC_DEVFN(0,0),
> -		.children = witherspoon_plx1_up,
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_npu0_slots[] = {
> -	{
> -		.etype = st_npu_slot,
> -		.location = ST_LOC_NPU_GROUP(0),
> -		.name = "GPU0",
> -	},
> -	{
> -		.etype = st_npu_slot,
> -		.location = ST_LOC_NPU_GROUP(1),
> -		.name = "GPU1",
> -	},
> -	{
> -		.etype = st_npu_slot,
> -		.location = ST_LOC_NPU_GROUP(2),
> -		.name = "GPU2",
> -	},
> -	{ .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_npu8_slots[] = {
> -	{
> -		.etype = st_npu_slot,
> -		.location = ST_LOC_NPU_GROUP(0),
> -		.name = "GPU3",
> -	},
> -	{
> -		.etype = st_npu_slot,
> -		.location = ST_LOC_NPU_GROUP(1),
> -		.name = "GPU4",
> -	},
> -	{
> -		.etype = st_npu_slot,
> -		.location = ST_LOC_NPU_GROUP(2),
> -		.name = "GPU5",
> -	},
> -	{ .etype = st_end },
> -};
> -
> -/*
> - * Slot numbering:
> - *
> - * slot 1 - x4 slot
> - * slot 2 - shared slot, 8x to each chip's PHB3
> - * slot 3 - 16x \w CAPI, second chip
> - * slot 4 - 16x \w CAPI, first chip
> - */
> -
> -static const struct slot_table_entry witherspoon_phb_table[] = {
> -	ST_PHB_ENTRY(0, 0, witherspoon_slot4),
> -	ST_PHB_ENTRY(0, 3, witherspoon_slot2_shared),
> -	ST_PHB_ENTRY(0, 4, witherspoon_plx0_phb),
> -	ST_PHB_ENTRY(0, 7, witherspoon_npu0_slots),
> -
> -	ST_PHB_ENTRY(8, 0, witherspoon_slot3),
> -	ST_PHB_ENTRY(8, 3, witherspoon_slot2_shared),
> -	ST_PHB_ENTRY(8, 4, witherspoon_slot1),
> -	ST_PHB_ENTRY(8, 5, witherspoon_plx1_phb),
> -	ST_PHB_ENTRY(8, 8, witherspoon_npu8_slots),
> -
> -	{ .etype = st_end },
> -};
> -
> -#define NPU_BASE 0x5011000
> -#define NPU_SIZE 0x2c
> -#define NPU_INDIRECT0	0x8000000009010c3f
> -#define NPU_INDIRECT1	0x800000000c010c3f
> -
> -static void create_link(struct dt_node *npu, int group, int index)
> -{
> -	struct dt_node *link;
> -	uint32_t lane_mask;
> -	uint64_t phy;
> -	char namebuf[32];
> -
> -	snprintf(namebuf, sizeof(namebuf), "link@%x", index);
> -	link = dt_new(npu, namebuf);
> -
> -	dt_add_property_string(link, "compatible", "ibm,npu-link");
> -	dt_add_property_cells(link, "ibm,npu-link-index", index);
> -
> -	if (!(index / 3))
> -		phy = NPU_INDIRECT0;
> -	else
> -		phy = NPU_INDIRECT1;
> -
> -	switch (index % 3) {
> -	case 0:
> -		lane_mask = 0xf1e000;
> -		break;
> -
> -	case 1:
> -		lane_mask = 0x0e1870;
> -		break;
> -
> -	case 2:
> -		lane_mask = 0x00078f;
> -		break;
> -
> -	default:
> -		assert(0);
> -	}
> -
> -	dt_add_property_u64s(link, "ibm,npu-phy", phy);
> -	dt_add_property_cells(link, "ibm,npu-lane-mask", lane_mask);
> -	dt_add_property_cells(link, "ibm,npu-group-id", group);
> -}
> -
> -static void dt_create_npu2(void)
> -{
> -        struct dt_node *xscom, *npu;
> -        char namebuf[32];
> -	int phb_index = 7;
> -	int npu_index = 0;
> -
> -	dt_for_each_compatible(dt_root, xscom, "ibm,xscom") {
> -		snprintf(namebuf, sizeof(namebuf), "npu@%x", NPU_BASE);
> -		npu = dt_new(xscom, namebuf);
> -		dt_add_property_cells(npu, "reg", NPU_BASE, NPU_SIZE);
> -		dt_add_property_strings(npu, "compatible", "ibm,power9-npu");
> -
> -		dt_add_property_cells(npu, "ibm,phb-index", phb_index++);
> -		dt_add_property_cells(npu, "ibm,npu-index", npu_index++);
> -		dt_add_property_cells(npu, "ibm,npu-links", 6);
> -
> -		create_link(npu, 0, 0);
> -		create_link(npu, 0, 1);
> -		create_link(npu, 1, 2);
> -		create_link(npu, 1, 3);
> -		create_link(npu, 2, 4);
> -		create_link(npu, 2, 5);
> -	}
> -}
> -
>   static bool witherspoon_probe(void)
>   {
>   	if (!dt_node_is_compatible(dt_root, "ibm,witherspoon"))
> @@ -351,37 +48,6 @@ static bool witherspoon_probe(void)
>   	return true;
>   }
>   
> -static bool old_witherspoon_probe(void)
> -{
> -	struct dt_node *slots, *npu;
> -
> -	if (!dt_node_is_compatible(dt_root, "ibm,witherspoon"))
> -		return false;
> -
> -	slots = dt_find_by_name(dt_root, "ibm,pcie-slots");
> -	npu  = dt_find_compatible_node(dt_root, NULL, "ibm,power9-npu");
> -
> -	if (slots && npu)
> -		return false;
> -
> -	/* Lot of common early inits here */
> -	astbmc_early_init();
> -
> -	/* Setup UART for use by OPAL (Linux hvc) */
> -	uart_set_console_policy(UART_CONSOLE_OPAL);
> -
> -
> -	/* Add NPU2 bindings */
> -	if (!npu)
> -		dt_create_npu2();
> -
> -	slot_table_init(witherspoon_phb_table);
> -
> -	prerror("!!! Old witherspoon firmware detected. Update hostboot and fix the Planar VPD !!!\n");
> -
> -	return true;
> -}
> -
>   static void phb4_activate_shared_slot_witherspoon(struct proc_chip *chip)
>   {
>   	uint64_t val;
> @@ -488,21 +154,3 @@ DECLARE_PLATFORM(witherspoon) = {
>   
>   	.pci_get_slot_info	= dt_slot_get_slot_info,
>   };
> -
> -DECLARE_PLATFORM(old_witherspoon) = {
> -	.name			= "Witherspoon (old)",
> -	.probe			= old_witherspoon_probe,
> -	.init			= astbmc_init,
> -	.pre_pci_fixup		= witherspoon_pre_pci_fixup,
> -	.start_preload_resource	= flash_start_preload_resource,
> -	.resource_loaded	= flash_resource_loaded,
> -	.bmc			= &astbmc_openbmc,
> -	.cec_power_down         = astbmc_ipmi_power_down,
> -	.cec_reboot             = astbmc_ipmi_reboot,
> -	.elog_commit		= ipmi_elog_commit,
> -	.exit			= ipmi_wdt_final_reset,
> -	.terminate		= ipmi_terminate,
> -
> -	.pci_get_slot_info	= slot_table_get_slot_info,
> -	.pci_probe_complete	= check_all_slot_table,
> -};
>
Reza Arbab Jan. 8, 2018, 2:48 p.m. | #2
On Mon, Jan 08, 2018 at 04:56:48PM +1100, Alistair Popple wrote:
>An old Witherspoon platform definition was added to aid the transition from
>versions of Hostboot which didn't have the correct NVLink HDAT information
>available and/or planar VPD. These system should now be updated so remove
>the possibly incorrect default assumption.

Fine by me!

Reviewed-by: Reza Arbab <arbab@linux.vnet.ibm.com>
Oliver Jan. 8, 2018, 10:16 p.m. | #3
On Mon, Jan 8, 2018 at 4:56 PM, Alistair Popple <alistair@popple.id.au> wrote:
> An old Witherspoon platform definition was added to aid the transition from
> versions of Hostboot which didn't have the correct NVLink HDAT information
> available and/or planar VPD. These system should now be updated so remove
> the possibly incorrect default assumption.
>
> This may disable NVLink on old out-dated systems but it can easily be
> restored with the appropriate FW and/or VPD updates. In any case there is a
> a 50% chance the existing default behaviour was incorrect as it only
> supports 6 GPU systems. Using an incorrect platform definition leads to
> undefined behaviour which is more difficult to detect/debug than not
> creating the NVLink devices so remove the possibly incorrect default
> behaviour.
>
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> ---
>  platforms/astbmc/witherspoon.c | 352 -----------------------------------------
>  1 file changed, 352 deletions(-)
>
> diff --git a/platforms/astbmc/witherspoon.c b/platforms/astbmc/witherspoon.c
> index b9ed2778..dc6459b9 100644
> --- a/platforms/astbmc/witherspoon.c
> +++ b/platforms/astbmc/witherspoon.c
> @@ -30,309 +30,6 @@
>
>  #include "astbmc.h"
>
> -static const struct slot_table_entry witherspoon_slot1[] = {
> -       {
> -               .etype = st_pluggable_slot,
> -               .location = ST_LOC_DEVFN(0,0),
> -               .name = "SLOT0"
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_slot2_shared[] = {
> -       {
> -               .etype = st_pluggable_slot,
> -               .location = ST_LOC_DEVFN(0,0),
> -               .name = "SLOT1"
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_slot3[] = {
> -       {
> -               .etype = st_pluggable_slot,
> -               .location = ST_LOC_DEVFN(0,0),
> -               .name = "SLOT2"
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_slot4[] = {
> -       {
> -               .etype = st_pluggable_slot,
> -               .location = ST_LOC_DEVFN(0,0),
> -               .name = "SLOT3"
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu0[] = {
> -       {
> -               .etype = st_pluggable_slot,
> -               .location = ST_LOC_DEVFN(0x80,0),
> -               .name = "GPU0",
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu1[] = {
> -       {
> -               .etype = st_pluggable_slot,
> -               .location = ST_LOC_DEVFN(0xa0,0),
> -               .name = "GPU1",
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu2[] = {
> -       {
> -               .etype = st_pluggable_slot,
> -               .location = ST_LOC_DEVFN(0xc0,0),
> -               .name = "GPU2",
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu3[] = {
> -       {
> -               .etype = st_pluggable_slot,
> -               .location = ST_LOC_DEVFN(0x60,0),
> -               .name = "GPU3",
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu4[] = {
> -       {
> -               .etype = st_pluggable_slot,
> -               .location = ST_LOC_DEVFN(0x80,0),
> -               .name = "GPU4",
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_gpu5[] = {
> -       {
> -               .etype = st_pluggable_slot,
> -               .location = ST_LOC_DEVFN(0xa0,0),
> -               .name = "GPU5",
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx0_down[] = {
> -       {
> -               .etype = st_builtin_dev,
> -               .location = ST_LOC_DEVFN(0x4a,0),
> -               .children = witherspoon_gpu0,
> -               .name = "GPU0 down",
> -       },
> -       {
> -               .etype = st_builtin_dev,
> -               .location = ST_LOC_DEVFN(0x4b,0),
> -               .children = witherspoon_gpu1,
> -               .name = "GPU1 down",
> -       },
> -       {
> -               .etype = st_builtin_dev,
> -               .location = ST_LOC_DEVFN(0x4c,0),
> -               .children = witherspoon_gpu2,
> -               .name = "GPU2 down",
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx1_down[] = {
> -       {
> -               .etype = st_builtin_dev,
> -               .location = ST_LOC_DEVFN(0x44,0),
> -               .children = witherspoon_gpu3,
> -               .name = "GPU3 down",
> -       },
> -       {
> -               .etype = st_builtin_dev,
> -               .location = ST_LOC_DEVFN(0x45,0),
> -               .children = witherspoon_gpu4,
> -               .name = "GPU4 down",
> -       },
> -       {
> -               .etype = st_builtin_dev,
> -               .location = ST_LOC_DEVFN(0x4d,0),
> -               .children = witherspoon_gpu5,
> -               .name = "GPU5 down",
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx0_up[] = {
> -       {
> -               .etype = st_builtin_dev,
> -               .location = ST_LOC_DEVFN(0x20,0),
> -               .children = witherspoon_plx0_down,
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx1_up[] = {
> -       {
> -               .etype = st_builtin_dev,
> -               .location = ST_LOC_DEVFN(0x20,0),
> -               .children = witherspoon_plx1_down,
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx0_phb[] = {
> -       {
> -               .etype = st_builtin_dev,
> -               .location = ST_LOC_DEVFN(0,0),
> -               .children = witherspoon_plx0_up,
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_plx1_phb[] = {
> -       {
> -               .etype = st_builtin_dev,
> -               .location = ST_LOC_DEVFN(0,0),
> -               .children = witherspoon_plx1_up,
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_npu0_slots[] = {
> -       {
> -               .etype = st_npu_slot,
> -               .location = ST_LOC_NPU_GROUP(0),
> -               .name = "GPU0",
> -       },
> -       {
> -               .etype = st_npu_slot,
> -               .location = ST_LOC_NPU_GROUP(1),
> -               .name = "GPU1",
> -       },
> -       {
> -               .etype = st_npu_slot,
> -               .location = ST_LOC_NPU_GROUP(2),
> -               .name = "GPU2",
> -       },
> -       { .etype = st_end },
> -};
> -
> -static const struct slot_table_entry witherspoon_npu8_slots[] = {
> -       {
> -               .etype = st_npu_slot,
> -               .location = ST_LOC_NPU_GROUP(0),
> -               .name = "GPU3",
> -       },
> -       {
> -               .etype = st_npu_slot,
> -               .location = ST_LOC_NPU_GROUP(1),
> -               .name = "GPU4",
> -       },
> -       {
> -               .etype = st_npu_slot,
> -               .location = ST_LOC_NPU_GROUP(2),
> -               .name = "GPU5",
> -       },
> -       { .etype = st_end },
> -};
> -
> -/*
> - * Slot numbering:
> - *
> - * slot 1 - x4 slot
> - * slot 2 - shared slot, 8x to each chip's PHB3
> - * slot 3 - 16x \w CAPI, second chip
> - * slot 4 - 16x \w CAPI, first chip
> - */
> -
> -static const struct slot_table_entry witherspoon_phb_table[] = {
> -       ST_PHB_ENTRY(0, 0, witherspoon_slot4),
> -       ST_PHB_ENTRY(0, 3, witherspoon_slot2_shared),
> -       ST_PHB_ENTRY(0, 4, witherspoon_plx0_phb),
> -       ST_PHB_ENTRY(0, 7, witherspoon_npu0_slots),
> -
> -       ST_PHB_ENTRY(8, 0, witherspoon_slot3),
> -       ST_PHB_ENTRY(8, 3, witherspoon_slot2_shared),
> -       ST_PHB_ENTRY(8, 4, witherspoon_slot1),
> -       ST_PHB_ENTRY(8, 5, witherspoon_plx1_phb),
> -       ST_PHB_ENTRY(8, 8, witherspoon_npu8_slots),
> -
> -       { .etype = st_end },
> -};
> -
> -#define NPU_BASE 0x5011000
> -#define NPU_SIZE 0x2c
> -#define NPU_INDIRECT0  0x8000000009010c3f
> -#define NPU_INDIRECT1  0x800000000c010c3f
> -
> -static void create_link(struct dt_node *npu, int group, int index)
> -{
> -       struct dt_node *link;
> -       uint32_t lane_mask;
> -       uint64_t phy;
> -       char namebuf[32];
> -
> -       snprintf(namebuf, sizeof(namebuf), "link@%x", index);
> -       link = dt_new(npu, namebuf);
> -
> -       dt_add_property_string(link, "compatible", "ibm,npu-link");
> -       dt_add_property_cells(link, "ibm,npu-link-index", index);
> -
> -       if (!(index / 3))
> -               phy = NPU_INDIRECT0;
> -       else
> -               phy = NPU_INDIRECT1;
> -
> -       switch (index % 3) {
> -       case 0:
> -               lane_mask = 0xf1e000;
> -               break;
> -
> -       case 1:
> -               lane_mask = 0x0e1870;
> -               break;
> -
> -       case 2:
> -               lane_mask = 0x00078f;
> -               break;
> -
> -       default:
> -               assert(0);
> -       }
> -
> -       dt_add_property_u64s(link, "ibm,npu-phy", phy);
> -       dt_add_property_cells(link, "ibm,npu-lane-mask", lane_mask);
> -       dt_add_property_cells(link, "ibm,npu-group-id", group);
> -}
> -
> -static void dt_create_npu2(void)
> -{
> -        struct dt_node *xscom, *npu;
> -        char namebuf[32];
> -       int phb_index = 7;
> -       int npu_index = 0;
> -
> -       dt_for_each_compatible(dt_root, xscom, "ibm,xscom") {
> -               snprintf(namebuf, sizeof(namebuf), "npu@%x", NPU_BASE);
> -               npu = dt_new(xscom, namebuf);
> -               dt_add_property_cells(npu, "reg", NPU_BASE, NPU_SIZE);
> -               dt_add_property_strings(npu, "compatible", "ibm,power9-npu");
> -
> -               dt_add_property_cells(npu, "ibm,phb-index", phb_index++);
> -               dt_add_property_cells(npu, "ibm,npu-index", npu_index++);
> -               dt_add_property_cells(npu, "ibm,npu-links", 6);
> -
> -               create_link(npu, 0, 0);
> -               create_link(npu, 0, 1);
> -               create_link(npu, 1, 2);
> -               create_link(npu, 1, 3);
> -               create_link(npu, 2, 4);
> -               create_link(npu, 2, 5);
> -       }
> -}
> -
>  static bool witherspoon_probe(void)
>  {
>         if (!dt_node_is_compatible(dt_root, "ibm,witherspoon"))
> @@ -351,37 +48,6 @@ static bool witherspoon_probe(void)
>         return true;
>  }
>
> -static bool old_witherspoon_probe(void)
> -{
> -       struct dt_node *slots, *npu;
> -
> -       if (!dt_node_is_compatible(dt_root, "ibm,witherspoon"))
> -               return false;
> -
> -       slots = dt_find_by_name(dt_root, "ibm,pcie-slots");
> -       npu  = dt_find_compatible_node(dt_root, NULL, "ibm,power9-npu");
> -
> -       if (slots && npu)
> -               return false;
> -
> -       /* Lot of common early inits here */
> -       astbmc_early_init();
> -
> -       /* Setup UART for use by OPAL (Linux hvc) */
> -       uart_set_console_policy(UART_CONSOLE_OPAL);
> -
> -
> -       /* Add NPU2 bindings */
> -       if (!npu)
> -               dt_create_npu2();
> -
> -       slot_table_init(witherspoon_phb_table);
> -
> -       prerror("!!! Old witherspoon firmware detected. Update hostboot and fix the Planar VPD !!!\n");
> -
> -       return true;
> -}
> -
>  static void phb4_activate_shared_slot_witherspoon(struct proc_chip *chip)
>  {
>         uint64_t val;
> @@ -488,21 +154,3 @@ DECLARE_PLATFORM(witherspoon) = {
>
>         .pci_get_slot_info      = dt_slot_get_slot_info,
>  };
> -
> -DECLARE_PLATFORM(old_witherspoon) = {
> -       .name                   = "Witherspoon (old)",
> -       .probe                  = old_witherspoon_probe,
> -       .init                   = astbmc_init,
> -       .pre_pci_fixup          = witherspoon_pre_pci_fixup,
> -       .start_preload_resource = flash_start_preload_resource,
> -       .resource_loaded        = flash_resource_loaded,
> -       .bmc                    = &astbmc_openbmc,
> -       .cec_power_down         = astbmc_ipmi_power_down,
> -       .cec_reboot             = astbmc_ipmi_reboot,
> -       .elog_commit            = ipmi_elog_commit,
> -       .exit                   = ipmi_wdt_final_reset,
> -       .terminate              = ipmi_terminate,
> -
> -       .pci_get_slot_info      = slot_table_get_slot_info,
> -       .pci_probe_complete     = check_all_slot_table,
> -};
> --
> 2.11.0
>

Two things:

a) This will need to go into the 5.9.x stable branch since that's what
OP910 builds use.
b) The remaining witherspoon_probe() will not match on systems that
don't have an npu-power9 node or the device-tree based PCIe slot
information. Odds are it'll fall back to the generic P9 platform so we
might want to amend witherspoon_probe() to be more accepting or have
it abort() if finds a system with out of date firmware.

Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
Alistair Popple Jan. 11, 2018, 4:38 a.m. | #4
On Tuesday, 9 January 2018 9:16:47 AM AEDT Oliver wrote:
> b) The remaining witherspoon_probe() will not match on systems that
> don't have an npu-power9 node or the device-tree based PCIe slot
> information. Odds are it'll fall back to the generic P9 platform so we
> might want to amend witherspoon_probe() to be more accepting or have
> it abort() if finds a system with out of date firmware.
> 

Good point. I will resubmit a V2 which removes the checks for npu-power9
and PCIe slot information as there shouldn't be any problem booting as a
Witherspoon system without those (the NPU devices won't get created, but
that is hardly a problem).

- Alistair

Patch

diff --git a/platforms/astbmc/witherspoon.c b/platforms/astbmc/witherspoon.c
index b9ed2778..dc6459b9 100644
--- a/platforms/astbmc/witherspoon.c
+++ b/platforms/astbmc/witherspoon.c
@@ -30,309 +30,6 @@ 
 
 #include "astbmc.h"
 
-static const struct slot_table_entry witherspoon_slot1[] = {
-	{
-		.etype = st_pluggable_slot,
-		.location = ST_LOC_DEVFN(0,0),
-		.name = "SLOT0"
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_slot2_shared[] = {
-	{
-		.etype = st_pluggable_slot,
-		.location = ST_LOC_DEVFN(0,0),
-		.name = "SLOT1"
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_slot3[] = {
-	{
-		.etype = st_pluggable_slot,
-		.location = ST_LOC_DEVFN(0,0),
-		.name = "SLOT2"
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_slot4[] = {
-	{
-		.etype = st_pluggable_slot,
-		.location = ST_LOC_DEVFN(0,0),
-		.name = "SLOT3"
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_gpu0[] = {
-	{
-		.etype = st_pluggable_slot,
-		.location = ST_LOC_DEVFN(0x80,0),
-		.name = "GPU0",
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_gpu1[] = {
-	{
-		.etype = st_pluggable_slot,
-		.location = ST_LOC_DEVFN(0xa0,0),
-		.name = "GPU1",
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_gpu2[] = {
-	{
-		.etype = st_pluggable_slot,
-		.location = ST_LOC_DEVFN(0xc0,0),
-		.name = "GPU2",
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_gpu3[] = {
-	{
-		.etype = st_pluggable_slot,
-		.location = ST_LOC_DEVFN(0x60,0),
-		.name = "GPU3",
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_gpu4[] = {
-	{
-		.etype = st_pluggable_slot,
-		.location = ST_LOC_DEVFN(0x80,0),
-		.name = "GPU4",
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_gpu5[] = {
-	{
-		.etype = st_pluggable_slot,
-		.location = ST_LOC_DEVFN(0xa0,0),
-		.name = "GPU5",
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_plx0_down[] = {
-	{
-		.etype = st_builtin_dev,
-		.location = ST_LOC_DEVFN(0x4a,0),
-		.children = witherspoon_gpu0,
-		.name = "GPU0 down",
-	},
-	{
-		.etype = st_builtin_dev,
-		.location = ST_LOC_DEVFN(0x4b,0),
-		.children = witherspoon_gpu1,
-		.name = "GPU1 down",
-	},
-	{
-		.etype = st_builtin_dev,
-		.location = ST_LOC_DEVFN(0x4c,0),
-		.children = witherspoon_gpu2,
-		.name = "GPU2 down",
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_plx1_down[] = {
-	{
-		.etype = st_builtin_dev,
-		.location = ST_LOC_DEVFN(0x44,0),
-		.children = witherspoon_gpu3,
-		.name = "GPU3 down",
-	},
-	{
-		.etype = st_builtin_dev,
-		.location = ST_LOC_DEVFN(0x45,0),
-		.children = witherspoon_gpu4,
-		.name = "GPU4 down",
-	},
-	{
-		.etype = st_builtin_dev,
-		.location = ST_LOC_DEVFN(0x4d,0),
-		.children = witherspoon_gpu5,
-		.name = "GPU5 down",
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_plx0_up[] = {
-	{
-		.etype = st_builtin_dev,
-		.location = ST_LOC_DEVFN(0x20,0),
-		.children = witherspoon_plx0_down,
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_plx1_up[] = {
-	{
-		.etype = st_builtin_dev,
-		.location = ST_LOC_DEVFN(0x20,0),
-		.children = witherspoon_plx1_down,
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_plx0_phb[] = {
-	{
-		.etype = st_builtin_dev,
-		.location = ST_LOC_DEVFN(0,0),
-		.children = witherspoon_plx0_up,
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_plx1_phb[] = {
-	{
-		.etype = st_builtin_dev,
-		.location = ST_LOC_DEVFN(0,0),
-		.children = witherspoon_plx1_up,
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_npu0_slots[] = {
-	{
-		.etype = st_npu_slot,
-		.location = ST_LOC_NPU_GROUP(0),
-		.name = "GPU0",
-	},
-	{
-		.etype = st_npu_slot,
-		.location = ST_LOC_NPU_GROUP(1),
-		.name = "GPU1",
-	},
-	{
-		.etype = st_npu_slot,
-		.location = ST_LOC_NPU_GROUP(2),
-		.name = "GPU2",
-	},
-	{ .etype = st_end },
-};
-
-static const struct slot_table_entry witherspoon_npu8_slots[] = {
-	{
-		.etype = st_npu_slot,
-		.location = ST_LOC_NPU_GROUP(0),
-		.name = "GPU3",
-	},
-	{
-		.etype = st_npu_slot,
-		.location = ST_LOC_NPU_GROUP(1),
-		.name = "GPU4",
-	},
-	{
-		.etype = st_npu_slot,
-		.location = ST_LOC_NPU_GROUP(2),
-		.name = "GPU5",
-	},
-	{ .etype = st_end },
-};
-
-/*
- * Slot numbering:
- *
- * slot 1 - x4 slot
- * slot 2 - shared slot, 8x to each chip's PHB3
- * slot 3 - 16x \w CAPI, second chip
- * slot 4 - 16x \w CAPI, first chip
- */
-
-static const struct slot_table_entry witherspoon_phb_table[] = {
-	ST_PHB_ENTRY(0, 0, witherspoon_slot4),
-	ST_PHB_ENTRY(0, 3, witherspoon_slot2_shared),
-	ST_PHB_ENTRY(0, 4, witherspoon_plx0_phb),
-	ST_PHB_ENTRY(0, 7, witherspoon_npu0_slots),
-
-	ST_PHB_ENTRY(8, 0, witherspoon_slot3),
-	ST_PHB_ENTRY(8, 3, witherspoon_slot2_shared),
-	ST_PHB_ENTRY(8, 4, witherspoon_slot1),
-	ST_PHB_ENTRY(8, 5, witherspoon_plx1_phb),
-	ST_PHB_ENTRY(8, 8, witherspoon_npu8_slots),
-
-	{ .etype = st_end },
-};
-
-#define NPU_BASE 0x5011000
-#define NPU_SIZE 0x2c
-#define NPU_INDIRECT0	0x8000000009010c3f
-#define NPU_INDIRECT1	0x800000000c010c3f
-
-static void create_link(struct dt_node *npu, int group, int index)
-{
-	struct dt_node *link;
-	uint32_t lane_mask;
-	uint64_t phy;
-	char namebuf[32];
-
-	snprintf(namebuf, sizeof(namebuf), "link@%x", index);
-	link = dt_new(npu, namebuf);
-
-	dt_add_property_string(link, "compatible", "ibm,npu-link");
-	dt_add_property_cells(link, "ibm,npu-link-index", index);
-
-	if (!(index / 3))
-		phy = NPU_INDIRECT0;
-	else
-		phy = NPU_INDIRECT1;
-
-	switch (index % 3) {
-	case 0:
-		lane_mask = 0xf1e000;
-		break;
-
-	case 1:
-		lane_mask = 0x0e1870;
-		break;
-
-	case 2:
-		lane_mask = 0x00078f;
-		break;
-
-	default:
-		assert(0);
-	}
-
-	dt_add_property_u64s(link, "ibm,npu-phy", phy);
-	dt_add_property_cells(link, "ibm,npu-lane-mask", lane_mask);
-	dt_add_property_cells(link, "ibm,npu-group-id", group);
-}
-
-static void dt_create_npu2(void)
-{
-        struct dt_node *xscom, *npu;
-        char namebuf[32];
-	int phb_index = 7;
-	int npu_index = 0;
-
-	dt_for_each_compatible(dt_root, xscom, "ibm,xscom") {
-		snprintf(namebuf, sizeof(namebuf), "npu@%x", NPU_BASE);
-		npu = dt_new(xscom, namebuf);
-		dt_add_property_cells(npu, "reg", NPU_BASE, NPU_SIZE);
-		dt_add_property_strings(npu, "compatible", "ibm,power9-npu");
-
-		dt_add_property_cells(npu, "ibm,phb-index", phb_index++);
-		dt_add_property_cells(npu, "ibm,npu-index", npu_index++);
-		dt_add_property_cells(npu, "ibm,npu-links", 6);
-
-		create_link(npu, 0, 0);
-		create_link(npu, 0, 1);
-		create_link(npu, 1, 2);
-		create_link(npu, 1, 3);
-		create_link(npu, 2, 4);
-		create_link(npu, 2, 5);
-	}
-}
-
 static bool witherspoon_probe(void)
 {
 	if (!dt_node_is_compatible(dt_root, "ibm,witherspoon"))
@@ -351,37 +48,6 @@  static bool witherspoon_probe(void)
 	return true;
 }
 
-static bool old_witherspoon_probe(void)
-{
-	struct dt_node *slots, *npu;
-
-	if (!dt_node_is_compatible(dt_root, "ibm,witherspoon"))
-		return false;
-
-	slots = dt_find_by_name(dt_root, "ibm,pcie-slots");
-	npu  = dt_find_compatible_node(dt_root, NULL, "ibm,power9-npu");
-
-	if (slots && npu)
-		return false;
-
-	/* Lot of common early inits here */
-	astbmc_early_init();
-
-	/* Setup UART for use by OPAL (Linux hvc) */
-	uart_set_console_policy(UART_CONSOLE_OPAL);
-
-
-	/* Add NPU2 bindings */
-	if (!npu)
-		dt_create_npu2();
-
-	slot_table_init(witherspoon_phb_table);
-
-	prerror("!!! Old witherspoon firmware detected. Update hostboot and fix the Planar VPD !!!\n");
-
-	return true;
-}
-
 static void phb4_activate_shared_slot_witherspoon(struct proc_chip *chip)
 {
 	uint64_t val;
@@ -488,21 +154,3 @@  DECLARE_PLATFORM(witherspoon) = {
 
 	.pci_get_slot_info	= dt_slot_get_slot_info,
 };
-
-DECLARE_PLATFORM(old_witherspoon) = {
-	.name			= "Witherspoon (old)",
-	.probe			= old_witherspoon_probe,
-	.init			= astbmc_init,
-	.pre_pci_fixup		= witherspoon_pre_pci_fixup,
-	.start_preload_resource	= flash_start_preload_resource,
-	.resource_loaded	= flash_resource_loaded,
-	.bmc			= &astbmc_openbmc,
-	.cec_power_down         = astbmc_ipmi_power_down,
-	.cec_reboot             = astbmc_ipmi_reboot,
-	.elog_commit		= ipmi_elog_commit,
-	.exit			= ipmi_wdt_final_reset,
-	.terminate		= ipmi_terminate,
-
-	.pci_get_slot_info	= slot_table_get_slot_info,
-	.pci_probe_complete	= check_all_slot_table,
-};