Hi
On 4/17/23 3:11 PM, Lukas Wunner wrote:
> On Mon, Apr 17, 2023 at 11:04:31AM +0800, Rongguang Wei wrote:
>> On 4/16/23 11:18 PM, Lukas Wunner wrote:
>>> On Mon, Apr 03, 2023 at 01:46:19PM +0800, Rongguang Wei wrote:
>>>> --- a/drivers/pci/hotplug/pciehp_ctrl.c
>>>> +++ b/drivers/pci/hotplug/pciehp_ctrl.c
>>>> @@ -232,6 +232,7 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events)
>>>> */
>>>> mutex_lock(&ctrl->state_lock);
>>>> switch (ctrl->state) {
>>>> + case BLINKINGON_STATE:
>>>> case BLINKINGOFF_STATE:
>>>> cancel_delayed_work(&ctrl->button_work);
>>>> fallthrough;
>>>
>>> This solution has the disadvantage that a gratuitous "Card not present"
>>> message is emitted even if the slot is occupied.
>>
>> I think when the "Card not present" is emitted, it may not consider the
>> slot status from the beginning.
>
> I don't quite follow. With the change you're proposing, if the Attention
> Button has been pressed and there's a card in the slot, after five seconds
> you'll emit an erroneous "Card not present" message. Erroneous because
> there's a card in the slot.
>
>
>> If the slot is in ON_STATE and is occupied, turn the slot off and then
>> back on. The message is also emitted at first.
>
> That's intentional. If the slot is occupied and a Presence Detect Changed
> event was received, it means the card in the slot may be a different one.
> So the "Card not present" message relates to the card that was
> *previously* in the slot.
>
> If the slot is still (or again) occupied, we'll then try to bring it up
> and that will lead to a subsequent "Card present" message.
>
>
>> Maybe I can rework to add like this to prevent the gratuitous message:
>
> Could you just test if the 1-line change I suggested in my previous e-mail
> fixes the issue for you?
>
> Thanks,
>
> Lukas
>
I test the 1-line change and it make the test failed. The dmesg like this:
pcieport 0000:00:01.5: pciehp: Slot(0-5): Attention button pressed
pcieport 0000:00:01.5: pciehp: Slot(0-5): Powering off due to button press
pcieport 0000:00:01.5: pciehp: Slot(0-5): Attention button pressed
pcieport 0000:00:01.5: pciehp: Slot(0-5): Ignoring invalid state 0x4
all ABP event are print "Ignoring invalid state 0x4".
I was add 1 line to disable slot and it works. This looks like what was done
before.
diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
index 529c34808440..462a61fc7313 100644
--- a/drivers/pci/hotplug/pciehp_ctrl.c
+++ b/drivers/pci/hotplug/pciehp_ctrl.c
@@ -256,7 +256,9 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events)
present = pciehp_card_present(ctrl);
link_active = pciehp_check_link_active(ctrl);
if (present <= 0 && link_active <= 0) {
+ ctrl->state = POWEROFF_STATE;
mutex_unlock(&ctrl->state_lock);
+ pciehp_disable_slot(ctrl, SURPRISE_REMOVAL);
return;
}
