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

[Lukas Wunner <lukas@wunner.de>]

On Tue, May 23, 2023 at 10:40:39AM -0500, Bjorn Helgaas wrote:
> I was trying to make it OFF/ON case parallel to the BLINKING case that
> only has one ctrl_info(), but I think that makes it a little harder to
> read in addition to being less efficient.
> 
> And the language is definitely confusing.   How about this?

Perfect, feel free to add my

Reviewed-by: Lukas Wunner <lukas@wunner.de>

to commit 6d433b9ddfda on pci/hotplug.

Thanks a lot!

Lukas

> @@ -166,11 +166,11 @@ void pciehp_handle_button_press(struct controller *ctrl)
>  	case ON_STATE:
>  		if (ctrl->state == ON_STATE) {
>  			ctrl->state = BLINKINGOFF_STATE;
> -			ctrl_info(ctrl, "Slot(%s): Powering off due to button press\n",
> +			ctrl_info(ctrl, "Slot(%s): Button press: will power off in 5 sec\n",
>  				  slot_name(ctrl));
>  		} else {
>  			ctrl->state = BLINKINGON_STATE;
> -			ctrl_info(ctrl, "Slot(%s) Powering on due to button press\n",
> +			ctrl_info(ctrl, "Slot(%s): Button press: will power on in 5 sec\n",
>  				  slot_name(ctrl));
>  		}
>  		/* blink power indicator and turn off attention */
> @@ -185,22 +185,23 @@ void pciehp_handle_button_press(struct controller *ctrl)
>  		 * press the attention again before the 5 sec. limit
>  		 * expires to cancel hot-add or hot-remove
>  		 */
> -		ctrl_info(ctrl, "Slot(%s): Button cancel\n", slot_name(ctrl));
>  		cancel_delayed_work(&ctrl->button_work);
>  		if (ctrl->state == BLINKINGOFF_STATE) {
>  			ctrl->state = ON_STATE;
>  			pciehp_set_indicators(ctrl, PCI_EXP_SLTCTL_PWR_IND_ON,
>  					      PCI_EXP_SLTCTL_ATTN_IND_OFF);
> +			ctrl_info(ctrl, "Slot(%s): Button press: canceling request to power off\n",
> +				  slot_name(ctrl));
>  		} else {
>  			ctrl->state = OFF_STATE;
>  			pciehp_set_indicators(ctrl, PCI_EXP_SLTCTL_PWR_IND_OFF,
>  					      PCI_EXP_SLTCTL_ATTN_IND_OFF);
> +			ctrl_info(ctrl, "Slot(%s): Button press: canceling request to power on\n",
> +				  slot_name(ctrl));
>  		}
> -		ctrl_info(ctrl, "Slot(%s): Action canceled due to button press\n",
> -			  slot_name(ctrl));
>  		break;