On Mon, Aug 22, 2016 at 09:08:29AM -0500, 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. > > > > > >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. > > > 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. > > > > > >>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 s/should be/shouldn't be/ > 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? 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. > > > > > 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; > > > > > Regards, > > > > Hans > > Regards, > -Bin. > -- 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
