On Tue, May 31, 2016 at 10:48:29AM +0200, Oliver Neukum wrote: > On Tue, 2016-05-31 at 11:31 +0300, Heikki Krogerus wrote: > > 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 > > Those are not exclusive conditions. > > > 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. > > Yes. But somebody needs to handle high level errors. > > > 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. > > That would be the first step. If that doesn't work you will at that > point either give up or use the next largest hammer. > In principle you could do that in kernel space, but that implies > that the kernel can detect all failures. That is unlikely. Any PD communication failures the kernel has to be able to detect, so I guess you mean failures with the alternate modes themselves, right? In that case, surely exiting the mode is enough to "reset" it? When it is re-entered, it has to be completely re-configured in any case. I don't see how resetting the whole port or cable would guarantee that a mode would become any more functional in case of failures? It will however make also the other active modes to de-activate even if they are functioning fine. > > 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. > > User space can call reboot. Actually that does not help. > Reset is an operation that is intended for error handling. > If all else fails, we will need to use it. > Its bad consequences apply whether you trigger this from kernel or > user space. In fact, an operation that may potentially crash the system > should involve user space. > > > > > 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. > > Please explain. It is not clear to me what you are proposing here. So is it possible to have a folder in sysfs for the alternate modes a port supports? So instead of: /sysfs/class/type-c/usbc0/svid:xxx1/ /sysfs/class/type-c/usbc0/svid:xxx2/ /sysfs/class/type-c/usbc0/svid:xxx3/ ... We would have something like: /sysfs/class/type-c/usbc0/supported_alternate_modes/svid:xxx1/ /sysfs/class/type-c/usbc0/supported_alternate_modes/svid:xxx2/ /sysfs/class/type-c/usbc0/supported_alternate_modes/svid:xxx3/ ... Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html