Hi Peter, sorry for the delayed reply, I got distracted by other things and ran into issues with my mail setup. On Thu, Sep 03, 2020 at 01:46:18AM +0000, Peter Chen wrote: > On 20-09-02 10:45:36, Matthias Kaehlcke wrote: > > > > > > Hi Matthias, > > > > > > I did similar several years ago [1], but the concept (power sequence) > > > doesn't be accepted by power maintainer. > > > > Yeah, I saw that, I think I even reviewed or tested some early version > > of it :) > > > > > Your patch introduce an new way to fix this long-term issue, I have an > > > idea to fix it more generally. > > > > > - Create a table (say usb_pm_table) for USB device which needs to do > > > initial power on and power management during suspend suspend/resume based > > > on VID and PID, example: usb/core/quirks.c > > > - After hub (both roothub and intermediate hub) device is created, search > > > the DT node under this hub, and see if the device is in usb_pm_table. If > > > it is in it, create a platform device, say usb-power-supply, and the > > > related driver is like your usb_hub_psupply.c, the parent of this device > > > is controller device. > > > > This part isn't clear to me. How would the DT look like? Would it have a > > single node per physical hub chip or one node for each 'logical' hub? > > Similarly, would there be a single plaform device or multiple? > > One power supply platform device for one physical device, and DT only > describes physical device. HUB driver only probes non-SS HUB port to > avoid create two power supply device for SS HUB, there should be no > SS-only HUB. I agree that there should be only one platform device per physical device. Probing only the non-SS hub should work to avoid multiple instances, however it doesn't work for the extended use case, where the hub is powered off during system suspend, but only when no wakeup capable devices are connected to any of the 'logical' hubs. For this to work the driver that controls the regulators, GPIOs, ... needs to have knowledge of all 'logical' hubs. I just sent v1 of this driver, which reworks things a bit, but for now there is still one platform device instantiated through the DT, and one DT entry for every 'logical' hub. I'm open to keep discussing alternative designs, as long as they can also cover the use case of conditionally powering the hub off during system suspend. We can probably continue the discussion on v1, unless it takes me longer than
