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

[Lukas Wunner <lukas@wunner.de>]

On Fri, May 19, 2023 at 03:55:35PM -0500, Bjorn Helgaas wrote:
> On Wed, May 17, 2023 at 04:02:01PM -0500, Bjorn Helgaas wrote:
> > On Fri, May 12, 2023 at 10:15:18AM +0800, Rongguang Wei wrote:
> > > From: Rongguang Wei <weirongguang@kylinos.cn>
> > > 
> > > pciehp's behavior is incorrect if the Attention Button is pressed
> > > on an unoccupied slot.
> > > 
> > > When a Presence Detect Changed event has occurred, the slot status
> > > in either BLINKINGOFF_STATE or OFF_STATE, turn it off unconditionally.
> 
> Was this supposed to say "BLINKINGOFF_STATE or ON_STATE"
> (not "OFF_STATE")?

Yes I think you're right.


> I propose the following commit log:
[...]
>   pciehp_queue_pushbutton_work() synthesizes a Presence Detect Changed
>   event, and pciehp_handle_presence_or_link_change() exits when it
>   finds the slot empty, leaving the slot in BLINKINGON_STATE with the
>   Power Indicator blinking.
> 
>   To fix the indefinitely blinking Power Indicator, change
>   pciehp_handle_presence_or_link_change() to put the empty slot back
>   in OFF_STATE and turn off the Power Indicator before exiting.

The indefinitely blinking Power Indicator is only one half of the problem.
The other half is that the next button press doesn't result in slot
bringup, even if the slot is occupied and the 5 second timeout has
elapsed.  Suggested wording, feel free to rephrase as you see fit:

  Because the slot was previously left in BLINKINGON_STATE, the next
  button press was interpreted as a "button cancel" event, even if the
  slot was occupied upon that next button press:  pciehp stopped blinking
  and did not perform another slot bringup attempt.

  By putting the slot in OFF_STATE, such user-unfriendly behavior is
  avoided:  Instead, the next button press will result in the slot
  starting to blink again and another bringup attempt after 5 seconds.

Thanks,

Lukas