Re: [PATCH v8 36/45] powerpc/powernv: Support PCI slot ID

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 


On 04/20/2016 12:28 PM, Gavin Shan wrote:

On Tue, Apr 19, 2016 at 07:28:20PM +1000, Alexey Kardashevskiy wrote:

On 02/17/2016 02:44 PM, Gavin Shan wrote:

PowerNV platforms runs on top of skiboot firmware that includes
changes to support PCI slots. PCI slots are identified by PHB's
ID or the combo of that and PCI slot ID.

This changes the EEH PowerNV backend to support PCI slots:

    * Rename arguments of opal_pci_reset() and opal_pci_poll().
    * One more argument (PCI slot's state) added to opal_pci_poll().
    * Drop pnv_eeh_phb_poll() and introduce a enhanced similar
      function pnv_pci_poll() that will be used by PowerNV hotplug
      backends.

Signed-off-by: Gavin Shan <gwshan@xxxxxxxxxxxxxxxxxx>
---
  arch/powerpc/include/asm/opal.h              |  4 +--
  arch/powerpc/platforms/powernv/eeh-powernv.c | 42 ++++++----------------------
  arch/powerpc/platforms/powernv/pci.c         | 21 ++++++++++++++
  arch/powerpc/platforms/powernv/pci.h         |  1 +
  4 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 07a99e6..9e0039f 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -131,7 +131,7 @@ int64_t opal_pci_map_pe_dma_window(uint64_t phb_id, uint16_t pe_number, uint16_t
  int64_t opal_pci_map_pe_dma_window_real(uint64_t phb_id, uint16_t pe_number,
  					uint16_t dma_window_number, uint64_t pci_start_addr,
  					uint64_t pci_mem_size);
-int64_t opal_pci_reset(uint64_t phb_id, uint8_t reset_scope, uint8_t assert_state);
+int64_t opal_pci_reset(uint64_t id, uint8_t reset_scope, uint8_t assert_state);

  int64_t opal_pci_get_hub_diag_data(uint64_t hub_id, void *diag_buffer,
  				   uint64_t diag_buffer_len);
@@ -148,7 +148,7 @@ int64_t opal_get_dpo_status(__be64 *dpo_timeout);
  int64_t opal_set_system_attention_led(uint8_t led_action);
  int64_t opal_pci_next_error(uint64_t phb_id, __be64 *first_frozen_pe,
  			    __be16 *pci_error_type, __be16 *severity);
-int64_t opal_pci_poll(uint64_t phb_id);
+int64_t opal_pci_poll(uint64_t id, uint8_t *state);
  int64_t opal_return_cpu(void);
  int64_t opal_check_token(uint64_t token);
  int64_t opal_reinit_cpus(uint64_t flags);
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index c7454ba..e23b063 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -717,28 +717,11 @@ static int pnv_eeh_get_state(struct eeh_pe *pe, int *delay)
  	return ret;
  }

-static s64 pnv_eeh_phb_poll(struct pnv_phb *phb)
-{
-	s64 rc = OPAL_HARDWARE;
-
-	while (1) {
-		rc = opal_pci_poll(phb->opal_id);
-		if (rc <= 0)
-			break;
-
-		if (system_state < SYSTEM_RUNNING)
-			udelay(1000 * rc);
-		else
-			msleep(rc);
-	}
-
-	return rc;
-}
-
  int pnv_eeh_phb_reset(struct pci_controller *hose, int option)
  {
  	struct pnv_phb *phb = hose->private_data;
  	s64 rc = OPAL_HARDWARE;
+	int ret;

  	pr_debug("%s: Reset PHB#%x, option=%d\n",
  		 __func__, hose->global_number, option);
@@ -753,8 +736,6 @@ int pnv_eeh_phb_reset(struct pci_controller *hose, int option)
  		rc = opal_pci_reset(phb->opal_id,
  				    OPAL_RESET_PHB_COMPLETE,
  				    OPAL_DEASSERT_RESET);
-	if (rc < 0)
-		goto out;

  	/*
  	 * Poll state of the PHB until the request is done
@@ -762,24 +743,22 @@ int pnv_eeh_phb_reset(struct pci_controller *hose, int option)
  	 * reset followed by hot reset on root bus. So we also
  	 * need the PCI bus settlement delay.
  	 */
-	rc = pnv_eeh_phb_poll(phb);
-	if (option == EEH_RESET_DEACTIVATE) {
+	ret = pnv_pci_poll(phb->opal_id, rc, NULL);
+	if (option == EEH_RESET_DEACTIVATE && !ret) {
  		if (system_state < SYSTEM_RUNNING)
  			udelay(1000 * EEH_PE_RST_SETTLE_TIME);
  		else
  			msleep(EEH_PE_RST_SETTLE_TIME);
  	}
-out:
-	if (rc != OPAL_SUCCESS)
-		return -EIO;

-	return 0;
+	return ret;
  }

  static int pnv_eeh_root_reset(struct pci_controller *hose, int option)
  {
  	struct pnv_phb *phb = hose->private_data;
  	s64 rc = OPAL_HARDWARE;
+	int ret;

  	pr_debug("%s: Reset PHB#%x, option=%d\n",
  		 __func__, hose->global_number, option);
@@ -801,18 +780,13 @@ static int pnv_eeh_root_reset(struct pci_controller *hose, int option)
  		rc = opal_pci_reset(phb->opal_id,
  				    OPAL_RESET_PCI_HOT,
  				    OPAL_DEASSERT_RESET);
-	if (rc < 0)
-		goto out;

  	/* Poll state of the PHB until the request is done */
-	rc = pnv_eeh_phb_poll(phb);
-	if (option == EEH_RESET_DEACTIVATE)
+	ret = pnv_pci_poll(phb->opal_id, rc, NULL);
+	if (option == EEH_RESET_DEACTIVATE && !ret)
  		msleep(EEH_PE_RST_SETTLE_TIME);
-out:
-	if (rc != OPAL_SUCCESS)
-		return -EIO;

-	return 0;
+	return ret;
  }

  static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option)
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index b87a315..a458703 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -42,6 +42,27 @@
  #define cfg_dbg(fmt...)	do { } while(0)
  //#define cfg_dbg(fmt...)	printk(fmt)

+int pnv_pci_poll(uint64_t id, int64_t rval, uint8_t *state)
+{
+	while (rval > 0) {
+		if (system_state < SYSTEM_RUNNING)
+			udelay(1000 * rval);
+		else
+			msleep(rval);
+
+		rval = opal_pci_poll(id, state);
+	}
+
+	/*
+	 * The caller expects to retrieve additional
+	 * information if the last argument isn't NULL.
+	 */
+	if (rval == OPAL_SUCCESS && state)
+		rval = opal_pci_poll(id, state);



Old OPAL won't touch @state so whatever garbage was there will stay there as
the only caller which is passing not-NULL there is pnv_php_get_power_state()
and it does not initialize @power_state (it is in "[PATCH v8 45/45]
PCI/hotplug: PowerPC PowerNV PCI hotplug driver").



Old OPAL without exposing hotpluggable slots won't have this case. I mean
pnv_php_get_power_state() won't be called on old OPAL.



What exactly does guarantee that hotplug will work with new OPAL only?





btw how will new OPAL react if old kernel is running, i.e. not passing @state
at all? If it is initialized to NULL somewher - fine but what exactly does
this initialization and makes sure that OPAL won't see garbage as a second
parameter?



@state is always NULL for old kernel + new OPAL.
What piece of code writes NULL to "state"? Old kernel will do opal_pci_poll(id) and omit the "state" so something has to take care of it and write NULL where OPAL expects to see a pointer to "state" (which we set to NULL) and that place would be some GPR which may have garbage. 

I am looking at
#define OPAL_CALL(name, token)
and cannot see where the second parameter (which old kernel omits) is reset, i.e. gpr4 (or gpr5?) is cleared. 



@state is used in
PCI hotplug functionality in OPAL only. As old kernel doesn't support
PCI hotplug, @state is never used. I'm not sure it's the answer you
want?


No, it is not :)


When ABI like this changes, I expect to see opal_pci_poll2() or
opal_pci_poll_ex() rather than just an additional parameter to
opal_pci_poll()...



It's a good suggestion but it would be nicer if you raised this
early. One question I have: current opal_pci_poll() is enough
to cover all cases, why we need introduce and maintain another
similar one? Sorry that I don't see the reason from your context
and could you please provide more details?


+
+	return (rval == OPAL_SUCCESS) ? 0 : -EIO;
+}
+
  #ifdef CONFIG_PCI_MSI
  int pnv_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
  {
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 0cddde3..6857703 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -192,6 +192,7 @@ extern int pnv_tce_xchg(struct iommu_table *tbl, long index,
  		unsigned long *hpa, enum dma_data_direction *direction);
  extern unsigned long pnv_tce_get(struct iommu_table *tbl, long index);

+int pnv_pci_poll(uint64_t id, int64_t rval, uint8_t *state);
  void pnv_pci_dump_phb_diag_data(struct pci_controller *hose,
  				unsigned char *log_buff);
  int pnv_pci_cfg_read(struct pci_dn *pdn,




--
Alexey



--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




--
Alexey
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux