On Fri, Feb 24, 2017 at 9:17 AM, Raj, Ashok <ashok.raj@xxxxxxxxx> wrote: > On Thu, Feb 23, 2017 at 10:54:35PM -0800, Yinghai Lu wrote: >> +++ linux-2.6/drivers/pci/hotplug/pciehp_ctrl.c >> @@ -71,9 +71,6 @@ static void set_slot_off(struct controll >> */ >> msleep(1000); >> } >> - >> - pciehp_green_led_off(pslot); >> - pciehp_set_attention_status(pslot, 1); > > Re using set_slot_off() in remove_board() make sense.. but i'm not sure > why these are pulled out? It seems to be functionally complete > when these are done in set_slot_off(). Just don't want to the led operation is wrapped too deep inside. In board_added(), has led operation code directly for success path. so put led operation code directly for error path make them more symmetrically. > >> } >> >> /** >> @@ -126,6 +123,8 @@ static int board_added(struct slot *p_sl >> >> err_exit: >> set_slot_off(ctrl, p_slot); >> + pciehp_green_led_off(p_slot); >> + pciehp_set_attention_status(p_slot, 1); >> return retval; >> } >> >> @@ -142,16 +141,7 @@ static int remove_board(struct slot *p_s >> if (retval) >> return retval; >> >> - if (POWER_CTRL(ctrl)) { >> - 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 >> - * removed from the slot/adapter. >> - */ >> - msleep(1000); >> - } >> + set_slot_off(ctrl, p_slot); >> >> /* turn off Green LED */ >> pciehp_green_led_off(p_slot); > Don't we need the pciehp_set_attention_status() here? that attention led could be on if previous power on is not done successfully Thanks Yinghai.