Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1587318 > unrolled thread
| Started by | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| First post | 2017-02-24 08:00 +0100 |
| Last post | 2017-02-24 19:40 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] PCI, pciehp: Reuse set_slot_off() Yinghai Lu <yinghai@kernel.org> - 2017-02-24 08:00 +0100
Re: [PATCH] PCI, pciehp: Reuse set_slot_off() "Raj, Ashok" <ashok.raj@intel.com> - 2017-02-24 18:30 +0100
Re: [PATCH] PCI, pciehp: Reuse set_slot_off() Yinghai Lu <yinghai@kernel.org> - 2017-02-24 19:40 +0100
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2017-02-24 08:00 +0100 |
| Subject | [PATCH] PCI, pciehp: Reuse set_slot_off() |
| Message-ID | <tei1b-4PN-3@gated-at.bofh.it> |
Now set_slot_off() is used in board_added() err path.
We could reuse in remove_board.
Also need to move green_led and attention_status out it.
and make code more readable.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/pci/hotplug/pciehp_ctrl.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
Index: linux-2.6/drivers/pci/hotplug/pciehp_ctrl.c
===================================================================
--- linux-2.6.orig/drivers/pci/hotplug/pciehp_ctrl.c
+++ 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);
}
/**
@@ -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);
[toc] | [next] | [standalone]
| From | "Raj, Ashok" <ashok.raj@intel.com> |
|---|---|
| Date | 2017-02-24 18:30 +0100 |
| Message-ID | <terQR-3oh-5@gated-at.bofh.it> |
| In reply to | #1587318 |
Hi Yinghai
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().
> }
>
> /**
> @@ -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?
Cheers,
Ashok
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2017-02-24 19:40 +0100 |
| Message-ID | <tesWC-46i-7@gated-at.bofh.it> |
| In reply to | #1587819 |
On Fri, Feb 24, 2017 at 9:17 AM, Raj, Ashok <ashok.raj@intel.com> 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.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web