@@ -0,0 +1,31 @@+The power sequence for generic USB Devices++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. The+power sequence will be done before this device can be found at USB+bus.++Required properties:+- compatible : contains "usb-pwrseq-generic".++Optional properties:+- 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.+- enable-gpios: Should specify the GPIO for enable.+- power-supply: The regulator of the power++Example:++usb2415_pwrseq: usb2415_pwrseq {+ compatible = "usb-pwrseq-generic";+ clocks = <&clks IMX6QDL_CLK_CKO>;+ reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;+ reset-duration-us = <3000>;+ enable-gpios = <&gpio7 11 GPIO_ACTIVE_LOW>;+ power-supply = <®_usb_power>;+};
@@ -12,6 +12,7 @@ Required properties: for usbVID,PID. - reg: the port number which this device is connecting to, the range is 1-31.+- usb-pwrseq: phandle for power sequence. Example:
@@ -0,0 +1,31 @@+The power sequence for generic USB Devices++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. The+power sequence will be done before this device can be found at USB+bus.++Required properties:+- compatible : contains "usb-pwrseq-generic".
In case I have not been clear, no.
I am not going to accept anything along the lines of the current mmc
pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
an added property and not a duplication of information. I'd suggest
you figure out how to make the kernel work with that rather than
trying to work-around whatever kernel limitations there are.
Rob
@@ -0,0 +1,31 @@+The power sequence for generic USB Devices++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. The+power sequence will be done before this device can be found at USB+bus.++Required properties:+- compatible : contains "usb-pwrseq-generic".
In case I have not been clear, no.
I am not going to accept anything along the lines of the current mmc
pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
an added property and not a duplication of information. I'd suggest
you figure out how to make the kernel work with that rather than
trying to work-around whatever kernel limitations there are.
I see.
Would you agree with below:
&usbotg1 {
vbus-supply = <®_usb_otg1_vbus>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_usb_otg1_id>;
status = "okay";
#address-cells = <1>;
#size-cells = <0>;
hub: genesys at 1 {
compatible = "usb5e3,608";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
reset-duration-us = <10>;
clocks = <&clks IMX6SX_CLK_CKO>;
#address-cells = <1>;
#size-cells = <0>;
ethernet: asix at 1 {
compatible = "usbb95,1708";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
reset-duration-us = <15>;
clocks = <&clks IMX6SX_CLK_IPG>;
};
};
};
If the node has property "power-sequence", the pwrseq core will create
related platform device, and the driver under pwrseq driver will handle
power sequence stuffs. The property below "power-sequence" will be
handled at pwrseq driver.
--
Best Regards,
Peter Chen
@@ -0,0 +1,31 @@+The power sequence for generic USB Devices++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. The+power sequence will be done before this device can be found at USB+bus.++Required properties:+- compatible : contains "usb-pwrseq-generic".
In case I have not been clear, no.
I am not going to accept anything along the lines of the current mmc
pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
an added property and not a duplication of information. I'd suggest
you figure out how to make the kernel work with that rather than
trying to work-around whatever kernel limitations there are.
I see.
Would you agree with below:
&usbotg1 {
vbus-supply = <®_usb_otg1_vbus>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_usb_otg1_id>;
status = "okay";
#address-cells = <1>;
#size-cells = <0>;
hub: genesys at 1 {
compatible = "usb5e3,608";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
reset-duration-us = <10>;
clocks = <&clks IMX6SX_CLK_CKO>;
#address-cells = <1>;
#size-cells = <0>;
ethernet: asix at 1 {
compatible = "usbb95,1708";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
reset-duration-us = <15>;
clocks = <&clks IMX6SX_CLK_IPG>;
};
};
};
If the node has property "power-sequence", the pwrseq core will create
related platform device, and the driver under pwrseq driver will handle
power sequence stuffs. The property below "power-sequence" will be
handled at pwrseq driver.
Isn't this binding what Krzysztof proposed? His series also provides example
code for enabling USB device power sequencing. Does it not work for you?
Having 2 series by 2 separate people doing the same thing but slightly
different is very confusing.
Regards
ChenYu
@@ -0,0 +1,31 @@+The power sequence for generic USB Devices++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. The+power sequence will be done before this device can be found at USB+bus.++Required properties:+- compatible : contains "usb-pwrseq-generic".
In case I have not been clear, no.
I am not going to accept anything along the lines of the current mmc
pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
an added property and not a duplication of information. I'd suggest
you figure out how to make the kernel work with that rather than
trying to work-around whatever kernel limitations there are.
I see.
Would you agree with below:
In general, yes. There are some points being discussed in the other
thread.
I wonder if this really needs to be specified. A sufficiently long
default should be good enough for 90 something % of devices.
clocks = <&clks IMX6SX_CLK_CKO>;
#address-cells = <1>;
#size-cells = <0>;
ethernet: asix at 1 {
compatible = "usbb95,1708";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
reset-duration-us = <15>;
clocks = <&clks IMX6SX_CLK_IPG>;
};
};
};
If the node has property "power-sequence", the pwrseq core will create
related platform device, and the driver under pwrseq driver will handle
power sequence stuffs.
This I have issue with. If you are creating a platform device here, you
are trying to work-around limitations in the linux driver model. Either
we need some sort of pre-probe hook to the drivers to call or each
parent node driver is responsible for checking and calling pwr-seq
functions for child nodes. e.g. The host controller calls pwr-seq for
the hub, the hub driver calls the power seq for the asix chip. Soon as
we have a case too complex for the generic pwr-seq, we're going to need
the pre-probe hook as I don't want to see a continual expansion of
generic pwr-seq binding for ever more complex cases.
The property below "power-sequence" will be
handled at pwrseq driver.
You cannot rely on order other than for logical readability. There are
no guarantees of either the dtb or kernel ordering of properties.
Rob
From: Mark Brown <hidden> Date: 2016-06-20 17:06:10
On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
quoted
If the node has property "power-sequence", the pwrseq core will create
related platform device, and the driver under pwrseq driver will handle
power sequence stuffs.
This I have issue with. If you are creating a platform device here, you
are trying to work-around limitations in the linux driver model. Either
we need some sort of pre-probe hook to the drivers to call or each
parent node driver is responsible for checking and calling pwr-seq
functions for child nodes. e.g. The host controller calls pwr-seq for
the hub, the hub driver calls the power seq for the asix chip. Soon as
we have a case too complex for the generic pwr-seq, we're going to need
the pre-probe hook as I don't want to see a continual expansion of
generic pwr-seq binding for ever more complex cases.
I think it's fairly clear that we need one or both of these mechanisms
for enumerable buses in embedded contexts - it's something that keeps
croping up.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160620/fb903add/attachment.sig>
@@ -0,0 +1,31 @@+The power sequence for generic USB Devices++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. The+power sequence will be done before this device can be found at USB+bus.++Required properties:+- compatible : contains "usb-pwrseq-generic".
In case I have not been clear, no.
I am not going to accept anything along the lines of the current mmc
pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
an added property and not a duplication of information. I'd suggest
you figure out how to make the kernel work with that rather than
trying to work-around whatever kernel limitations there are.
I see.
Would you agree with below:
In general, yes. There are some points being discussed in the other
thread.
I wonder if this really needs to be specified. A sufficiently long
default should be good enough for 90 something % of devices.
This property is optional, there is a default value in pwrseq driver.
quoted
clocks = <&clks IMX6SX_CLK_CKO>;
#address-cells = <1>;
#size-cells = <0>;
ethernet: asix at 1 {
compatible = "usbb95,1708";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
reset-duration-us = <15>;
clocks = <&clks IMX6SX_CLK_IPG>;
};
};
};
If the node has property "power-sequence", the pwrseq core will create
related platform device, and the driver under pwrseq driver will handle
power sequence stuffs.
This I have issue with. If you are creating a platform device here, you
are trying to work-around limitations in the linux driver model.
My current solution like below, but it seems you didn't agree with that.
I just double confirm here, if you don't, I give up the solution for
using generic power sequence framework.
In drivers/usb/core/hub.c
for_each_child_of_node(parent->of_node, node) {
hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
if (!pwrseq_node) {
ret = -ENOMEM;
goto err1;
}
/* power on sequence */
ret = pwrseq_pre_power_on(hdev_pwrseq);
if (ret)
goto err2;
pwrseq_node->pwrseq_on = hdev_pwrseq;
list_add(&pwrseq_node->list, &hub->pwrseq_list);
} else if (IS_ERR(hdev_pwrseq)) {
return PTR_ERR(hdev_pwrseq);
}
}
In drivers/power/pwrseq/core.c
struct pwrseq *pwrseq_alloc(struct device_node *np, const char *dev_name)
{
struct pwrseq *p, *pwrseq = NULL;
bool created;
/* If there is no device is associated with this node, create it */
if (!of_find_device_by_node(np)) {
if (of_platform_device_create(np, dev_name, NULL))
created = true;
else
return ERR_PTR(-ENODEV);
}
mutex_lock(&pwrseq_list_mutex);
list_for_each_entry(p, &pwrseq_list, pwrseq_node) {
if (p->dev->of_node == np) {
if (!try_module_get(p->owner))
dev_err(p->dev,
"increasing module refcount failed\n");
else
pwrseq = p;
break;
}
}
And there is a platform driver at drivers/power/pwrseq/pwrseq_usb_generic.c
to handle these pwrseq properties, see my patch 9/12.
Either
we need some sort of pre-probe hook to the drivers to call or each
parent node driver is responsible for checking and calling pwr-seq
functions for child nodes. e.g. The host controller calls pwr-seq for
the hub, the hub driver calls the power seq for the asix chip. Soon as
we have a case too complex for the generic pwr-seq, we're going to need
the pre-probe hook as I don't want to see a continual expansion of
generic pwr-seq binding for ever more complex cases.
How the driver know what it needs to handle (eg, gpio, clock) if there
is no device for it? The most important we need to consider is which
device owns there power sequence properties, then the corresponding
driver can handle it.
--
Best Regards,
Peter Chen
@@ -0,0 +1,31 @@+The power sequence for generic USB Devices++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. The+power sequence will be done before this device can be found at USB+bus.++Required properties:+- compatible : contains "usb-pwrseq-generic".
In case I have not been clear, no.
I am not going to accept anything along the lines of the current mmc
pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
an added property and not a duplication of information. I'd suggest
you figure out how to make the kernel work with that rather than
trying to work-around whatever kernel limitations there are.
I see.
Would you agree with below:
&usbotg1 {
vbus-supply = <®_usb_otg1_vbus>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_usb_otg1_id>;
status = "okay";
#address-cells = <1>;
#size-cells = <0>;
hub: genesys at 1 {
compatible = "usb5e3,608";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
reset-duration-us = <10>;
clocks = <&clks IMX6SX_CLK_CKO>;
#address-cells = <1>;
#size-cells = <0>;
ethernet: asix at 1 {
compatible = "usbb95,1708";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
reset-duration-us = <15>;
clocks = <&clks IMX6SX_CLK_IPG>;
};
};
};
If the node has property "power-sequence", the pwrseq core will create
related platform device, and the driver under pwrseq driver will handle
power sequence stuffs. The property below "power-sequence" will be
handled at pwrseq driver.
Isn't this binding what Krzysztof proposed? His series also provides example
code for enabling USB device power sequencing. Does it not work for you?
In this series, I just change RFC to formal patch, and finished his
example code. USB power sequence should be described on USB bus, not
platform bus.
--
Best Regards,
Peter Chen
From: Rob Herring <robh@kernel.org> Date: 2016-06-21 21:26:53
On Tue, Jun 21, 2016 at 10:11:17AM +0800, Peter Chen wrote:
On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
quoted
On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
quoted
On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
quoted
On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen [off-list ref] wrote:
quoted
Add binding doc for generic usb power sequence driver, and update
generic usb device binding-doc accordingly.
[...]
quoted
quoted
clocks = <&clks IMX6SX_CLK_CKO>;
#address-cells = <1>;
#size-cells = <0>;
ethernet: asix at 1 {
compatible = "usbb95,1708";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
reset-duration-us = <15>;
clocks = <&clks IMX6SX_CLK_IPG>;
};
};
};
If the node has property "power-sequence", the pwrseq core will create
related platform device, and the driver under pwrseq driver will handle
power sequence stuffs.
This I have issue with. If you are creating a platform device here, you
are trying to work-around limitations in the linux driver model.
My current solution like below, but it seems you didn't agree with that.
I just double confirm here, if you don't, I give up the solution for
using generic power sequence framework.
In drivers/usb/core/hub.c
for_each_child_of_node(parent->of_node, node) {
hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
if (!pwrseq_node) {
ret = -ENOMEM;
goto err1;
}
/* power on sequence */
ret = pwrseq_pre_power_on(hdev_pwrseq);
Why does this function need to do anything more than:
- Check if the child has a "power-sequence" property
- Get the "reset-gpios" GPIO
- Assert reset for specified/default time
- Deassert reset
Then continue on as normal. That seems straight-forward to me.
There is no reason you need a platform device in the mix. Perhaps trying
to move the MMC pwr-seq code is pointless as it adds needless
complexity.
[...]
quoted
Either
we need some sort of pre-probe hook to the drivers to call or each
parent node driver is responsible for checking and calling pwr-seq
functions for child nodes. e.g. The host controller calls pwr-seq for
the hub, the hub driver calls the power seq for the asix chip. Soon as
we have a case too complex for the generic pwr-seq, we're going to need
the pre-probe hook as I don't want to see a continual expansion of
generic pwr-seq binding for ever more complex cases.
How the driver know what it needs to handle (eg, gpio, clock) if there
is no device for it? The most important we need to consider is which
device owns there power sequence properties, then the corresponding
driver can handle it.
What can be handled by is defined by presence of power-sequence
property. There can be 1 driver for the device. That is the USB hub
driver in this example. You should not have 2 "devices".
Rob
From: Peter Chen <hidden> Date: 2016-06-22 01:14:39
On Tue, Jun 21, 2016 at 04:26:53PM -0500, Rob Herring wrote:
On Tue, Jun 21, 2016 at 10:11:17AM +0800, Peter Chen wrote:
quoted
On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
quoted
On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
quoted
On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
quoted
On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen [off-list ref] wrote:
quoted
Add binding doc for generic usb power sequence driver, and update
generic usb device binding-doc accordingly.
[...]
quoted
quoted
quoted
clocks = <&clks IMX6SX_CLK_CKO>;
#address-cells = <1>;
#size-cells = <0>;
ethernet: asix at 1 {
compatible = "usbb95,1708";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
reset-duration-us = <15>;
clocks = <&clks IMX6SX_CLK_IPG>;
};
};
};
If the node has property "power-sequence", the pwrseq core will create
related platform device, and the driver under pwrseq driver will handle
power sequence stuffs.
This I have issue with. If you are creating a platform device here, you
are trying to work-around limitations in the linux driver model.
My current solution like below, but it seems you didn't agree with that.
I just double confirm here, if you don't, I give up the solution for
using generic power sequence framework.
In drivers/usb/core/hub.c
for_each_child_of_node(parent->of_node, node) {
hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
if (!pwrseq_node) {
ret = -ENOMEM;
goto err1;
}
/* power on sequence */
ret = pwrseq_pre_power_on(hdev_pwrseq);
Why does this function need to do anything more than:
- Check if the child has a "power-sequence" property
- Get the "reset-gpios" GPIO
- Assert reset for specified/default time
- Deassert reset
Besides gpios, it may exist clock and regulator operation, and the
operation may be failed. I thought these operations can be easy
done belongs to a device, but now, let me try this straight-forward
way, thanks.
Peter
Then continue on as normal. That seems straight-forward to me.
There is no reason you need a platform device in the mix. Perhaps trying
to move the MMC pwr-seq code is pointless as it adds needless
complexity.
[...]
quoted
quoted
Either
we need some sort of pre-probe hook to the drivers to call or each
parent node driver is responsible for checking and calling pwr-seq
functions for child nodes. e.g. The host controller calls pwr-seq for
the hub, the hub driver calls the power seq for the asix chip. Soon as
we have a case too complex for the generic pwr-seq, we're going to need
the pre-probe hook as I don't want to see a continual expansion of
generic pwr-seq binding for ever more complex cases.
How the driver know what it needs to handle (eg, gpio, clock) if there
is no device for it? The most important we need to consider is which
device owns there power sequence properties, then the corresponding
driver can handle it.
What can be handled by is defined by presence of power-sequence
property. There can be 1 driver for the device. That is the USB hub
driver in this example. You should not have 2 "devices".
Rob
On 21 June 2016 at 23:26, Rob Herring [off-list ref] wrote:
On Tue, Jun 21, 2016 at 10:11:17AM +0800, Peter Chen wrote:
quoted
On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
quoted
On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
quoted
On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
quoted
On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen [off-list ref] wrote:
quoted
Add binding doc for generic usb power sequence driver, and update
generic usb device binding-doc accordingly.
[...]
quoted
quoted
quoted
clocks = <&clks IMX6SX_CLK_CKO>;
#address-cells = <1>;
#size-cells = <0>;
ethernet: asix at 1 {
compatible = "usbb95,1708";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
reset-duration-us = <15>;
clocks = <&clks IMX6SX_CLK_IPG>;
};
};
};
If the node has property "power-sequence", the pwrseq core will create
related platform device, and the driver under pwrseq driver will handle
power sequence stuffs.
This I have issue with. If you are creating a platform device here, you
are trying to work-around limitations in the linux driver model.
I somewhat understand your point.
Although, having the option to use a driver (which requires a device)
has turned out to be quite convenient from many aspects - at least in
the mmc case.
Certainly one can do without it, but in the end using a driver avoids
open coding.
quoted
My current solution like below, but it seems you didn't agree with that.
I just double confirm here, if you don't, I give up the solution for
using generic power sequence framework.
In drivers/usb/core/hub.c
for_each_child_of_node(parent->of_node, node) {
hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
if (!pwrseq_node) {
ret = -ENOMEM;
goto err1;
}
/* power on sequence */
ret = pwrseq_pre_power_on(hdev_pwrseq);
Why does this function need to do anything more than:
- Check if the child has a "power-sequence" property
- Get the "reset-gpios" GPIO
- Assert reset for specified/default time
- Deassert reset
Then continue on as normal. That seems straight-forward to me.
There is no reason you need a platform device in the mix. Perhaps trying
to move the MMC pwr-seq code is pointless as it adds needless
complexity.
Complexity?
The problem we are tying to solve, is to make the various platform/SoC
specific power sequences to be able to live in generic drivers.
One could decide to encode the sequences inside the driver code
itself, but it will soon turn into a mess and more importantly, lots
of open coding as to support different platforms/SoCs. To most kernel
hackers I don't think this is an option to consider.
The MMC pwr-seq code/drivers tries to address these issues - in a
somewhat generic way.
Initially we have decided to start with only a few flavours of
supported sequences and so far these have been sufficient.
Finally, I am indeed concerned that it so hard to agree on a solution
to deal with generic power sequences. There have been many attempts
throughout the last ~4-5 years, but peoples strong opinions about
different approaches mad them all fail. Isn't it time to finally just
pick a solution and stick with it, even if it doesn't meet all peoples
expectations?
[...]
Kind regards
Uffe
From: Rob Herring <robh@kernel.org> Date: 2016-06-24 15:25:05
On Wed, Jun 22, 2016 at 4:09 AM, Ulf Hansson [off-list ref] wrote:
On 21 June 2016 at 23:26, Rob Herring [off-list ref] wrote:
quoted
On Tue, Jun 21, 2016 at 10:11:17AM +0800, Peter Chen wrote:
quoted
On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
quoted
On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
quoted
On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
quoted
On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen [off-list ref] wrote:
quoted
Add binding doc for generic usb power sequence driver, and update
generic usb device binding-doc accordingly.
[...]
quoted
quoted
quoted
clocks = <&clks IMX6SX_CLK_CKO>;
#address-cells = <1>;
#size-cells = <0>;
ethernet: asix at 1 {
compatible = "usbb95,1708";
reg = <1>;
power-sequence;
reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
reset-duration-us = <15>;
clocks = <&clks IMX6SX_CLK_IPG>;
};
};
};
If the node has property "power-sequence", the pwrseq core will create
related platform device, and the driver under pwrseq driver will handle
power sequence stuffs.
This I have issue with. If you are creating a platform device here, you
are trying to work-around limitations in the linux driver model.
I somewhat understand your point.
Although, having the option to use a driver (which requires a device)
has turned out to be quite convenient from many aspects - at least in
the mmc case.
Certainly one can do without it, but in the end using a driver avoids
open coding.
Why would it be open coded? Just create library functions to parse the
node and implement the generic pwr-seq steps.
quoted
quoted
My current solution like below, but it seems you didn't agree with that.
I just double confirm here, if you don't, I give up the solution for
using generic power sequence framework.
In drivers/usb/core/hub.c
for_each_child_of_node(parent->of_node, node) {
hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
if (!pwrseq_node) {
ret = -ENOMEM;
goto err1;
}
/* power on sequence */
ret = pwrseq_pre_power_on(hdev_pwrseq);
Why does this function need to do anything more than:
- Check if the child has a "power-sequence" property
- Get the "reset-gpios" GPIO
- Assert reset for specified/default time
- Deassert reset
Then continue on as normal. That seems straight-forward to me.
There is no reason you need a platform device in the mix. Perhaps trying
to move the MMC pwr-seq code is pointless as it adds needless
complexity.
Complexity?
The problem we are tying to solve, is to make the various platform/SoC
specific power sequences to be able to live in generic drivers.
One could decide to encode the sequences inside the driver code
itself, but it will soon turn into a mess and more importantly, lots
of open coding as to support different platforms/SoCs. To most kernel
hackers I don't think this is an option to consider.
I'm not proposing open coding, but having generic library functions.
I'm not saying the mmc pwr-seq has to change from being a driver
either. Leave it as is. I'm only talking about Krzysztof's new
proposal.
The MMC pwr-seq code/drivers tries to address these issues - in a
somewhat generic way.
Initially we have decided to start with only a few flavours of
supported sequences and so far these have been sufficient.
Only a few because we've pushed back against defining state machines in DT.
Finally, I am indeed concerned that it so hard to agree on a solution
to deal with generic power sequences. There have been many attempts
throughout the last ~4-5 years, but peoples strong opinions about
different approaches mad them all fail. Isn't it time to finally just
pick a solution and stick with it, even if it doesn't meet all peoples
expectations?
No. I'm pretty that is not how kernel development works. Features get
merged when all are happy (or not paying attention).
From what I recall, all attempts have worked around the problem that
the driver model has no way to either force probe or provide a
pre-probe hook on probeable buses. This then leads to the work-around
defining the DT binding.
Rob