diff mbox

[2/3] doc: dt-binding: generic onboard USB HUB

Message ID 1449538670-7954-3-git-send-email-peter.chen@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Chen Dec. 8, 2015, 1:37 a.m. UTC
Add dt-binding documentation for generic onboard USB HUB.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 .../bindings/usb/generic-onboard-hub.txt           | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/generic-onboard-hub.txt

Comments

Fabio Estevam Dec. 8, 2015, 2:30 a.m. UTC | #1
On Mon, Dec 7, 2015 at 11:37 PM, Peter Chen <peter.chen@freescale.com> wrote:
> Add dt-binding documentation for generic onboard USB HUB.
>
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  .../bindings/usb/generic-onboard-hub.txt           | 28 ++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> new file mode 100644
> index 0000000..ea92205
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> @@ -0,0 +1,28 @@
> +Generic Onboard USB HUB
> +
> +Required properties:
> +- compatible: should be "generic-onboard-hub"
> +
> +Optional properties:
> +- clocks: the input clock for HUB.
> +
> +- clock-names: Should be "external_clk"
> +
> +- hub-reset-gpios: Should specify the GPIO for reset.
> +
> +- hub-reset-active-high: the active reset signal is high, if this property is
> +  not set, the active reset signal is low.
> +
> +- hub-reset-duration-us: the duration for assert reset signal, the time unit
> +  is microsecond.
> +
> +Example:
> +
> +       usb_hub1 {
> +               compatible = "generic-onboard-hub";
> +               clocks = <&clks IMX6QDL_CLK_CKO>;
> +               clock-names = "external_clk";
> +               hub-reset-gpios = <&gpio7 12 0>;
> +               hub-reset-active-high;

I think you could drop the 'hub-reset-active-high' property and do
like this instead:

hub-reset-gpios = <&gpio7 12 GPIO_ACTIVE_HIGH>;

or

hub-reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
Peter Chen Dec. 8, 2015, 9:20 a.m. UTC | #2
On Tue, Dec 08, 2015 at 12:30:59AM -0200, Fabio Estevam wrote:
> On Mon, Dec 7, 2015 at 11:37 PM, Peter Chen <peter.chen@freescale.com> wrote:
> > Add dt-binding documentation for generic onboard USB HUB.
> >
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > ---
> >  .../bindings/usb/generic-onboard-hub.txt           | 28 ++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> >
> > diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > new file mode 100644
> > index 0000000..ea92205
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > @@ -0,0 +1,28 @@
> > +Generic Onboard USB HUB
> > +
> > +Required properties:
> > +- compatible: should be "generic-onboard-hub"
> > +
> > +Optional properties:
> > +- clocks: the input clock for HUB.
> > +
> > +- clock-names: Should be "external_clk"
> > +
> > +- hub-reset-gpios: Should specify the GPIO for reset.
> > +
> > +- hub-reset-active-high: the active reset signal is high, if this property is
> > +  not set, the active reset signal is low.
> > +
> > +- hub-reset-duration-us: the duration for assert reset signal, the time unit
> > +  is microsecond.
> > +
> > +Example:
> > +
> > +       usb_hub1 {
> > +               compatible = "generic-onboard-hub";
> > +               clocks = <&clks IMX6QDL_CLK_CKO>;
> > +               clock-names = "external_clk";
> > +               hub-reset-gpios = <&gpio7 12 0>;
> > +               hub-reset-active-high;
> 
> I think you could drop the 'hub-reset-active-high' property and do
> like this instead:
> 
> hub-reset-gpios = <&gpio7 12 GPIO_ACTIVE_HIGH>;
> 
> or
> 
> hub-reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;

Yes, you are right, would you have a test to see if can work for
udoo board?
Arnd Bergmann Dec. 8, 2015, 9:45 a.m. UTC | #3
On Tuesday 08 December 2015 17:20:46 Peter Chen wrote:
> On Tue, Dec 08, 2015 at 12:30:59AM -0200, Fabio Estevam wrote:
> > On Mon, Dec 7, 2015 at 11:37 PM, Peter Chen <peter.chen@freescale.com> wrote:
> > > Add dt-binding documentation for generic onboard USB HUB.
> > >
> > > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > > ---
> > >  .../bindings/usb/generic-onboard-hub.txt           | 28 ++++++++++++++++++++++
> > >  1 file changed, 28 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > new file mode 100644
> > > index 0000000..ea92205
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > @@ -0,0 +1,28 @@
> > > +Generic Onboard USB HUB
> > > +
> > > +Required properties:
> > > +- compatible: should be "generic-onboard-hub"
> > > +
> > > +Optional properties:
> > > +- clocks: the input clock for HUB.
> > > +
> > > +- clock-names: Should be "external_clk"

Make this just "external", or remove the name and use it as an anonymous clock.

> > > +- hub-reset-gpios: Should specify the GPIO for reset.
> > > +
> > > +- hub-reset-active-high: the active reset signal is high, if this property is
> > > +  not set, the active reset signal is low.
> > > +
> > > +- hub-reset-duration-us: the duration for assert reset signal, the time unit
> > > +  is microsecond.

Remove the "hub-" prefix here.

> > > +Example:
> > > +
> > > +       usb_hub1 {
> > > +               compatible = "generic-onboard-hub";
> > > +               clocks = <&clks IMX6QDL_CLK_CKO>;
> > > +               clock-names = "external_clk";
> > > +               hub-reset-gpios = <&gpio7 12 0>;
> > > +               hub-reset-active-high;
> > 
> > I think you could drop the 'hub-reset-active-high' property and do
> > like this instead:
> > 
> > hub-reset-gpios = <&gpio7 12 GPIO_ACTIVE_HIGH>;
> > 
> > or
> > 
> > hub-reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
> 
> Yes, you are right, would you have a test to see if can work for
> udoo board?

Yes, it should be done like this.

	Arnd
Philipp Zabel Dec. 8, 2015, 9:50 a.m. UTC | #4
Hi Peter,

Am Dienstag, den 08.12.2015, 09:37 +0800 schrieb Peter Chen:
> Add dt-binding documentation for generic onboard USB HUB.
> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  .../bindings/usb/generic-onboard-hub.txt           | 28 ++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> new file mode 100644
> index 0000000..ea92205
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> @@ -0,0 +1,28 @@
> +Generic Onboard USB HUB
>+
> +Required properties:
> +- compatible: should be "generic-onboard-hub"

This something we don't have to define ad-hoc. The hub hangs off an USB
controller, right? The "Open Firmware recommended practice: USB"
document already describes how to represent USB devices in a generic
manner:
http://www.firmware.org/1275/bindings/usb/usb-1_0.ps

Is there a reason not to reuse this?

The usb hub node would be a child of the usb controller node, and it
could use
	compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */


> +Optional properties:
> +- clocks: the input clock for HUB.
> +
> +- clock-names: Should be "external_clk"

Is clock-names necessary for a single clock?

> +- hub-reset-gpios: Should specify the GPIO for reset.

I'd prefer that to be just "reset-gpios", it is the only reset property
in the hub node after all. And use the GPIO_ACTIVE_HIGH/LOW flags to
indicate polarity.

> +- hub-reset-active-high: the active reset signal is high, if this property is
> +  not set, the active reset signal is low.

Then this could be dropped.

> +- hub-reset-duration-us: the duration for assert reset signal, the time unit
> +  is microsecond.

And consequently, this could just be called "reset-duration-us".
It might make sense to define the same for other reset GPIOs, too.
The Freescale FEC, for example, has "phy-reset-duration" (in ms)
already.

> +
> +Example:
> +
> +	usb_hub1 {
> +		compatible = "generic-onboard-hub";
> +		clocks = <&clks IMX6QDL_CLK_CKO>;
> +		clock-names = "external_clk";
> +		hub-reset-gpios = <&gpio7 12 0>;
> +		hub-reset-active-high;
> +		hub-reset-duration-us = <2>;
> +	};

best regards
Philipp
Arnd Bergmann Dec. 8, 2015, 9:58 a.m. UTC | #5
On Tuesday 08 December 2015 10:50:49 Philipp Zabel wrote:
> Am Dienstag, den 08.12.2015, 09:37 +0800 schrieb Peter Chen:
> > Add dt-binding documentation for generic onboard USB HUB.
> > 
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > ---
> >  .../bindings/usb/generic-onboard-hub.txt           | 28 ++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > new file mode 100644
> > index 0000000..ea92205
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > @@ -0,0 +1,28 @@
> > +Generic Onboard USB HUB
> >+
> > +Required properties:
> > +- compatible: should be "generic-onboard-hub"
> 
> This something we don't have to define ad-hoc. The hub hangs off an USB
> controller, right? The "Open Firmware recommended practice: USB"
> document already describes how to represent USB devices in a generic
> manner:
> http://www.firmware.org/1275/bindings/usb/usb-1_0.ps
> 
> Is there a reason not to reuse this?
> 
> The usb hub node would be a child of the usb controller node, and it
> could use
> 	compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */

Good point, I had not thought of that when I looked at the patches.

Yes, let's do this way. I don't know if we ever implemented the simple
patch to associate a USB device with a device_node, but if not, then
let's do it now for this driver. A lot of people have asked for it in
the past.

	Arnd
Rob Herring (Arm) Dec. 9, 2015, 3:24 a.m. UTC | #6
On Tue, Dec 08, 2015 at 10:58:48AM +0100, Arnd Bergmann wrote:
> On Tuesday 08 December 2015 10:50:49 Philipp Zabel wrote:
> > Am Dienstag, den 08.12.2015, 09:37 +0800 schrieb Peter Chen:
> > > Add dt-binding documentation for generic onboard USB HUB.
> > > 
> > > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > > ---
> > >  .../bindings/usb/generic-onboard-hub.txt           | 28 ++++++++++++++++++++++
> > >  1 file changed, 28 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > new file mode 100644
> > > index 0000000..ea92205
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > @@ -0,0 +1,28 @@
> > > +Generic Onboard USB HUB
> > >+
> > > +Required properties:
> > > +- compatible: should be "generic-onboard-hub"
> > 
> > This something we don't have to define ad-hoc. The hub hangs off an USB
> > controller, right? The "Open Firmware recommended practice: USB"
> > document already describes how to represent USB devices in a generic
> > manner:
> > http://www.firmware.org/1275/bindings/usb/usb-1_0.ps
> > 
> > Is there a reason not to reuse this?
> > 
> > The usb hub node would be a child of the usb controller node, and it
> > could use
> > 	compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */
> 
> Good point, I had not thought of that when I looked at the patches.
>  
> Yes, let's do this way. I don't know if we ever implemented the simple
> patch to associate a USB device with a device_node, but if not, then
> let's do it now for this driver. A lot of people have asked for it in
> the past.

Agreed. Also, some hubs have I2C buses as well, but I still think under 
the USB bus is the right place.

However, one complication here is often (probably this case) these 
addtional signals need to be controlled before the device enumerates.

Rob
Peter Chen Dec. 9, 2015, 8:09 a.m. UTC | #7
On Tue, Dec 08, 2015 at 10:50:49AM +0100, Philipp Zabel wrote:
> Hi Peter,
> 
> Am Dienstag, den 08.12.2015, 09:37 +0800 schrieb Peter Chen:
> > Add dt-binding documentation for generic onboard USB HUB.
> > 
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > ---
> >  .../bindings/usb/generic-onboard-hub.txt           | 28 ++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > new file mode 100644
> > index 0000000..ea92205
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > @@ -0,0 +1,28 @@
> > +Generic Onboard USB HUB
> >+
> > +Required properties:
> > +- compatible: should be "generic-onboard-hub"
> 
> This something we don't have to define ad-hoc. The hub hangs off an USB
> controller, right? The "Open Firmware recommended practice: USB"
> document already describes how to represent USB devices in a generic
> manner:
> http://www.firmware.org/1275/bindings/usb/usb-1_0.ps
> 
> Is there a reason not to reuse this?
> 
> The usb hub node would be a child of the usb controller node, and it
> could use
> 	compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */
> 

Before writing this driver, I did consider how to let the USB core
handle device tree, but without good solution.

The controller (platform) driver should not handle specific USB device
described at device tree, since we want this handling to be generic.
And the USB device may not be recognized by controller without handling
its properties, like clock, reset pins.

With your suggestion, the dts likes below

usbh1: usb@02184000 {
	compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
	reg = <0x02184000 0x200>;
	interrupts = <0 43 IRQ_TYPE_LEVEL_HIGH>;
	clocks = <&clks IMX6QDL_CLK_USBOH3>;
	usb_hub@1 {
		compatible = "usb,class9"
		clocks = <&clks USB_HUB_CLK>;
		....
	};
};

I don't know how to handle above at USB bus, if you have any
suggestions, give me some tips please.

> 
> > +Optional properties:
> > +- clocks: the input clock for HUB.
> > +
> > +- clock-names: Should be "external_clk"
> 
> Is clock-names necessary for a single clock?
> 
> > +- hub-reset-gpios: Should specify the GPIO for reset.
> 
> I'd prefer that to be just "reset-gpios", it is the only reset property
> in the hub node after all. And use the GPIO_ACTIVE_HIGH/LOW flags to
> indicate polarity.

I will change to "clk" and "reset-gpios".

> 
> > +- hub-reset-active-high: the active reset signal is high, if this property is
> > +  not set, the active reset signal is low.
> 
> Then this could be dropped.
> 
> > +- hub-reset-duration-us: the duration for assert reset signal, the time unit
> > +  is microsecond.
> 
> And consequently, this could just be called "reset-duration-us".
> It might make sense to define the same for other reset GPIOs, too.
> The Freescale FEC, for example, has "phy-reset-duration" (in ms)
> already.
> 

Agree.

By the way: Felipe suggest using generic reset gpio driver to handle
above, I find you have written similar things before [1], why it can't be
accepted?

[1]http://comments.gmane.org/gmane.linux.ports.arm.kernel/255129
Peter Chen Dec. 9, 2015, 8:12 a.m. UTC | #8
On Tue, Dec 08, 2015 at 09:24:03PM -0600, Rob Herring wrote:
> On Tue, Dec 08, 2015 at 10:58:48AM +0100, Arnd Bergmann wrote:
> > On Tuesday 08 December 2015 10:50:49 Philipp Zabel wrote:
> > > Am Dienstag, den 08.12.2015, 09:37 +0800 schrieb Peter Chen:
> > > > Add dt-binding documentation for generic onboard USB HUB.
> > > > 
> > > > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > > > ---
> > > >  .../bindings/usb/generic-onboard-hub.txt           | 28 ++++++++++++++++++++++
> > > >  1 file changed, 28 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > > new file mode 100644
> > > > index 0000000..ea92205
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > > @@ -0,0 +1,28 @@
> > > > +Generic Onboard USB HUB
> > > >+
> > > > +Required properties:
> > > > +- compatible: should be "generic-onboard-hub"
> > > 
> > > This something we don't have to define ad-hoc. The hub hangs off an USB
> > > controller, right? The "Open Firmware recommended practice: USB"
> > > document already describes how to represent USB devices in a generic
> > > manner:
> > > http://www.firmware.org/1275/bindings/usb/usb-1_0.ps
> > > 
> > > Is there a reason not to reuse this?
> > > 
> > > The usb hub node would be a child of the usb controller node, and it
> > > could use
> > > 	compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */
> > 
> > Good point, I had not thought of that when I looked at the patches.
> >  
> > Yes, let's do this way. I don't know if we ever implemented the simple
> > patch to associate a USB device with a device_node, but if not, then
> > let's do it now for this driver. A lot of people have asked for it in
> > the past.
> 
> Agreed. Also, some hubs have I2C buses as well, but I still think under 
> the USB bus is the right place.
> 
> However, one complication here is often (probably this case) these 
> addtional signals need to be controlled before the device enumerates.
> 

Yes, I did not find a way to let the USB bus code handle it, so I had to
write a platform driver to do it
Arnd Bergmann Dec. 9, 2015, 8:55 a.m. UTC | #9
On Wednesday 09 December 2015 16:12:24 Peter Chen wrote:
> On Tue, Dec 08, 2015 at 09:24:03PM -0600, Rob Herring wrote:
> > On Tue, Dec 08, 2015 at 10:58:48AM +0100, Arnd Bergmann wrote:
> > > On Tuesday 08 December 2015 10:50:49 Philipp Zabel wrote:
> > > > This something we don't have to define ad-hoc. The hub hangs off an USB
> > > > controller, right? The "Open Firmware recommended practice: USB"
> > > > document already describes how to represent USB devices in a generic
> > > > manner:
> > > > http://www.firmware.org/1275/bindings/usb/usb-1_0.ps
> > > > 
> > > > Is there a reason not to reuse this?
> > > > 
> > > > The usb hub node would be a child of the usb controller node, and it
> > > > could use
> > > > 	compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */
> > > 
> > > Good point, I had not thought of that when I looked at the patches.
> > >  
> > > Yes, let's do this way. I don't know if we ever implemented the simple
> > > patch to associate a USB device with a device_node, but if not, then
> > > let's do it now for this driver. A lot of people have asked for it in
> > > the past.
> > 
> > Agreed. Also, some hubs have I2C buses as well, but I still think under 
> > the USB bus is the right place.
> > 
> > However, one complication here is often (probably this case) these 
> > addtional signals need to be controlled before the device enumerates.
> > 
> 
> Yes, I did not find a way to let the USB bus code handle it, so I had to
> write a platform driver to do it

Looping in Ulf, he solved the same problem for SDIO devices recently,
and probably remembers the details best.

	Arnd
Ulf Hansson Dec. 15, 2015, 8:21 p.m. UTC | #10
On 9 December 2015 at 09:55, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 09 December 2015 16:12:24 Peter Chen wrote:
>> On Tue, Dec 08, 2015 at 09:24:03PM -0600, Rob Herring wrote:
>> > On Tue, Dec 08, 2015 at 10:58:48AM +0100, Arnd Bergmann wrote:
>> > > On Tuesday 08 December 2015 10:50:49 Philipp Zabel wrote:
>> > > > This something we don't have to define ad-hoc. The hub hangs off an USB
>> > > > controller, right? The "Open Firmware recommended practice: USB"
>> > > > document already describes how to represent USB devices in a generic
>> > > > manner:
>> > > > http://www.firmware.org/1275/bindings/usb/usb-1_0.ps
>> > > >
>> > > > Is there a reason not to reuse this?
>> > > >
>> > > > The usb hub node would be a child of the usb controller node, and it
>> > > > could use
>> > > >         compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */
>> > >
>> > > Good point, I had not thought of that when I looked at the patches.
>> > >
>> > > Yes, let's do this way. I don't know if we ever implemented the simple
>> > > patch to associate a USB device with a device_node, but if not, then
>> > > let's do it now for this driver. A lot of people have asked for it in
>> > > the past.
>> >
>> > Agreed. Also, some hubs have I2C buses as well, but I still think under
>> > the USB bus is the right place.
>> >
>> > However, one complication here is often (probably this case) these
>> > addtional signals need to be controlled before the device enumerates.
>> >
>>
>> Yes, I did not find a way to let the USB bus code handle it, so I had to
>> write a platform driver to do it
>
> Looping in Ulf, he solved the same problem for SDIO devices recently,
> and probably remembers the details best.
>

Thanks Arnd!

Yes, that was a kind of a long outstanding issue we have had for SDIO devices.

Several generic attempts was made to have a framework/library
available to support so called "power sequences" for exactly the same
reasons as above.
Others and myself failed to get those attempts accepted.

Instead, I invented a mmc subsystem specific DT based solution, the
"mmc-pwrseq".

DT documentation:
Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
Documentation/devicetree/bindings/mmc/mmc.txt

Long story short: The mmc host device may contain a phandle to a
mmc-pwrseq, which will describe the resources needed to power on/off
the SDIO card.

The code is available at:
drivers/mmc/core/pwrseq*

We didn't implement this as platform driver, but that was mostly
because I initially wanted things to be simple. Although, nothing
prevents us from converting to this as a follow up, which would make
the solution a bit less "hacky".

Kind regards
Uffe
Peter Chen Dec. 16, 2015, 2:46 a.m. UTC | #11
On Tue, Dec 15, 2015 at 09:21:09PM +0100, Ulf Hansson wrote:
> On 9 December 2015 at 09:55, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 09 December 2015 16:12:24 Peter Chen wrote:
> >> On Tue, Dec 08, 2015 at 09:24:03PM -0600, Rob Herring wrote:
> >> > On Tue, Dec 08, 2015 at 10:58:48AM +0100, Arnd Bergmann wrote:
> >> > > On Tuesday 08 December 2015 10:50:49 Philipp Zabel wrote:
> >> > > > This something we don't have to define ad-hoc. The hub hangs off an USB
> >> > > > controller, right? The "Open Firmware recommended practice: USB"
> >> > > > document already describes how to represent USB devices in a generic
> >> > > > manner:
> >> > > > http://www.firmware.org/1275/bindings/usb/usb-1_0.ps
> >> > > >
> >> > > > Is there a reason not to reuse this?
> >> > > >
> >> > > > The usb hub node would be a child of the usb controller node, and it
> >> > > > could use
> >> > > >         compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */
> >> > >
> >> > > Good point, I had not thought of that when I looked at the patches.
> >> > >
> >> > > Yes, let's do this way. I don't know if we ever implemented the simple
> >> > > patch to associate a USB device with a device_node, but if not, then
> >> > > let's do it now for this driver. A lot of people have asked for it in
> >> > > the past.
> >> >
> >> > Agreed. Also, some hubs have I2C buses as well, but I still think under
> >> > the USB bus is the right place.
> >> >
> >> > However, one complication here is often (probably this case) these
> >> > addtional signals need to be controlled before the device enumerates.
> >> >
> >>
> >> Yes, I did not find a way to let the USB bus code handle it, so I had to
> >> write a platform driver to do it
> >
> > Looping in Ulf, he solved the same problem for SDIO devices recently,
> > and probably remembers the details best.
> >
> 
> Thanks Arnd!
> 
> Yes, that was a kind of a long outstanding issue we have had for SDIO devices.
> 
> Several generic attempts was made to have a framework/library
> available to support so called "power sequences" for exactly the same
> reasons as above.
> Others and myself failed to get those attempts accepted.
> 
> Instead, I invented a mmc subsystem specific DT based solution, the
> "mmc-pwrseq".
> 
> DT documentation:
> Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> Documentation/devicetree/bindings/mmc/mmc.txt
> 
> Long story short: The mmc host device may contain a phandle to a
> mmc-pwrseq, which will describe the resources needed to power on/off
> the SDIO card.
> 
> The code is available at:
> drivers/mmc/core/pwrseq*
> 
> We didn't implement this as platform driver, but that was mostly
> because I initially wanted things to be simple. Although, nothing
> prevents us from converting to this as a follow up, which would make
> the solution a bit less "hacky".
> 

Thanks for your information, Ulf.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
new file mode 100644
index 0000000..ea92205
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
@@ -0,0 +1,28 @@ 
+Generic Onboard USB HUB
+
+Required properties:
+- compatible: should be "generic-onboard-hub"
+
+Optional properties:
+- clocks: the input clock for HUB.
+
+- clock-names: Should be "external_clk"
+
+- hub-reset-gpios: Should specify the GPIO for reset.
+
+- hub-reset-active-high: the active reset signal is high, if this property is
+  not set, the active reset signal is low.
+
+- hub-reset-duration-us: the duration for assert reset signal, the time unit
+  is microsecond.
+
+Example:
+
+	usb_hub1 {
+		compatible = "generic-onboard-hub";
+		clocks = <&clks IMX6QDL_CLK_CKO>;
+		clock-names = "external_clk";
+		hub-reset-gpios = <&gpio7 12 0>;
+		hub-reset-active-high;
+		hub-reset-duration-us = <2>;
+	};