From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757689AbcFAIfP (ORCPT ); Wed, 1 Jun 2016 04:35:15 -0400 Received: from mx2.suse.de ([195.135.220.15]:55643 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757656AbcFAIfL (ORCPT ); Wed, 1 Jun 2016 04:35:11 -0400 Message-ID: <1464769892.4051.2.camel@suse.com> Subject: Re: [RFC PATCHv2] usb: USB Type-C Connector Class From: Oliver Neukum To: Heikki Krogerus Cc: Guenter Roeck , Andy Shevchenko , Rajaram R , Felipe Balbi , Mathias Nyman , Greg KH , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org Date: Wed, 01 Jun 2016 10:31:32 +0200 In-Reply-To: <20160601082336.GE10084@kuha.fi.intel.com> References: <1463661894-22820-1-git-send-email-heikki.krogerus@linux.intel.com> <20160530131951.GA13055@kuha.fi.intel.com> <1464616767.5364.5.camel@suse.com> <20160531083121.GA10084@kuha.fi.intel.com> <1464684509.10800.16.camel@suse.com> <20160531120901.GB10084@kuha.fi.intel.com> <20160531124355.GC10084@kuha.fi.intel.com> <20160531172033.GB14007@roeck-us.net> <20160601082336.GE10084@kuha.fi.intel.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2016-06-01 at 11:23 +0300, Heikki Krogerus wrote: > On Tue, May 31, 2016 at 10:20:34AM -0700, Guenter Roeck wrote: > > On Tue, May 31, 2016 at 03:43:56PM +0300, Heikki Krogerus wrote: > > > On Tue, May 31, 2016 at 03:09:01PM +0300, Heikki Krogerus wrote: > > > > 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. > > > > > > Forget about it, I'll just add the reset attributes. I'm still not > > > clear about their usefulness, but instead they will just create a small > > > risk, but I can live with that. > > > > > > > Given my experience over the last few weeks, I think the added risk > > may not just be small, and I think the added benefit is questionable. > > Reset handling is not well implemented in all devices, and manually > > triggered resets in an unexpected state may make the situation worse. > > > > Can you make it optional ? I may choose not to support it to avoid > > the risk. > > Maybe I gave up on this too hastily... I changing my mind about this, > I'm not going to add them. Having them optional is not enough. It > changes nothing when they are implemented. I think there is a change > that we would actually end up having to remove the attributes, which > would be really bad. > > I think we can still add them later if they are still seen as > necessity later on, tough I seriously doubt it. It would not be > ideal, but adding an attribute should not really break anything, > right? Removing would. That is true. So let's leave it out for now. I still think sane error handling will require it eventually, but that will be in the future. Regards Oliver