Date:   Thu, 3 Sep 2020 01:46:18 +0000
From:   Peter Chen <peter.chen@....com>
To:     Matthias Kaehlcke <mka@...omium.org>
CC:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Ravi Chandra Sadineni <ravisadineni@...omium.org>,
        Douglas Anderson <dianders@...omium.org>,
        Bastien Nocera <hadess@...ess.net>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Stephen Boyd <swboyd@...omium.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        Alan Stern <stern@...land.harvard.edu>,
        "Alexander A. Klimov" <grandmaster@...klimov.de>,
        Masahiro Yamada <masahiroy@...nel.org>
Subject: 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 = <&reg_vdd_hub_usb2415>;

		 genesys: ethernet@1 {
			 compatible = "usb5e3,608";
			 reg = <1>;
			 vdd-supply = <&reg_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.chen@nxp.com/

-- 

Thanks,
Peter Chen

Powered by blists - more mailing lists