On Thu, Jun 02, 2022 at 03:16:27PM -0400, Alan Stern wrote:
On Thu, Jun 02, 2022 at 04:59:18PM +0200, Michael Grzeschik wrote:On Thu, Jun 02, 2022 at 10:39:54AM -0400, Alan Stern wrote: > You might want to disable the new sysfs file (don't create it or have it > return -EOPNOTSUPP) if the hub doesn't support per-port power switching. Is it possible to read out if this feature is not working by the hub?Actually, I don't think so. You can get some information about ganged power switching, and there's the hub_is_port_power_switchable() test, but that's all. The situation is discussed in section 11.11 (Hub Port Power Control) of the USB-2.0 spec.
I think we can skip this check then, since this sysfs file could still work with hubs, that do not support port power switching by at least disabling the port connection.
The most hubs, that I was working with, did not really toggle the vbus, because the physical logic to switch a regulator was completely missing in the hardware. But with removing the other PORT_FEATURES the hub behaved like the port is just not powered any more.Yes, that's how most hubs work. There are a few, however, which really do switch port Vbus power on and off.Because of that; I am currently curious if we just should rename that property to something more generic like "enable" or "disable". So that as the real vbus power switching is missing, the hubs port switching does still function like intended.That makes sense. But the question arises, does this patch really do what you want? The patch description talks about the need to disable devices or re-enumerate them. You can disable a device right now by writing -1 to the bConfigurationValue sysfs file, and you can force a device to be re-enumerated by resetting it (using the USBDEVFS_RESET usbfs ioctl). About the only thing you can't currently do is actually turn off power to the port. This patch will allow users to do that, but only if the hub supports power switching. (Okay, there's one other thing: The patch also allows users to disable a port, so that devices plugged into that port get ignored. Maybe that's what you really had in mind...?)
Yes, that is what I had in mind. If you agree, I would still keep the name "port_power" since it is the main function, but skip the hub_is_port_power_switchable check.
> Finally, you should add a test to port_event() in hub.c, probably where > the pm_runtime_active() check is. If the port is powered off, return > before doing any of the warm_reset or connect_change processing. I don't understand jet why this is needed. In all my tests, the hubs port was just not functioning any more. Regardless if the hub was capable of real vbus switching or not. Just as described above. Is it possible that this is already handled correctly because of the other cleared port_features I mentioned?The USB spec does say that hubs should ignore connections to ports whose port_power feature flag is off. The test I suggested was meant as a "just in case" sort of thing, for hubs that don't comply with the spec's requirement. In the end it may not be necessary, and it can be done in a separate patch.
Agreed.
In v3 I will also add port_power_show to make it possible for the userspace to read out the current port status but returning the value of test_bit(port1, hub->power_bits);.Good idea; I should have thought of it.
:) Thanks, Michael -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature
