Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1436417

Re: [PATCHv4 1/2] usb: USB Type-C connector class

Show all headers | View raw

Hi Guenter,

On Sun, Jul 03, 2016 at 02:28:44PM -0700, Guenter Roeck wrote:
> On 07/03/2016 12:38 PM, Heikki Krogerus wrote:
> > On Fri, Jul 01, 2016 at 07:33:12AM -0700, Guenter Roeck wrote:
> > > On Fri, Jul 01, 2016 at 03:05:35PM +0300, Heikki Krogerus wrote:
> > > > I've updated my github branch with a commit where both of these issues
> > > > should be fixed. Can you give it a try?
> > > > 
> > > 
> > > This is going to be very difficult. So far, calls such as
> > > typec_set_data_role() were independent (asynchronous) of role change
> > > requests through sysfs, meaning they happened whenever lower level
> > > confirmed that a role change was complete, for whatever reason. Now
> > > I have to check if a role change request through the class code
> > > is pending and not call typec_set_data_role() and friends in that case.
> > 
> > I'm sorry about the misunderstanding.
> > 
> > What you want is basically that we only support non-blocking mode with
> > this interface, and we can't do that.
> > 
> 
> No, that is not what I said or asked for. The problems I am seeing are due
> to locking in the class code. "Asynchronous" above referred to the state
> machine vs. role change requests by the class code, which operates independent
> of each other in my code. Set requests from the class code still wait for
> the state machine to complete.
> 
> The problem is that the state machine now needs to know if the class code
> has a set role request pending, because in that case it can no longer report
> role changes directly to the class code. This includes role changes unrelated
> to the actual set role request. That code is now much more complicated, especially
> since each callback into the typec code is handled differently. For example,
> typec_disconnect() does not require a lock (unless I missed it), but many
> other functions do.

It seems that I have misunderstood your whole point. I'm really sorry.
I was in a hurry to start my vacation.

So let's just fix the locking.

<snip>

> > > > I've also removed the supports_usb_power_delivery and id_header_vdo
> > > > attributes from partners and cables. We really have to put all the USB
> > > > PD specific attributes to its own group, and that group can't be in
> > > > control of this class. We will simply not always have access to the
> > > > kind of USB PD specific details you want to show, for example details
> > > > that you get from discover identity command, even when USB PD is fully
> > > > supported. For example on systems that use UCSI.
> > > > 
> > > 
> > > I think we should have a single unified ABI, independent of the underlying
> > > driver, that informs the user about the partner device. I really don't
> > > quite understand why the class code should not be able to report what device
> > > it is connected to (while at the same time being able to report the alternate
> > > modes it supports).
> > 
> > OK, let's plan this more then. Maybe we can make for example a layer
> > that creates the groups for the PD specific details to the class?
> > 
> > The problem is still that we can only provide results from for example
> > request identity command reliably when the PD protocol layer is
> > completely inside kernel, and that is not always the case. So we
> > really need to group those details in its own group, and that group
> > will basically indicate is the PD stack inside the kernel or not.
> > 
> > We should not forget also that the userspace can never rely on those
> > details because of the fact that they simply will not always be
> > available.
> > 
> On the other side, not being able to rely on a well defined ABI makes the
> ABI much less useful.

What do we do about this?


Thanks,

-- 
heikki

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCHv4 0/2] USB Type-C Connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-29 15:40 +0200
  [PATCHv4 2/2] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-29 15:40 +0200
  Re: [PATCHv4 1/2] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-06-30 19:20 +0200
    Re: [PATCHv4 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-07-01 09:40 +0200
      Re: [PATCHv4 1/2] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-07-01 16:50 +0200
  Re: [PATCHv4 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-07-01 09:20 +0200
    Re: [PATCHv4 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-07-01 14:10 +0200
      Re: [PATCHv4 1/2] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-07-01 16:40 +0200
        Re: [PATCHv4 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-07-03 21:40 +0200
          Re: [PATCHv4 1/2] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-07-03 23:30 +0200
            Re: [PATCHv4 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-07-04 19:20 +0200
              Re: [PATCHv4 1/2] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-07-04 19:50 +0200
                Re: [PATCHv4 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-07-05 20:50 +0200

csiph-web