Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Mon, 30 Oct 2023 09:30:34 +0200
From:   Heikki Krogerus <heikki.krogerus@linux.intel.com>
To:     RD Babiera <rdbabiera@google.com>
Cc:     gregkh@linuxfoundation.org, linux@roeck-us.net,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        badhri@google.com, stable@vger.kernel.org
Subject: Re: [PATCH v1] usb: typec: tcpm: only discover modes the port
 supports svids for
Message-ID: <ZT9bBnKPMTDqy6aW@kuha.fi.intel.com>
References: <20231016232816.3355132-2-rdbabiera@google.com>
 <ZTDkIGLmjmL9HwJP@kuha.fi.intel.com>
 <CALzBnUF-EZjFEHCc4XRLdFr5yP8dCq7De4SaNif32LcL5=tMYA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CALzBnUF-EZjFEHCc4XRLdFr5yP8dCq7De4SaNif32LcL5=tMYA@mail.gmail.com>
Precedence: bulk

Hi,

I'm sorry to keep you waiting.

<snip>

> > If you need the modes to be discovered in some specific order, then we
> > need the framework to allow you to do that. So perhaps the tcpci
> > drivers should be able to supply the preferred order to the tcpm?
> >
> > But as such, unless I'm mistaken, this patch will change the logic so
> > that only the partner alt modes that the port supports get registered,
> > and that way are exposed to the user. You can't do that - right now
> > it's the only way we can inform the user about them. All partner
> > alternate modes (at least the SVIDs) must be exposed to the user one
> > way or the other, regardless does the port support them or not.
> 
> The test this patch tries to fix could just be written without consideration
> of this. My guess is that the test was designed such that the SVIDs before
> the DisplayPort SVID are unknown to the port under test so the mentality
> could have been "why should a port care about SVIDs it doesn't know
> about?"
> 
> A defense I could make for it is that the USB PD CTS doesn't test
> to see if a port under test sends Discover Modes for every SVID returned
> in a Discover SVIDs ACK, so the interpretation isn't invalid. I've seen other
> tcpm implementations handle Discover Modes this way as well.
> 
> Regardless, you're definitely right that the user should know about all
> Alt Modes/SVIDs - the port would lose SVID information without
> registering a partner altmode for it. Currently I think the approaches to pass
> this test look like:
>     1. Your suggestion - let the tcpci decide if there should be a
> discovery order.
> Alternatively, let the tcpci decide if it wants to opt into this
> patch's behavior of
> only discovering modes for known SVIDs - a strict discovery flag.
>     2. Send a Discover Mode message to known SVIDs first in the order
> they come in, and then to unknown SVIDs. The test passes and no information
> is lost, but it's unnecessary refactoring just to pass one test for
> one Alt Mode.
>     3. Don't send a Discover Mode message to unknown SVIDs, but do register
> an Alt Mode with blank info for that SVID. It passes the test without having to
> do any reordering compared to the first option and it preserves supported
> SVIDs. But, the port would lose information such as each SVID's Alt Modes
> VDO plus each SVID can support more than one Alt Mode.
> 
> Let me know if any of these approaches sound worth pursuing; I do think
> Option 1 does make more sense than the others.

I would like to hear what Guenter thinks. Guenter, do you have time to
take a look at this?

thanks,

-- 
heikki