From 49ed2b4963cd00993eab518b820a6700f94f222d Mon Sep 17 00:00:00 2001 From: Kenji Kaneshige Date: Fri, 22 Sep 2006 10:17:10 -0700 Subject: pciehp: fix improper info messages The slot number displayed in info messages would cause a confusion because those are displayed in several ways (decimal and hex). Furthermore, those slot number is not same as slot name (directory name). This patch fixes those improper info messages. Signed-off-by: Kenji Kaneshige Signed-off-by: Kristen Carlson Accardi Signed-off-by: Greg Kroah-Hartman --- drivers/pci/hotplug/pciehp_ctrl.c | 47 ++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 18 deletions(-) (limited to 'drivers/pci/hotplug/pciehp_ctrl.c') diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index 41290a106bd8..f602b042adcf 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -43,6 +43,11 @@ static int event_finished; static unsigned long pushbutton_pending; /* = 0 */ static unsigned long surprise_rm_pending; /* = 0 */ +static inline char *slot_name(struct slot *p_slot) +{ + return p_slot->hotplug_slot->name; +} + u8 pciehp_handle_attention_button(u8 hp_slot, void *inst_id) { struct controller *ctrl = (struct controller *) inst_id; @@ -68,7 +73,7 @@ u8 pciehp_handle_attention_button(u8 hp_slot, void *inst_id) /* * Button pressed - See if need to TAKE ACTION!!! */ - info("Button pressed on Slot(%d)\n", ctrl->first_slot + hp_slot); + info("Button pressed on Slot(%s)\n", slot_name(p_slot)); taskInfo->event_type = INT_BUTTON_PRESS; if ((p_slot->state == BLINKINGON_STATE) @@ -78,7 +83,7 @@ u8 pciehp_handle_attention_button(u8 hp_slot, void *inst_id) * or hot-remove */ taskInfo->event_type = INT_BUTTON_CANCEL; - info("Button cancel on Slot(%d)\n", ctrl->first_slot + hp_slot); + info("Button cancel on Slot(%s)\n", slot_name(p_slot)); } else if ((p_slot->state == POWERON_STATE) || (p_slot->state == POWEROFF_STATE)) { /* Ignore if the slot is on power-on or power-off state; this @@ -86,7 +91,7 @@ u8 pciehp_handle_attention_button(u8 hp_slot, void *inst_id) * hot-remove is undergoing */ taskInfo->event_type = INT_BUTTON_IGNORE; - info("Button ignore on Slot(%d)\n", ctrl->first_slot + hp_slot); + info("Button ignore on Slot(%s)\n", slot_name(p_slot)); } if (rc) @@ -122,13 +127,13 @@ u8 pciehp_handle_switch_change(u8 hp_slot, void *inst_id) /* * Switch opened */ - info("Latch open on Slot(%d)\n", ctrl->first_slot + hp_slot); + info("Latch open on Slot(%s)\n", slot_name(p_slot)); taskInfo->event_type = INT_SWITCH_OPEN; } else { /* * Switch closed */ - info("Latch close on Slot(%d)\n", ctrl->first_slot + hp_slot); + info("Latch close on Slot(%s)\n", slot_name(p_slot)); taskInfo->event_type = INT_SWITCH_CLOSE; } @@ -166,13 +171,13 @@ u8 pciehp_handle_presence_change(u8 hp_slot, void *inst_id) /* * Card Present */ - info("Card present on Slot(%d)\n", ctrl->first_slot + hp_slot); + info("Card present on Slot(%s)\n", slot_name(p_slot)); taskInfo->event_type = INT_PRESENCE_ON; } else { /* * Not Present */ - info("Card not present on Slot(%d)\n", ctrl->first_slot + hp_slot); + info("Card not present on Slot(%s)\n", slot_name(p_slot)); taskInfo->event_type = INT_PRESENCE_OFF; } @@ -206,13 +211,13 @@ u8 pciehp_handle_power_fault(u8 hp_slot, void *inst_id) /* * power fault Cleared */ - info("Power fault cleared on Slot(%d)\n", ctrl->first_slot + hp_slot); + info("Power fault cleared on Slot(%s)\n", slot_name(p_slot)); taskInfo->event_type = INT_POWER_FAULT_CLEAR; } else { /* * power fault */ - info("Power fault on Slot(%d)\n", ctrl->first_slot + hp_slot); + info("Power fault on Slot(%s)\n", slot_name(p_slot)); taskInfo->event_type = INT_POWER_FAULT; info("power fault bit %x set\n", hp_slot); } @@ -654,7 +659,7 @@ static void interrupt_event_handler(struct controller *ctrl) warn("Not a valid state\n"); return; } - info(msg_button_cancel, p_slot->number); + info(msg_button_cancel, slot_name(p_slot)); p_slot->state = STATIC_STATE; } /* ***********Button Pressed (No action on 1st press...) */ @@ -667,12 +672,12 @@ static void interrupt_event_handler(struct controller *ctrl) /* slot is on */ dbg("slot is on\n"); p_slot->state = BLINKINGOFF_STATE; - info(msg_button_off, p_slot->number); + info(msg_button_off, slot_name(p_slot)); } else { /* slot is off */ dbg("slot is off\n"); p_slot->state = BLINKINGON_STATE; - info(msg_button_on, p_slot->number); + info(msg_button_on, slot_name(p_slot)); } /* Wait for exclusive access to hardware */ @@ -760,14 +765,16 @@ int pciehp_enable_slot(struct slot *p_slot) rc = p_slot->hpc_ops->get_adapter_status(p_slot, &getstatus); if (rc || !getstatus) { - info("%s: no adapter on slot(%x)\n", __FUNCTION__, p_slot->number); + info("%s: no adapter on slot(%s)\n", __FUNCTION__, + slot_name(p_slot)); mutex_unlock(&p_slot->ctrl->crit_sect); return -ENODEV; } if (MRL_SENS(p_slot->ctrl->ctrlcap)) { rc = p_slot->hpc_ops->get_latch_status(p_slot, &getstatus); if (rc || getstatus) { - info("%s: latch open on slot(%x)\n", __FUNCTION__, p_slot->number); + info("%s: latch open on slot(%s)\n", __FUNCTION__, + slot_name(p_slot)); mutex_unlock(&p_slot->ctrl->crit_sect); return -ENODEV; } @@ -776,7 +783,8 @@ int pciehp_enable_slot(struct slot *p_slot) if (POWER_CTRL(p_slot->ctrl->ctrlcap)) { rc = p_slot->hpc_ops->get_power_status(p_slot, &getstatus); if (rc || getstatus) { - info("%s: already enabled on slot(%x)\n", __FUNCTION__, p_slot->number); + info("%s: already enabled on slot(%s)\n", __FUNCTION__, + slot_name(p_slot)); mutex_unlock(&p_slot->ctrl->crit_sect); return -EINVAL; } @@ -811,7 +819,8 @@ int pciehp_disable_slot(struct slot *p_slot) if (!HP_SUPR_RM(p_slot->ctrl->ctrlcap)) { ret = p_slot->hpc_ops->get_adapter_status(p_slot, &getstatus); if (ret || !getstatus) { - info("%s: no adapter on slot(%x)\n", __FUNCTION__, p_slot->number); + info("%s: no adapter on slot(%s)\n", __FUNCTION__, + slot_name(p_slot)); mutex_unlock(&p_slot->ctrl->crit_sect); return -ENODEV; } @@ -820,7 +829,8 @@ int pciehp_disable_slot(struct slot *p_slot) if (MRL_SENS(p_slot->ctrl->ctrlcap)) { ret = p_slot->hpc_ops->get_latch_status(p_slot, &getstatus); if (ret || getstatus) { - info("%s: latch open on slot(%x)\n", __FUNCTION__, p_slot->number); + info("%s: latch open on slot(%s)\n", __FUNCTION__, + slot_name(p_slot)); mutex_unlock(&p_slot->ctrl->crit_sect); return -ENODEV; } @@ -829,7 +839,8 @@ int pciehp_disable_slot(struct slot *p_slot) if (POWER_CTRL(p_slot->ctrl->ctrlcap)) { ret = p_slot->hpc_ops->get_power_status(p_slot, &getstatus); if (ret || !getstatus) { - info("%s: already disabled slot(%x)\n", __FUNCTION__, p_slot->number); + info("%s: already disabled slot(%s)\n", __FUNCTION__, + slot_name(p_slot)); mutex_unlock(&p_slot->ctrl->crit_sect); return -EINVAL; } -- cgit v1.2.3 From dd5619cb4407e830a8921a93c949be37c81105b5 Mon Sep 17 00:00:00 2001 From: Kenji Kaneshige Date: Fri, 22 Sep 2006 10:17:29 -0700 Subject: pciehp - add missing locking This patch fixes the problem that system will panic if multiple power on/off operations are issued to the same slot in parallel. This problem can be easily reproduced by commands below. # while true; do echo 1 > power; echo 0 > power; done & # while true; do echo 1 > power; echo 0 > power; done & The cause is lack of locking for enable/disable operations. This patch fixes this problem. Signed-off-by: Kenji Kaneshige Signed-off-by: Kristen Carlson Accardi Signed-off-by: Greg Kroah-Hartman --- drivers/pci/hotplug/pciehp_ctrl.c | 54 +++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 27 deletions(-) (limited to 'drivers/pci/hotplug/pciehp_ctrl.c') diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index f602b042adcf..c206a3d63b63 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -234,13 +234,13 @@ u8 pciehp_handle_power_fault(u8 hp_slot, void *inst_id) static void set_slot_off(struct controller *ctrl, struct slot * pslot) { /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); /* turn off slot, turn on Amber LED, turn off Green LED if supported*/ if (POWER_CTRL(ctrl->ctrlcap)) { if (pslot->hpc_ops->power_off_slot(pslot)) { err("%s: Issue of Slot Power Off command failed\n", __FUNCTION__); - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); return; } wait_for_ctrl_irq (ctrl); @@ -254,14 +254,14 @@ static void set_slot_off(struct controller *ctrl, struct slot * pslot) if (ATTN_LED(ctrl->ctrlcap)) { if (pslot->hpc_ops->set_attention_status(pslot, 1)) { err("%s: Issue of Set Attention Led command failed\n", __FUNCTION__); - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); return; } wait_for_ctrl_irq (ctrl); } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); } /** @@ -284,13 +284,13 @@ static int board_added(struct slot *p_slot) ctrl->slot_device_offset, hp_slot); /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); if (POWER_CTRL(ctrl->ctrlcap)) { /* Power on slot */ rc = p_slot->hpc_ops->power_on_slot(p_slot); if (rc) { - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); return -1; } @@ -306,7 +306,7 @@ static int board_added(struct slot *p_slot) } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); /* Wait for ~1 second */ wait_for_ctrl_irq (ctrl); @@ -340,7 +340,7 @@ static int board_added(struct slot *p_slot) pci_fixup_device(pci_fixup_final, ctrl->pci_dev); if (PWR_LED(ctrl->ctrlcap)) { /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); p_slot->hpc_ops->green_led_on(p_slot); @@ -348,7 +348,7 @@ static int board_added(struct slot *p_slot) wait_for_ctrl_irq (ctrl); /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); } return 0; @@ -380,14 +380,14 @@ static int remove_board(struct slot *p_slot) dbg("In %s, hp_slot = %d\n", __FUNCTION__, hp_slot); /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); if (POWER_CTRL(ctrl->ctrlcap)) { /* power off slot */ rc = p_slot->hpc_ops->power_off_slot(p_slot); if (rc) { err("%s: Issue of Slot Disable command failed\n", __FUNCTION__); - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); return rc; } /* Wait for the command to complete */ @@ -403,7 +403,7 @@ static int remove_board(struct slot *p_slot) } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); return 0; } @@ -450,7 +450,7 @@ static void pciehp_pushbutton_thread(unsigned long slot) if (pciehp_enable_slot(p_slot) && PWR_LED(p_slot->ctrl->ctrlcap)) { /* Wait for exclusive access to hardware */ - mutex_lock(&p_slot->ctrl->crit_sect); + mutex_lock(&p_slot->ctrl->ctrl_lock); p_slot->hpc_ops->green_led_off(p_slot); @@ -458,7 +458,7 @@ static void pciehp_pushbutton_thread(unsigned long slot) wait_for_ctrl_irq (p_slot->ctrl); /* Done with exclusive hardware access */ - mutex_unlock(&p_slot->ctrl->crit_sect); + mutex_unlock(&p_slot->ctrl->ctrl_lock); } p_slot->state = STATIC_STATE; } @@ -500,7 +500,7 @@ static void pciehp_surprise_rm_thread(unsigned long slot) if (pciehp_enable_slot(p_slot) && PWR_LED(p_slot->ctrl->ctrlcap)) { /* Wait for exclusive access to hardware */ - mutex_lock(&p_slot->ctrl->crit_sect); + mutex_lock(&p_slot->ctrl->ctrl_lock); p_slot->hpc_ops->green_led_off(p_slot); @@ -508,7 +508,7 @@ static void pciehp_surprise_rm_thread(unsigned long slot) wait_for_ctrl_irq (p_slot->ctrl); /* Done with exclusive hardware access */ - mutex_unlock(&p_slot->ctrl->crit_sect); + mutex_unlock(&p_slot->ctrl->ctrl_lock); } p_slot->state = STATIC_STATE; } @@ -621,7 +621,7 @@ static void interrupt_event_handler(struct controller *ctrl) switch (p_slot->state) { case BLINKINGOFF_STATE: /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); if (PWR_LED(ctrl->ctrlcap)) { p_slot->hpc_ops->green_led_on(p_slot); @@ -635,11 +635,11 @@ static void interrupt_event_handler(struct controller *ctrl) wait_for_ctrl_irq (ctrl); } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); break; case BLINKINGON_STATE: /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); if (PWR_LED(ctrl->ctrlcap)) { p_slot->hpc_ops->green_led_off(p_slot); @@ -652,7 +652,7 @@ static void interrupt_event_handler(struct controller *ctrl) wait_for_ctrl_irq (ctrl); } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); break; default: @@ -681,7 +681,7 @@ static void interrupt_event_handler(struct controller *ctrl) } /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); /* blink green LED and turn off amber */ if (PWR_LED(ctrl->ctrlcap)) { @@ -698,7 +698,7 @@ static void interrupt_event_handler(struct controller *ctrl) } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); init_timer(&p_slot->task_event); p_slot->task_event.expires = jiffies + 5 * HZ; /* 5 second delay */ @@ -713,7 +713,7 @@ static void interrupt_event_handler(struct controller *ctrl) if (POWER_CTRL(ctrl->ctrlcap)) { dbg("power fault\n"); /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); if (ATTN_LED(ctrl->ctrlcap)) { p_slot->hpc_ops->set_attention_status(p_slot, 1); @@ -726,7 +726,7 @@ static void interrupt_event_handler(struct controller *ctrl) } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); } } /***********SURPRISE REMOVAL********************/ @@ -789,7 +789,6 @@ int pciehp_enable_slot(struct slot *p_slot) return -EINVAL; } } - mutex_unlock(&p_slot->ctrl->crit_sect); p_slot->hpc_ops->get_latch_status(p_slot, &getstatus); @@ -801,6 +800,7 @@ int pciehp_enable_slot(struct slot *p_slot) if (p_slot) update_slot_info(p_slot); + mutex_unlock(&p_slot->ctrl->crit_sect); return rc; } @@ -846,10 +846,10 @@ int pciehp_disable_slot(struct slot *p_slot) } } - mutex_unlock(&p_slot->ctrl->crit_sect); - ret = remove_board(p_slot); update_slot_info(p_slot); + + mutex_unlock(&p_slot->ctrl->crit_sect); return ret; } -- cgit v1.2.3 From 9ef9977cabc1b2c1718ef6eb883caec8dcb80b4c Mon Sep 17 00:00:00 2001 From: Eric Sesterhenn Date: Mon, 25 Sep 2006 00:56:53 +0200 Subject: pciehp: Remove unnecessary check in pciehp_ctrl.c this was spotted by coverity (cid #819). We dereference p_slot earlier in the function, and i found no way it could become NULL anywhere. Signed-off-by: Eric Sesterhenn Signed-off-by: Greg Kroah-Hartman --- drivers/pci/hotplug/pciehp_ctrl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/pci/hotplug/pciehp_ctrl.c') diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index c206a3d63b63..372c63e35aa9 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -797,8 +797,7 @@ int pciehp_enable_slot(struct slot *p_slot) p_slot->hpc_ops->get_latch_status(p_slot, &getstatus); } - if (p_slot) - update_slot_info(p_slot); + update_slot_info(p_slot); mutex_unlock(&p_slot->ctrl->crit_sect); return rc; -- cgit v1.2.3