linux-kernel - Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPTae5+SdP7qhoB1S9JLC8Wv3JbL-ANZUj1esQ1-wVzkW4Zdow@mail.gmail.com>
Date:   Wed, 17 May 2017 11:05:58 -0700
From:   Badhri Jagan Sridharan <badhri@...gle.com>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     Guenter Roeck <linux@...ck-us.net>,
        Oliver Neukum <oneukum@...e.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        USB <linux-usb@...r.kernel.org>
Subject: Re: [PATCH] usb: typec: Defer checking of valid power role swap to
 low level drivers

For non-pd case, Should I instead say that the low level driver might optionally
choose to perform a best case effort of performing a role swap by disconnect
and reconnect ?

Does the following description look better ?

Description:
                The supported power roles. This attribute can be used to request
                power role swap. Swapping is supported as synchronous
operation, so
                write(2) to the attribute will not return until the operation
                has finished. The attribute is notified about role changes so
                that poll(2) on the attribute wakes up. Change on the role will
                also generate uevent KOBJ_CHANGE. The current role is show in
                brackets, for example "[source] sink" when in source mode.
                When both the port and the port-partner supports USB Power
                Delivery, the PR_SWAP command is used to perform the role swap.
                For non-pd case, the low level driver might optionally perform
                a best effort approach to swap port roles by forcing
                a disconnect and reconnect.

Thanks,
Badhri

On Wed, May 17, 2017 at 6:51 AM, Heikki Krogerus
<heikki.krogerus@...ux.intel.com> wrote:
>
> On Wed, May 17, 2017 at 06:02:47AM -0700, Guenter Roeck wrote:
> > On 05/17/2017 05:38 AM, Heikki Krogerus wrote:
> > > Hi guys,
> > >
> > > On Wed, May 17, 2017 at 02:36:44AM -0700, Guenter Roeck wrote:
> > > > On 05/17/2017 12:34 AM, Oliver Neukum wrote:
> > > > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
> > > > > Sridharan:
> > > > >
> > > > > Hi,
> > > > >
> > > > > > "Two independent set of mechanisms are defined to allow a USB Type-C
> > > > > > DRP to functionally swap power and data roles. When USB PD is
> > > > > > supported, power and data role swapping is performed as a subsequent
> > > > > > step following the initial connection process. For non-PD implementations,
> > > > > > power/data role swapping can optionally be dealt with as part of the initial
> > > > > > connection process."
> > > > >
> > > > > Well, as I read it, without PD once a connection is established, you
> > > > > are stuck with your role. So it seems to me that blocking a later
> > > > > attempt to change it makes sense.
> > > > >
> > > >
> > > > That seems to be a harsh and not very user friendly reading of the specification.
> > > >
> > > > I would argue that the user doesn't care if the partner supports PD or not
> > > > when selecting a role, and I would prefer to provide an implementation which is
> > > > as user friendly as possible.
> > >
> > > I agree. But I have to point out that at least with UCSI, the role
> > > swapping is usually not possible without USB PD connection.
> > >
> > > So what I'm trying to say is that we can't really promise that role
> > > swapping will ever be possible in all cases without PD, which may mean
> > > different behavior depending on the platform. I don't know if that is
> > > a huge problem.
> > >
> > > How predictable do we want this interface to function with role
> > > swapping?
> > >
> >
> > We can only do as good as we can, but we should not preclude it either.
> > PR_SWAP and other swap operations fail a lot in practice with many dongles,
> > just because the PD protocol implementation isn't always stable. So even that
> > won't be predictable for some time to come.
> >
> > As Badhri pointed out earlier, at least some low cost devices won't support PD.
> > Since we are talking about high volume products, we _have_ to make it as
> > user friendly as we can, if for nothing else to reduce the number of customer
> > service calls, repeated bug filings, and, last but not least, to avoid bad press.
>
> OK. So I think we just need to explain in the documentation that there
> are no guarantees with role swapping.
>
>
> Thanks,
>
> --
> heikki