Re: [PATCH v2 05/22] doc: dt-binding: usb: add otg related properties

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 10, 2015 at 03:37:37PM +0800, Roger Quadros wrote:
> On Tue, 9 Jun 2015 11:33:11 -0500
> Rob Herring <robherring2@xxxxxxxxx> wrote:
> 
> > On Tue, Jun 9, 2015 at 10:29 AM, Roger Quadros <rogerq@xxxxxx> wrote:
> > > Rob,
> > >
> > > On Tue, 9 Jun 2015 08:26:20 -0500
> > > Rob Herring <robherring2@xxxxxxxxx> wrote:
> > >
> > >> On Mon, Jun 8, 2015 at 8:18 PM, Li Jun <b47624@xxxxxxxxxxxxx> 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.li@xxxxxxxxxxxxx> 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.li@xxxxxxxxxxxxx>
> > >> >> > ---
> > >> >> >  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.

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. 
if use disable flag, this platform has to add adp-disable property
otherwise it will report ADP support to the host.

But with enable flags, I can check all those 3 properties,
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.
 
> > 
> > > 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.
 
> cheers,
> -roger
--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux