diff mbox series

[RFC] USB: misc: Add usb_hub_pwr driver

Message ID 20200901132005.RFC.1.I7c9a1f1d6ced41dd8310e8a03da666a32364e790@changeid
State New
Headers show
Series [RFC] USB: misc: Add usb_hub_pwr driver | expand

Commit Message

Matthias Kaehlcke Sept. 1, 2020, 8:21 p.m. UTC
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 <ravisadineni@chromium.org>
Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
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

Comments

Matthias Kaehlcke Sept. 14, 2020, 6:42 p.m. UTC | #1
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
diff mbox series

Patch

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 000000000000..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 <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+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_psupply_dev *usb_hub_psupply = dev_get_drvdata(dev);
+	int err;
+
+	err = regulator_disable(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_off);
+
+static int usb_hub_psupply_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct usb_hub_psupply_dev *usb_hub_psupply;
+
+	usb_hub_psupply = devm_kzalloc(dev, sizeof(*usb_hub_psupply), GFP_KERNEL);
+	if (!usb_hub_psupply)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, usb_hub_psupply);
+
+	usb_hub_psupply->vdd = devm_regulator_get(dev, "vdd");
+	if (IS_ERR(usb_hub_psupply->vdd))
+		return PTR_ERR(usb_hub_psupply->vdd);
+
+	return usb_hub_psupply_on(dev);
+}
+
+static int usb_hub_psupply_remove(struct platform_device *pdev)
+{
+	return usb_hub_psupply_off(&pdev->dev);
+}
+
+static int usb_hub_psupply_suspend(struct platform_device *pdev, pm_message_t msg)
+{
+	return usb_hub_psupply_off(&pdev->dev);
+}
+
+static int usb_hub_psupply_resume(struct platform_device *pdev)
+{
+	return usb_hub_psupply_on(&pdev->dev);
+}
+
+static const struct of_device_id usb_hub_psupply_match[] = {
+	{ .compatible = "linux,usb_hub_psupply" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, usb_hub_psupply_match);
+
+static struct platform_driver usb_hub_psupply_driver = {
+	.probe = usb_hub_psupply_probe,
+	.remove = usb_hub_psupply_remove,
+	.suspend = usb_hub_psupply_suspend,
+	.resume = usb_hub_psupply_resume,
+	.driver = {
+		.name = "usb-hub-psupply",
+		.of_match_table = usb_hub_psupply_match,
+	},
+};
+
+static int __init usb_hub_psupply_init(void)
+{
+	return platform_driver_register(&usb_hub_psupply_driver);
+}
+device_initcall(usb_hub_psupply_init);
+
+static void __exit usb_hub_psupply_exit(void)
+{
+	platform_driver_unregister(&usb_hub_psupply_driver);
+}
+module_exit(usb_hub_psupply_exit);
+
+MODULE_AUTHOR("Matthias Kaehlcke <mka@chromium.org>");
+MODULE_DESCRIPTION("USB Hub Power Supply");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/usb/misc/usb_hub_psupply.h b/drivers/usb/misc/usb_hub_psupply.h
new file mode 100644
index 000000000000..284e88f45fcf
--- /dev/null
+++ b/drivers/usb/misc/usb_hub_psupply.h
@@ -0,0 +1,9 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _USB_HUB_PSUPPLY_H
+#define _USB_HUB_PSUPPLY_H
+
+int usb_hub_psupply_on(struct device *dev);
+int usb_hub_psupply_off(struct device *dev);
+
+#endif /* _USB_HUB_PSUPPLY_H */
diff --git a/drivers/usb/misc/usb_hub_pwr.c b/drivers/usb/misc/usb_hub_pwr.c
new file mode 100644
index 000000000000..33945ca4a8c0
--- /dev/null
+++ b/drivers/usb/misc/usb_hub_pwr.c
@@ -0,0 +1,177 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * USB hub power control
+ *
+ * Copyright (c) 2020, Google LLC
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/power_supply.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+#include "../core/usb.h"
+#include "usb_hub_psupply.h"
+
+#define VENDOR_ID_REALTEK	0x0bda
+
+struct usb_hub_pwr_dev {
+	struct regulator *vdd;
+	struct device *psupply_dev;
+	bool powered_off;
+};
+
+static struct device *usb_pwr_find_psupply_dev(struct device *dev)
+{
+	const phandle *ph;
+	struct device_node *np;
+	struct platform_device *pdev;
+
+	ph = of_get_property(dev->of_node, "psupply", NULL);
+	if (!ph) {
+		dev_err(dev, "failed to read 'psupply' property\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	np = of_find_node_by_phandle(be32_to_cpu(*ph));
+	if (!np) {
+		dev_err(dev, "failed find device node for power supply\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pdev = of_find_device_by_node(np);
+	of_node_put(np);
+	if (!pdev)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return &pdev->dev;
+}
+
+static int usb_hub_pwr_probe(struct usb_device *udev)
+{
+	struct device *dev = &udev->dev;
+	struct usb_hub_pwr_dev *uhpw;
+	struct device *psupply_dev;
+	int err;
+
+	/* ignore supported hubs without device tree node */
+	if (!dev->of_node)
+		return -ENODEV;
+
+	psupply_dev = usb_pwr_find_psupply_dev(dev);
+	if (IS_ERR(psupply_dev))
+		return PTR_ERR(psupply_dev);
+
+	err = usb_generic_driver_probe(udev);
+	if (err) {
+		put_device(psupply_dev);
+		return err;
+	}
+
+	uhpw = devm_kzalloc(dev, sizeof(*uhpw), GFP_KERNEL);
+	if (!uhpw) {
+		put_device(psupply_dev);
+		return -ENOMEM;
+	}
+
+	dev_set_drvdata(&udev->dev, uhpw);
+
+	uhpw->psupply_dev = psupply_dev;
+
+	err = usb_hub_psupply_on(psupply_dev);
+	if (err) {
+		dev_err(dev, "failed to enable regulator: %d\n", err);
+		put_device(psupply_dev);
+		return err;
+	}
+
+	return 0;
+}
+
+static void usb_hub_pwr_disconnect(struct usb_device *udev)
+{
+	struct usb_hub_pwr_dev *uhpw = dev_get_drvdata(&udev->dev);
+
+	usb_hub_psupply_off(uhpw->psupply_dev);
+	put_device(uhpw->psupply_dev);
+}
+
+static int usb_hub_pwr_suspend(struct usb_device *udev, pm_message_t msg)
+{
+	struct usb_hub_pwr_dev *uhpw = dev_get_drvdata(&udev->dev);
+	int err;
+
+	err = usb_generic_driver_suspend(udev, msg);
+	if (err)
+		return err;
+
+	if (!usb_wakeup_enabled_descendants(udev)) {
+		usb_port_disable(udev);
+
+		err = usb_hub_psupply_off(uhpw->psupply_dev);
+		if (err)
+			return err;
+
+		uhpw->powered_off = true;
+	}
+
+	return 0;
+}
+
+static int usb_hub_pwr_resume(struct usb_device *udev, pm_message_t msg)
+{
+	struct usb_hub_pwr_dev *uhpw = dev_get_drvdata(&udev->dev);
+	int err;
+
+	if (uhpw->powered_off) {
+		err = usb_hub_psupply_on(uhpw->psupply_dev);
+		if (err)
+			return err;
+
+		uhpw->powered_off = false;
+	}
+
+	return usb_generic_driver_resume(udev, msg);
+}
+
+static const struct usb_device_id hub_id_table[] = {
+	{ .idVendor = VENDOR_ID_REALTEK,
+	  .idProduct = 0x0411, /* RTS5411 USB 3.0 */
+	  .match_flags = USB_DEVICE_ID_MATCH_DEVICE },
+	{ .idVendor = VENDOR_ID_REALTEK,
+	  .idProduct = 0x5411, /* RTS5411 USB 2.0 */
+	  .match_flags = USB_DEVICE_ID_MATCH_DEVICE },
+	{},
+};
+
+MODULE_DEVICE_TABLE(usb, hub_id_table);
+
+static struct usb_device_driver usb_hub_pwr_driver = {
+
+	.name = "usb-hub-pwr",
+	.probe = usb_hub_pwr_probe,
+	.disconnect = usb_hub_pwr_disconnect,
+	.suspend = usb_hub_pwr_suspend,
+	.resume = usb_hub_pwr_resume,
+	.id_table = hub_id_table,
+};
+
+static int __init usb_hub_pwr_driver_init(void)
+{
+	return usb_register_device_driver(&usb_hub_pwr_driver, THIS_MODULE);
+}
+
+static void __exit usb_hub_pwr_driver_exit(void)
+{
+	usb_deregister_device_driver(&usb_hub_pwr_driver);
+}
+
+module_init(usb_hub_pwr_driver_init);
+module_exit(usb_hub_pwr_driver_exit);
+
+MODULE_AUTHOR("Matthias Kaehlcke <mka@chromium.org>");
+MODULE_DESCRIPTION("USB Hub Power Control");
+MODULE_LICENSE("GPL v2");