Re: [PATCH 7/8] usb: typec: altmodes/displayport: Make dp_altmode_notify() more generic

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Imre Deak <imre.deak@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Guenter Roeck <linux@roeck-us.net>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel@lists.freedesktop.org,
	platform-driver-x86@vger.kernel.org, linux-usb@vger.kernel.org
Subject: Re: [PATCH 7/8] usb: typec: altmodes/displayport: Make dp_altmode_notify() more generic
Date: Tue, 11 May 2021 09:55:36 +0300	[thread overview]
Message-ID: <YJoqaKps8L5QNJoU@kuha.fi.intel.com> (raw)
In-Reply-To: <20210505162415.531876-8-hdegoede@redhat.com>

On Wed, May 05, 2021 at 06:24:14PM +0200, Hans de Goede wrote:
> Make dp_altmode_notify() handle the dp->data.conf == 0 case too,
> rather then having separate code-paths for this in various places
> which call it.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/altmodes/displayport.c | 35 +++++++++---------------
>  1 file changed, 13 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index b7f094435b00..aa669b9cf70e 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -66,10 +66,17 @@ struct dp_altmode {
>  
>  static int dp_altmode_notify(struct dp_altmode *dp)
>  {
> -	u8 state = get_count_order(DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
> +	unsigned long conf;
> +	u8 state;
> +
> +	if (dp->data.conf) {
> +		state = get_count_order(DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
> +		conf = TYPEC_MODAL_STATE(state);
> +	} else {
> +		conf = TYPEC_STATE_USB;
> +	}
>  
> -	return typec_altmode_notify(dp->alt, TYPEC_MODAL_STATE(state),
> -				   &dp->data);
> +	return typec_altmode_notify(dp->alt, conf, &dp->data);
>  }
>  
>  static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
> @@ -137,21 +144,10 @@ static int dp_altmode_status_update(struct dp_altmode *dp)
>  
>  static int dp_altmode_configured(struct dp_altmode *dp)
>  {
> -	int ret;
> -
>  	sysfs_notify(&dp->alt->dev.kobj, "displayport", "configuration");
> -
> -	if (!dp->data.conf)
> -		return typec_altmode_notify(dp->alt, TYPEC_STATE_USB,
> -					    &dp->data);
> -
> -	ret = dp_altmode_notify(dp);
> -	if (ret)
> -		return ret;
> -
>  	sysfs_notify(&dp->alt->dev.kobj, "displayport", "pin_assignment");
>  
> -	return 0;
> +	return dp_altmode_notify(dp);
>  }
>  
>  static int dp_altmode_configure_vdm(struct dp_altmode *dp, u32 conf)
> @@ -172,13 +168,8 @@ static int dp_altmode_configure_vdm(struct dp_altmode *dp, u32 conf)
>  	}
>  
>  	ret = typec_altmode_vdm(dp->alt, header, &conf, 2);
> -	if (ret) {
> -		if (DP_CONF_GET_PIN_ASSIGN(dp->data.conf))
> -			dp_altmode_notify(dp);
> -		else
> -			typec_altmode_notify(dp->alt, TYPEC_STATE_USB,
> -					     &dp->data);
> -	}
> +	if (ret)
> +		dp_altmode_notify(dp);
>  
>  	return ret;
>  }
> -- 
> 2.31.1

-- 
heikki