diff mbox

[v3] phb4: Activate shared PCI slot on witherspoon

Message ID 20170606145945.26521-1-fbarrat@linux.vnet.ibm.com
State Superseded
Headers show

Commit Message

Frederic Barrat June 6, 2017, 2:59 p.m. UTC
Witherspoon systems come with a 'shared' PCI slot: physically, it
looks like a x16 slot, but it's actually two x8 slots connected to two
PHBs of two different chips. Taking advantage of it requires some
logic on the PCI adapter. Only the Mellanox CX5 adapter is known to
support it at the time of this writing.

This patch enables support for the shared slot on witherspoon if a x16
adapter is detected. Each x8 slot has a presence bit, so both bits
need to be set for the activation to take place. Slot sharing is
activated through a gpio.

Note that there's no easy way to be sure that the card is indeed a
shared-slot compatible PCI adapter and not a normal x16 card. Plugging
a normal x16 adapter on the shared slot should be avoided on
witherspoon, as the link won't train on the second slot, resulting in
a timeout and a longer boot time. Only the first slot is usable and
the x16 adapter will end up using only half the lines.

If the PCI card plugged on the physical slot is only x8 (or less),
then the presence bit of the second slot is not set, so this patch
does nothing. The x8 (or less) adapter should work like on any other
physical slot.

Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
---
Changelog:
v3: fix in case there's only one chip left (Mikey)
v2: address comment from Ben and Mikey

 core/pci.c                     |  3 ++
 hw/phb4.c                      | 90 +++++++++++++++++++++++++++++++++++++++++-
 include/platform.h             |  5 +++
 include/skiboot.h              |  1 +
 include/xscom-p9-regs.h        |  3 ++
 platforms/astbmc/witherspoon.c |  6 +++
 6 files changed, 107 insertions(+), 1 deletion(-)

Comments

Stewart Smith June 7, 2017, 10:27 a.m. UTC | #1
Frederic Barrat <fbarrat@linux.vnet.ibm.com> writes:
> Witherspoon systems come with a 'shared' PCI slot: physically, it
> looks like a x16 slot, but it's actually two x8 slots connected to two
> PHBs of two different chips. Taking advantage of it requires some
> logic on the PCI adapter. Only the Mellanox CX5 adapter is known to
> support it at the time of this writing.
>
> This patch enables support for the shared slot on witherspoon if a x16
> adapter is detected. Each x8 slot has a presence bit, so both bits
> need to be set for the activation to take place. Slot sharing is
> activated through a gpio.
>
> Note that there's no easy way to be sure that the card is indeed a
> shared-slot compatible PCI adapter and not a normal x16 card. Plugging
> a normal x16 adapter on the shared slot should be avoided on
> witherspoon, as the link won't train on the second slot, resulting in
> a timeout and a longer boot time. Only the first slot is usable and
> the x16 adapter will end up using only half the lines.
>
> If the PCI card plugged on the physical slot is only x8 (or less),
> then the presence bit of the second slot is not set, so this patch
> does nothing. The x8 (or less) adapter should work like on any other
> physical slot.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> ---
> Changelog:
> v3: fix in case there's only one chip left (Mikey)
> v2: address comment from Ben and Mikey
>
>  core/pci.c                     |  3 ++
>  hw/phb4.c                      | 90 +++++++++++++++++++++++++++++++++++++++++-
>  include/platform.h             |  5 +++
>  include/skiboot.h              |  1 +
>  include/xscom-p9-regs.h        |  3 ++
>  platforms/astbmc/witherspoon.c |  6 +++
>  6 files changed, 107 insertions(+), 1 deletion(-)
>
> diff --git a/core/pci.c b/core/pci.c
> index 2d3c4f88..e5ec97e8 100644
> --- a/core/pci.c
> +++ b/core/pci.c
> @@ -1646,6 +1646,9 @@ void pci_init_slots(void)
>  	 */
>  	time_wait_ms(20);
>
> +	if (platform.pre_pci_fixup)
> +		platform.pre_pci_fixup();
> +
>  	prlog(PR_NOTICE, "PCI: Resetting PHBs...\n");
>  	pci_do_jobs(pci_reset_phb);
>
> diff --git a/hw/phb4.c b/hw/phb4.c
> index b53dadfb..ee8e03f7 100644
> --- a/hw/phb4.c
> +++ b/hw/phb4.c
> @@ -77,6 +77,94 @@ static void phb4_init_hw(struct phb4 *p, bool first_init);
>  #define PHBLOGCFG(p, fmt, a...) do {} while (0)
>  #endif
>
> +#define PHB4_PER_CHIP                        6 /* Max 6 PHBs per chip on p9 */
> +#define PHB4_SHARED_SLOT_IDX_WITHERSPOON     3
> +
> +static int phb4_get_opal_id(unsigned int chip_id, unsigned int index)
> +{
> +	return chip_id * PHB4_PER_CHIP + index;
> +}
> +
> +static void phb4_activate_shared_slot_witherspoon(struct proc_chip *chip)
> +{
> +	uint64_t val;

I moved things around a bit in preparation for merging this (will send
patch in a tic) so that all the platform specific code was over in
witherspoon.c rather than in the phb4 source.

Then, as if somebody heard me about to merge something, the one working
witherspoon I could find to test on has bricked itself in the middle of
running the test suite... so I'm a bit nervous about hitting merge today
until I dig into it tomorrow.

Thoughts on my revised patch appreciated though.
Stewart Smith June 7, 2017, 10:50 a.m. UTC | #2
Stewart Smith <stewart@linux.vnet.ibm.com> writes:
> Frederic Barrat <fbarrat@linux.vnet.ibm.com> writes:
>> Witherspoon systems come with a 'shared' PCI slot: physically, it
>> looks like a x16 slot, but it's actually two x8 slots connected to two
>> PHBs of two different chips. Taking advantage of it requires some
>> logic on the PCI adapter. Only the Mellanox CX5 adapter is known to
>> support it at the time of this writing.
>>
>> This patch enables support for the shared slot on witherspoon if a x16
>> adapter is detected. Each x8 slot has a presence bit, so both bits
>> need to be set for the activation to take place. Slot sharing is
>> activated through a gpio.
>>
>> Note that there's no easy way to be sure that the card is indeed a
>> shared-slot compatible PCI adapter and not a normal x16 card. Plugging
>> a normal x16 adapter on the shared slot should be avoided on
>> witherspoon, as the link won't train on the second slot, resulting in
>> a timeout and a longer boot time. Only the first slot is usable and
>> the x16 adapter will end up using only half the lines.
>>
>> If the PCI card plugged on the physical slot is only x8 (or less),
>> then the presence bit of the second slot is not set, so this patch
>> does nothing. The x8 (or less) adapter should work like on any other
>> physical slot.
>>
>> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
>> ---
>> Changelog:
>> v3: fix in case there's only one chip left (Mikey)
>> v2: address comment from Ben and Mikey
>>
>>  core/pci.c                     |  3 ++
>>  hw/phb4.c                      | 90 +++++++++++++++++++++++++++++++++++++++++-
>>  include/platform.h             |  5 +++
>>  include/skiboot.h              |  1 +
>>  include/xscom-p9-regs.h        |  3 ++
>>  platforms/astbmc/witherspoon.c |  6 +++
>>  6 files changed, 107 insertions(+), 1 deletion(-)
>>
>> diff --git a/core/pci.c b/core/pci.c
>> index 2d3c4f88..e5ec97e8 100644
>> --- a/core/pci.c
>> +++ b/core/pci.c
>> @@ -1646,6 +1646,9 @@ void pci_init_slots(void)
>>  	 */
>>  	time_wait_ms(20);
>>
>> +	if (platform.pre_pci_fixup)
>> +		platform.pre_pci_fixup();
>> +
>>  	prlog(PR_NOTICE, "PCI: Resetting PHBs...\n");
>>  	pci_do_jobs(pci_reset_phb);
>>
>> diff --git a/hw/phb4.c b/hw/phb4.c
>> index b53dadfb..ee8e03f7 100644
>> --- a/hw/phb4.c
>> +++ b/hw/phb4.c
>> @@ -77,6 +77,94 @@ static void phb4_init_hw(struct phb4 *p, bool first_init);
>>  #define PHBLOGCFG(p, fmt, a...) do {} while (0)
>>  #endif
>>
>> +#define PHB4_PER_CHIP                        6 /* Max 6 PHBs per chip on p9 */
>> +#define PHB4_SHARED_SLOT_IDX_WITHERSPOON     3
>> +
>> +static int phb4_get_opal_id(unsigned int chip_id, unsigned int index)
>> +{
>> +	return chip_id * PHB4_PER_CHIP + index;
>> +}
>> +
>> +static void phb4_activate_shared_slot_witherspoon(struct proc_chip *chip)
>> +{
>> +	uint64_t val;
>
> I moved things around a bit in preparation for merging this (will send
> patch in a tic) so that all the platform specific code was over in
> witherspoon.c rather than in the phb4 source.
>
> Then, as if somebody heard me about to merge something, the one working
> witherspoon I could find to test on has bricked itself in the middle of
> running the test suite... so I'm a bit nervous about hitting merge today
> until I dig into it tomorrow.

...and the witherspoon came back, and appears to pass some tests. I'll
still wait until the morning though :)

> Thoughts on my revised patch appreciated though.
diff mbox

Patch

diff --git a/core/pci.c b/core/pci.c
index 2d3c4f88..e5ec97e8 100644
--- a/core/pci.c
+++ b/core/pci.c
@@ -1646,6 +1646,9 @@  void pci_init_slots(void)
 	 */
 	time_wait_ms(20);
 
+	if (platform.pre_pci_fixup)
+		platform.pre_pci_fixup();
+
 	prlog(PR_NOTICE, "PCI: Resetting PHBs...\n");
 	pci_do_jobs(pci_reset_phb);
 
diff --git a/hw/phb4.c b/hw/phb4.c
index b53dadfb..ee8e03f7 100644
--- a/hw/phb4.c
+++ b/hw/phb4.c
@@ -77,6 +77,94 @@  static void phb4_init_hw(struct phb4 *p, bool first_init);
 #define PHBLOGCFG(p, fmt, a...) do {} while (0)
 #endif
 
+#define PHB4_PER_CHIP                        6 /* Max 6 PHBs per chip on p9 */
+#define PHB4_SHARED_SLOT_IDX_WITHERSPOON     3
+
+static int phb4_get_opal_id(unsigned int chip_id, unsigned int index)
+{
+	return chip_id * PHB4_PER_CHIP + index;
+}
+
+static void phb4_activate_shared_slot_witherspoon(struct proc_chip *chip)
+{
+	uint64_t val;
+
+	/*
+	 * Shared slot activation is done by raising a GPIO line on the
+	 * chip with the secondary slot. It will somehow activate the
+	 * sideband signals between the slots.
+	 * Need to wait 100us for stability.
+	 */
+	xscom_read(chip->id, P9_GPIO_DATA_OUT_ENABLE, &val);
+	val |= PPC_BIT(2);
+	xscom_write(chip->id, P9_GPIO_DATA_OUT_ENABLE, val);
+
+	xscom_read(chip->id, P9_GPIO_DATA_OUT, &val);
+	val |= PPC_BIT(2);
+	xscom_write(chip->id, P9_GPIO_DATA_OUT, val);
+	time_wait_us(100);
+	prlog(PR_INFO, "Shared PCI slot activated\n");
+}
+
+void phb4_pre_pci_fixup_witherspoon(void)
+{
+	struct pci_slot *slot0, *slot1;
+	struct proc_chip *chip0, *chip1;
+	uint8_t p0 = 0, p1 = 0;
+
+	/*
+	 * Detect if a x16 card is present on the shared slot and
+	 * do some extra configuration if it is.
+	 *
+	 * The shared slot, a.k.a "Slot 2" in the documentation, is
+	 * connected to PEC2 phb index 3 on both chips. From skiboot,
+	 * it looks like two x8 slots, each with its own presence bit.
+	 *
+	 * Here is the matrix of possibilities for the presence bits:
+	 *
+	 * slot0 presence     slot1 presence
+	 *    0                  0               => no card
+	 *    1                  0               => x8 or less card detected
+	 *    1                  1               => x16 card detected
+	 *    0                  1               => invalid combination
+	 *
+	 * We only act if a x16 card is detected ('1 1' combination above).
+	 *
+	 * One issue is that we don't really know if it is a
+	 * shared-slot-compatible card (such as Mellanox CX5) or
+	 * a 'normal' x16 PCI card. We activate the shared slot in both cases,
+	 * as it doesn't seem to hurt.
+	 *
+	 * If the card is a normal x16 PCI card, the link won't train on the
+	 * second slot (nothing to do with the shared slot activation), the
+	 * procedure will timeout, thus adding some delay to the boot time.
+	 * Therefore the recommendation is that we shouldn't use a normal
+	 * x16 card on the shared slot of a witherspoon.
+	 *
+	 * Plugging a x8 or less adapter on the shared slot should work
+	 * like any other physical slot.
+	 */
+	chip0 = next_chip(NULL);
+	chip1 = next_chip(chip0);
+	if (!chip1 || next_chip(chip1)) {
+		prlog(PR_WARNING,
+			"Unexpected number of chips, skipping shared slot detection\n");
+		return;
+	}
+	slot0 = pci_slot_find(phb4_get_opal_id(chip0->id,
+					PHB4_SHARED_SLOT_IDX_WITHERSPOON));
+	slot1 = pci_slot_find(phb4_get_opal_id(chip1->id,
+					PHB4_SHARED_SLOT_IDX_WITHERSPOON));
+	if (slot0 && slot1) {
+		if (slot0->ops.get_presence_state)
+			slot0->ops.get_presence_state(slot0, &p0);
+		if (slot1->ops.get_presence_state)
+			slot1->ops.get_presence_state(slot1, &p1);
+		if (p0 == 1 && p1 == 1)
+			phb4_activate_shared_slot_witherspoon(chip1);
+	}
+}
+
 /* Note: The "ASB" name is historical, practically this means access via
  * the XSCOM backdoor
  */
@@ -3290,7 +3378,7 @@  static void phb4_create(struct dt_node *np)
 	/* We register the PHB before we initialize it so we
 	 * get a useful OPAL ID for it
 	 */
-	pci_register_phb(&p->phb, p->chip_id * 6 + p->index); //6 PHBs per chip?
+	pci_register_phb(&p->phb, phb4_get_opal_id(p->chip_id, p->index));
 
 	/* Create slot structure */
 	slot = phb4_slot_create(&p->phb);
diff --git a/include/platform.h b/include/platform.h
index 91332042..4dcdb335 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -101,6 +101,11 @@  struct platform {
 	void		(*pci_setup_phb)(struct phb *phb, unsigned int index);
 
 	/*
+	 * This is called before resetting the PHBs (lift PERST) and
+	 * probing the devices. The PHBs have already been initialized.
+	 */
+	void		(*pre_pci_fixup)(void);
+	/*
 	 * Called during PCI scan for each device. For bridges, this is
 	 * called before its children are probed. This is called for
 	 * every device and for the PHB itself with a NULL pd though
diff --git a/include/skiboot.h b/include/skiboot.h
index 5c8b0c85..14953cab 100644
--- a/include/skiboot.h
+++ b/include/skiboot.h
@@ -218,6 +218,7 @@  extern int phb3_preload_capp_ucode(void);
 extern void phb3_preload_vpd(void);
 extern int phb4_preload_capp_ucode(void);
 extern void phb4_preload_vpd(void);
+extern void phb4_pre_pci_fixup_witherspoon(void);
 extern void probe_npu(void);
 extern void probe_npu2(void);
 extern void uart_init(void);
diff --git a/include/xscom-p9-regs.h b/include/xscom-p9-regs.h
index 7da404ba..4738e812 100644
--- a/include/xscom-p9-regs.h
+++ b/include/xscom-p9-regs.h
@@ -18,4 +18,7 @@ 
 #define P9X_EX_NCU_DARN_BAR			0x11011
 #define  P9X_EX_NCU_DARN_BAR_EN			PPC_BIT(0)
 
+#define P9_GPIO_DATA_OUT_ENABLE			0x00000000000B0054ull
+#define P9_GPIO_DATA_OUT			0x00000000000B0051ull
+
 #endif /* __XSCOM_P9_REGS_H__ */
diff --git a/platforms/astbmc/witherspoon.c b/platforms/astbmc/witherspoon.c
index abaa7c9b..083e7cff 100644
--- a/platforms/astbmc/witherspoon.c
+++ b/platforms/astbmc/witherspoon.c
@@ -38,10 +38,16 @@  static bool witherspoon_probe(void)
 	return true;
 }
 
+static void witherspoon_pre_pci_fixup(void)
+{
+	phb4_pre_pci_fixup_witherspoon();
+}
+
 DECLARE_PLATFORM(witherspoon_platform) = {
 	.name			= "Witherspoon",
 	.probe			= 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			= NULL, /* FIXME: Add openBMC */