Re: [PATCH] PCI: pciehp: Simplify Attention Button logging

[Lukas Wunner <lukas@wunner.de>]

On Mon, May 22, 2023 at 04:40:51PM -0500, Bjorn Helgaas wrote:
> --- a/drivers/pci/hotplug/pciehp_ctrl.c
> +++ b/drivers/pci/hotplug/pciehp_ctrl.c
> @@ -164,15 +164,13 @@ void pciehp_handle_button_press(struct controller *ctrl)
>  	switch (ctrl->state) {
>  	case OFF_STATE:
>  	case ON_STATE:
> -		if (ctrl->state == ON_STATE) {
> +		if (ctrl->state == ON_STATE)
>  			ctrl->state = BLINKINGOFF_STATE;
> -			ctrl_info(ctrl, "Slot(%s): Powering off due to button press\n",
> -				  slot_name(ctrl));
> -		} else {
> +		else
>  			ctrl->state = BLINKINGON_STATE;
> -			ctrl_info(ctrl, "Slot(%s) Powering on due to button press\n",
> -				  slot_name(ctrl));
> -		}
> +		ctrl_info(ctrl, "Slot(%s): Button press: powering %s\n",
> +			  slot_name(ctrl),
> +			  ctrl->state == BLINKINGON_STATE ? "on" : "off");

This results in double checks of ctrl->state (because the ctrl_info()
is pulled out of the if/else statement), so is slightly less
efficient than before.  Not a huge issue, but noting nonetheless.

I think the "powering on" (and "powering off") message is (and
has always been) confusing because it's present participle, yet
the powering on / off occurs in the future, hence "powering on in 5 sec"
or "will power on" or "power on pending" would probably be more
more correct.

Thanks,

Lukas