Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20181206030227.9507-1-kyletso@google.com> <20181206175625.GA11808@roeck-us.net>
In-Reply-To: <20181206175625.GA11808@roeck-us.net>
From:   Kyle Tso <kyletso@google.com>
Date:   Wed, 12 Dec 2018 09:38:26 +0800
Message-ID: <CAGZ6i=0TBGKGaUuHtjnv-d=UqqVNfujSian-STj3oT7HePT-JA@mail.gmail.com>
Subject: Re: [PATCH] usb: typec: tcpm: Extend the matching rules on PPS APDO selection
To:     linux@roeck-us.net
Cc:     heikki.krogerus@linux.intel.com, gregkh@linuxfoundation.org,
        Badhri Jagan Sridharan <badhri@google.com>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Dec 7, 2018 at 1:56 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Thu, Dec 06, 2018 at 11:02:27AM +0800, Kyle Tso wrote:
> > Current matching rules ensure that the voltage range of selected Source
> > Capability is entirely within the range defined in one of the Sink
> > Capabilities. This is reasonable but not practical because Sink may not
> > support wide range of voltage when sinking power while Source could
> > advertise its capabilities in raletively wider range. For example, a
> > Source PDO advertising 3.3V-11V@3A (9V Prog of Fixed Nominal Voltage)
> > will not be selected if the Sink requires 5V-12V@3A PPS power. However,
> > the Sink could work well if the requested voltage range in RDOs is
> > 5V-11V@3A.
> >
>
> Maybe a graphical description would help ?
>
> Currently accepted:
>                 |--------- source -----|
>         |----------- sink ---------------|
>
> Currently not accepted:
>
>                         |--------- source -----|
>         |----------- sink ---------------|
>
>         |--------- source -----|
>                 |----------- sink ---------------|
>
>         |--------- source -----------------|
>                 |------ sink -------|
>

Sorry for late reply. I was on vacation.
Thanks for the suggestion. I will update the commit message.

> > To improve the usability, change the matching rules to what listed
> > below:
> > a. The Source PDO is selectable if any portion of the voltage range
> >    overlaps one of the Sink PDO's voltage range.
> > b. The maximum operational voltage will be the lower one between the
> >    selected Source PDO and the matching Sink PDO.
> > c. The maximum power will be the maximum operational voltage times the
> >    maximum current defined in the selected Source PDO
> > d. Select the Source PDO with the highest maximum power
> >
> > Signed-off-by: Kyle Tso <kyletso@google.com>
>
> Makes sense to me. I am a bit concerned that it might cause odd regressions,
> though. Did you test it with a few adapters ?
>
> With the expectation that you did,
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>

Yes I have tested this patch with 3 different brands of adapters with
PPS capabilities.

thanks,
Kyle