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 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?

>>
>> >> > + - 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.

>> > 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?

If they are truly debugging, then they would belong in debugfs rather
than sysfs.

>> >> 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?

How is ID pin detect supposed to be supported? Do we need dr_mode = "idpin"?

>> > 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.

> 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?

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux