Re: [PATCH 1/2] platform/chrome: cros_ec_typec: Enforce priority-based mode selection

[Andrei Kuchynski]

On Fri, Jan 30, 2026 at 4:27 AM Tzung-Bi Shih <tzungbi@xxxxxxxxxx> wrote:
>
> On Thu, Jan 29, 2026 at 01:19:27PM +0000, Andrei Kuchynski wrote:
> > The driver sets mode_selection bit for each Alternate mode, thereby
> > preventing individual altmode drivers from activating their respective
> > modes.
> > Once the registration of all Alternate Modes is complete, the driver
> > verifies if USB4 mode is supported by both the port and the partner and
> > activates USB4 mode. In cases where USB4 is not supported, the driver
> > invokes typec_mode_selection_start to initiate the mode selection process
> > based on mode priorities.
> > The driver communicates the current Type-C mode to the mode selection
> > process via typec_altmode_state_update.
>
> I find the message doesn't help me too much for understanding the patch.
> Can you rephrase them and assuming readers might not have too much context?
>

This patch is part of a larger series introducing the USB Type-C Mode
Selection feature. The goal is to allow the kernel to manage and negotiate
specific functional modes (like DisplayPort or Thunderbolt).
https://lore.kernel.org/all/20260119131824.2529334-1-akuchynski@xxxxxxxxxxxx/
Apologies, I should have included the cover letter; it can be found at the
link provided above. The series implements the mode selection feature and
UCSI support.
These specific patches provide the necessary support for cros_ec_typec.

> > ---
> >  drivers/platform/chrome/cros_ec_typec.c      | 39 +++++++++++++++++++-
> >  drivers/platform/chrome/cros_typec_altmode.c |  8 +++-
> >  2 files changed, 44 insertions(+), 3 deletions(-)
>
> The patch uses some exported symbols which aren't yet in upstream
> (drivers/usb/typec/mode_selection.c).  Ideally, you should mention that in
> either cover letter or note after "---".
>

I should have included that detail. The mode selection feature is now
integrated into the core from the main repository.

> > +/* Delay between mode entry/exit attempts, ms */
> > +static const unsigned int mode_selection_delay = 1000;
> > +/* Timeout for a mode entry attempt, ms */
> > +static const unsigned int mode_selection_timeout = 4000;
>
> The commit message probably needs some description about why these values.

These values match the UCSI implementation. The timeouts provide the EC
enough time to complete previous operations; they aren't mathematically
derived but based on test experience. We've tested them for six months and
haven't seen any reason to change.

Thank you for the review!
Andrei