All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Andrei Kuchynski <akuchynski@chromium.org>
Cc: Benson Leung <bleung@chromium.org>,
	Jameson Thies <jthies@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] usb: typec: Add helper to check cable altmode support
Date: Tue, 23 Jun 2026 11:30:19 +0300	[thread overview]
Message-ID: <ajpEG7ryQi8im7U-@kuha> (raw)
In-Reply-To: <CAMMMRMf0kLTJeVztQwDsz8Zm4wBXX52agNpvnKttkFqupQX=4w@mail.gmail.com>

On Mon, Jun 22, 2026 at 02:03:48PM +0200, Andrei Kuchynski wrote:
> Hi Heikki,
> 
> On Mon, Jun 22, 2026 at 12:18 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > Hi Andrei,
> >
> > > +bool typec_cable_altmode_unsupported(struct typec_altmode *alt)
> > > +{
> > > +     return unsupported;
> >
> > So if typec_cable_get() doesn't return a cable, this function will now
> > always return false - i.e. the cable is supported? Is that intentional?
> >
> 
> Yes, this is intentional.
> The UCSI GET_CABLE_PROPERTY command is optional. typec_register_cable()
> function can also be called without the desc->identity field.
> Failing to provide cable information shouldn't be a reason to reject the
> alternate mode.
> Also, the cable can be registered after the partner's altmodes. We
> encounter this scenario with the cros_ec_typec driver. I'm planning to fix
> it, but for now, this approach will preserve the previous behavior.
> 
> Therefore, the idea is to reject the altmode only if we know the cable
> doesn't support it. That's why the function is called "unsupported".
> It returns true if a limitation is detected. Otherwise, the function
> returns false.

One day and I'm completely confused again :). I opened the code for
myself (not compiled) to get the idea again. Please consider that, or
something like it - the important part for me is the enum. The enum
does not cost that many lines, but it does make the idea more clear,
at least for me.


enum typec_cable_altmode_support {
        CABLE_SUPPORT_UNKNOWN,
        CABLE_SUPPORTED,
        CABLE_NOT_SUPPORTED,
};

static enum typec_cable_altmode_support
typec_cable_check_altmode_support(struct typec_cable *cable,
                                  struct typec_altmode *alt)
{
        struct typec_altmode *plug;
        const u32 speed;

        plug = typec_altmode_get_plug(alt);
        if (plug) {
                typec_altmode_put_plug(plug);
                return CABLE_SUPPORTED;
        }

        /* Maybe UCSI, or the something else, does no supply the identity */
        if (!cable->identity)
                return ALTMODE_SUPPORT_UNKNOWN;

        /* Non-e-marked cable */
        if (!cable->identity->id_header)
                return CABLE_NOT_SUPPORTED;

        speed = VDO_TYPEC_CABLE_SPEED(cable->identity->vdo[0]);

        switch (PD_IDH_PTYPE(cable->identity->id_header)) {
        case IDH_PTYPE_PCABLE:
                if (speed == CABLE_USB2_ONLY)
                        return CABLE_NOT_SUPPORTED;
                break;
        case IDH_PTYPE_ACABLE:
                /*
                 * Active cables must establish an SOP' communication
                 * node. Since that check failed at the beginning of
                 * this function, this active cable does not support
                 * this specific altmode.
                 */
                return CABLE_NOT_SUPPORTED;
        default:
                break;
        }

        return CABLE_SUPPORT_UNKNOWN;
}

bool typec_altmode_cable_not_supported(struct typec_altmode *alt)
{
        enum typec_cable_altmode_support support = CABLE_SUPPORT_UNKNOWN;
        struct typec_cable *cable;

        cable = typec_cable_get(typec_altmode2port(alt));
        if (cable) {
                support = typec_cable_check_altmode_support(cable, alt);
                typec_cable_put(cable);
        }

        return support == CABLE_NOT_SUPPORTED;
}


thanks,

-- 
heikki

  reply	other threads:[~2026-06-23  8:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-22  9:39 [PATCH v2 0/3] Restrict alternate modes based on cable capabilities Andrei Kuchynski
2026-06-22  9:39 ` [PATCH v2 1/3] usb: typec: Add helper to check cable altmode support Andrei Kuchynski
2026-06-22 10:18   ` Heikki Krogerus
2026-06-22 12:03     ` Andrei Kuchynski
2026-06-23  8:30       ` Heikki Krogerus [this message]
2026-06-22  9:39 ` [PATCH v2 2/3] usb: typec: thunderbolt: Check " Andrei Kuchynski
2026-06-22  9:39 ` [PATCH v2 3/3] usb: typec: displayport: " Andrei Kuchynski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajpEG7ryQi8im7U-@kuha \
    --to=heikki.krogerus@linux.intel.com \
    --cc=akuchynski@chromium.org \
    --cc=bleung@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jthies@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.