Merge branch 'pci/pciehp' into next

* pci/pciehp:
  PCI: pciehp: Move Attention & Power Indicator support tests to accessors
  PCI: pciehp: Use symbolic constants for Slot Control fields
  PCI: pciehp: Use symbolic constants, not hard-coded bitmask
  PCI: pciehp: Simplify "Power Fault Detected" checking/clearing
  PCI: pciehp: Announce slot capabilities (slot #, button, LEDs, etc)
  PCI: pciehp: Make various functions void since they can't fail
  PCI: pciehp: Remove error checks when accessing PCIe Capability
  PCI: pciehp: Drop pciehp_readw()/pciehp_writew() wrappers

5 files changed, 177 insertions, 328 deletions

diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index 21e865d..ffe6a6b 100644
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -140,15 +140,15 @@ struct controller *pcie_init(struct pcie_device *dev);
 int pcie_init_notification(struct controller *ctrl);
 int pciehp_enable_slot(struct slot *p_slot);
 int pciehp_disable_slot(struct slot *p_slot);
-int pcie_enable_notification(struct controller *ctrl);
+void pcie_enable_notification(struct controller *ctrl);
 int pciehp_power_on_slot(struct slot *slot);
-int pciehp_power_off_slot(struct slot *slot);
-int pciehp_get_power_status(struct slot *slot, u8 *status);
-int pciehp_get_attention_status(struct slot *slot, u8 *status);
+void pciehp_power_off_slot(struct slot *slot);
+void pciehp_get_power_status(struct slot *slot, u8 *status);
+void pciehp_get_attention_status(struct slot *slot, u8 *status);
 
-int pciehp_set_attention_status(struct slot *slot, u8 status);
-int pciehp_get_latch_status(struct slot *slot, u8 *status);
-int pciehp_get_adapter_status(struct slot *slot, u8 *status);
+void pciehp_set_attention_status(struct slot *slot, u8 status);
+void pciehp_get_latch_status(struct slot *slot, u8 *status);
+void pciehp_get_adapter_status(struct slot *slot, u8 *status);
 int pciehp_query_power_fault(struct slot *slot);
 void pciehp_green_led_on(struct slot *slot);
 void pciehp_green_led_off(struct slot *slot);
diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index bbd48bb..143a389 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -160,7 +160,8 @@ static int set_attention_status(struct hotplug_slot *hotplug_slot, u8 status)
 	ctrl_dbg(slot->ctrl, "%s: physical_slot = %s\n",
 		  __func__, slot_name(slot));
 
-	return pciehp_set_attention_status(slot, status);
+	pciehp_set_attention_status(slot, status);
+	return 0;
 }
 
 
@@ -192,7 +193,8 @@ static int get_power_status(struct hotplug_slot *hotplug_slot, u8 *value)
 	ctrl_dbg(slot->ctrl, "%s: physical_slot = %s\n",
 		  __func__, slot_name(slot));
 
-	return pciehp_get_power_status(slot, value);
+	pciehp_get_power_status(slot, value);
+	return 0;
 }
 
 static int get_attention_status(struct hotplug_slot *hotplug_slot, u8 *value)
@@ -202,7 +204,8 @@ static int get_attention_status(struct hotplug_slot *hotplug_slot, u8 *value)
 	ctrl_dbg(slot->ctrl, "%s: physical_slot = %s\n",
 		  __func__, slot_name(slot));
 
-	return pciehp_get_attention_status(slot, value);
+	pciehp_get_attention_status(slot, value);
+	return 0;
 }
 
 static int get_latch_status(struct hotplug_slot *hotplug_slot, u8 *value)
@@ -212,7 +215,8 @@ static int get_latch_status(struct hotplug_slot *hotplug_slot, u8 *value)
 	ctrl_dbg(slot->ctrl, "%s: physical_slot = %s\n",
 		 __func__, slot_name(slot));
 
-	return pciehp_get_latch_status(slot, value);
+	pciehp_get_latch_status(slot, value);
+	return 0;
 }
 
 static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
@@ -222,7 +226,8 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
 	ctrl_dbg(slot->ctrl, "%s: physical_slot = %s\n",
 		 __func__, slot_name(slot));
 
-	return pciehp_get_adapter_status(slot, value);
+	pciehp_get_adapter_status(slot, value);
+	return 0;
 }
 
 static int reset_slot(struct hotplug_slot *hotplug_slot, int probe)
diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
index 38f0186..5062848 100644
--- a/drivers/pci/hotplug/pciehp_ctrl.c
+++ b/drivers/pci/hotplug/pciehp_ctrl.c
@@ -158,11 +158,8 @@ static void set_slot_off(struct controller *ctrl, struct slot * pslot)
 {
 	/* turn off slot, turn on Amber LED, turn off Green LED if supported*/
 	if (POWER_CTRL(ctrl)) {
-		if (pciehp_power_off_slot(pslot)) {
-			ctrl_err(ctrl,
-				 "Issue of Slot Power Off command failed\n");
-			return;
-		}
+		pciehp_power_off_slot(pslot);
+
 		/*
 		 * After turning power off, we must wait for at least 1 second
 		 * before taking any action that relies on power having been
@@ -171,16 +168,8 @@ static void set_slot_off(struct controller *ctrl, struct slot * pslot)
 		msleep(1000);
 	}
 
-	if (PWR_LED(ctrl))
-		pciehp_green_led_off(pslot);
-
-	if (ATTN_LED(ctrl)) {
-		if (pciehp_set_attention_status(pslot, 1)) {
-			ctrl_err(ctrl,
-				 "Issue of Set Attention Led command failed\n");
-			return;
-		}
-	}
+	pciehp_green_led_off(pslot);
+	pciehp_set_attention_status(pslot, 1);
 }
 
 /**
@@ -203,8 +192,7 @@ static int board_added(struct slot *p_slot)
 			return retval;
 	}
 
-	if (PWR_LED(ctrl))
-		pciehp_green_led_blink(p_slot);
+	pciehp_green_led_blink(p_slot);
 
 	/* Check link training status */
 	retval = pciehp_check_link_status(ctrl);
@@ -227,9 +215,7 @@ static int board_added(struct slot *p_slot)
 		goto err_exit;
 	}
 
-	if (PWR_LED(ctrl))
-		pciehp_green_led_on(p_slot);
-
+	pciehp_green_led_on(p_slot);
 	return 0;
 
 err_exit:
@@ -243,7 +229,7 @@ err_exit:
  */
 static int remove_board(struct slot *p_slot)
 {
-	int retval = 0;
+	int retval;
 	struct controller *ctrl = p_slot->ctrl;
 
 	retval = pciehp_unconfigure_device(p_slot);
@@ -251,13 +237,8 @@ static int remove_board(struct slot *p_slot)
 		return retval;
 
 	if (POWER_CTRL(ctrl)) {
-		/* power off slot */
-		retval = pciehp_power_off_slot(p_slot);
-		if (retval) {
-			ctrl_err(ctrl,
-				 "Issue of Slot Disable command failed\n");
-			return retval;
-		}
+		pciehp_power_off_slot(p_slot);
+
 		/*
 		 * After turning power off, we must wait for at least 1 second
 		 * before taking any action that relies on power having been
@@ -267,9 +248,7 @@ static int remove_board(struct slot *p_slot)
 	}
 
 	/* turn off Green LED */
-	if (PWR_LED(ctrl))
-		pciehp_green_led_off(p_slot);
-
+	pciehp_green_led_off(p_slot);
 	return 0;
 }
 
@@ -305,7 +284,7 @@ static void pciehp_power_thread(struct work_struct *work)
 		break;
 	case POWERON_STATE:
 		mutex_unlock(&p_slot->lock);
-		if (pciehp_enable_slot(p_slot) && PWR_LED(p_slot->ctrl))
+		if (pciehp_enable_slot(p_slot))
 			pciehp_green_led_off(p_slot);
 		mutex_lock(&p_slot->lock);
 		p_slot->state = STATIC_STATE;
@@ -372,11 +351,8 @@ static void handle_button_press_event(struct slot *p_slot)
 				  "press.\n", slot_name(p_slot));
 		}
 		/* blink green LED and turn off amber */
-		if (PWR_LED(ctrl))
-			pciehp_green_led_blink(p_slot);
-		if (ATTN_LED(ctrl))
-			pciehp_set_attention_status(p_slot, 0);
-
+		pciehp_green_led_blink(p_slot);
+		pciehp_set_attention_status(p_slot, 0);
 		queue_delayed_work(p_slot->wq, &p_slot->work, 5*HZ);
 		break;
 	case BLINKINGOFF_STATE:
@@ -389,14 +365,11 @@ static void handle_button_press_event(struct slot *p_slot)
 		ctrl_info(ctrl, "Button cancel on Slot(%s)\n", slot_name(p_slot));
 		cancel_delayed_work(&p_slot->work);
 		if (p_slot->state == BLINKINGOFF_STATE) {
-			if (PWR_LED(ctrl))
-				pciehp_green_led_on(p_slot);
+			pciehp_green_led_on(p_slot);
 		} else {
-			if (PWR_LED(ctrl))
-				pciehp_green_led_off(p_slot);
+			pciehp_green_led_off(p_slot);
 		}
-		if (ATTN_LED(ctrl))
-			pciehp_set_attention_status(p_slot, 0);
+		pciehp_set_attention_status(p_slot, 0);
 		ctrl_info(ctrl, "PCI slot #%s - action canceled "
 			  "due to button press\n", slot_name(p_slot));
 		p_slot->state = STATIC_STATE;
@@ -456,10 +429,8 @@ static void interrupt_event_handler(struct work_struct *work)
 	case INT_POWER_FAULT:
 		if (!POWER_CTRL(ctrl))
 			break;
-		if (ATTN_LED(ctrl))
-			pciehp_set_attention_status(p_slot, 1);
-		if (PWR_LED(ctrl))
-			pciehp_green_led_off(p_slot);
+		pciehp_set_attention_status(p_slot, 1);
+		pciehp_green_led_off(p_slot);
 		break;
 	case INT_PRESENCE_ON:
 	case INT_PRESENCE_OFF:
@@ -482,14 +453,14 @@ int pciehp_enable_slot(struct slot *p_slot)
 	int rc;
 	struct controller *ctrl = p_slot->ctrl;
 
-	rc = pciehp_get_adapter_status(p_slot, &getstatus);
-	if (rc || !getstatus) {
+	pciehp_get_adapter_status(p_slot, &getstatus);
+	if (!getstatus) {
 		ctrl_info(ctrl, "No adapter on slot(%s)\n", slot_name(p_slot));
 		return -ENODEV;
 	}
 	if (MRL_SENS(p_slot->ctrl)) {
-		rc = pciehp_get_latch_status(p_slot, &getstatus);
-		if (rc || getstatus) {
+		pciehp_get_latch_status(p_slot, &getstatus);
+		if (getstatus) {
 			ctrl_info(ctrl, "Latch open on slot(%s)\n",
 				  slot_name(p_slot));
 			return -ENODEV;
@@ -497,8 +468,8 @@ int pciehp_enable_slot(struct slot *p_slot)
 	}
 
 	if (POWER_CTRL(p_slot->ctrl)) {
-		rc = pciehp_get_power_status(p_slot, &getstatus);
-		if (rc || getstatus) {
+		pciehp_get_power_status(p_slot, &getstatus);
+		if (getstatus) {
 			ctrl_info(ctrl, "Already enabled on slot(%s)\n",
 				  slot_name(p_slot));
 			return -EINVAL;
@@ -518,15 +489,14 @@ int pciehp_enable_slot(struct slot *p_slot)
 int pciehp_disable_slot(struct slot *p_slot)
 {
 	u8 getstatus = 0;
-	int ret = 0;
 	struct controller *ctrl = p_slot->ctrl;
 
 	if (!p_slot->ctrl)
 		return 1;
 
 	if (!HP_SUPR_RM(p_slot->ctrl)) {
-		ret = pciehp_get_adapter_status(p_slot, &getstatus);
-		if (ret || !getstatus) {
+		pciehp_get_adapter_status(p_slot, &getstatus);
+		if (!getstatus) {
 			ctrl_info(ctrl, "No adapter on slot(%s)\n",
 				  slot_name(p_slot));
 			return -ENODEV;
@@ -534,8 +504,8 @@ int pciehp_disable_slot(struct slot *p_slot)
 	}
 
 	if (MRL_SENS(p_slot->ctrl)) {
-		ret = pciehp_get_latch_status(p_slot, &getstatus);
-		if (ret || getstatus) {
+		pciehp_get_latch_status(p_slot, &getstatus);
+		if (getstatus) {
 			ctrl_info(ctrl, "Latch open on slot(%s)\n",
 				  slot_name(p_slot));
 			return -ENODEV;
@@ -543,8 +513,8 @@ int pciehp_disable_slot(struct slot *p_slot)
 	}
 
 	if (POWER_CTRL(p_slot->ctrl)) {
-		ret = pciehp_get_power_status(p_slot, &getstatus);
-		if (ret || !getstatus) {
+		pciehp_get_power_status(p_slot, &getstatus);
+		if (!getstatus) {
 			ctrl_info(ctrl, "Already disabled on slot(%s)\n",
 				  slot_name(p_slot));
 			return -EINVAL;
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 3eea3fd..14acfcc 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -41,34 +41,11 @@
 #include "../pci.h"
 #include "pciehp.h"
 
-static inline int pciehp_readw(struct controller *ctrl, int reg, u16 *value)
+static inline struct pci_dev *ctrl_dev(struct controller *ctrl)
 {
-	struct pci_dev *dev = ctrl->pcie->port;
-	return pcie_capability_read_word(dev, reg, value);
+	return ctrl->pcie->port;
 }
 
-static inline int pciehp_readl(struct controller *ctrl, int reg, u32 *value)
-{
-	struct pci_dev *dev = ctrl->pcie->port;
-	return pcie_capability_read_dword(dev, reg, value);
-}
-
-static inline int pciehp_writew(struct controller *ctrl, int reg, u16 value)
-{
-	struct pci_dev *dev = ctrl->pcie->port;
-	return pcie_capability_write_word(dev, reg, value);
-}
-
-static inline int pciehp_writel(struct controller *ctrl, int reg, u32 value)
-{
-	struct pci_dev *dev = ctrl->pcie->port;
-	return pcie_capability_write_dword(dev, reg, value);
-}
-
-/* Power Control Command */
-#define POWER_ON	0
-#define POWER_OFF	PCI_EXP_SLTCTL_PCC
-
 static irqreturn_t pcie_isr(int irq, void *dev_id);
 static void start_int_poll_timer(struct controller *ctrl, int sec);
 
@@ -129,20 +106,23 @@ static inline void pciehp_free_irq(struct controller *ctrl)
 
 static int pcie_poll_cmd(struct controller *ctrl)
 {
+	struct pci_dev *pdev = ctrl_dev(ctrl);
 	u16 slot_status;
-	int err, timeout = 1000;
+	int timeout = 1000;
 
-	err = pciehp_readw(ctrl, PCI_EXP_SLTSTA, &slot_status);
-	if (!err && (slot_status & PCI_EXP_SLTSTA_CC)) {
-		pciehp_writew(ctrl, PCI_EXP_SLTSTA, PCI_EXP_SLTSTA_CC);
+	pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
+	if (slot_status & PCI_EXP_SLTSTA_CC) {
+		pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
+					   PCI_EXP_SLTSTA_CC);
 		return 1;
 	}
 	while (timeout > 0) {
 		msleep(10);
 		timeout -= 10;
-		err = pciehp_readw(ctrl, PCI_EXP_SLTSTA, &slot_status);
-		if (!err && (slot_status & PCI_EXP_SLTSTA_CC)) {
-			pciehp_writew(ctrl, PCI_EXP_SLTSTA, PCI_EXP_SLTSTA_CC);
+		pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
+		if (slot_status & PCI_EXP_SLTSTA_CC) {
+			pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
+						   PCI_EXP_SLTSTA_CC);
 			return 1;
 		}
 	}
@@ -169,21 +149,15 @@ static void pcie_wait_cmd(struct controller *ctrl, int poll)
  * @cmd:  command value written to slot control register
  * @mask: bitmask of slot control register to be modified
  */
-static int pcie_write_cmd(struct controller *ctrl, u16 cmd, u16 mask)
+static void pcie_write_cmd(struct controller *ctrl, u16 cmd, u16 mask)
 {
-	int retval = 0;
+	struct pci_dev *pdev = ctrl_dev(ctrl);
 	u16 slot_status;
 	u16 slot_ctrl;
 
 	mutex_lock(&ctrl->ctrl_lock);
 
-	retval = pciehp_readw(ctrl, PCI_EXP_SLTSTA, &slot_status);
-	if (retval) {
-		ctrl_err(ctrl, "%s: Cannot read SLOTSTATUS register\n",
-			 __func__);
-		goto out;
-	}
-
+	pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
 	if (slot_status & PCI_EXP_SLTSTA_CC) {
 		if (!ctrl->no_cmd_complete) {
 			/*
@@ -207,24 +181,17 @@ static int pcie_write_cmd(struct controller *ctrl, u16 cmd, u16 mask)
 		}
 	}
 
-	retval = pciehp_readw(ctrl, PCI_EXP_SLTCTL, &slot_ctrl);
-	if (retval) {
-		ctrl_err(ctrl, "%s: Cannot read SLOTCTRL register\n", __func__);
-		goto out;
-	}
-
+	pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
 	slot_ctrl &= ~mask;
 	slot_ctrl |= (cmd & mask);
 	ctrl->cmd_busy = 1;
 	smp_mb();
-	retval = pciehp_writew(ctrl, PCI_EXP_SLTCTL, slot_ctrl);
-	if (retval)
-		ctrl_err(ctrl, "Cannot write to SLOTCTRL register\n");
+	pcie_capability_write_word(pdev, PCI_EXP_SLTCTL, slot_ctrl);
 
 	/*
 	 * Wait for command completion.
 	 */
-	if (!retval && !ctrl->no_cmd_complete) {
+	if (!ctrl->no_cmd_complete) {
 		int poll = 0;
 		/*
 		 * if hotplug interrupt is not enabled or command
@@ -236,19 +203,16 @@ static int pcie_write_cmd(struct controller *ctrl, u16 cmd, u16 mask)
 			poll = 1;
                 pcie_wait_cmd(ctrl, poll);
 	}
- out:
 	mutex_unlock(&ctrl->ctrl_lock);
-	return retval;
 }
 
 static bool check_link_active(struct controller *ctrl)
 {
-	bool ret = false;
+	struct pci_dev *pdev = ctrl_dev(ctrl);
 	u16 lnk_status;
+	bool ret;
 
-	if (pciehp_readw(ctrl, PCI_EXP_LNKSTA, &lnk_status))
-		return ret;
-
+	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
 	ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
 
 	if (ret)
@@ -311,9 +275,9 @@ static bool pci_bus_check_dev(struct pci_bus *bus, int devfn)
 
 int pciehp_check_link_status(struct controller *ctrl)
 {
+	struct pci_dev *pdev = ctrl_dev(ctrl);
+	bool found;
 	u16 lnk_status;
-	int retval = 0;
-	bool found = false;
 
         /*
          * Data Link Layer Link Active Reporting must be capable for
@@ -330,52 +294,37 @@ int pciehp_check_link_status(struct controller *ctrl)
 	found = pci_bus_check_dev(ctrl->pcie->port->subordinate,
 					PCI_DEVFN(0, 0));
 
-	retval = pciehp_readw(ctrl, PCI_EXP_LNKSTA, &lnk_status);
-	if (retval) {
-		ctrl_err(ctrl, "Cannot read LNKSTATUS register\n");
-		return retval;
-	}
-
+	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
 	ctrl_dbg(ctrl, "%s: lnk_status = %x\n", __func__, lnk_status);
 	if ((lnk_status & PCI_EXP_LNKSTA_LT) ||
 	    !(lnk_status & PCI_EXP_LNKSTA_NLW)) {
 		ctrl_err(ctrl, "Link Training Error occurs \n");
-		retval = -1;
-		return retval;
+		return -1;
 	}
 
 	pcie_update_link_speed(ctrl->pcie->port->subordinate, lnk_status);
 
-	if (!found && !retval)
-		retval = -1;
+	if (!found)
+		return -1;
 
-	return retval;
+	return 0;
 }
 
 static int __pciehp_link_set(struct controller *ctrl, bool enable)
 {
+	struct pci_dev *pdev = ctrl_dev(ctrl);
 	u16 lnk_ctrl;
-	int retval = 0;
 
-	retval = pciehp_readw(ctrl, PCI_EXP_LNKCTL, &lnk_ctrl);
-	if (retval) {
-		ctrl_err(ctrl, "Cannot read LNKCTRL register\n");
-		return retval;
-	}
+	pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &lnk_ctrl);
 
 	if (enable)
 		lnk_ctrl &= ~PCI_EXP_LNKCTL_LD;
 	else
 		lnk_ctrl |= PCI_EXP_LNKCTL_LD;
 
-	retval = pciehp_writew(ctrl, PCI_EXP_LNKCTL, lnk_ctrl);
-	if (retval) {
-		ctrl_err(ctrl, "Cannot write LNKCTRL register\n");
-		return retval;
-	}
+	pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, lnk_ctrl);
 	ctrl_dbg(ctrl, "%s: lnk_ctrl = %x\n", __func__, lnk_ctrl);
-
-	return retval;
+	return 0;
 }
 
 static int pciehp_link_enable(struct controller *ctrl)
@@ -388,223 +337,165 @@ static int pciehp_link_disable(struct controller *ctrl)
 	return __pciehp_link_set(ctrl, false);
 }
 
-int pciehp_get_attention_status(struct slot *slot, u8 *status)
+void pciehp_get_attention_status(struct slot *slot, u8 *status)
 {
 	struct controller *ctrl = slot->ctrl;
+	struct pci_dev *pdev = ctrl_dev(ctrl);
 	u16 slot_ctrl;
-	u8 atten_led_state;
-	int retval = 0;
-
-	retval = pciehp_readw(ctrl, PCI_EXP_SLTCTL, &slot_ctrl);
-	if (retval) {
-		ctrl_err(ctrl, "%s: Cannot read SLOTCTRL register\n", __func__);
-		return retval;
-	}
 
+	pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
 	ctrl_dbg(ctrl, "%s: SLOTCTRL %x, value read %x\n", __func__,
 		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_ctrl);
 
-	atten_led_state = (slot_ctrl & PCI_EXP_SLTCTL_AIC) >> 6;
-
-	switch (atten_led_state) {
-	case 0:
-		*status = 0xFF;	/* Reserved */
-		break;
-	case 1:
+	switch (slot_ctrl & PCI_EXP_SLTCTL_AIC) {
+	case PCI_EXP_SLTCTL_ATTN_IND_ON:
 		*status = 1;	/* On */
 		break;
-	case 2:
+	case PCI_EXP_SLTCTL_ATTN_IND_BLINK:
 		*status = 2;	/* Blink */
 		break;
-	case 3:
+	case PCI_EXP_SLTCTL_ATTN_IND_OFF:
 		*status = 0;	/* Off */
 		break;
 	default:
 		*status = 0xFF;
 		break;
 	}
-
-	return 0;
 }
 
-int pciehp_get_power_status(struct slot *slot, u8 *status)
+void pciehp_get_power_status(struct slot *slot, u8 *status)
 {
 	struct controller *ctrl = slot->ctrl;
+	struct pci_dev *pdev = ctrl_dev(ctrl);
 	u16 slot_ctrl;
-	u8 pwr_state;
-	int	retval = 0;
 
-	retval = pciehp_readw(ctrl, PCI_EXP_SLTCTL, &slot_ctrl);
-	if (retval) {
-		ctrl_err(ctrl, "%s: Cannot read SLOTCTRL register\n", __func__);
-		return retval;
-	}
+	pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
 	ctrl_dbg(ctrl, "%s: SLOTCTRL %x value read %x\n", __func__,
 		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_ctrl);
 
-	pwr_state = (slot_ctrl & PCI_EXP_SLTCTL_PCC) >> 10;
-
-	switch (pwr_state) {
-	case 0:
-		*status = 1;
+	switch (slot_ctrl & PCI_EXP_SLTCTL_PCC) {
+	case PCI_EXP_SLTCTL_PWR_ON:
+		*status = 1;	/* On */
 		break;
-	case 1:
-		*status = 0;
+	case PCI_EXP_SLTCTL_PWR_OFF:
+		*status = 0;	/* Off */
 		break;
 	default:
 		*status = 0xFF;
 		break;
 	}
-
-	return retval;
 }
 
-int pciehp_get_latch_status(struct slot *slot, u8 *status)
+void pciehp_get_latch_status(struct slot *slot, u8 *status)
 {
-	struct controller *ctrl = slot->ctrl;
+	struct pci_dev *pdev = ctrl_dev(slot->ctrl);
 	u16 slot_status;
-	int retval;
 
-	retval = pciehp_readw(ctrl, PCI_EXP_SLTSTA, &slot_status);
-	if (retval) {
-		ctrl_err(ctrl, "%s: Cannot read SLOTSTATUS register\n",
-			 __func__);
-		return retval;
-	}
+	pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
 	*status = !!(slot_status & PCI_EXP_SLTSTA_MRLSS);
-	return 0;
 }
 
-int pciehp_get_adapter_status(struct slot *slot, u8 *status)
+void pciehp_get_adapter_status(struct slot *slot, u8 *status)
 {
-	struct controller *ctrl = slot->ctrl;
+	struct pci_dev *pdev = ctrl_dev(slot->ctrl);
 	u16 slot_status;
-	int retval;
 
-	retval = pciehp_readw(ctrl, PCI_EXP_SLTSTA, &slot_status);
-	if (retval) {
-		ctrl_err(ctrl, "%s: Cannot read SLOTSTATUS register\n",
-			 __func__);
-		return retval;
-	}
+	pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
 	*status = !!(slot_status & PCI_EXP_SLTSTA_PDS);
-	return 0;
 }
 
 int pciehp_query_power_fault(struct slot *slot)
 {
-	struct controller *ctrl = slot->ctrl;
+	struct pci_dev *pdev = ctrl_dev(slot->ctrl);
 	u16 slot_status;
-	int retval;
 
-	retval = pciehp_readw(ctrl, PCI_EXP_SLTSTA, &slot_status);
-	if (retval) {
-		ctrl_err(ctrl, "Cannot check for power fault\n");
-		return retval;
-	}
+	pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
 	return !!(slot_status & PCI_EXP_SLTSTA_PFD);
 }
 
-int pciehp_set_attention_status(struct slot *slot, u8 value)
+void pciehp_set_attention_status(struct slot *slot, u8 value)
 {
 	struct controller *ctrl = slot->ctrl;
 	u16 slot_cmd;
-	u16 cmd_mask;
 
-	cmd_mask = PCI_EXP_SLTCTL_AIC;
+	if (!ATTN_LED(ctrl))
+		return;
+
 	switch (value) {
 	case 0 :	/* turn off */
-		slot_cmd = 0x00C0;
+		slot_cmd = PCI_EXP_SLTCTL_ATTN_IND_OFF;
 		break;
 	case 1:		/* turn on */
-		slot_cmd = 0x0040;
+		slot_cmd = PCI_EXP_SLTCTL_ATTN_IND_ON;
 		break;
 	case 2:		/* turn blink */
-		slot_cmd = 0x0080;
+		slot_cmd = PCI_EXP_SLTCTL_ATTN_IND_BLINK;
 		break;
 	default:
-		return -EINVAL;
+		return;
 	}
 	ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
 		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_cmd);
-	return pcie_write_cmd(ctrl, slot_cmd, cmd_mask);
+	pcie_write_cmd(ctrl, slot_cmd, PCI_EXP_SLTCTL_AIC);
 }
 
 void pciehp_green_led_on(struct slot *slot)
 {
 	struct controller *ctrl = slot->ctrl;
-	u16 slot_cmd;
-	u16 cmd_mask;
 
-	slot_cmd = 0x0100;
-	cmd_mask = PCI_EXP_SLTCTL_PIC;
-	pcie_write_cmd(ctrl, slot_cmd, cmd_mask);
+	if (!PWR_LED(ctrl))
+		return;
+
+	pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_IND_ON, PCI_EXP_SLTCTL_PIC);
 	ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
-		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_cmd);
+		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL,
+		 PCI_EXP_SLTCTL_PWR_IND_ON);
 }
 
 void pciehp_green_led_off(struct slot *slot)
 {
 	struct controller *ctrl = slot->ctrl;
-	u16 slot_cmd;
-	u16 cmd_mask;
 
-	slot_cmd = 0x0300;
-	cmd_mask = PCI_EXP_SLTCTL_PIC;
-	pcie_write_cmd(ctrl, slot_cmd, cmd_mask);
+	if (!PWR_LED(ctrl))
+		return;
+
+	pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_IND_OFF, PCI_EXP_SLTCTL_PIC);
 	ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
-		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_cmd);
+		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL,
+		 PCI_EXP_SLTCTL_PWR_IND_OFF);
 }
 
 void pciehp_green_led_blink(struct slot *slot)
 {
 	struct controller *ctrl = slot->ctrl;
-	u16 slot_cmd;
-	u16 cmd_mask;
 
-	slot_cmd = 0x0200;
-	cmd_mask = PCI_EXP_SLTCTL_PIC;
-	pcie_write_cmd(ctrl, slot_cmd, cmd_mask);
+	if (!PWR_LED(ctrl))
+		return;
+
+	pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_IND_BLINK, PCI_EXP_SLTCTL_PIC);
 	ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
-		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_cmd);
+		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL,
+		 PCI_EXP_SLTCTL_PWR_IND_BLINK);
 }
 
 int pciehp_power_on_slot(struct slot * slot)
 {
 	struct controller *ctrl = slot->ctrl;
-	u16 slot_cmd;
-	u16 cmd_mask;
+	struct pci_dev *pdev = ctrl_dev(ctrl);
 	u16 slot_status;
-	int retval = 0;
+	int retval;
 
 	/* Clear sticky power-fault bit from previous power failures */
-	retval = pciehp_readw(ctrl, PCI_EXP_SLTSTA, &slot_status);
-	if (retval) {
-		ctrl_err(ctrl, "%s: Cannot read SLOTSTATUS register\n",
-			 __func__);
-		return retval;
-	}
-	slot_status &= PCI_EXP_SLTSTA_PFD;
-	if (slot_status) {
-		retval = pciehp_writew(ctrl, PCI_EXP_SLTSTA, slot_status);
-		if (retval) {
-			ctrl_err(ctrl,
-				 "%s: Cannot write to SLOTSTATUS register\n",
-				 __func__);
-			return retval;
-		}
-	}
+	pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
+	if (slot_status & PCI_EXP_SLTSTA_PFD)
+		pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
+					   PCI_EXP_SLTSTA_PFD);
 	ctrl->power_fault_detected = 0;
 
-	slot_cmd = POWER_ON;
-	cmd_mask = PCI_EXP_SLTCTL_PCC;
-	retval = pcie_write_cmd(ctrl, slot_cmd, cmd_mask);
-	if (retval) {
-		ctrl_err(ctrl, "Write %x command failed!\n", slot_cmd);
-		return retval;
-	}
+	pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_ON, PCI_EXP_SLTCTL_PCC);
 	ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
-		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_cmd);
+		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL,
+		 PCI_EXP_SLTCTL_PWR_ON);
 
 	retval = pciehp_link_enable(ctrl);
 	if (retval)
@@ -613,12 +504,9 @@ int pciehp_power_on_slot(struct slot * slot)
 	return retval;
 }
 
-int pciehp_power_off_slot(struct slot * slot)
+void pciehp_power_off_slot(struct slot * slot)
 {
 	struct controller *ctrl = slot->ctrl;
-	u16 slot_cmd;
-	u16 cmd_mask;
-	int retval;
 
 	/* Disable the link at first */
 	pciehp_link_disable(ctrl);
@@ -628,21 +516,16 @@ int pciehp_power_off_slot(struct slot * slot)
 	else
 		msleep(1000);
 
-	slot_cmd = POWER_OFF;
-	cmd_mask = PCI_EXP_SLTCTL_PCC;
-	retval = pcie_write_cmd(ctrl, slot_cmd, cmd_mask);
-	if (retval) {
-		ctrl_err(ctrl, "Write command failed!\n");
-		return retval;
-	}
+	pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_OFF, PCI_EXP_SLTCTL_PCC);
 	ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
-		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, slot_cmd);
-	return 0;
+		 pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL,
+		 PCI_EXP_SLTCTL_PWR_OFF);
 }
 
 static irqreturn_t pcie_isr(int irq, void *dev_id)
 {
 	struct controller *ctrl = (struct controller *)dev_id;
+	struct pci_dev *pdev = ctrl_dev(ctrl);
 	struct slot *slot = ctrl->slot;
 	u16 detected, intr_loc;
 
@@ -653,11 +536,7 @@ static irqreturn_t pcie_isr(int irq, void *dev_id)
 	 */
 	intr_loc = 0;
 	do {
-		if (pciehp_readw(ctrl, PCI_EXP_SLTSTA, &detected)) {
-			ctrl_err(ctrl, "%s: Cannot read SLOTSTATUS\n",
-				 __func__);
-			return IRQ_NONE;
-		}
+		pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &detected);
 
 		detected &= (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD |
 			     PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC |
@@ -666,11 +545,9 @@ static irqreturn_t pcie_isr(int irq, void *dev_id)
 		intr_loc |= detected;
 		if (!intr_loc)
 			return IRQ_NONE;
-		if (detected && pciehp_writew(ctrl, PCI_EXP_SLTSTA, intr_loc)) {
-			ctrl_err(ctrl, "%s: Cannot write to SLOTSTATUS\n",
-				 __func__);
-			return IRQ_NONE;
-		}
+		if (detected)
+			pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
+						   intr_loc);
 	} while (detected);
 
 	ctrl_dbg(ctrl, "%s: intr_loc %x\n", __func__, intr_loc);
@@ -705,7 +582,7 @@ static irqreturn_t pcie_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-int pcie_enable_notification(struct controller *ctrl)
+void pcie_enable_notification(struct controller *ctrl)
 {
 	u16 cmd, mask;
 
@@ -731,22 +608,18 @@ int pcie_enable_notification(struct controller *ctrl)
 		PCI_EXP_SLTCTL_MRLSCE | PCI_EXP_SLTCTL_PFDE |
 		PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE);
 
-	if (pcie_write_cmd(ctrl, cmd, mask)) {
-		ctrl_err(ctrl, "Cannot enable software notification\n");
-		return -1;
-	}
-	return 0;
+	pcie_write_cmd(ctrl, cmd, mask);
 }
 
 static void pcie_disable_notification(struct controller *ctrl)
 {
 	u16 mask;
+
 	mask = (PCI_EXP_SLTCTL_PDCE | PCI_EXP_SLTCTL_ABPE |
 		PCI_EXP_SLTCTL_MRLSCE | PCI_EXP_SLTCTL_PFDE |
 		PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE |
 		PCI_EXP_SLTCTL_DLLSCE);
-	if (pcie_write_cmd(ctrl, 0, mask))
-		ctrl_warn(ctrl, "Cannot disable software notification\n");
+	pcie_write_cmd(ctrl, 0, mask);
 }
 
 /*
@@ -758,6 +631,7 @@ static void pcie_disable_notification(struct controller *ctrl)
 int pciehp_reset_slot(struct slot *slot, int probe)
 {
 	struct controller *ctrl = slot->ctrl;
+	struct pci_dev *pdev = ctrl_dev(ctrl);
 
 	if (probe)
 		return 0;
@@ -771,7 +645,8 @@ int pciehp_reset_slot(struct slot *slot, int probe)
 	pci_reset_bridge_secondary_bus(ctrl->pcie->port);
 
 	if (HP_SUPR_RM(ctrl)) {
-		pciehp_writew(ctrl, PCI_EXP_SLTSTA, PCI_EXP_SLTSTA_PDC);
+		pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
+					   PCI_EXP_SLTSTA_PDC);
 		pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PDCE, PCI_EXP_SLTCTL_PDCE);
 		if (pciehp_poll_mode)
 			int_poll_timeout(ctrl->poll_timer.data);
@@ -784,10 +659,7 @@ int pcie_init_notification(struct controller *ctrl)
 {
 	if (pciehp_request_irq(ctrl))
 		return -1;
-	if (pcie_enable_notification(ctrl)) {
-		pciehp_free_irq(ctrl);
-		return -1;
-	}
+	pcie_enable_notification(ctrl);
 	ctrl->notification_enabled = 1;
 	return 0;
 }
@@ -875,12 +747,14 @@ static inline void dbg_ctrl(struct controller *ctrl)
 		  EMI(ctrl)        ? "yes" : "no");
 	ctrl_info(ctrl, "  Command Completed    : %3s\n",
 		  NO_CMD_CMPL(ctrl) ? "no" : "yes");
-	pciehp_readw(ctrl, PCI_EXP_SLTSTA, &reg16);
+	pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &reg16);
 	ctrl_info(ctrl, "Slot Status            : 0x%04x\n", reg16);
-	pciehp_readw(ctrl, PCI_EXP_SLTCTL, &reg16);
+	pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &reg16);
 	ctrl_info(ctrl, "Slot Control           : 0x%04x\n", reg16);
 }
 
+#define FLAG(x,y)	(((x) & (y)) ? '+' : '-')
+
 struct controller *pcie_init(struct pcie_device *dev)
 {
 	struct controller *ctrl;
@@ -893,11 +767,7 @@ struct controller *pcie_init(struct pcie_device *dev)
 		goto abort;
 	}
 	ctrl->pcie = dev;
-	if (pciehp_readl(ctrl, PCI_EXP_SLTCAP, &slot_cap)) {
-		ctrl_err(ctrl, "Cannot read SLOTCAP register\n");
-		goto abort_ctrl;
-	}
-
+	pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &slot_cap);
 	ctrl->slot_cap = slot_cap;
 	mutex_init(&ctrl->ctrl_lock);
 	init_waitqueue_head(&ctrl->queue);
@@ -913,25 +783,31 @@ struct controller *pcie_init(struct pcie_device *dev)
 	    ctrl->no_cmd_complete = 1;
 
         /* Check if Data Link Layer Link Active Reporting is implemented */
-        if (pciehp_readl(ctrl, PCI_EXP_LNKCAP, &link_cap)) {
-                ctrl_err(ctrl, "%s: Cannot read LNKCAP register\n", __func__);
-                goto abort_ctrl;
-        }
+        pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, &link_cap);
         if (link_cap & PCI_EXP_LNKCAP_DLLLARC) {
                 ctrl_dbg(ctrl, "Link Active Reporting supported\n");
                 ctrl->link_active_reporting = 1;
         }
 
 	/* Clear all remaining event bits in Slot Status register */
-	if (pciehp_writew(ctrl, PCI_EXP_SLTSTA, 0x1f))
-		goto abort_ctrl;
+	pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
+		PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD |
+		PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC |
+		PCI_EXP_SLTSTA_CC);
 
 	/* Disable software notification */
 	pcie_disable_notification(ctrl);
 
-	ctrl_info(ctrl, "HPC vendor_id %x device_id %x ss_vid %x ss_did %x\n",
-		  pdev->vendor, pdev->device, pdev->subsystem_vendor,
-		  pdev->subsystem_device);
+	ctrl_info(ctrl, "Slot #%d AttnBtn%c AttnInd%c PwrInd%c PwrCtrl%c MRL%c Interlock%c NoCompl%c LLActRep%c\n",
+		(slot_cap & PCI_EXP_SLTCAP_PSN) >> 19,
+		FLAG(slot_cap, PCI_EXP_SLTCAP_ABP),
+		FLAG(slot_cap, PCI_EXP_SLTCAP_AIP),
+		FLAG(slot_cap, PCI_EXP_SLTCAP_PIP),
+		FLAG(slot_cap, PCI_EXP_SLTCAP_PCP),
+		FLAG(slot_cap, PCI_EXP_SLTCAP_MRLSP),
+		FLAG(slot_cap, PCI_EXP_SLTCAP_EIP),
+		FLAG(slot_cap, PCI_EXP_SLTCAP_NCCS),
+		FLAG(link_cap, PCI_EXP_LNKCAP_DLLLARC));
 
 	if (pcie_init_slot(ctrl))
 		goto abort_ctrl;
diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index 0e0d0f7..1983551 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -78,7 +78,7 @@ int pciehp_configure_device(struct slot *p_slot)
 
 int pciehp_unconfigure_device(struct slot *p_slot)
 {
-	int ret, rc = 0;
+	int rc = 0;
 	u8 bctl = 0;
 	u8 presence = 0;
 	struct pci_dev *dev, *temp;
@@ -88,9 +88,7 @@ int pciehp_unconfigure_device(struct slot *p_slot)
 
 	ctrl_dbg(ctrl, "%s: domain:bus:dev = %04x:%02x:00\n",
 		 __func__, pci_domain_nr(parent), parent->number);
-	ret = pciehp_get_adapter_status(p_slot, &presence);
-	if (ret)
-		presence = 0;
+	pciehp_get_adapter_status(p_slot, &presence);
 
 	/*
 	 * Stopping an SR-IOV PF device removes all the associated VFs,