Re: [PATCH v1] PCI: pciehp: Fix the slot in BLINKINGON_STATE when Presence Detect Changed event occurred

[Lukas Wunner <lukas@wunner.de>]

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