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.
>
>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. @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?
>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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
