lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 9 Apr 2024 11:05:09 +0300
From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	linux-usb@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/5] usb: typec: ucsi: make it orientation-aware

Hi Dmitry,

On Mon, Apr 08, 2024 at 07:30:52AM +0300, Dmitry Baryshkov wrote:
> The UCSI 1.0 is not orientation aware. Only UCSI 2.0 has added
> orientation status to GET_CONNECTOR_STATUS data. However the glue code
> can be able to detect cable orientation on its own (and report it via
> corresponding typec API). Add a flag to let UCSI mark registered ports
> as orientation aware.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> ---
>  drivers/usb/typec/ucsi/ucsi.c | 2 ++
>  drivers/usb/typec/ucsi/ucsi.h | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index 7ad544c968e4..6f5adc335980 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -1551,6 +1551,8 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
>  	cap->svdm_version = SVDM_VER_2_0;
>  	cap->prefer_role = TYPEC_NO_PREFERRED_ROLE;
>  
> +	cap->orientation_aware = !!(ucsi->quirks & UCSI_ORIENTATION_AWARE);
> +
>  	if (con->cap.op_mode & UCSI_CONCAP_OPMODE_AUDIO_ACCESSORY)
>  		*accessory++ = TYPEC_ACCESSORY_AUDIO;
>  	if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DEBUG_ACCESSORY)
> diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
> index 6599fbd09bee..e92be45e4c1c 100644
> --- a/drivers/usb/typec/ucsi/ucsi.h
> +++ b/drivers/usb/typec/ucsi/ucsi.h
> @@ -410,6 +410,7 @@ struct ucsi {
>  	unsigned long quirks;
>  #define UCSI_NO_PARTNER_PDOS	BIT(0)	/* Don't read partner's PDOs */
>  #define UCSI_DELAY_DEVICE_PDOS	BIT(1)	/* Reading PDOs fails until the parter is in PD mode */
> +#define UCSI_ORIENTATION_AWARE	BIT(2)	/* UCSI is orientation aware */

You are not using that flag anywhere in this series. But why would
orientation need a "quirk" in the first place?

I'm not sure where you are going with this, but please try to avoid
the quirk flags in general in this driver rather than considering them
as the first way of solving things. Use them only as the last resort.

I don't want this driver to end up like xhci and some other drivers,
where refactoring is almost impossible because every place is full of
conditions checking the quirks, and where in worst case a quirk meant
to solve a problem on one hardware causes problems on another.

thanks,

-- 
heikki

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ