| Message ID | 20241206153813.v4.4.I083bf9188947be8cb7460211cfdf3233370a28f6@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Thunderbolt and DP altmode support for cros-ec-typec | expand |
Quoting Abhishek Pandit-Subedi (2024-12-06 15:38:15) > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c > index c7781aea0b88..e3eabe5e42ac 100644 > --- a/drivers/platform/chrome/cros_ec_typec.c > +++ b/drivers/platform/chrome/cros_ec_typec.c > @@ -676,6 +677,16 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num, > port->mux_flags); > } > > + /* Iterate all partner alt-modes and set the active alternate mode. */ > + list_for_each_entry_safe(node, n, &port->partner_mode_list, list) { This can just be list_for_each_entry() because the list isn't changing during iteration. > + if (port->state.alt != NULL && > + node->amode->svid == port->state.alt->svid) { > + typec_altmode_update_active(node->amode, true); > + } else { > + typec_altmode_update_active(node->amode, false); > + } It could also be shorter: list_for_each_entry(node, &port->partner_mode_list, list) { typec_altmode_update_active(node->amode, port->state.alt && node->amode->svid == port->state.alt->svid); } As far as I can tell, cros_typec_configure_mux() is called when the HPD state changes. We'll iterate through here unnecessarily in that case. Can that be avoided somehow? > + > mux_ack: > if (!typec->needs_mux_ack) > return ret;
On Tue, Dec 10, 2024 at 3:32 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Abhishek Pandit-Subedi (2024-12-06 15:38:15) > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c > > index c7781aea0b88..e3eabe5e42ac 100644 > > --- a/drivers/platform/chrome/cros_ec_typec.c > > +++ b/drivers/platform/chrome/cros_ec_typec.c > > @@ -676,6 +677,16 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num, > > port->mux_flags); > > } > > > > + /* Iterate all partner alt-modes and set the active alternate mode. */ > > + list_for_each_entry_safe(node, n, &port->partner_mode_list, list) { > > This can just be list_for_each_entry() because the list isn't changing > during iteration. Done > > > + if (port->state.alt != NULL && > > + node->amode->svid == port->state.alt->svid) { > > + typec_altmode_update_active(node->amode, true); > > + } else { > > + typec_altmode_update_active(node->amode, false); > > + } > > It could also be shorter: > > list_for_each_entry(node, &port->partner_mode_list, list) { > typec_altmode_update_active(node->amode, > port->state.alt && node->amode->svid == port->state.alt->svid); > } Done > > As far as I can tell, cros_typec_configure_mux() is called when the HPD > state changes. We'll iterate through here unnecessarily in that case. > Can that be avoided somehow? Writing the same value to `typec_altmode_update_active` is a no-op. I'd prefer to leave this code as-is since it's quite simple (having to determine HPD vs non-HPD, whether DP is currently active, etc. is going to be more work than it saves from not calling this loop). > > > + > > mux_ack: > > if (!typec->needs_mux_ack) > > return ret;
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c index c7781aea0b88..e3eabe5e42ac 100644 --- a/drivers/platform/chrome/cros_ec_typec.c +++ b/drivers/platform/chrome/cros_ec_typec.c @@ -618,6 +618,7 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num, }; struct ec_params_usb_pd_mux_ack mux_ack; enum typec_orientation orientation; + struct cros_typec_altmode_node *node, *n; int ret; ret = cros_ec_cmd(typec->ec, 0, EC_CMD_USB_PD_MUX_INFO, @@ -676,6 +677,16 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num, port->mux_flags); } + /* Iterate all partner alt-modes and set the active alternate mode. */ + list_for_each_entry_safe(node, n, &port->partner_mode_list, list) { + if (port->state.alt != NULL && + node->amode->svid == port->state.alt->svid) { + typec_altmode_update_active(node->amode, true); + } else { + typec_altmode_update_active(node->amode, false); + } + } + mux_ack: if (!typec->needs_mux_ack) return ret;
Mux configuration is often the final piece of mode entry and can be used to determine whether a partner altmode is active. When mux configuration is done, use the active port altmode's SVID to set the partner active field for all partner alt modes. Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> --- (no changes since v1) drivers/platform/chrome/cros_ec_typec.c | 11 +++++++++++ 1 file changed, 11 insertions(+)