Hi Jameson, On Thu, Mar 14, 2024 at 11:55:54PM +0000, Jameson Thies wrote: > Check the UCSI_CAP_GET_PD_MESSAGE bit before sending GET_PD_MESSAGE to > discover partner and cable identity, check UCSI_CAP_CABLE_DETAILS before > sending GET_CABLE_PROPERTY to discover the cable and check > UCSI_CAP_ALT_MODE_DETAILS before registering the a cable plug. > > Suggested-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> > Signed-off-by: Jameson Thies <jthies@xxxxxxxxxx> Since Neil pointed out that this error appeared starting in 38ca416597b0, I think it would be appropriate to tag this commit with a Fixes: tag. See here on instructions how to describe a Fixes: https://www.kernel.org/doc/html/v4.10/process/submitting-patches.html#describe-changes > --- > Confirmed a device which supports GET_PD_MESSAGE, GET_CABLE_PROPERTY and > GET_ALTERNATE_MODES still requested identity and cable information. > > This moves 8 bits from "reserved_1" to "features" in the ucsi_capability > struct. Really, this should be 24 bits to reflect the field size in UCSI. > But, as of UCSI 3.0, this will not overflow becasue the bmOptionalFeatures > description only defines 14 bits. Are you sure you wanted to include this information below the --- ? This won't be incorporated into the commit message when this is merged. Thanks, Benson > > drivers/usb/typec/ucsi/ucsi.c | 34 +++++++++++++++++++++------------- > drivers/usb/typec/ucsi/ucsi.h | 5 +++-- > 2 files changed, 24 insertions(+), 15 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index cf52cb34d2859..958dc82989b60 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -1133,17 +1133,21 @@ static int ucsi_check_cable(struct ucsi_connector *con) > if (ret < 0) > return ret; > > - ret = ucsi_get_cable_identity(con); > - if (ret < 0) > - return ret; > + if (con->ucsi->cap.features & UCSI_CAP_GET_PD_MESSAGE) { > + ret = ucsi_get_cable_identity(con); > + if (ret < 0) > + return ret; > + } > > - ret = ucsi_register_plug(con); > - if (ret < 0) > - return ret; > + if (con->ucsi->cap.features & UCSI_CAP_ALT_MODE_DETAILS) { > + ret = ucsi_register_plug(con); > + if (ret < 0) > + return ret; > > - ret = ucsi_register_altmodes(con, UCSI_RECIPIENT_SOP_P); > - if (ret < 0) > - return ret; > + ret = ucsi_register_altmodes(con, UCSI_RECIPIENT_SOP_P); > + if (ret < 0) > + return ret; > + } > > return 0; > } > @@ -1189,8 +1193,10 @@ static void ucsi_handle_connector_change(struct work_struct *work) > ucsi_register_partner(con); > ucsi_partner_task(con, ucsi_check_connection, 1, HZ); > ucsi_partner_task(con, ucsi_check_connector_capability, 1, HZ); > - ucsi_partner_task(con, ucsi_get_partner_identity, 1, HZ); > - ucsi_partner_task(con, ucsi_check_cable, 1, HZ); > + if (con->ucsi->cap.features & UCSI_CAP_GET_PD_MESSAGE) > + ucsi_partner_task(con, ucsi_get_partner_identity, 1, HZ); > + if (con->ucsi->cap.features & UCSI_CAP_CABLE_DETAILS) > + ucsi_partner_task(con, ucsi_check_cable, 1, HZ); > > if (UCSI_CONSTAT_PWR_OPMODE(con->status.flags) == > UCSI_CONSTAT_PWR_OPMODE_PD) > @@ -1589,8 +1595,10 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con) > ucsi_register_partner(con); > ucsi_pwr_opmode_change(con); > ucsi_port_psy_changed(con); > - ucsi_get_partner_identity(con); > - ucsi_check_cable(con); > + if (con->ucsi->cap.features & UCSI_CAP_GET_PD_MESSAGE) > + ucsi_get_partner_identity(con); > + if (con->ucsi->cap.features & UCSI_CAP_CABLE_DETAILS) > + ucsi_check_cable(con); > } > > /* Only notify USB controller if partner supports USB data */ > diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h > index 32daf5f586505..0e7c92eb1b227 100644 > --- a/drivers/usb/typec/ucsi/ucsi.h > +++ b/drivers/usb/typec/ucsi/ucsi.h > @@ -206,7 +206,7 @@ struct ucsi_capability { > #define UCSI_CAP_ATTR_POWER_OTHER BIT(10) > #define UCSI_CAP_ATTR_POWER_VBUS BIT(14) > u8 num_connectors; > - u8 features; > + u16 features; > #define UCSI_CAP_SET_UOM BIT(0) > #define UCSI_CAP_SET_PDM BIT(1) > #define UCSI_CAP_ALT_MODE_DETAILS BIT(2) > @@ -215,7 +215,8 @@ struct ucsi_capability { > #define UCSI_CAP_CABLE_DETAILS BIT(5) > #define UCSI_CAP_EXT_SUPPLY_NOTIFICATIONS BIT(6) > #define UCSI_CAP_PD_RESET BIT(7) > - u16 reserved_1; > +#define UCSI_CAP_GET_PD_MESSAGE BIT(8) > + u8 reserved_1; > u8 num_alt_modes; > u8 reserved_2; > u16 bc_version; > -- > 2.44.0.291.gc1ea87d7ee-goog >
Attachment:
signature.asc
Description: PGP signature
