Remove the check for is_b_host upon enumerating otg devices as it
can trigger some weird behaviors on otg sessions. Some devices claim
to be b_host even though they have an a_connector attached to it.
Checking b_hnp_enable flag should be secure enough or terms of
otg compliancy.
Signed-off-by: Felipe Balbi <[email protected]>
---
drivers/usb/core/hub.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 68fc521..963cf9b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1315,7 +1315,7 @@ static int usb_configure_device_otg(struct usb_device *udev)
/* Maybe it can talk to us, though we can't talk to it.
* (Includes HNP test device.)
*/
- if (udev->bus->b_hnp_enable || udev->bus->is_b_host) {
+ if (udev->bus->b_hnp_enable) {
err = usb_port_suspend(udev);
if (err < 0)
dev_dbg(&udev->dev, "HNP fail, %d\n", err);
--
1.5.4.34.g053d9
On Tuesday 12 February 2008, Felipe Balbi wrote:
> Some devices claim
> to be b_host even though they have an a_connector attached to it.
Why not just fix that bug? Remember that's Linux code.
On Tue, Feb 12, 2008 at 02:28:53AM -0800, David Brownell wrote:
> On Tuesday 12 February 2008, Felipe Balbi wrote:
> > Some devices claim
> > to be b_host even though they have an a_connector attached to it.
>
> Why not just fix that bug? Remember that's Linux code.
The device claiming to be b_host is not linux based. In any case this
is_b_host check won't do nothing here as we should check is
SetFeature(b_hnp_enbable) has been succesfull.
A device in b_host state is not enough for allowing hnp to happen.
Remember some devices doesn't support hnp in B roles, only a_hnp_support
is mandatory.
--
- Balbi
On Tuesday 12 February 2008, Felipe Balbi wrote:
> On Tue, Feb 12, 2008 at 02:28:53AM -0800, David Brownell wrote:
> > On Tuesday 12 February 2008, Felipe Balbi wrote:
> > > Some devices claim
> > > to be b_host even though they have an a_connector attached to it.
> >
> > Why not just fix that bug? Remember that's Linux code.
>
> The device claiming to be b_host is not linux based.
Wrong ... the meaning of that flag is: *THIS* system is a
Linux-USB OTG device which came up in B-peripheral role, and
then through the magic of HNP morphed into the B-host role.
Linux is acting as a host at that point. So either it's
being the A-host, or the B-host. That flag says which. If
the other device has the A-connector, yet we're the B-host,
then right now it is acting as an A-peripheral. That's
exactly what HNP is designed to do.
> In any case this
> is_b_host check won't do nothing here as we should check is
> SetFeature(b_hnp_enbable) has been succesfull.
Again wrong ... if the host side's b_hnp_enable flag is set,
that means this is a Linux-USB OTG device which came up in
A-host role and enumerated some vendor's OTG device, and
then set the b_hnp_enable flag. (So it could in the future
use HNP to become an A-peripheral, despite having started
as an A-host.)
> A device in b_host state is not enough for allowing hnp to happen.
If the system is in b_host state, then HNP *ALREADY* happened.
So it can happen again, to go back into the b_peripheral state.
(And of course, we can't set the b_hnp_enable flag in a device
which is in the a_peripheral state...)
Seems like the root cause of the problem here is that you
have some correctly functioning code, but for some reason
you're surprised by that correct functioning. (Maybe this
is a case where neither device is on the other's whitelist?)
- Dave
p.s. http://www.linux-usb.org/gadget/h2-otg.html
does talk about those two bits, albeit succinctly.
On Tue, Feb 12, 2008 at 05:02:47AM -0800, David Brownell wrote:
> On Tuesday 12 February 2008, Felipe Balbi wrote:
> > On Tue, Feb 12, 2008 at 02:28:53AM -0800, David Brownell wrote:
> > > On Tuesday 12 February 2008, Felipe Balbi wrote:
> > > > Some devices claim
> > > > to be b_host even though they have an a_connector attached to it.
> > >
> > > Why not just fix that bug? Remember that's Linux code.
> >
> > The device claiming to be b_host is not linux based.
>
> Wrong ... the meaning of that flag is: *THIS* system is a
> Linux-USB OTG device which came up in B-peripheral role, and
> then through the magic of HNP morphed into the B-host role.
Don't be sarcastic I won't even fall into what Jean Delvare told you.
> Linux is acting as a host at that point. So either it's
> being the A-host, or the B-host. That flag says which. If
> the other device has the A-connector, yet we're the B-host,
> then right now it is acting as an A-peripheral. That's
> exactly what HNP is designed to do.
True, but allowing hnp nowing we're b_host doesn't sound correct. If
we're b_host it can get two scenarios we would get to fall into this
state:
1. We asked to become host; or
2. A_device allowed us become host because it can't handle us.
In both cases, HNP is quite useless happening again, don't you think?
We should at least try to enumerate a_peripheral, if it can't due to
our power capabilities i.e. 100mA, we'll fall into overcurrent
protection and we'll suspend the bus and disconnect, which would give
a_device another change to enumerate us.
This sound much better to me.
> > In any case this
> > is_b_host check won't do nothing here as we should check is
> > SetFeature(b_hnp_enbable) has been succesfull.
>
> Again wrong ... if the host side's b_hnp_enable flag is set,
> that means this is a Linux-USB OTG device which came up in
> A-host role and enumerated some vendor's OTG device, and
> then set the b_hnp_enable flag. (So it could in the future
> use HNP to become an A-peripheral, despite having started
> as an A-host.)
>
>
> > A device in b_host state is not enough for allowing hnp to happen.
>
> If the system is in b_host state, then HNP *ALREADY* happened.
True, but...
> So it can happen again, to go back into the b_peripheral state.
> (And of course, we can't set the b_hnp_enable flag in a device
> which is in the a_peripheral state...)
it shouldn't happen again at this point. If we're b_host, we should try to
enumerate a_peripheral. If it draws more than 100mA the session should
end, otherwise we'd fall into hnp loops until we start getting hub port
errors and linux decide to suspend roothub and end the session.
> Seems like the root cause of the problem here is that you
> have some correctly functioning code, but for some reason
> you're surprised by that correct functioning. (Maybe this
> is a case where neither device is on the other's whitelist?)
True, neither is in other's whitelist, but the point is:
1. Currently standard-a device supports hnp only on A states
2. Currently standard-b device supports hnp on both roles
3. A-device enumerates b-device, checks it's not tpl'ed and allow hnp
4. B-device become b_host, checks a_peripheral is not on its tpl, even
though it has support for that device class and it draws less than
100mA.
According to otg tpl clarification, they should work in this scenario,
shouldn't they ? Or should they stay in an hnp loop until someone
decides to end the session ?
--
- Balbi
> > > > > Some devices claim
> > > > > to be b_host even though they have an a_connector attached to it.
> > > >
> > > > Why not just fix that bug? Remember that's Linux code.
> > >
> > > The device claiming to be b_host is not linux based.
> >
> > Wrong ... the meaning of that flag is: *THIS* system is a
> > Linux-USB OTG device which came up in B-peripheral role, and
> > then through the magic of HNP morphed into the B-host role.
>
> Don't be sarcastic I won't even fall into what Jean Delvare told you.
I see no sarcasm there. People seem to want different degrees
of handholding when being told they're wrong. First time gently,
second time less so ...
I think there are limits to how long you can realistically expect
someone (in this case, me) to let that drag on.
(You object to "magic"? Most LKML readers know squat about OTG,
and that seemed like a succinct introduction to what it does.)
> > Linux is acting as a host at that point. So either it's
> > being the A-host, or the B-host. That flag says which. If
> > the other device has the A-connector, yet we're the B-host,
> > then right now it is acting as an A-peripheral. That's
> > exactly what HNP is designed to do.
>
> True, but allowing hnp nowing we're b_host doesn't sound correct.
Why not? The B-host can't proceed in that case. The mechanism
for not-proceeding is inside this:
if (!is_targeted(udev)) {
if (udev->bus->b_hnp_enable || udev->bus->is_b_host) {
err = usb_port_suspend(udev);
if (err < 0)
dev_dbg(&udev->dev, "HNP fail, %d\n", err);
}
err = -ENOTSUPP;
goto fail;
}
Your proposal is to strike the "is_b_host" check. In terms of the
OTG (1.3) state machine, that removes a b_host --> b_peripheral
state transition.
BUT: notice it doesn't replace it with the ONLY other valid state
transition, b_host --> b_idle. In fact, that can not be initiated
by the B-device...
So the current code *is* correct.
> If
> we're b_host it can get two scenarios we would get to fall into this
> state:
>
> 1. We asked to become host; or
This is always the case... the other side can enable HNP all
it wants, but it won't proceed unless the b-peripheral does
its side of the dance. (In terms of spec: b_bus_req must be
true before HNP can proceed.)
> 2. A_device allowed us become host because it can't handle us.
We actually can't know why it started HNP; it may well have
had other reasons. All we know is that the roles switched.
Maybe it finished its initial task, and decided to give our
side the opportunity to accomplish some tasks it couldn't
know about in advance.
> In both cases, HNP is quite useless happening again, don't you think?
No. You have no way of knowing what the A-device may or may not
do when it becomes host again. It's getting a chance to do
more work, in conformance with what OTG is supposed to do.
> We should at least try to enumerate a_peripheral, if it can't due to
> our power capabilities i.e. 100mA, we'll fall into overcurrent
> protection and we'll suspend the bus and disconnect, which would give
> a_device another change to enumerate us.
>
> This sound much better to me.
You're overlooking something: this is the "!is_targeted(udev)"
case, so we have already enumerated the a_peripheral as much as
we can. There is *nothing* more we can do with it. Sitting
around in the b_host state would be utterly pointless.
> > So it can happen again, to go back into the b_peripheral state.
> > (And of course, we can't set the b_hnp_enable flag in a device
> > which is in the a_peripheral state...)
>
> it shouldn't happen again at this point. If we're b_host, we should try to
> enumerate a_peripheral. If it draws more than 100mA the session should
> end, otherwise we'd fall into hnp loops until we start getting hub port
> errors and linux decide to suspend roothub and end the session.
Notice that because we're b_host we are not supplying power. :)
Re HNP loops, go look again at the B-device state diagram. Now
tell me how the B-device could terminate such a loop ... it can't.
But look at the A-device diagram ... it obviously *CAN* terminate
such a loop (by turning off VBUS power).
Though I don't think Linux currently *will* terminate that loop.
It needs to disable port power, which is still problematic. And
also it would need to *decide* to disable port power.
> > Seems like the root cause of the problem here is that you
> > have some correctly functioning code, but for some reason
> > you're surprised by that correct functioning. (Maybe this
> > is a case where neither device is on the other's whitelist?)
>
> True, neither is in other's whitelist, but the point is:
>
> 1. Currently standard-a device supports hnp only on A states
That would be a bug; the a_peripheral --> a_wait_bcon transition
has always been part of OTG.
> 2. Currently standard-b device supports hnp on both roles
Correct; that conforms with OTG requirements.
> 3. A-device enumerates b-device, checks it's not tpl'ed and allow hnp
OK ...
> 4. B-device become b_host, checks a_peripheral is not on its tpl, even
> though it has support for that device class
If it supports a device class I'd say that should be included
in the TPL ("whitelist"). Though if it tries to do that, it's
an explicit violation of the OTG spec (sect 3.3):
The Targeted Peripheral List shall not list supported
USB Classes or “similar” devices.
> and it draws less than 100mA.
Irrelevent, though possibly the Linux code there needs to be
taught to ignore the power constraints when it is_b_host ...
because the B-device is never supplying power to the A-device,
even when it's in the b_host state.
> According to otg tpl clarification, they should work in this scenario,
> shouldn't they ? Or should they stay in an hnp loop until someone
> decides to end the session ?
Shouldn't work, because your points 1 and 4 violate specs.
Though it would make sense to me to have the host record
whether it already gave up on the peripheral once ... and
if it did, then power it off.
- Dave
On Tue, Feb 12, 2008 at 12:32:34PM -0800, David Brownell wrote:
> > > > > > Some devices claim
> > > > > > to be b_host even though they have an a_connector attached to it.
> > > > >
> > > > > Why not just fix that bug? Remember that's Linux code.
> > > >
> > > > The device claiming to be b_host is not linux based.
> > >
> > > Wrong ... the meaning of that flag is: *THIS* system is a
> > > Linux-USB OTG device which came up in B-peripheral role, and
> > > then through the magic of HNP morphed into the B-host role.
> >
> > Don't be sarcastic I won't even fall into what Jean Delvare told you.
>
> I see no sarcasm there. People seem to want different degrees
Ok, sorry here. Misunderstood the point of "magic"
> of handholding when being told they're wrong. First time gently,
> second time less so ...
>
> I think there are limits to how long you can realistically expect
> someone (in this case, me) to let that drag on.
>
> (You object to "magic"? Most LKML readers know squat about OTG,
> and that seemed like a succinct introduction to what it does.)
>
>
> > > Linux is acting as a host at that point. So either it's
> > > being the A-host, or the B-host. That flag says which. If
> > > the other device has the A-connector, yet we're the B-host,
> > > then right now it is acting as an A-peripheral. That's
> > > exactly what HNP is designed to do.
> >
> > True, but allowing hnp nowing we're b_host doesn't sound correct.
>
> Why not? The B-host can't proceed in that case. The mechanism
> for not-proceeding is inside this:
>
> if (!is_targeted(udev)) {
> if (udev->bus->b_hnp_enable || udev->bus->is_b_host) {
> err = usb_port_suspend(udev);
> if (err < 0)
> dev_dbg(&udev->dev, "HNP fail, %d\n", err);
> }
> err = -ENOTSUPP;
> goto fail;
> }
>
> Your proposal is to strike the "is_b_host" check. In terms of the
> OTG (1.3) state machine, that removes a b_host --> b_peripheral
> state transition.
Not at all, we're just not doing transition right now.
>
> BUT: notice it doesn't replace it with the ONLY other valid state
> transition, b_host --> b_idle. In fact, that can not be initiated
> by the B-device...
>
> So the current code *is* correct.
>
>
> > If
> > we're b_host it can get two scenarios we would get to fall into this
> > state:
> >
> > 1. We asked to become host; or
>
> This is always the case... the other side can enable HNP all
> it wants, but it won't proceed unless the b-peripheral does
> its side of the dance. (In terms of spec: b_bus_req must be
> true before HNP can proceed.)
>
>
> > 2. A_device allowed us become host because it can't handle us.
>
> We actually can't know why it started HNP; it may well have
> had other reasons. All we know is that the roles switched.
Got your point.
>
> Maybe it finished its initial task, and decided to give our
> side the opportunity to accomplish some tasks it couldn't
> know about in advance.
>
>
> > In both cases, HNP is quite useless happening again, don't you think?
>
> No. You have no way of knowing what the A-device may or may not
> do when it becomes host again. It's getting a chance to do
> more work, in conformance with what OTG is supposed to do.
>
>
> > We should at least try to enumerate a_peripheral, if it can't due to
> > our power capabilities i.e. 100mA, we'll fall into overcurrent
> > protection and we'll suspend the bus and disconnect, which would give
> > a_device another change to enumerate us.
> >
> > This sound much better to me.
>
> You're overlooking something: this is the "!is_targeted(udev)"
> case, so we have already enumerated the a_peripheral as much as
> we can. There is *nothing* more we can do with it. Sitting
> around in the b_host state would be utterly pointless.
And you're overlooking something: tpl is not an enumeration blocker at
all. The only blockers are drivers and power limits.
Imagine what happens on n810, we have 3 mass storage devices listed on
its tpl: 770, n800 and itself. Besides that, we allow enumeration of any
mass storage device as long as it draws less than 100mA. These tips I
got from otg chairman himself and from OTG Clarification document, you
can find it in compliance.usb.org. So, what I'm trying to say is if
a_device let us become host, we shold at least try to do, let's call it,
full enumeration. Which means attaching any unlisted mass storage device
should work.
> > > So it can happen again, to go back into the b_peripheral state.
> > > (And of course, we can't set the b_hnp_enable flag in a device
> > > which is in the a_peripheral state...)
> >
> > it shouldn't happen again at this point. If we're b_host, we should try to
> > enumerate a_peripheral. If it draws more than 100mA the session should
> > end, otherwise we'd fall into hnp loops until we start getting hub port
> > errors and linux decide to suspend roothub and end the session.
>
> Notice that because we're b_host we are not supplying power. :)
True, sorry. :-p
>
> Re HNP loops, go look again at the B-device state diagram. Now
> tell me how the B-device could terminate such a loop ... it can't.
> But look at the A-device diagram ... it obviously *CAN* terminate
> such a loop (by turning off VBUS power).
aka ending the session, yeah. But once again I'm still thinking we
should try to enumerate the device and, in mass storage case, start scsi
emulation, mount fs and so on.
> Though I don't think Linux currently *will* terminate that loop.
> It needs to disable port power, which is still problematic. And
> also it would need to *decide* to disable port power.
But then a-device should end the session, not b. If we really can't use
that a_peripheral, we suspend and roles swtich again. In this case
a_host should know we're still that non-working b_peripheral and end the
session.
> > > Seems like the root cause of the problem here is that you
> > > have some correctly functioning code, but for some reason
> > > you're surprised by that correct functioning. (Maybe this
> > > is a case where neither device is on the other's whitelist?)
> >
> > True, neither is in other's whitelist, but the point is:
> >
> > 1. Currently standard-a device supports hnp only on A states
>
> That would be a bug; the a_peripheral --> a_wait_bcon transition
> has always been part of OTG.
OTG 1.3 page 25:
OTG products are required to support HNP as an A-device. OTG products
must support HNP as a B-device if they can support any OTG product as
a peripheral. OTG products that cannot support any OTG product
as a peripheral are not required to support HNP as a B-device.
> > 2. Currently standard-b device supports hnp on both roles
>
> Correct; that conforms with OTG requirements.
>
>
> > 3. A-device enumerates b-device, checks it's not tpl'ed and allow hnp
>
> OK ...
>
> > 4. B-device become b_host, checks a_peripheral is not on its tpl, even
> > though it has support for that device class
>
> If it supports a device class I'd say that should be included
> in the TPL ("whitelist"). Though if it tries to do that, it's
> an explicit violation of the OTG spec (sect 3.3):
We can't support classes. What I'm trying to say is we have usb mass
storage support enabled in kernel and we can handle _any_ mass storage
devices as long as they meet otg requirements.
>
> The Targeted Peripheral List shall not list supported
> USB Classes or ???similar??? devices.
>
>
> > and it draws less than 100mA.
>
> Irrelevent, though possibly the Linux code there needs to be
> taught to ignore the power constraints when it is_b_host ...
> because the B-device is never supplying power to the A-device,
> even when it's in the b_host state.
>
>
> > According to otg tpl clarification, they should work in this scenario,
> > shouldn't they ? Or should they stay in an hnp loop until someone
> > decides to end the session ?
>
> Shouldn't work, because your points 1 and 4 violate specs.
Not true, check again Chapter 6 Host Negotiation Protocol and OTG
Clarification regarding TPL.
>
> Though it would make sense to me to have the host record
> whether it already gave up on the peripheral once ... and
> if it did, then power it off.
Good catch. That'd be nice.
--
- Balbi
On Wednesday 13 February 2008, Felipe Balbi wrote:
> On Tue, Feb 12, 2008 at 12:32:34PM -0800, David Brownell wrote:
> >
> > Your proposal is to strike the "is_b_host" check. In terms of the
> > OTG (1.3) state machine, that removes a b_host --> b_peripheral
> > state transition.
>
> Not at all, we're just not doing transition right now.
When *would* it happen then? And why not do it as soon as it's
known that the device on the other end of the cable is unsupported?
> > BUT: notice it doesn't replace it with the ONLY other valid state
> > transition, b_host --> b_idle. In fact, that can not be initiated
> > by the B-device...
> >
> > So the current code *is* correct.
> >
> > ... deletia ...
> >
> > > We should at least try to enumerate a_peripheral, if it can't due to
> > > our power capabilities i.e. 100mA, we'll fall into overcurrent
> > > protection and we'll suspend the bus and disconnect, which would give
> > > a_device another change to enumerate us.
> > >
> > > This sound much better to me.
> >
> > You're overlooking something: this is the "!is_targeted(udev)"
> > case, so we have already enumerated the a_peripheral as much as
> > we can. There is *nothing* more we can do with it. Sitting
> > around in the b_host state would be utterly pointless.
>
> And you're overlooking something: tpl is not an enumeration blocker at
> all. The only blockers are drivers and power limits.
The device has already been enumerated at this point. This is just
whether to set up communication with, and use, the device ... which
activity *IS* predicated on device support, "on the TPL" (as it says
in 6.8.1.4 of the spec for the A_HOST role, later reusing that language
where it describes the B_HOST role).
> Imagine what happens on n810, we have 3 mass storage devices listed on
> its tpl: 770, n800 and itself. Besides that, we allow enumeration of any
> mass storage device as long as it draws less than 100mA.
While it makes sense to me that an OTG device support things like
mass storage class devices, I still see that spec language saying
that classes can't be on the TPL ... and that only devices on the
TPL get communication beyond what's needed for enumeration.
Regardless, that's a red herring. If you allow sane things like
arbitrary flash memory sticks to be used, you've effectively put
a device class on the TPL. But this discussion is about things
that are NOT on the TPL.
> These tips I
> got from otg chairman himself and from OTG Clarification document, you
> can find it in compliance.usb.org.
Well, I'm glad at least one person there is talking sanely about
the TPL, but there is no such clarification that I can find.
I've observed USB-IF to be slow to address bugs in their specs,
maybe that's what's going on here. Bugs are often written into
specs as political compromises, and they can linger for various
reasons. (Including members with hidden agendas to block success,
and people who prefer to deny inconvenient realities.)
> So, what I'm trying to say is if
> a_device let us become host, we shold at least try to do, let's call it,
> full enumeration. Which means attaching any unlisted mass storage device
> should work.
Attaching works, sure. If you are willing to communicate with
that device, then it's on the Targeted Peripherals List (TPL),
so this code branch doesn't apply.
Put it this way: there is no *OTHER* mechanism for deciding not
to talk with a device; and that's the entire purpose of the TPL.
> > > 4. B-device become b_host, checks a_peripheral is not on its tpl, even
> > > though it has support for that device class
> >
> > If it supports a device class I'd say that should be included
> > in the TPL ("whitelist"). Though if it tries to do that, it's
> > an explicit violation of the OTG spec (sect 3.3):
>
> We can't support classes. What I'm trying to say is we have usb mass
> storage support enabled in kernel and we can handle _any_ mass storage
> devices as long as they meet otg requirements.
I think that detail of the OTG spec is stupid and misguided.
Of *course* it should be possible to support, for example,
devices that weren't shipped before your product's TPL was
defined. How ever you define the list of Targeted/supported
peripherals, that's got to be able to include classes and
similar mechanisms. That spec stupidity is one issue;
for the scope of this discussion I'll assume the TPL (which
has an algorithmic check) can pass things like flash sticks.
The other issue is what you call the part of your product
documentation which says what devices it can talk to. The
only term for such stuff in the OTG spec is "Targeted
Peripherals List". If you're assuming some other design
abstraction packages that notion, that doesn't match what
the Linux-USB code does.
If you wanted to *add* such an abstraction, you'd have to put
it everywhere the current whitelist is used ... better IMO to
just update the current whitelist/TPL code to handle classes.
Right where the comment says to do it.
> > > According to otg tpl clarification, they should work in this scenario,
> > > shouldn't they ? Or should they stay in an hnp loop until someone
> > > decides to end the session ?
> >
> > Shouldn't work, because your points 1 and 4 violate specs.
>
> Not true, check again Chapter 6 Host Negotiation Protocol and OTG
> Clarification regarding TPL.
There is no such "OTG Clarification" on the website I just
looked at.
> > Though it would make sense to me to have the host record
> > whether it already gave up on the peripheral once ... and
> > if it did, then power it off.
>
> Good catch. That'd be nice.
That's a patch I wouldn't NAK. :)
- Dave
On Wed, Feb 13, 2008 at 01:36:00PM -0800, David Brownell wrote:
> On Wednesday 13 February 2008, Felipe Balbi wrote:
> > On Tue, Feb 12, 2008 at 12:32:34PM -0800, David Brownell wrote:
> > >
> > > Your proposal is to strike the "is_b_host" check. In terms of the
> > > OTG (1.3) state machine, that removes a b_host --> b_peripheral
> > > state transition.
> >
> > Not at all, we're just not doing transition right now.
>
> When *would* it happen then? And why not do it as soon as it's
> known that the device on the other end of the cable is unsupported?
Whenever we finnish using the bus we'd suspend it. This would sinalize
a_peripheral switch back to a_host. Also, even though it's not on our
TPL it doesn't mean we can't work with it.
>
>
> > > BUT: notice it doesn't replace it with the ONLY other valid state
> > > transition, b_host --> b_idle. In fact, that can not be initiated
> > > by the B-device...
> > >
> > > So the current code *is* correct.
> > >
> > > ... deletia ...
> > >
> > > > We should at least try to enumerate a_peripheral, if it can't due to
> > > > our power capabilities i.e. 100mA, we'll fall into overcurrent
> > > > protection and we'll suspend the bus and disconnect, which would give
> > > > a_device another change to enumerate us.
> > > >
> > > > This sound much better to me.
> > >
> > > You're overlooking something: this is the "!is_targeted(udev)"
> > > case, so we have already enumerated the a_peripheral as much as
> > > we can. There is *nothing* more we can do with it. Sitting
> > > around in the b_host state would be utterly pointless.
> >
> > And you're overlooking something: tpl is not an enumeration blocker at
> > all. The only blockers are drivers and power limits.
>
> The device has already been enumerated at this point. This is just
> whether to set up communication with, and use, the device ... which
> activity *IS* predicated on device support, "on the TPL" (as it says
> in 6.8.1.4 of the spec for the A_HOST role, later reusing that language
> where it describes the B_HOST role).
>
It only tell us to output a message to the user indicating it's not
supported. Same for b_host. It never says we should end the session at
that point.
OTG Rev 1.3 is unclear about this. That's why I consulted Richard Pietre
and asked him what would be the most sane way of handling hnp and he
told me to implement it always trying to work with the device even
though it's not tpl'ed.
> > Imagine what happens on n810, we have 3 mass storage devices listed on
> > its tpl: 770, n800 and itself. Besides that, we allow enumeration of any
> > mass storage device as long as it draws less than 100mA.
>
> While it makes sense to me that an OTG device support things like
> mass storage class devices, I still see that spec language saying
> that classes can't be on the TPL ... and that only devices on the
> TPL get communication beyond what's needed for enumeration.
It doesn't say your last point.
Besides, it says:
"The Targeted Peripheral List shall not list supported USB Classes or
similar devices."
By "similar" we can understand several mass storage devices. It doesn't
really matter on usb otg point of view whether we support mass storage
device A or mass storage device B, both should work if they meet otg
requirements (power consumption being the most significant issue).
We can also see the TPL example on otg rev 1.3 is not listing "similar"
devices. They are all different. Quoting from otg rev 1.3 Section 3.3:
Vendor Model Speed Transport VID PID Description
1. Logitech M-BJ58 LS Int 0x046D 0xC00E USB Wheel Mouse
2. Yamaha YST-MS35D FS Isoch 0x0499 0x3002 USB Speakers
3. TEAC Corporation FD-05PUB FS Bulk 0x0644 0x0000 USB Floppy Drive
4. Hewlett Packard D125XI FS Bulk 0x03F0 0x2311 All-In-One Printer/Scanner/Copier
This sounds like another clue that we should be able to "try" to work
with any mouse, any speekers, any floppy and any printer as long as they
meet power requirements because we have support for them built into the
otg device.
> Regardless, that's a red herring. If you allow sane things like
> arbitrary flash memory sticks to be used, you've effectively put
> a device class on the TPL. But this discussion is about things
> that are NOT on the TPL.
No cuz I'm still notifying my user the device is unsupported even though
it's not even a requirement, if the device meet otg
constraints*(explained below). Which means it might or might not work.
If it happen to work, that's fine, if it doesn't the user has been
notified about that possibility. One other thing "Device is Not
Supported" is just an example message.
* In section 3.4 OTG says:
(...) and the OTG device does not support the type of communitcation
being requested by the user, then the OTG device shall provide messages
to the user that allow him or her to understand the problem (...)"
Which means that we should notify our user if, and only if, we _really_
can't work with the device.
TPL is a list of well tested devices against the otg device under use,
it's not meant to be a list of the only workable devices we have.
> > These tips I
> > got from otg chairman himself and from OTG Clarification document, you
> > can find it in compliance.usb.org.
>
> Well, I'm glad at least one person there is talking sanely about
> the TPL, but there is no such clarification that I can find.
I'll ask him to send me a copy and I'll mail you, that's a not a problem
:-)
> I've observed USB-IF to be slow to address bugs in their specs,
> maybe that's what's going on here. Bugs are often written into
> specs as political compromises, and they can linger for various
> reasons. (Including members with hidden agendas to block success,
> and people who prefer to deny inconvenient realities.)
I agree with you here :-)
That's why I joined OTG working group and I'm trying hard to get in
touch with them about such bugs in the specs.
I can say to you next otg spec will be much clearer about tpl ;-)
> > So, what I'm trying to say is if
> > a_device let us become host, we shold at least try to do, let's call it,
> > full enumeration. Which means attaching any unlisted mass storage device
> > should work.
>
> Attaching works, sure. If you are willing to communicate with
> that device, then it's on the Targeted Peripherals List (TPL),
> so this code branch doesn't apply.
>
> Put it this way: there is no *OTHER* mechanism for deciding not
> to talk with a device; and that's the entire purpose of the TPL.
Well there is. Kernel drivers (on the case of b_host) and kernel drivers
plus power constraints (on the case of a_host).
If we don't have kernel drivers, we, for sure, can't communicate with
that device. Or maybe if we have kernel drivers for mass storage but
we attach some kind of messed up storage device using iso endpoints,
which we don't have support on musb (hypothetical situation :-p), we
can't, again, communicate with that device.
> > > > 4. B-device become b_host, checks a_peripheral is not on its tpl, even
> > > > though it has support for that device class
> > >
> > > If it supports a device class I'd say that should be included
> > > in the TPL ("whitelist"). Though if it tries to do that, it's
> > > an explicit violation of the OTG spec (sect 3.3):
> >
> > We can't support classes. What I'm trying to say is we have usb mass
> > storage support enabled in kernel and we can handle _any_ mass storage
> > devices as long as they meet otg requirements.
>
> I think that detail of the OTG spec is stupid and misguided.
>
> Of *course* it should be possible to support, for example,
> devices that weren't shipped before your product's TPL was
> defined. How ever you define the list of Targeted/supported
> peripherals, that's got to be able to include classes and
> similar mechanisms. That spec stupidity is one issue;
> for the scope of this discussion I'll assume the TPL (which
> has an algorithmic check) can pass things like flash sticks.
Gotcha
>
> The other issue is what you call the part of your product
> documentation which says what devices it can talk to. The
> only term for such stuff in the OTG spec is "Targeted
> Peripherals List". If you're assuming some other design
> abstraction packages that notion, that doesn't match what
> the Linux-USB code does.
TPL is a list of well tested devices, like I said up there. We ship the
device with such a list, warning the user that any other device might or
might not work. This is how it should be according otg working group.
> If you wanted to *add* such an abstraction, you'd have to put
> it everywhere the current whitelist is used ... better IMO to
> just update the current whitelist/TPL code to handle classes.
> Right where the comment says to do it.
Did you see my patches proposing a tpl rework ?
I'm only missing the match class thing so I'm still notifying the user,
even though we allow communication with such devices.
> > > > According to otg tpl clarification, they should work in this scenario,
> > > > shouldn't they ? Or should they stay in an hnp loop until someone
> > > > decides to end the session ?
> > >
> > > Shouldn't work, because your points 1 and 4 violate specs.
> >
> > Not true, check again Chapter 6 Host Negotiation Protocol and OTG
> > Clarification regarding TPL.
>
> There is no such "OTG Clarification" on the website I just
> looked at.
Mailing otg working group. I'll get it for everybody interested ;-)
> > > Though it would make sense to me to have the host record
> > > whether it already gave up on the peripheral once ... and
> > > if it did, then power it off.
> >
> > Good catch. That'd be nice.
>
> That's a patch I wouldn't NAK. :)
heh, I'll try to make it work.
Good we're trying to address things here.
Thanks for all your time Dave :-)
--
- Balbi