Re: [PATCH 1/3] usb: core: add power sequence for USB devices

2 messages, 2 authors, 2016-03-05 · open the first message on its own page

Re: [PATCH 1/3] usb: core: add power sequence for USB devices

From: Rob Herring <hidden>
Date: 2016-03-05 04:28:54

On Fri, Mar 04, 2016 at 10:37:50AM +0800, Peter Chen wrote:
On Fri, Mar 04, 2016 at 03:23:05AM +0100, Andrew Lunn wrote:
quoted
On Fri, Mar 04, 2016 at 10:02:42AM +0800, Peter Chen wrote:
quoted
On Thu, Mar 03, 2016 at 02:54:55PM -0600, Rob Herring wrote:
quoted
On Thu, Mar 3, 2016 at 4:01 AM, Peter Chen [off-list ref] wrote:
quoted
Some hard-wired USB devices need to do power sequence to let the
device work normally, the typical power sequence like: enable USB
PHY clock, toggle reset pin, etc. But current Linux USB driver
lacks of such code to do it, it may cause some hard-wired USB devices
works abnormal or can't be recognized by controller at all.

In this patch, it will do power on sequence at hub's probe for all
devices under this hub (includes root hub) if this device is described
at dts and there is a phandle "usb-pwrseq" for it. At hub_disconnect,
it will do power off sequence.

Signed-off-by: Peter Chen <redacted>
---
 .../devicetree/bindings/usb/usb-device.txt         |  41 +++++-
 drivers/usb/core/Makefile                          |   2 +-
 drivers/usb/core/hub.c                             |  32 +++++
 drivers/usb/core/pwrseq.c                          | 149 +++++++++++++++++++++
 include/linux/usb/of.h                             |  10 ++
 5 files changed, 232 insertions(+), 2 deletions(-)
 create mode 100644 drivers/usb/core/pwrseq.c
diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt b/Documentation/devicetree/bindings/usb/usb-device.txt
index 1c35e7b..c7a298c 100644
--- a/Documentation/devicetree/bindings/usb/usb-device.txt
+++ b/Documentation/devicetree/bindings/usb/usb-device.txt
@@ -13,8 +13,37 @@ Required properties:
 - reg: the port number which this device is connecting to, the range
   is 1-31.

+Optional properties:
+- usb-pwrseq: the power sequence handler which need to do before this USB
+  device can work.
+- clocks: the input clock for USB device.
+- clock-frequency: the frequency for device's clock.
+- reset-gpios: Should specify the GPIO for reset.
+- reset-duration-us: the duration in microsecond for assert reset signal.
So we sorted out how to describe USB devices in DT, but now we don't
use it?
We only know USB device after USB bus finds it, but without power
on sequence, the USB bus can't find it.
Not really true. Device tree says it exists, you just cannot see it
yet. The fact it is in device tree means it is soldered on the board
and really is there. So when the host controller enumerates the bus,
it should run the power sequence of all child nodes to make them
appear.
Exactly.
Yes, it is my patch doing. My words "We only know USB device after USB
bus finds it" means the USB common code only create the USB device, and
assign its .of_node after USB bus finds it.
I understand the problem. It is just as easy for you to search power 
sequence child nodes as searching the *actual* child nodes for devices. 
The main difference is that a power sequence node indicates that power 
sequencing can be handled generically. With the device nodes, you have 
to assume that the presence of standard properties like reset-gpios 
means you can handle it generically. 

Either solution suffers from not handling cases that can't be handled 
generically. No doubt, we'll just see continued extensions to keep 
things generic when they really shouldn't be.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: core: add power sequence for USB devices

From: Peter Chen <hidden>
Date: 2016-03-05 08:33:48

On Fri, Mar 04, 2016 at 10:28:54PM -0600, Rob Herring wrote:
On Fri, Mar 04, 2016 at 10:37:50AM +0800, Peter Chen wrote:
quoted
On Fri, Mar 04, 2016 at 03:23:05AM +0100, Andrew Lunn wrote:
quoted
On Fri, Mar 04, 2016 at 10:02:42AM +0800, Peter Chen wrote:
quoted
On Thu, Mar 03, 2016 at 02:54:55PM -0600, Rob Herring wrote:
quoted
On Thu, Mar 3, 2016 at 4:01 AM, Peter Chen [off-list ref] wrote:
quoted
Some hard-wired USB devices need to do power sequence to let the
device work normally, the typical power sequence like: enable USB
PHY clock, toggle reset pin, etc. But current Linux USB driver
lacks of such code to do it, it may cause some hard-wired USB devices
works abnormal or can't be recognized by controller at all.

In this patch, it will do power on sequence at hub's probe for all
devices under this hub (includes root hub) if this device is described
at dts and there is a phandle "usb-pwrseq" for it. At hub_disconnect,
it will do power off sequence.

Signed-off-by: Peter Chen <redacted>
---
 .../devicetree/bindings/usb/usb-device.txt         |  41 +++++-
 drivers/usb/core/Makefile                          |   2 +-
 drivers/usb/core/hub.c                             |  32 +++++
 drivers/usb/core/pwrseq.c                          | 149 +++++++++++++++++++++
 include/linux/usb/of.h                             |  10 ++
 5 files changed, 232 insertions(+), 2 deletions(-)
 create mode 100644 drivers/usb/core/pwrseq.c
diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt b/Documentation/devicetree/bindings/usb/usb-device.txt
index 1c35e7b..c7a298c 100644
--- a/Documentation/devicetree/bindings/usb/usb-device.txt
+++ b/Documentation/devicetree/bindings/usb/usb-device.txt
@@ -13,8 +13,37 @@ Required properties:
 - reg: the port number which this device is connecting to, the range
   is 1-31.

+Optional properties:
+- usb-pwrseq: the power sequence handler which need to do before this USB
+  device can work.
+- clocks: the input clock for USB device.
+- clock-frequency: the frequency for device's clock.
+- reset-gpios: Should specify the GPIO for reset.
+- reset-duration-us: the duration in microsecond for assert reset signal.
So we sorted out how to describe USB devices in DT, but now we don't
use it?
We only know USB device after USB bus finds it, but without power
on sequence, the USB bus can't find it.
Not really true. Device tree says it exists, you just cannot see it
yet. The fact it is in device tree means it is soldered on the board
and really is there. So when the host controller enumerates the bus,
it should run the power sequence of all child nodes to make them
appear.
Exactly.
quoted
Yes, it is my patch doing. My words "We only know USB device after USB
bus finds it" means the USB common code only create the USB device, and
assign its .of_node after USB bus finds it.
I understand the problem. It is just as easy for you to search power 
sequence child nodes as searching the *actual* child nodes for devices. 
The main difference is that a power sequence node indicates that power 
sequencing can be handled generically. With the device nodes, you have 
to assume that the presence of standard properties like reset-gpios 
means you can handle it generically. 

Either solution suffers from not handling cases that can't be handled 
generically. No doubt, we'll just see continued extensions to keep 
things generic when they really shouldn't be.
So, would you like to accept the generic solution like below:

- Create a generic power sequence driver, and it will be probed
according to compatible string at device tree. At its probe,
we can create a power sequence structure, and let this structure
as the private data for this power sequence device.

struct pwrseq
{
	(*on) (struct pwrseq *);
	(*off) (struct pwrseq *);
	...
};

At power sequence driver ->probe
{
	pwrseq = devm_kzalloc(dev, sizeof(*pwrseq), GFP_KERNEL);
	pwrseq->on = generic_pwrseq_on(pwrseq);
	pwrseq->on = generic_pwrseq_off(pwrseq);
	dev_set_drvdata(dev, pwrseq);
}


- At device driver which need to use power sequence operation
	- Get the node according to phandle, and get its private data
	accordingly. The driver can assign its pwrseq to its structure
       	or not.
	- The driver can call its pwrseq->on and pwrseq->off
	accordingly.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help