Hi,
On 22-08-16 16:08, Bin Liu wrote:
Hi,
On Sun, Aug 21, 2016 at 11:29:02AM +0200, Hans de Goede wrote:
Hi,
On 19-08-16 23:25, Bin Liu wrote:
Hi,
On Mon, Aug 15, 2016 at 09:21:25PM +0200, Hans de Goede wrote:
Hi All,
Here is a patch series which implements run-time changing the dr-mode
of sunxi musb controllers through the (already existing) musb "mode"
sysfs attribute.
This is useful on boards where there is no id pin, e.g. some tv-boxes
use the musb controller to get an extra usb A port without needing
a hub chip. Except for the missing id pin when using a usb A<->A cable
these ports can do peripheral mode just fine. This series makes it
possible to do e.g. this by doing echo "peripheral" > mode before
plugging in the usb A<->A cable.
Well, this is an illegal usecase. A-A cable is invalid by USB Spec.
Yet they exist and there are USB devices (e.g. harddisk enclosures)
which use a USB-A connector even though they are a device not
a host. I own several such devices myself. I agree that this is
wrong but these devices exist, typical case of reality trumping
the spec.
I know those products exist, but they are different from yours. Those
existing devices are usb peripheral-only, which never source vbus so do
not hurt anything other than violate the USB Spec. But your usecase
allows the device switching to host mode, which could damage the usb
host on the other end of the connection due to vbus contention.
Actually the sun4i phy driver avoids this by checking vbus-detect for
external vbus presence before enabling the boards own Vbus.
More importantly, not supporting peripheral mode on musb attached to an
USB-A receptacle will not stop users from trying to use it and insert
an A<->A cable at which point the damage is already done.
I understand why you dislike this, but we cannot stop a user
from trying this. So it is better to actually be prepared for this
(e.g. do the vbus-detect check we're already doing) then it is
to not support this.
For example:
Lets say we do not have vbus-detect and the user plugs in
an A<->A cable causing Vbus being driven from 2 sides.
Not good, now we have current flowing between the 2
power supplies and things are heating up.
Now there are 2 possible follow-up scenarios:
1) We support peripheral mode, the user does
"echo peripheral > mode" in sysfs and we disable
the boards Vbus, no more current, things are cooling
down again before components burn up.
2) We do not support peripheral mode (which is what you're
suggesting), the user does "echo peripheral > mode" in sysfs,
nothing happens. Now we can only happen the user unplugs
the A<->A cable soon, instead of trying a bunch of other things.
Neither is ideal, but do you see how not allowing peripheral
mode is not helping here ? As soon as the user plugs in
an A<->A cable bad things may happen, we cannot avoid that
from the software side.
With type-A receptacle the controller should be in host-only mode,
switching to peripheral mode should not be allowed.
Peripheral mode can still be quite useful, e.g. to do ethernet
over USB (some of these devices lack wired ethernet).
For such usecase, a micro-AB connector should be used, or use two
connectors - type-A and type-B which are mux'd to the same usb port.
Also the manual of these devices typically instructs users to
use an A<->A cable for firmware updates, since the bootROM in
(Nobody reads the manual, we all know it...) This does not prevent users
to connect two hosts together then damage the hardware. So this design
should not be allowed.
Nothing prevents an user from doing that, as soon as they have
an A<->A cable they can easily do that, one can only hope they
will not do it.
the SoC does not care anmd will happily put the device in
Allwinner's custom DFU mode using the USB-A connector.
Although I agree with this being not ideal, the fact is that
an USB-A connector combined with a A<->A cable is electrically
100% identical to a USB-B connector with its id-pin not connected
(on the PCB side), which also happens see below.
they are two different cables. A-B cable does not provide a chance to
connect two hosts together, but A-A cable does.
Right, but an A-B cable with its id-pin wrongly connected to ground
(some so called otg charging hubs do this) will cause the exact same issue.
This is why usb-ports and especially otg ports should have current limiting
on their Vbus and why the sun4i phy driver checks vbus-detect not reporting
an external vbus before enabling Vbus.
This series has both sun4i-usb-phy driver and sunxi-musb-glue changes,
both are necessary for the run-time changing to work, but they can be
merged independently without breaking anything.
Changed in v2:
After sleeping on it a night I realized that always passing port_mode =
DUAL_ROLE to the musb-core was wrong. There is a distintion between
the id-pin not working properly which we can work around with software
mode switching (and we want to register both the host and udc driver
in this case) vs cases where we really only want to register a host
(wifi module connected to musb soldered onto the PCB).
So v2 of this series drops the
"musb: sunxi: Always register both host and udc when build with dual-role support"
patch.
Instead systems which are dual-role capable, but lack an id-pin should use
dr_mode = "otg" in the dts file. There is one problem with this, some
systems are dual-role capable but use a female USB-A connector connected
to the musb controller. These should come up in host mode by default,
This is not a problem. With a type-A connector, the dual-role controller
should work in host-only mode.
Role switching should only be allowed if an AB connector is used.
Yet people may want to use it in peripheral mode, I strongly believe
that we should not be telling people what they can and cannot do
with their hardware. That is policy and the kernel does not set
I agree with the policy, but we are responsible to tell people don't do
something which is not correct.
such policy, that is up to userspace. OTOH the port should clearly
default to host mode hence the new property.
Due to the problem with A-A cable I explained above, A-A cable should
not be used in role-switching cases (it should be used at the first
place), so this new property is unnecessary.
Using the sysfs entry to switch roles for generic purpose is really a
bad idea, it opens up ton of problems.
Yet the sysfs entry exists already, and the problems depend on how
you hook it up. I'm merely using the sysfs entry as an id-pin
for cases where there is no id pin hooked up. If you look at the
patches you will see that all that is changed by writing the
sysfs entry is the phy reporting a different id-pin value to
the musb ip. Everything else is handled the same as for any normal
otg mode port.
For systems which lack of id-pin should use a discrete circuit (for
example GPIO) to detect the id pin in the AB receptacle, then the USB
driver will handle the role switching transparently.
You're again talking theory here in reality there is hardware where
for whatever reasons the PCB uses a mini or micro usb receptacle,
but they did not bother to _physically_ hook up the id pin.
Who are they?
One example of such a device is the quite popular mk802 tv stick
(the original version) with A10s SoC.
> the manufactures who use musb controller to make products,
so as Linux or musb drivers, we have control, it is our responsibility
to tell those manufactures to do things in the right way - design the hw
correctly at the first place. We can't compromise by using an A-A cable
to provide a change to damage the usb host.
This is different from the cases, for example, from the xhci controller
perspective, the xhci driver has to compromise any usb thumb drives
which do not follow the USB Specs, to make them work on PCs.
That makes no sense, why would the xhci driver have to compromise
but we don't ? Either actually having things working is more important
then following the spec or the spec is more important...
And in reality as your xhci example points out having things
working (and that means offering all possible functionality) is
more important.
As said before reality trumps the Spec / theory.
rather then peripheral mode which is the default for systems which lack an
id-pin. This patch set introduces:
"phy-sun4i-usb: Add "allwinner,usb0-usb-a-connector" dt property"
Which allows specifying the use of a female USB-A connector for the
musb controller in the phy dt node, the presence of this dt property
changes the default to host mode.
This is unnecessary, if using a type-A connector, dr_mode should be
"host" in DT.
Which means we're telling users what they can and cannot do with
their hardware, which we should not be doing.
To summary up my opinion from my lengthy comments above,
A-A cable should not be used, especially in role-switching cases, so
the DT prop is unnecessary;
A<_>A cables exist, users will try to plug them in, those are facts.
So we'd better be prepared to deal with this, denying this scenario
exists is not helpful, not supporting it is equally unhelpful.
Regards,
Hans
--
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
