On Wed, 10 Jun 2015 21:47:51 +0800 Li Jun <b47...@freescale.com> wrote:
> On Wed, Jun 10, 2015 at 03:37:37PM +0800, Roger Quadros wrote: > > On Tue, 9 Jun 2015 11:33:11 -0500 > > Rob Herring <robherri...@gmail.com> wrote: > > > > > On Tue, Jun 9, 2015 at 10:29 AM, Roger Quadros <rog...@ti.com> wrote: > > > > Rob, > > > > > > > > On Tue, 9 Jun 2015 08:26:20 -0500 > > > > Rob Herring <robherri...@gmail.com> wrote: > > > > > > > >> On Mon, Jun 8, 2015 at 8:18 PM, Li Jun <b47...@freescale.com> wrote: > > > >> > On Mon, Jun 08, 2015 at 11:06:49AM -0500, Rob Herring wrote: > > > >> >> On Mon, Jun 8, 2015 at 10:02 AM, Li Jun <jun...@freescale.com> > > > >> >> wrote: > > > >> >> > Add otg version, srp, hnp and adp support for usb OTG port, then > > > >> >> > those OTG > > > >> >> > features don't have to be decided by usb gadget drivers. > > > >> >> > > > > >> >> > Signed-off-by: Li Jun <jun...@freescale.com> > > > >> >> > --- > > > >> >> > Documentation/devicetree/bindings/usb/generic.txt | 10 ++++++++++ > > > >> >> > 1 file changed, 10 insertions(+) > > > >> >> > > > > >> >> > diff --git a/Documentation/devicetree/bindings/usb/generic.txt > > > >> >> > b/Documentation/devicetree/bindings/usb/generic.txt > > > >> >> > index 477d5bb..7386f4a 100644 > > > >> >> > --- a/Documentation/devicetree/bindings/usb/generic.txt > > > >> >> > +++ b/Documentation/devicetree/bindings/usb/generic.txt > > > >> >> > @@ -11,6 +11,12 @@ Optional properties: > > > >> >> > "peripheral" and "otg". In case this > > > >> >> > attribute isn't > > > >> >> > passed via DT, USB DRD controllers should > > > >> >> > default to > > > >> >> > OTG. > > > >> >> > + - otg-rev: tells usb driver the release number of the OTG and > > > >> >> > EH supplement > > > >> >> > + with which the device and its descriptors > > > >> >> > are compliant, > > > >> >> > + in binary-coded decimal (i.e. 2.0 is > > > >> >> > 0200H). > > > >> >> > > > >> >> I would assume OTG 2.0 is somehow backwards compatible? Is this a > > > >> >> h/w > > > >> >> dependency or a driver feature? > > > >> >> > > > >> > Not fully compatible, OTG 2.0 extend the usb_otg_descriptor by > > > >> > adding a new > > > >> > member bcdOTG to identify the OTG version, this descriptor needs to > > > >> > be sent > > > >> > to OTG host with correct size and content, so we have to know which > > > >> > release > > > >> > version the OTG device is compliant with, either by menuconfig > > > >> > config or pass > > > >> > via DT. > > > >> > > > >> So you have to change the version depending on the host you are > > > >> connected to? That really seems strange that plugging in a OTG 2.0 > > > >> device to an OTG 1.3 host would not work and doesn't make for a good > > > >> user experience. > > > > > > > > No. The OTG version in the OTG descriptor for any device is usually > > > > fixed for the > > > > lifetime of the product. > > > > > > > > Let's assume it is 2.0. > > > > > > > > If you plug this to OTG 1.0 host, it won't be an issue as OTG 1.0 host > > > > doesn't > > > > read the BCD version. > > > > > > That makes sense, but there was some discussion about the size mattering. > > > > > > So is there a reason not to always report 2.0 with any kernel that has > > > 2.0 support? > > > > A 2.0 host would still need to know if the attached OTG device is 1.0 or 2.0 > > so we don't want to force existing 1.0 devices to 2.0. > > > > > > > > >> > > > >> >> > + - srp-support: tells OTG controllers we want to enable SRP. > > > >> >> > + - hnp-support: tells OTG controllers we want to enable HNP. > > > >> >> > + - adp-support: tells OTG controllers we want to enable ADP. > > > >> >> > > > >> >> I've recently run into a problem[1] and found that I have to disable > > > >> >> OTG in the kernel to get my device to work. Having to turn-off OTG > > > >> >> seems like the wrong solution, and shifting the problem to DT seems > > > >> >> wrong too. Why is this not a user configurable option (within > > > >> >> whatever > > > >> >> h/w constraints there are)? > > > >> > The problem of below link, seems your device is claiming it's a HNP > > > >> > capable > > > >> > OTG device, but connecting to a non-OTG port of your Host, assume > > > >> > your Host > > > >> > does have a OTG port, your Host issue a A_ALT_HNP_SUPPORT request to > > > >> > your > > > >> > OTG device to remind it can use another port with HNP, but the > > > >> > request failed > > > >> > (maybe STALL by your device, this request is defined in OTG 1.3 but > > > >> > obsolete > > > >> > in OTG 2.0), so your Host just stopped enumeration of your device, > > > >> > this is not > > > >> > reasonable because current OTG code is some out of data. > > > >> > > > >> Do PCs have OTG ports typically? My expectation is that if I plug in > > > >> an OTG device as a B device to any host port, that it will work as a > > > >> device no matter what the host OTG capabilities are. If I have to > > > >> change the kernel config or DT, that is a problem. > > > > > > > > AFAIK PCs don't have OTG ports. > > > > > > > > If you plug in OTG device to a non-otg host port it will work as normal > > > > B-device. > > > > The host doesn't request for OTG descriptors and doesn't care what OTG > > > > features it > > > > supports or not. > > > > > > That is what I would expect. My testing and the bug report show otherwise. > > > > what kernel and platform are you on? > > > > > > > > >> > I am trying to make those OTG feaures to be configurable options, > > > >> > you mean > > > >> > by sys? > > > >> > > > >> Yes. > > > > > > > > why do you need OTG features to be sysfs configurable other than for > > > > debugging? > > > > > > I don't know. Buggy host perhaps? Why do you need them in DT? > > > > I'll explain why we need in DT below. > > > > > > > > If they are truly debugging, then they would belong in debugfs rather > > > than sysfs. > > > > agreed. > > > > > > >> >> What are the valid combinations? When do we want these enabled or > > > >> >> not? > > > >> >> Wouldn't default enabled be better? > > > >> > > > > >> > We want to enable all those support in kernel driver, but some > > > >> > platform or > > > >> > hardware may not want to enable any or some of them, so those > > > >> > hardware > > > >> > can disable it by not pass the property in dt, the 3 sub features of > > > >> > OTG are > > > >> > not mandatory for so called OTG device, normally we at least enable > > > >> > HNP, and > > > >> > SRP and ADP are optional. > > > >> > > > >> Please answer my questions in the doc. > > > >> > > > >> >> > > > >> >> We already have dr_mode property. How is it related to these? > > > > > > > > dr_mode states what mode the controller will operate in. > > > > > > > > for dr_mode == "host" we don't care about these otg flags. > > > > > > > > for dr_mode == "peripheral" or dr_mode == "otg" > > > > we care about these OTG flags to create our OTG descriptor on the fly. > > > > > > Then how do I specify my device is peripheral only even though I have > > > a DR controller? > > > > by specifying dr_mode = "peripheral" in the DT. > > > > > > > > How is ID pin detect supposed to be supported? Do we need dr_mode = > > > "idpin"? > > > > ID pin is not used in single role mode. It will be used only when dr_mode = > > "otg". > > > > > > > > >> > dr_mode is to tell the device it will work at OTG mode(there is > > > >> > another simple > > > >> > dual role mode which is commom used but not HNP), srp/hnp/adp can > > > >> > further specify > > > >> > which protocol the OTG device will support. > > > >> > > > >> By simple DR, you mean ID pin detect, right. So please define how you > > > >> support just ID pin detect vs. other levels of capability. Does only > > > >> dr_mode = otg mean ID pin detect? That may be a problem for existing > > > >> DTs if you disable other OTG functions because they have not been > > > >> added to the DT, then that is a problem. > > > >> > > > >> I'm feeling less convinced that this belongs in DT at all. Please > > > >> convince me otherwise. > > > > > > > > Yes not specifying anything in DT should work and default to the > > > > best OTG version and features supported by the OTG controller. > > > > > > Right, hence why I suggested disable flags, not enable flags. > > > > I second that. They must be disable flags. > > > > Disable flags may not work with current situation of gadget driver: > Currently each gadget class driver hard coded the OTG attributes > to be HNP | SRP, independent of controller driver. That is wrong in the first place. Gadget drivers shouldn't decide the OTG attributes. Platform code/DT should. If gadget drivers define OTG flags then they cannot be used on different platforms with different OTG needs. > > E.g. some platform with OTG enabled: gadget->is_otg = 1 > HNP and SRP are enabled by gadget driver, ADP = 0, this OTG port > can really support HNP and SRP, but not ADP. What if the platform on which the gadget driver is used doesn't want SRP enabled? > if use disable flag, this platform has to add adp-disable property > otherwise it will report ADP support to the host. This issue won't happen if gadget driver doesn't define any OTG attributes. > > But with enable flags, I can check all those 3 properties, I don't see why you can't do that with disable flags. Note that there are 2 things. 1) disable flags from DT 2) support flags from controller. This information is already known to the controller. Based on these 2 you can decide what OTG features you want to set/clear. And you can't combine the 2 by just defining enable flags in DT. > 1)If none of them are passed, but gadget->is_otg == 1, I suppose it's > legacy platform, still set HNP and SRP as current gadget driver does, > works as before; > 2)If any one of them appear, I will set all those features by dt property. > 3)If some platform already based on those properties, wants to disable > all 3 OTG features, also not pass any one of them like 1), it will not > be a OTG device at all, set gadget->is_otg = 0 in its controller driver, > then no need set and report any OTG features, this can meet ID pin detect > case. With enable flags you don't get what you set. e.g. in DT, we might set enable-adp. but if controller doesn't support adp, you don't have ADP working. So this is misleading. cheers, -roger > > > > > > > > But if the device manufacturer wants to restrict the OTG version > > > > to something less or disable some OTG features then the DT flags come > > > > into play. > > > > > > Why would they? > > > > Maybe they just don't want some of the features. > > one example is DRD mode. In DRD mode the controller works as host or > > peripheral but doesn't support dynamic role switching or host negotiation. > > > > For that they can set > > > > dr_mode = "otg" > > disable-adp; > > disable-hnp; > > disable-srp; > > Other possible reasons maybe power saving, ADP need constantly do vbus > probe if there is no usb device connected, this will consume power. > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
