Hi Oliver, On Mon, May 30, 2016 at 03:59:27PM +0200, Oliver Neukum wrote: > On Mon, 2016-05-30 at 16:19 +0300, Heikki Krogerus wrote: > > Hi guys, > > > > I'm attaching a diff instead of full v3. I'm not yet adding attributes > > for the reset and cable_reset. I still don't understand what is the > > case where the userspace would need to be able to tricker reset? Why > > isn't it enough for the userspace to be able to enter/exit modes? > > Oliver! Can you please comment? > > 1. Because we need error handling. > Devices crash. Cables will crash. We will get out of sync. > You never put yourself in a place where you cannot handle an > IO error. > 2. Because it is in the spec. We do not second guess the spec. > We implement it.
Error conditions and crashes are the responsibility of the USB PD stack, not userspace. In those cases the stack can not wait for a command from the userspace. So for example if a timer like NoResponseTimer times out, the stack an its state machines will have to take care of the reset quite independently. If you get out of sync with an alternate mode, you reset that specific alternate mode by exiting and re-entering it, and you do not reset the entire PD connection, port, partner or cable. The resets from userspace would be purely unsolicited. What would the cases where we would need to tricker a reset like that? I want to be careful with exposing reset to userspace. Reset in USB PD is not just an IO related thing. When you tricker a reset with USB PD, even if it's a soft reset, it may lead into hard reset, which may potentially lead into sudden voltage and current drop, which may lead into the entire system crashing. We really need to understand the cases where it would be necessary to tricker a reset from userspace. Right now I don't see any. > > I also made a change to the partner devices so that they always have > > the port as their parent. That will have an effect on the location > > where the partner device is exposed in sysfs (so now always under the > > port). And because of that, I would like to get an OK from you guys. > > It is not very important. i am fine with it. > > > I'm a bit concerned about the current API for adding the alternate > > modes. Since we are passing the parent for an alternate mode as > > struct device, it makes it possible for the caller to place it into > > some wrong place. But I guess we can change the API even later. > > > > We also need to decide how the alternate modes a port support are > > exposed to the userspace. Do we just assume the port drivers will > > create them as devices under the port device itself, just like > > alternate modes of partners and cable plugs are exposed under the > > partners and cable plugs? That works for me, but again, the class does > > not have any effect on that, and it will be just a guideline. Maybe > > we can add some kind of helpers and force the port drivers to use > > them. > > What are the alternatives? Can we make a group for them under the port device somehow? Like the supported_alternate_modes I proposed. I guess it's not possible to add devices to a specific group in sysfs. And would it even be useful. Thanks, -- heikki
