Re: [PATCH v3 3/7] usb: typec: Check port is active before enter mode on probe

[Greg Kroah-Hartman]

On Thu, Nov 07, 2024 at 11:29:56AM -0800, Abhishek Pandit-Subedi wrote:
> Enforce the same requirement as when we attempt to activate altmode via
> sysfs (do not enter partner mode if port mode is not active). In order
> to set a default value for this field, also introduce the inactive field
> in struct typec_altmode_desc.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@xxxxxxxxxxxx>
> ---
> 
> Changes in v3:
> - Use port.active instead of introducing auto-enter field
> - Introduce inactive field to typec_altmode_desc to set default value
>   for active.
> - Always make port 'active' field writable
> 
>  drivers/usb/typec/altmodes/displayport.c | 7 +++++--
>  drivers/usb/typec/altmodes/thunderbolt.c | 6 +++++-
>  drivers/usb/typec/class.c                | 5 +++--
>  include/linux/usb/typec.h                | 2 ++
>  4 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index 3245e03d59e6..f4116e96f6a1 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -767,8 +767,11 @@ int dp_altmode_probe(struct typec_altmode *alt)
>  	if (plug)
>  		typec_altmode_set_drvdata(plug, dp);
>  
> -	dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER;
> -	schedule_work(&dp->work);
> +	/* Only attempt to enter altmode if port is active. */
> +	if (port->active) {
> +		dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER;
> +		schedule_work(&dp->work);
> +	}

What prevents active from changing right after you checked it?

> @@ -150,6 +151,7 @@ struct typec_altmode_desc {
>  	u32			vdo;
>  	/* Only used with ports */
>  	enum typec_port_data	roles;
> +	bool			inactive : 1;

A boolean bitfield?  That's not needed, please just do this properly.

thanks,

greg k-h