Re: [RFC PATCH] USB: misc: Add usb_hub_pwr driver
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 +, 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
Re: [RFC PATCH] USB: misc: Add usb_hub_pwr driver
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.
Below is the example of DT entry, there is a dwc3 host, and one USB
HUB on it, there is a onboard USB ethernet chip on HUB's port 1.
&usb_1_dwc3 {
status = "okay";
usb2415: hub@1 {
compatible = "usb424,2514";
reg = <1>;
clocks = <&clk, IMX6QDL_CLK_CKO>;
reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
reset-duration-us = <3000>;
vdd-supply = <®_vdd_hub_usb2415>;
genesys: ethernet@1 {
compatible = "usb5e3,608";
reg = <1>;
vdd-supply = <®_vdd_genesys>;
};
};
>
> I guess when registering the platform device we could pass it the
> corresponding DT node, to allow it to determine its regulators, GPIOs,
> etc during probe.
>
> > - After this usb-power-supply device is probed, do initial power on at
> > probe, eg, clock, regulator, reset-gpio.
> > - This usb-power-supply device system suspend operation should be called
> > after
> > onboard device has suspended since it is created before it. No runtime PM
> > ops
> > are needed for it.
> > - When the hub is removed, delete this platform device.
>
> What exactly is removal? It seems once the hub is 'removed' it could never be
> probed again because the platform device that is in charge of the
> initialization is only created when the USB controller is initialized. I have
> the impression that the platform device would have to exist as long as the USB
> controller.
This USB power supply device services USB devices on HUB ports (HUB is
also one of USB devices). For USB devices under the roothub, it will be
powered on when the roothub is probed.[1]
[1]
https://lore.kernel.org/lkml/1498027328-25078-5-git-send-email-peter.c...@nxp.com/
--
Thanks,
Peter Chen
Re: [RFC PATCH] USB: misc: Add usb_hub_pwr driver
Hi Greg, On Wed, Sep 02, 2020 at 08:07:44AM +0200, Greg Kroah-Hartman wrote: > On Tue, Sep 01, 2020 at 01:21:43PM -0700, Matthias Kaehlcke wrote: > > diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile > > index da39bddb0604..2bd02388ca62 100644 > > --- a/drivers/usb/misc/Makefile > > +++ b/drivers/usb/misc/Makefile > > @@ -31,3 +31,4 @@ obj-$(CONFIG_USB_CHAOSKEY)+= chaoskey.o > > > > obj-$(CONFIG_USB_SISUSBVGA)+= sisusbvga/ > > obj-$(CONFIG_USB_LINK_LAYER_TEST) += lvstest.o > > +obj-$(CONFIG_USB_HUB_PWR) += usb_hub_pwr.o usb_hub_psupply.o > > Why is this 2 files? Why can't it just be one? It's effectively two drivers that work together, it seemed cleaner to me to have a file for every driver. The 'usb_hub_psupply' driver (which probably should be named 'onboard_usb_hub' or similar) would even be useful on its own (taking care of powering the hub on and potentially other setup actions) with a bit of rework. > And isn't this much the same thing as many of the other "misc" hub > controller drivers we have? There are some commonalities, however most of these drivers seem to target USB hubs with an I2C bus, using custom 'commands' for initialization and putting the hub in/out of standby. The drivers in this patch have two goals: - provide a generic (configurable) solution for powering up/initializing a USB hub - enable support for powering down a USB hub during system suspend > And to make it easier to review, can you split out the device tree > descriptions so that the DT maintainers can review that? Sure, I wasn't sure if this is the right approach, hence I only sent an RFC without formal bindings to get initial feedback. As of now it seems that you are at least not frontally opposed it ;-)
Re: [RFC PATCH] USB: misc: Add usb_hub_pwr driver
Hi Peter, On Wed, Sep 02, 2020 at 05:31:06AM +, Peter Chen wrote: > On 20-09-01 13:21:43, Matthias Kaehlcke wrote: > > The driver combo usb_hub_pwr/usb_hub_psupply allows to control > > the power supply of an onboard USB hub. > > > > The drivers address two issues: > > - a USB hub needs to be powered before it can be discovered > > - battery powered devices may want to switch the USB hub off > >during suspend to extend battery life > > > > The regulator of the hub is controlled by the usb_hub_psupply > > platform driver. The regulator is switched on when the platform > > device is initialized, which enables discovery of the hub. The > > driver provides an external interface to enable/disable the > > power supply which is used by the usb_hub_pwr driver. > > > > The usb_hub_pwr extends the generic USB hub driver. The device is > > initialized when the hub is discovered by the USB subsystem. It > > uses the usb_hub_psupply interface to make its own request to > > enable the regulator (increasing the use count to 2). > > > > During system suspend usb_hub_pwr checks if any wakeup capable > > devices are connected to the hub. If not it 'disables' the hub > > regulator (decreasing the use count to 1, hence the regulator > > stays enabled for now). When the usb_hub_psupply device suspends > > it disables the hub regulator unconditionally (decreasing the use > > count to 0 or 1, depending on the actions of usb_hub_pwr). This > > is done to allow the usb_hub_pwr device to control the state of > > the regulator during system suspend. > > > > Upon resume usb_hub_psupply enables the regulator again, the > > usb_hub_pwr device does the same if it disabled the regulator > > during resume. > > 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? I guess when registering the platform device we could pass it the corresponding DT node, to allow it to determine its regulators, GPIOs, etc during probe. > - After this usb-power-supply device is probed, do initial power on at > probe, eg, clock, regulator, reset-gpio. > - This usb-power-supply device system suspend operation should be called after > onboard device has suspended since it is created before it. No runtime PM ops > are needed for it. > - When the hub is removed, delete this platform device. What exactly is removal? It seems once the hub is 'removed' it could never be probed again because the platform device that is in charge of the initialization is only created when the USB controller is initialized. I have the impression that the platform device would have to exist as long as the USB controller.
Re: [RFC PATCH] USB: misc: Add usb_hub_pwr driver
On Tue, Sep 01, 2020 at 01:21:43PM -0700, Matthias Kaehlcke wrote: > diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile > index da39bddb0604..2bd02388ca62 100644 > --- a/drivers/usb/misc/Makefile > +++ b/drivers/usb/misc/Makefile > @@ -31,3 +31,4 @@ obj-$(CONFIG_USB_CHAOSKEY) += chaoskey.o > > obj-$(CONFIG_USB_SISUSBVGA) += sisusbvga/ > obj-$(CONFIG_USB_LINK_LAYER_TEST)+= lvstest.o > +obj-$(CONFIG_USB_HUB_PWR)+= usb_hub_pwr.o usb_hub_psupply.o Why is this 2 files? Why can't it just be one? And isn't this much the same thing as many of the other "misc" hub controller drivers we have? And to make it easier to review, can you split out the device tree descriptions so that the DT maintainers can review that? thanks, greg k-h
Re: [RFC PATCH] USB: misc: Add usb_hub_pwr driver
On 20-09-01 13:21:43, Matthias Kaehlcke wrote:
> The driver combo usb_hub_pwr/usb_hub_psupply allows to control
> the power supply of an onboard USB hub.
>
> The drivers address two issues:
> - a USB hub needs to be powered before it can be discovered
> - battery powered devices may want to switch the USB hub off
>during suspend to extend battery life
>
> The regulator of the hub is controlled by the usb_hub_psupply
> platform driver. The regulator is switched on when the platform
> device is initialized, which enables discovery of the hub. The
> driver provides an external interface to enable/disable the
> power supply which is used by the usb_hub_pwr driver.
>
> The usb_hub_pwr extends the generic USB hub driver. The device is
> initialized when the hub is discovered by the USB subsystem. It
> uses the usb_hub_psupply interface to make its own request to
> enable the regulator (increasing the use count to 2).
>
> During system suspend usb_hub_pwr checks if any wakeup capable
> devices are connected to the hub. If not it 'disables' the hub
> regulator (decreasing the use count to 1, hence the regulator
> stays enabled for now). When the usb_hub_psupply device suspends
> it disables the hub regulator unconditionally (decreasing the use
> count to 0 or 1, depending on the actions of usb_hub_pwr). This
> is done to allow the usb_hub_pwr device to control the state of
> the regulator during system suspend.
>
> Upon resume usb_hub_psupply enables the regulator again, the
> usb_hub_pwr device does the same if it disabled the regulator
> during resume.
Hi Matthias,
I did similar several years ago [1], but the concept (power sequence)
doesn't be accepted by power maintainer. 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.
- After this usb-power-supply device is probed, do initial power on at
probe, eg, clock, regulator, reset-gpio.
- This usb-power-supply device system suspend operation should be called after
onboard device has suspended since it is created before it. No runtime PM ops
are needed for it.
- When the hub is removed, delete this platform device.
What's your opinion?
[1]
https://lore.kernel.org/lkml/1498027328-25078-1-git-send-email-peter.c...@nxp.com/
Peter
>
> Co-developed-by: Ravi Chandra Sadineni
> Signed-off-by: Ravi Chandra Sadineni
> Signed-off-by: Matthias Kaehlcke
> ---
> The driver currently only supports a single power supply. This should
> work for most/many configurations/hubs, support for multiple power
> supplies can be added later if needed.
>
> No DT bindings are included since this is just a RFC. Here is a DT
> example:
>
> usb_hub_psupply: usb-hub-psupply {
> compatible = "linux,usb_hub_psupply";
> vdd-supply = <&pp3300_hub>;
> };
>
> &usb_1_dwc3 {
> /* 2.0 hub on port 1 */
> hub@1 {
> compatible = "usbbda,5411";
> reg = <1>;
> psupply = <&usb_hub_psupply>;
> };
>
> /* 3.0 hub on port 2 */
> hub@2 {
> compatible = "usbbda,411";
> reg = <2>;
> psupply = <&usb_hub_psupply>;
> };
> };
>
> drivers/usb/misc/Kconfig | 14 +++
> drivers/usb/misc/Makefile | 1 +
> drivers/usb/misc/usb_hub_psupply.c | 112 ++
> drivers/usb/misc/usb_hub_psupply.h | 9 ++
> drivers/usb/misc/usb_hub_pwr.c | 177 +
> 5 files changed, 313 insertions(+)
> create mode 100644 drivers/usb/misc/usb_hub_psupply.c
> create mode 100644 drivers/usb/misc/usb_hub_psupply.h
> create mode 100644 drivers/usb/misc/usb_hub_pwr.c
>
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index 6818ea689cd9..79ed50e6a7bf 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -275,3 +275,17 @@ config USB_CHAOSKEY
>
> To compile this driver as a module, choose M here: the
> module will be called chaoskey.
> +
> +config USB_HUB_PWR
> + tristate "Control power supply for onboard USB hubs"
> + depends on PM
> + help
> + Say Y here if you want to control the power supply of an
> + onboard USB hub. The driver switches the power supply of the
> + hub on, to make sure the hub can be discovered. During system
> + suspend the power supply is switched off, unless a wakeup
> + capable device is connected to the hub. This may reduce power
> + consumption on battery powered devices.
> +
> + To compile this driver as a
Re: [RFC PATCH] USB: misc: Add usb_hub_pwr driver
Hi Doug,
On Tue, Sep 01, 2020 at 02:21:49PM -0700, Doug Anderson wrote:
> Hi,
>
> On Tue, Sep 1, 2020 at 1:21 PM Matthias Kaehlcke wrote:
> >
> > The driver combo usb_hub_pwr/usb_hub_psupply allows to control
> > the power supply of an onboard USB hub.
> >
> > The drivers address two issues:
> > - a USB hub needs to be powered before it can be discovered
> > - battery powered devices may want to switch the USB hub off
> >during suspend to extend battery life
> >
> > The regulator of the hub is controlled by the usb_hub_psupply
> > platform driver. The regulator is switched on when the platform
> > device is initialized, which enables discovery of the hub. The
> > driver provides an external interface to enable/disable the
> > power supply which is used by the usb_hub_pwr driver.
> >
> > The usb_hub_pwr extends the generic USB hub driver. The device is
> > initialized when the hub is discovered by the USB subsystem. It
> > uses the usb_hub_psupply interface to make its own request to
> > enable the regulator (increasing the use count to 2).
> >
> > During system suspend usb_hub_pwr checks if any wakeup capable
> > devices are connected to the hub. If not it 'disables' the hub
> > regulator (decreasing the use count to 1, hence the regulator
> > stays enabled for now). When the usb_hub_psupply device suspends
> > it disables the hub regulator unconditionally (decreasing the use
> > count to 0 or 1, depending on the actions of usb_hub_pwr). This
> > is done to allow the usb_hub_pwr device to control the state of
> > the regulator during system suspend.
> >
> > Upon resume usb_hub_psupply enables the regulator again, the
> > usb_hub_pwr device does the same if it disabled the regulator
> > during resume.
> >
> > Co-developed-by: Ravi Chandra Sadineni
> > Signed-off-by: Ravi Chandra Sadineni
> > Signed-off-by: Matthias Kaehlcke
> > ---
> > The driver currently only supports a single power supply. This should
> > work for most/many configurations/hubs, support for multiple power
> > supplies can be added later if needed.
> >
> > No DT bindings are included since this is just a RFC. Here is a DT
> > example:
> >
> > usb_hub_psupply: usb-hub-psupply {
> > compatible = "linux,usb_hub_psupply";
> > vdd-supply = <&pp3300_hub>;
> > };
>
> Definitely bikeshedding, but I would name this differently. The
> name/compatible you have makes this sound as if it's a software
> concept that we're sticking into DT. That's generally discouraged.
> ...if we name it slightly different then I think the driver can work
> the same but be more in the spirit of DT describing hardware.
> Specifically, I think it'd be better as:
>
> usb_hub: usb-hub {
> compatible = "realtek,rts5411", "onboard-usb-hub";
> vdd-supply = <&pp3300_hub>;
> };
>
> Now we're describing hardware that's on the board. We have a RTS5411
> hub and we've described its power supply. There's also precedent for
> describing an on-board USB hub in a lop-level node like this
> (smsc,usb3503).
You are right, it definitely describes existing hardware, not some
linux-ism.
I also envisioned that there might be eventually device specific
compatible strings to support multiple regulators or even hub specific
power on/off sequences.
> Now, I know what you're saying: we're already describing this hub
> underneath the USB port node below. I think this is OK to have
> different aspects of the device described in two places in the DT,
> though of course I could be corrected by someone more knowledgeable.
>
>
> > &usb_1_dwc3 {
> > /* 2.0 hub on port 1 */
> > hub@1 {
> > compatible = "usbbda,5411";
>
> What is "usbbda"? I would probably just call this:
>
> compatible = "realtek,rts5411-usb2", "onboard-usb-hub-usb2"
That's from the USB bindings:
Required properties for device nodes:
- compatible: "usbVID,PID", where VID is the vendor id and PID the product id.
The textual representation of VID and PID shall be in lower case hexadecimal
with leading zeroes suppressed. The other compatible strings from the above
standard binding could also be used, but a device adhering to this binding
may leave out all except for "usbVID,PID".
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/usb/usb-device.txt
>
>
> > reg = <1>;
> > psupply = <&usb_hub_psupply>;
>
> Calling this psupply sounds a bit too much like you're referring to a
> regulator (with the -supply suffix). Given that I've proposed calling
> the main device "usb-hub" what about just saying "hub = <&usb_hub>;"
I envisioned it as an abstract regulator (potentially consisting of
multiple regulators), but modelling it with the regulator framework
also didn't seem the right thing to do.
Using the hub abstraction sounds good to me. Maybe we should also come
up with a better name for 'usb_hub_psupply', I never was overly fond
of it.
> > };
> >
> > /* 3.0 hub on port 2 */
> > hub@2 {
> > compat
Re: [RFC PATCH] USB: misc: Add usb_hub_pwr driver
Hi,
On Tue, Sep 1, 2020 at 1:21 PM Matthias Kaehlcke wrote:
>
> The driver combo usb_hub_pwr/usb_hub_psupply allows to control
> the power supply of an onboard USB hub.
>
> The drivers address two issues:
> - a USB hub needs to be powered before it can be discovered
> - battery powered devices may want to switch the USB hub off
>during suspend to extend battery life
>
> The regulator of the hub is controlled by the usb_hub_psupply
> platform driver. The regulator is switched on when the platform
> device is initialized, which enables discovery of the hub. The
> driver provides an external interface to enable/disable the
> power supply which is used by the usb_hub_pwr driver.
>
> The usb_hub_pwr extends the generic USB hub driver. The device is
> initialized when the hub is discovered by the USB subsystem. It
> uses the usb_hub_psupply interface to make its own request to
> enable the regulator (increasing the use count to 2).
>
> During system suspend usb_hub_pwr checks if any wakeup capable
> devices are connected to the hub. If not it 'disables' the hub
> regulator (decreasing the use count to 1, hence the regulator
> stays enabled for now). When the usb_hub_psupply device suspends
> it disables the hub regulator unconditionally (decreasing the use
> count to 0 or 1, depending on the actions of usb_hub_pwr). This
> is done to allow the usb_hub_pwr device to control the state of
> the regulator during system suspend.
>
> Upon resume usb_hub_psupply enables the regulator again, the
> usb_hub_pwr device does the same if it disabled the regulator
> during resume.
>
> Co-developed-by: Ravi Chandra Sadineni
> Signed-off-by: Ravi Chandra Sadineni
> Signed-off-by: Matthias Kaehlcke
> ---
> The driver currently only supports a single power supply. This should
> work for most/many configurations/hubs, support for multiple power
> supplies can be added later if needed.
>
> No DT bindings are included since this is just a RFC. Here is a DT
> example:
>
> usb_hub_psupply: usb-hub-psupply {
> compatible = "linux,usb_hub_psupply";
> vdd-supply = <&pp3300_hub>;
> };
Definitely bikeshedding, but I would name this differently. The
name/compatible you have makes this sound as if it's a software
concept that we're sticking into DT. That's generally discouraged.
...if we name it slightly different then I think the driver can work
the same but be more in the spirit of DT describing hardware.
Specifically, I think it'd be better as:
usb_hub: usb-hub {
compatible = "realtek,rts5411", "onboard-usb-hub";
vdd-supply = <&pp3300_hub>;
};
Now we're describing hardware that's on the board. We have a RTS5411
hub and we've described its power supply. There's also precedent for
describing an on-board USB hub in a lop-level node like this
(smsc,usb3503).
Now, I know what you're saying: we're already describing this hub
underneath the USB port node below. I think this is OK to have
different aspects of the device described in two places in the DT,
though of course I could be corrected by someone more knowledgeable.
> &usb_1_dwc3 {
> /* 2.0 hub on port 1 */
> hub@1 {
> compatible = "usbbda,5411";
What is "usbbda"? I would probably just call this:
compatible = "realtek,rts5411-usb2", "onboard-usb-hub-usb2"
> reg = <1>;
> psupply = <&usb_hub_psupply>;
Calling this psupply sounds a bit too much like you're referring to a
regulator (with the -supply suffix). Given that I've proposed calling
the main device "usb-hub" what about just saying "hub = <&usb_hub>;"
> };
>
> /* 3.0 hub on port 2 */
> hub@2 {
> compatible = "usbbda,411";
Similar to above:
compatible = "realtek,rts5411-usb3", "onboard-usb-hub-usb3"
> reg = <2>;
> psupply = <&usb_hub_psupply>;
> };
> };
>
> drivers/usb/misc/Kconfig | 14 +++
> drivers/usb/misc/Makefile | 1 +
> drivers/usb/misc/usb_hub_psupply.c | 112 ++
> drivers/usb/misc/usb_hub_psupply.h | 9 ++
> drivers/usb/misc/usb_hub_pwr.c | 177 +
> 5 files changed, 313 insertions(+)
> create mode 100644 drivers/usb/misc/usb_hub_psupply.c
> create mode 100644 drivers/usb/misc/usb_hub_psupply.h
> create mode 100644 drivers/usb/misc/usb_hub_pwr.c
>
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index 6818ea689cd9..79ed50e6a7bf 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -275,3 +275,17 @@ config USB_CHAOSKEY
>
> To compile this driver as a module, choose M here: the
> module will be called chaoskey.
> +
> +config USB_HUB_PWR
> + tristate "Control power supply for onboard USB hubs"
> + depends on PM
> + help
> + Say Y here if you want to control the power supply of an
> + onboard USB hub. The driver switches the power supply of the
> + hub on, to make sure the hub can be discovered. During system
> + suspend the power
[RFC PATCH] USB: misc: Add usb_hub_pwr driver
The driver combo usb_hub_pwr/usb_hub_psupply allows to control
the power supply of an onboard USB hub.
The drivers address two issues:
- a USB hub needs to be powered before it can be discovered
- battery powered devices may want to switch the USB hub off
during suspend to extend battery life
The regulator of the hub is controlled by the usb_hub_psupply
platform driver. The regulator is switched on when the platform
device is initialized, which enables discovery of the hub. The
driver provides an external interface to enable/disable the
power supply which is used by the usb_hub_pwr driver.
The usb_hub_pwr extends the generic USB hub driver. The device is
initialized when the hub is discovered by the USB subsystem. It
uses the usb_hub_psupply interface to make its own request to
enable the regulator (increasing the use count to 2).
During system suspend usb_hub_pwr checks if any wakeup capable
devices are connected to the hub. If not it 'disables' the hub
regulator (decreasing the use count to 1, hence the regulator
stays enabled for now). When the usb_hub_psupply device suspends
it disables the hub regulator unconditionally (decreasing the use
count to 0 or 1, depending on the actions of usb_hub_pwr). This
is done to allow the usb_hub_pwr device to control the state of
the regulator during system suspend.
Upon resume usb_hub_psupply enables the regulator again, the
usb_hub_pwr device does the same if it disabled the regulator
during resume.
Co-developed-by: Ravi Chandra Sadineni
Signed-off-by: Ravi Chandra Sadineni
Signed-off-by: Matthias Kaehlcke
---
The driver currently only supports a single power supply. This should
work for most/many configurations/hubs, support for multiple power
supplies can be added later if needed.
No DT bindings are included since this is just a RFC. Here is a DT
example:
usb_hub_psupply: usb-hub-psupply {
compatible = "linux,usb_hub_psupply";
vdd-supply = <&pp3300_hub>;
};
&usb_1_dwc3 {
/* 2.0 hub on port 1 */
hub@1 {
compatible = "usbbda,5411";
reg = <1>;
psupply = <&usb_hub_psupply>;
};
/* 3.0 hub on port 2 */
hub@2 {
compatible = "usbbda,411";
reg = <2>;
psupply = <&usb_hub_psupply>;
};
};
drivers/usb/misc/Kconfig | 14 +++
drivers/usb/misc/Makefile | 1 +
drivers/usb/misc/usb_hub_psupply.c | 112 ++
drivers/usb/misc/usb_hub_psupply.h | 9 ++
drivers/usb/misc/usb_hub_pwr.c | 177 +
5 files changed, 313 insertions(+)
create mode 100644 drivers/usb/misc/usb_hub_psupply.c
create mode 100644 drivers/usb/misc/usb_hub_psupply.h
create mode 100644 drivers/usb/misc/usb_hub_pwr.c
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 6818ea689cd9..79ed50e6a7bf 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -275,3 +275,17 @@ config USB_CHAOSKEY
To compile this driver as a module, choose M here: the
module will be called chaoskey.
+
+config USB_HUB_PWR
+ tristate "Control power supply for onboard USB hubs"
+ depends on PM
+ help
+ Say Y here if you want to control the power supply of an
+ onboard USB hub. The driver switches the power supply of the
+ hub on, to make sure the hub can be discovered. During system
+ suspend the power supply is switched off, unless a wakeup
+ capable device is connected to the hub. This may reduce power
+ consumption on battery powered devices.
+
+ To compile this driver as a module, choose M here: the
+ module will be called usb_hub_pwr.
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index da39bddb0604..2bd02388ca62 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_USB_CHAOSKEY)+= chaoskey.o
obj-$(CONFIG_USB_SISUSBVGA)+= sisusbvga/
obj-$(CONFIG_USB_LINK_LAYER_TEST) += lvstest.o
+obj-$(CONFIG_USB_HUB_PWR) += usb_hub_pwr.o usb_hub_psupply.o
diff --git a/drivers/usb/misc/usb_hub_psupply.c
b/drivers/usb/misc/usb_hub_psupply.c
new file mode 100644
index ..6a155ae1f831
--- /dev/null
+++ b/drivers/usb/misc/usb_hub_psupply.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, Google LLC
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct usb_hub_psupply_dev {
+ struct regulator *vdd;
+};
+
+int usb_hub_psupply_on(struct device *dev)
+{
+ struct usb_hub_psupply_dev *usb_hub_psupply = dev_get_drvdata(dev);
+ int err;
+
+ err = regulator_enable(usb_hub_psupply->vdd);
+ if (err) {
+ dev_err(dev, "failed to enable regulator: %d\n", err);
+ return err;
+ }
+
+ return 0;
+
+}
+EXPORT_SYMBOL_GPL(usb_hub_psupply_on);
+
+int usb_hub_psupply_off(struct device *dev)
+{
+ struct usb_hub_psuppl
