[RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Krzysztof Kozlowski]

Hi,

This is a different, second try to fix usb3503+lan on Odroid U3 board
if it was initialized by bootloader (e.g. for TFTP boot).

First version:
http://www.spinics.net/lists/linux-usb/msg140042.html


Problem
=======
When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
is required, e.g. by suspend to RAM. The actual TFTP boot does
not have to happen. Just "usb start" from U-Boot is sufficient.

>From the schematics, the regulator is a supply only to LAN, however
without toggling it off/on, the usb3503 hub won appear neither.


Solution
========
This is very similar to the MMC pwrseq behavior so the idea is to:
1. Move MMC pwrseq drivers to generic place,
2. Extend the pwrseq-simple with regulator toggling,
3. Add support to USB hub and port core for pwrseq,
4. Toggle the regulator when needed.


Issues
======
I am not familiar with USB subsystem, so please kindly guide me
where USB related code should be placed.

In the code there are still some issues to solve (FIXME/TODO notes).
If the approach is okay, I will improve the patchset. However at this
point - IT WORKS, which is nice. :)


Best regards,
Krzysztof

Krzysztof Kozlowski (13):
  usb: misc: usb3503: Clean up on driver unbind
  power/mmc: Move pwrseq drivers to power/pwrseq
  MAINTAINERS: Retain Ulf Hansson as the same maintainer of pwrseq
  power: pwrseq: Enable COMPILE_TEST for drivers
  power: pwrseq: Remove mmc prefix from mmc_pwrseq
  power: pwrseq: Generalize mmc_pwrseq operations by removing mmc prefix
  power: pwrseq: simple: Add support for toggling regulator
  usb: hub: Handle deferred probe
  power: pwrseq: Add support for USB hubs with external power
  usb: hub: Power sequence the ports on activation
  usb: port: Parse pwrseq phandle from Device Tree
  ARM: dts: exynos: Switch the buck8 to GPIO mode on Odroid U3
  ARM: dts: exynos: Fix LAN and HUB after bootloader initialization on
    Odroid U3

 .../devicetree/bindings/mmc/mmc-pwrseq-simple.txt  |  2 +
 MAINTAINERS                                        |  8 +++
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi    |  2 +-
 arch/arm/boot/dts/exynos4412-odroidu3.dts          |  7 ++
 drivers/mmc/Kconfig                                |  2 -
 drivers/mmc/core/Makefile                          |  3 -
 drivers/mmc/core/core.c                            |  8 +--
 drivers/mmc/core/host.c                            |  2 +-
 drivers/mmc/core/pwrseq.h                          | 52 --------------
 drivers/power/Kconfig                              |  1 +
 drivers/power/Makefile                             |  1 +
 drivers/{mmc/core => power/pwrseq}/Kconfig         | 21 ++++--
 drivers/power/pwrseq/Makefile                      |  3 +
 drivers/{mmc/core => power/pwrseq}/pwrseq.c        | 80 +++++++++++++++++-----
 drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c   | 15 ++--
 drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c | 73 ++++++++++++++++----
 drivers/usb/core/hub.c                             | 17 ++++-
 drivers/usb/core/hub.h                             |  3 +
 drivers/usb/core/port.c                            | 15 ++++
 drivers/usb/misc/usb3503.c                         | 28 ++++++++
 include/linux/mmc/host.h                           |  4 +-
 include/linux/pwrseq.h                             | 60 ++++++++++++++++
 22 files changed, 294 insertions(+), 113 deletions(-)
 delete mode 100644 drivers/mmc/core/pwrseq.h
 rename drivers/{mmc/core => power/pwrseq}/Kconfig (65%)
 create mode 100644 drivers/power/pwrseq/Makefile
 rename drivers/{mmc/core => power/pwrseq}/pwrseq.c (50%)
 rename drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c (89%)
 rename drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c (64%)
 create mode 100644 include/linux/pwrseq.h

-- 
1.9.1

--
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

[RFC v2 01/13] usb: misc: usb3503: Clean up on driver unbind

[Krzysztof Kozlowski]

The driver should clean up after itself by unpreparing the clock when it
is unbound.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/usb/misc/usb3503.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c
index b45cb77c0744..a464636675a6 100644
--- a/drivers/usb/misc/usb3503.c
+++ b/drivers/usb/misc/usb3503.c
@@ -330,6 +330,19 @@ static int usb3503_i2c_probe(struct i2c_client *i2c,
 	return usb3503_probe(hub);
 }
 
+static int usb3503_i2c_remove(struct i2c_client *i2c)
+{
+	struct usb3503 *hub;
+
+	hub = i2c_get_clientdata(i2c);
+	if (hub) {
+		if (hub->clk)
+			clk_disable_unprepare(hub->clk);
+	}
+
+	return 0;
+}
+
 static int usb3503_platform_probe(struct platform_device *pdev)
 {
 	struct usb3503 *hub;
@@ -342,6 +355,19 @@ static int usb3503_platform_probe(struct platform_device *pdev)
 	return usb3503_probe(hub);
 }
 
+static int usb3503_platform_remove(struct platform_device *pdev)
+{
+	struct usb3503 *hub;
+
+	hub = platform_get_drvdata(pdev);
+	if (hub) {
+		if (hub->clk)
+			clk_disable_unprepare(hub->clk);
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int usb3503_i2c_suspend(struct device *dev)
 {
@@ -395,6 +421,7 @@ static struct i2c_driver usb3503_i2c_driver = {
 		.of_match_table = of_match_ptr(usb3503_of_match),
 	},
 	.probe		= usb3503_i2c_probe,
+	.remove		= usb3503_i2c_remove,
 	.id_table	= usb3503_id,
 };
 
@@ -404,6 +431,7 @@ static struct platform_driver usb3503_platform_driver = {
 		.of_match_table = of_match_ptr(usb3503_of_match),
 	},
 	.probe		= usb3503_platform_probe,
+	.remove		= usb3503_platform_remove,
 };
 
 static int __init usb3503_init(void)
-- 
1.9.1

Re: [RFC v2 01/13] usb: misc: usb3503: Clean up on driver unbind

[Javier Martinez Canillas]

Hello Krzysztof

Patch looks good to me, I just have a trivial comment below.

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> The driver should clean up after itself by unpreparing the clock when it
> is unbound.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  drivers/usb/misc/usb3503.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c
> index b45cb77c0744..a464636675a6 100644
> --- a/drivers/usb/misc/usb3503.c
> +++ b/drivers/usb/misc/usb3503.c
> @@ -330,6 +330,19 @@ static int usb3503_i2c_probe(struct i2c_client *i2c,
>  	return usb3503_probe(hub);
>  }
>  
> +static int usb3503_i2c_remove(struct i2c_client *i2c)
> +{
> +	struct usb3503 *hub;
> +
> +	hub = i2c_get_clientdata(i2c);
> +	if (hub) {
> +		if (hub->clk)
> +			clk_disable_unprepare(hub->clk);
> +	}

I think the following is simpler and easier to read:

if (hub && hub->clk)
	clk_disable_unprepare(hub->clk);

> +
> +	return 0;
> +}
> +
>  static int usb3503_platform_probe(struct platform_device *pdev)
>  {
>  	struct usb3503 *hub;
> @@ -342,6 +355,19 @@ static int usb3503_platform_probe(struct platform_device *pdev)
>  	return usb3503_probe(hub);
>  }
>  
> +static int usb3503_platform_remove(struct platform_device *pdev)
> +{
> +	struct usb3503 *hub;
> +
> +	hub = platform_get_drvdata(pdev);
> +	if (hub) {
> +		if (hub->clk)
> +			clk_disable_unprepare(hub->clk);
> +	}
> +

Ditto.

> +	return 0;
> +}
> +
>  #ifdef CONFIG_PM_SLEEP
>  static int usb3503_i2c_suspend(struct device *dev)
>  {
> @@ -395,6 +421,7 @@ static struct i2c_driver usb3503_i2c_driver = {
>  		.of_match_table = of_match_ptr(usb3503_of_match),
>  	},
>  	.probe		= usb3503_i2c_probe,
> +	.remove		= usb3503_i2c_remove,
>  	.id_table	= usb3503_id,
>  };
>  
> @@ -404,6 +431,7 @@ static struct platform_driver usb3503_platform_driver = {
>  		.of_match_table = of_match_ptr(usb3503_of_match),
>  	},
>  	.probe		= usb3503_platform_probe,
> +	.remove		= usb3503_platform_remove,
>  };
>  
>  static int __init usb3503_init(void)
> 

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [RFC v2 01/13] usb: misc: usb3503: Clean up on driver unbind

[Krzysztof Kozlowski]

On 05/05/2016 08:32 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof
> 
> Patch looks good to me, I just have a trivial comment below.
> 
> On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
>> The driver should clean up after itself by unpreparing the clock when it
>> is unbound.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> ---
>>  drivers/usb/misc/usb3503.c | 28 ++++++++++++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c
>> index b45cb77c0744..a464636675a6 100644
>> --- a/drivers/usb/misc/usb3503.c
>> +++ b/drivers/usb/misc/usb3503.c
>> @@ -330,6 +330,19 @@ static int usb3503_i2c_probe(struct i2c_client *i2c,
>>  	return usb3503_probe(hub);
>>  }
>>  
>> +static int usb3503_i2c_remove(struct i2c_client *i2c)
>> +{
>> +	struct usb3503 *hub;
>> +
>> +	hub = i2c_get_clientdata(i2c);
>> +	if (hub) {
>> +		if (hub->clk)
>> +			clk_disable_unprepare(hub->clk);
>> +	}
> 
> I think the following is simpler and easier to read:
> 
> if (hub && hub->clk)
> 	clk_disable_unprepare(hub->clk);
> 

Sure, thanks.

Best regards,
Krzysztof

>> +
>> +	return 0;
>> +}
>> +
>>  static int usb3503_platform_probe(struct platform_device *pdev)
>>  {
>>  	struct usb3503 *hub;
>> @@ -342,6 +355,19 @@ static int usb3503_platform_probe(struct platform_device *pdev)
>>  	return usb3503_probe(hub);
>>  }
>>  
>> +static int usb3503_platform_remove(struct platform_device *pdev)
>> +{
>> +	struct usb3503 *hub;
>> +
>> +	hub = platform_get_drvdata(pdev);
>> +	if (hub) {
>> +		if (hub->clk)
>> +			clk_disable_unprepare(hub->clk);
>> +	}
>> +
> 
> Ditto.
> 
>> +	return 0;
>> +}
>> +
>>  #ifdef CONFIG_PM_SLEEP
>>  static int usb3503_i2c_suspend(struct device *dev)
>>  {
>> @@ -395,6 +421,7 @@ static struct i2c_driver usb3503_i2c_driver = {
>>  		.of_match_table = of_match_ptr(usb3503_of_match),
>>  	},
>>  	.probe		= usb3503_i2c_probe,
>> +	.remove		= usb3503_i2c_remove,
>>  	.id_table	= usb3503_id,
>>  };
>>  
>> @@ -404,6 +431,7 @@ static struct platform_driver usb3503_platform_driver = {
>>  		.of_match_table = of_match_ptr(usb3503_of_match),
>>  	},
>>  	.probe		= usb3503_platform_probe,
>> +	.remove		= usb3503_platform_remove,
>>  };
>>  
>>  static int __init usb3503_init(void)
>>
> 
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> Best regards,
> 

[RFC v2 02/13] power/mmc: Move pwrseq drivers to power/pwrseq

[Krzysztof Kozlowski]

The MMC power sequence drivers are useful also outside of MMC world: for
USB devices needed a hard-reset before probing. Before extending and
re-using pwrseq drivers, move them to a new place.

The commit does not introduce significant changes in the pwrseq drivers
code so still all the functions are prefixed with "mmc_pwrseq". However
the MMC-specific pwrseq functions has to be now exported and everything
is hidden not by CONFIG_OF but by new CONFIG_POWER_SEQ option.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/mmc/Kconfig                                |  2 --
 drivers/mmc/core/Makefile                          |  3 ---
 drivers/mmc/core/core.c                            |  2 +-
 drivers/mmc/core/host.c                            |  2 +-
 drivers/power/Kconfig                              |  1 +
 drivers/power/Makefile                             |  1 +
 drivers/{mmc/core => power/pwrseq}/Kconfig         | 17 ++++++++++++-----
 drivers/power/pwrseq/Makefile                      |  3 +++
 drivers/{mmc/core => power/pwrseq}/pwrseq.c        |  8 ++++++--
 drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c   |  3 +--
 drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c |  3 +--
 {drivers/mmc/core => include/linux}/pwrseq.h       |  6 +++---
 12 files changed, 30 insertions(+), 21 deletions(-)
 rename drivers/{mmc/core => power/pwrseq}/Kconfig (71%)
 create mode 100644 drivers/power/pwrseq/Makefile
 rename drivers/{mmc/core => power/pwrseq}/pwrseq.c (90%)
 rename drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c (99%)
 rename drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c (99%)
 rename {drivers/mmc/core => include/linux}/pwrseq.h (94%)

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index f2eeb38efa65..7ade379e0634 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -21,8 +21,6 @@ config MMC_DEBUG
 
 if MMC
 
-source "drivers/mmc/core/Kconfig"
-
 source "drivers/mmc/card/Kconfig"
 
 source "drivers/mmc/host/Kconfig"
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index f007151dfdc6..a901d3cd09d3 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -8,7 +8,4 @@ mmc_core-y			:= core.o bus.o host.o \
 				   sdio.o sdio_ops.o sdio_bus.o \
 				   sdio_cis.o sdio_io.o sdio_irq.o \
 				   quirks.o slot-gpio.o
-mmc_core-$(CONFIG_OF)		+= pwrseq.o
-obj-$(CONFIG_PWRSEQ_SIMPLE)	+= pwrseq_simple.o
-obj-$(CONFIG_PWRSEQ_EMMC)	+= pwrseq_emmc.o
 mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 99275e40bf2f..0f145ff6e4f1 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -25,6 +25,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/pm_wakeup.h>
 #include <linux/suspend.h>
+#include <linux/pwrseq.h>
 #include <linux/fault-inject.h>
 #include <linux/random.h>
 #include <linux/slab.h>
@@ -43,7 +44,6 @@
 #include "bus.h"
 #include "host.h"
 #include "sdio_bus.h"
-#include "pwrseq.h"
 
 #include "mmc_ops.h"
 #include "sd_ops.h"
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index e0a3ee16c0d3..98164a352dfb 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -21,6 +21,7 @@
 #include <linux/export.h>
 #include <linux/leds.h>
 #include <linux/slab.h>
+#include <linux/pwrseq.h>
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
@@ -29,7 +30,6 @@
 #include "core.h"
 #include "host.h"
 #include "slot-gpio.h"
-#include "pwrseq.h"
 
 #define cls_dev_to_mmc_host(d)	container_of(d, struct mmc_host, class_dev)
 
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 421770ddafa3..2702aca6cd2c 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -511,5 +511,6 @@ config AXP20X_POWER
 
 endif # POWER_SUPPLY
 
+source "drivers/power/pwrseq/Kconfig"
 source "drivers/power/reset/Kconfig"
 source "drivers/power/avs/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index e46b75d448a5..02f9d5da2e76 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_POWER_AVS)		+= avs/
 obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
 obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
 obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
+obj-$(CONFIG_POWER_SEQ)		+= pwrseq/
 obj-$(CONFIG_POWER_RESET)	+= reset/
 obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
 obj-$(CONFIG_AXP288_CHARGER)	+= axp288_charger.o
diff --git a/drivers/mmc/core/Kconfig b/drivers/power/pwrseq/Kconfig
similarity index 71%
rename from drivers/mmc/core/Kconfig
rename to drivers/power/pwrseq/Kconfig
index 250f223aaa80..b5d2d6c65f28 100644
--- a/drivers/mmc/core/Kconfig
+++ b/drivers/power/pwrseq/Kconfig
@@ -1,7 +1,12 @@
-#
-# MMC core configuration
-#
-config PWRSEQ_EMMC
+menuconfig POWER_SEQ
+	default y if OF
+	bool "Hardware reset support for specific devices"
+	help
+	  Provides drivers which reset the specific device before...
+
+if POWER_SEQ
+
+config POWER_SEQ_EMMC
 	tristate "HW reset support for eMMC"
 	default y
 	depends on OF
@@ -12,7 +17,7 @@ config PWRSEQ_EMMC
 	  This driver can also be built as a module. If so, the module
 	  will be called pwrseq_emmc.
 
-config PWRSEQ_SIMPLE
+config POWER_SEQ_SIMPLE
 	tristate "Simple HW reset support for MMC"
 	default y
 	depends on OF
@@ -22,3 +27,5 @@ config PWRSEQ_SIMPLE
 
 	  This driver can also be built as a module. If so, the module
 	  will be called pwrseq_simple.
+
+endif
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
new file mode 100644
index 000000000000..9e40e4b9068b
--- /dev/null
+++ b/drivers/power/pwrseq/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_POWER_SEQ)		+= pwrseq.o
+obj-$(CONFIG_POWER_SEQ_SIMPLE)	+= pwrseq_simple.o
+obj-$(CONFIG_POWER_SEQ_EMMC)	+= pwrseq_emmc.o
diff --git a/drivers/mmc/core/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
similarity index 90%
rename from drivers/mmc/core/pwrseq.c
rename to drivers/power/pwrseq/pwrseq.c
index 9386c4771814..66310d7643cc 100644
--- a/drivers/mmc/core/pwrseq.c
+++ b/drivers/power/pwrseq/pwrseq.c
@@ -11,11 +11,10 @@
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/pwrseq.h>
 
 #include <linux/mmc/host.h>
 
-#include "pwrseq.h"
-
 static DEFINE_MUTEX(pwrseq_list_mutex);
 static LIST_HEAD(pwrseq_list);
 
@@ -51,6 +50,7 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
 
 void mmc_pwrseq_pre_power_on(struct mmc_host *host)
 {
@@ -59,6 +59,7 @@ void mmc_pwrseq_pre_power_on(struct mmc_host *host)
 	if (pwrseq && pwrseq->ops->pre_power_on)
 		pwrseq->ops->pre_power_on(host);
 }
+EXPORT_SYMBOL_GPL(mmc_pwrseq_pre_power_on);
 
 void mmc_pwrseq_post_power_on(struct mmc_host *host)
 {
@@ -67,6 +68,7 @@ void mmc_pwrseq_post_power_on(struct mmc_host *host)
 	if (pwrseq && pwrseq->ops->post_power_on)
 		pwrseq->ops->post_power_on(host);
 }
+EXPORT_SYMBOL_GPL(mmc_pwrseq_post_power_on);
 
 void mmc_pwrseq_power_off(struct mmc_host *host)
 {
@@ -75,6 +77,7 @@ void mmc_pwrseq_power_off(struct mmc_host *host)
 	if (pwrseq && pwrseq->ops->power_off)
 		pwrseq->ops->power_off(host);
 }
+EXPORT_SYMBOL_GPL(mmc_pwrseq_power_off);
 
 void mmc_pwrseq_free(struct mmc_host *host)
 {
@@ -85,6 +88,7 @@ void mmc_pwrseq_free(struct mmc_host *host)
 		host->pwrseq = NULL;
 	}
 }
+EXPORT_SYMBOL_GPL(mmc_pwrseq_free);
 
 int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
 {
diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/power/pwrseq/pwrseq_emmc.c
similarity index 99%
rename from drivers/mmc/core/pwrseq_emmc.c
rename to drivers/power/pwrseq/pwrseq_emmc.c
index adc9c0c614fb..a0583ed46d7f 100644
--- a/drivers/mmc/core/pwrseq_emmc.c
+++ b/drivers/power/pwrseq/pwrseq_emmc.c
@@ -17,11 +17,10 @@
 #include <linux/err.h>
 #include <linux/gpio/consumer.h>
 #include <linux/reboot.h>
+#include <linux/pwrseq.h>
 
 #include <linux/mmc/host.h>
 
-#include "pwrseq.h"
-
 struct mmc_pwrseq_emmc {
 	struct mmc_pwrseq pwrseq;
 	struct notifier_block reset_nb;
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/power/pwrseq/pwrseq_simple.c
similarity index 99%
rename from drivers/mmc/core/pwrseq_simple.c
rename to drivers/power/pwrseq/pwrseq_simple.c
index 450d907c6e6c..786f1db53a3f 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -16,11 +16,10 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/gpio/consumer.h>
+#include <linux/pwrseq.h>
 
 #include <linux/mmc/host.h>
 
-#include "pwrseq.h"
-
 struct mmc_pwrseq_simple {
 	struct mmc_pwrseq pwrseq;
 	bool clk_enabled;
diff --git a/drivers/mmc/core/pwrseq.h b/include/linux/pwrseq.h
similarity index 94%
rename from drivers/mmc/core/pwrseq.h
rename to include/linux/pwrseq.h
index d69e751f148b..e230670c1d8d 100644
--- a/drivers/mmc/core/pwrseq.h
+++ b/include/linux/pwrseq.h
@@ -23,7 +23,7 @@ struct mmc_pwrseq {
 	struct module *owner;
 };
 
-#ifdef CONFIG_OF
+#ifdef CONFIG_POWER_SEQ
 
 int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq);
 void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq);
@@ -34,7 +34,7 @@ void mmc_pwrseq_post_power_on(struct mmc_host *host);
 void mmc_pwrseq_power_off(struct mmc_host *host);
 void mmc_pwrseq_free(struct mmc_host *host);
 
-#else
+#else /* CONFIG_POWER_SEQ */
 
 static inline int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
 {
@@ -47,6 +47,6 @@ static inline void mmc_pwrseq_post_power_on(struct mmc_host *host) {}
 static inline void mmc_pwrseq_power_off(struct mmc_host *host) {}
 static inline void mmc_pwrseq_free(struct mmc_host *host) {}
 
-#endif
+#endif /* CONFIG_POWER_SEQ */
 
 #endif
-- 
1.9.1

Re: [RFC v2 02/13] power/mmc: Move pwrseq drivers to power/pwrseq

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> The MMC power sequence drivers are useful also outside of MMC world: for
> USB devices needed a hard-reset before probing. Before extending and
> re-using pwrseq drivers, move them to a new place.
> 
> The commit does not introduce significant changes in the pwrseq drivers
> code so still all the functions are prefixed with "mmc_pwrseq". However
> the MMC-specific pwrseq functions has to be now exported and everything
> is hidden not by CONFIG_OF but by new CONFIG_POWER_SEQ option.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---

[snip]

> --- a/drivers/mmc/core/Kconfig
> +++ b/drivers/power/pwrseq/Kconfig
> @@ -1,7 +1,12 @@
> -#
> -# MMC core configuration
> -#
> -config PWRSEQ_EMMC
> +menuconfig POWER_SEQ
> +	default y if OF
> +	bool "Hardware reset support for specific devices"
> +	help
> +	  Provides drivers which reset the specific device before...
> +

I think this text could be improved a little bit, maybe something like:

"Provides drivers that implements specific power sequences for chips,
using the generic power sequence management interface".

The rest looks good to me.

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [RFC v2 02/13] power/mmc: Move pwrseq drivers to power/pwrseq

[Krzysztof Kozlowski]

On 05/05/2016 08:44 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
>> The MMC power sequence drivers are useful also outside of MMC world: for
>> USB devices needed a hard-reset before probing. Before extending and
>> re-using pwrseq drivers, move them to a new place.
>>
>> The commit does not introduce significant changes in the pwrseq drivers
>> code so still all the functions are prefixed with "mmc_pwrseq". However
>> the MMC-specific pwrseq functions has to be now exported and everything
>> is hidden not by CONFIG_OF but by new CONFIG_POWER_SEQ option.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> ---
> 
> [snip]
> 
>> --- a/drivers/mmc/core/Kconfig
>> +++ b/drivers/power/pwrseq/Kconfig
>> @@ -1,7 +1,12 @@
>> -#
>> -# MMC core configuration
>> -#
>> -config PWRSEQ_EMMC
>> +menuconfig POWER_SEQ
>> +	default y if OF
>> +	bool "Hardware reset support for specific devices"
>> +	help
>> +	  Provides drivers which reset the specific device before...
>> +
> 
> I think this text could be improved a little bit, maybe something like:
> 
> "Provides drivers that implements specific power sequences for chips,
> using the generic power sequence management interface".
> 
> The rest looks good to me.
> 
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Yes, I wanted to code, not to focus on descriptions, so also commit
messages should be extended.

I'll fix this in next iteration.

BR,
Krzysztof

[RFC v2 03/13] MAINTAINERS: Retain Ulf Hansson as the same maintainer of pwrseq

[Krzysztof Kozlowski]

Before moving pwrseq drivers from drivers/mmc/core/ to drivers/power/,
they were maintained by Ulf Hansson.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b016f447c6c9..2c501b795d18 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8874,6 +8874,14 @@ F:	include/linux/power_supply.h
 F:	drivers/power/
 X:	drivers/power/avs/
 
+POWER SEQ CORE and DRIVERS
+M:	Ulf Hansson <ulf.hansson@linaro.org>
+L:	linux-mmc@vger.kernel.org
+T:	git git://git.linaro.org/people/ulf.hansson/mmc.git
+S:	Maintained
+F:	drivers/power/pwrseq/
+F:	include/linux/pwrseq.h
+
 POWER STATE COORDINATION INTERFACE (PSCI)
 M:	Mark Rutland <mark.rutland@arm.com>
 M:	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
-- 
1.9.1

Re: [RFC v2 03/13] MAINTAINERS: Retain Ulf Hansson as the same maintainer of pwrseq

[Javier Martinez Canillas]

Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Before moving pwrseq drivers from drivers/mmc/core/ to drivers/power/,
> they were maintained by Ulf Hansson.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[RFC v2 06/13] power: pwrseq: Generalize mmc_pwrseq operations by removing mmc prefix

[Krzysztof Kozlowski]

The power sequence hooks (mmc_pwrseq_pre_power_on(),
mmc_pwrseq_post_power_on() and mmc_pwrseq_power_off()) can be made more
generic to allow re-use in other subsystems. They do not need to take
pointer to struct mmc_host but instead the struct pwrseq should be
sufficient.

Remove the "mmc" prefix and use the pointer to struct pwrseq as
argument.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/mmc/core/core.c              |  6 +++---
 drivers/power/pwrseq/pwrseq.c        | 24 +++++++++---------------
 drivers/power/pwrseq/pwrseq_emmc.c   |  4 ++--
 drivers/power/pwrseq/pwrseq_simple.c | 12 ++++++------
 include/linux/pwrseq.h               | 18 +++++++++---------
 5 files changed, 29 insertions(+), 35 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 0f145ff6e4f1..dfc4681054a8 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1719,7 +1719,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 	if (host->ios.power_mode == MMC_POWER_ON)
 		return;
 
-	mmc_pwrseq_pre_power_on(host);
+	pwrseq_pre_power_on(host->pwrseq);
 
 	host->ios.vdd = fls(ocr) - 1;
 	host->ios.power_mode = MMC_POWER_UP;
@@ -1740,7 +1740,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 	 */
 	mmc_delay(10);
 
-	mmc_pwrseq_post_power_on(host);
+	pwrseq_post_power_on(host->pwrseq);
 
 	host->ios.clock = host->f_init;
 
@@ -1759,7 +1759,7 @@ void mmc_power_off(struct mmc_host *host)
 	if (host->ios.power_mode == MMC_POWER_OFF)
 		return;
 
-	mmc_pwrseq_power_off(host);
+	pwrseq_power_off(host->pwrseq);
 
 	host->ios.clock = 0;
 	host->ios.vdd = 0;
diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
index 9c665821f890..495a19d3c30b 100644
--- a/drivers/power/pwrseq/pwrseq.c
+++ b/drivers/power/pwrseq/pwrseq.c
@@ -52,32 +52,26 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
 }
 EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
 
-void mmc_pwrseq_pre_power_on(struct mmc_host *host)
+void pwrseq_pre_power_on(struct pwrseq *pwrseq)
 {
-	struct pwrseq *pwrseq = host->pwrseq;
-
 	if (pwrseq && pwrseq->ops->pre_power_on)
-		pwrseq->ops->pre_power_on(host);
+		pwrseq->ops->pre_power_on(pwrseq);
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_pre_power_on);
+EXPORT_SYMBOL_GPL(pwrseq_pre_power_on);
 
-void mmc_pwrseq_post_power_on(struct mmc_host *host)
+void pwrseq_post_power_on(struct pwrseq *pwrseq)
 {
-	struct pwrseq *pwrseq = host->pwrseq;
-
 	if (pwrseq && pwrseq->ops->post_power_on)
-		pwrseq->ops->post_power_on(host);
+		pwrseq->ops->post_power_on(pwrseq);
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_post_power_on);
+EXPORT_SYMBOL_GPL(pwrseq_post_power_on);
 
-void mmc_pwrseq_power_off(struct mmc_host *host)
+void pwrseq_power_off(struct pwrseq *pwrseq)
 {
-	struct pwrseq *pwrseq = host->pwrseq;
-
 	if (pwrseq && pwrseq->ops->power_off)
-		pwrseq->ops->power_off(host);
+		pwrseq->ops->power_off(pwrseq);
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_power_off);
+EXPORT_SYMBOL_GPL(pwrseq_power_off);
 
 void mmc_pwrseq_free(struct mmc_host *host)
 {
diff --git a/drivers/power/pwrseq/pwrseq_emmc.c b/drivers/power/pwrseq/pwrseq_emmc.c
index a68ac9a68e04..82327d0223f2 100644
--- a/drivers/power/pwrseq/pwrseq_emmc.c
+++ b/drivers/power/pwrseq/pwrseq_emmc.c
@@ -37,9 +37,9 @@ static void __mmc_pwrseq_emmc_reset(struct mmc_pwrseq_emmc *pwrseq)
 	udelay(200);
 }
 
-static void mmc_pwrseq_emmc_reset(struct mmc_host *host)
+static void mmc_pwrseq_emmc_reset(struct pwrseq *_pwrseq)
 {
-	struct mmc_pwrseq_emmc *pwrseq =  to_pwrseq_emmc(host->pwrseq);
+	struct mmc_pwrseq_emmc *pwrseq = to_pwrseq_emmc(_pwrseq);
 
 	__mmc_pwrseq_emmc_reset(pwrseq);
 }
diff --git a/drivers/power/pwrseq/pwrseq_simple.c b/drivers/power/pwrseq/pwrseq_simple.c
index d5fbd653153e..ab0098412690 100644
--- a/drivers/power/pwrseq/pwrseq_simple.c
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -46,9 +46,9 @@ static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
 	}
 }
 
-static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
+static void mmc_pwrseq_simple_pre_power_on(struct pwrseq *_pwrseq)
 {
-	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(host->pwrseq);
+	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
 
 	if (!IS_ERR(pwrseq->ext_clk) && !pwrseq->clk_enabled) {
 		clk_prepare_enable(pwrseq->ext_clk);
@@ -58,16 +58,16 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
 	mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
 }
 
-static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
+static void mmc_pwrseq_simple_post_power_on(struct pwrseq *_pwrseq)
 {
-	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(host->pwrseq);
+	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
 
 	mmc_pwrseq_simple_set_gpios_value(pwrseq, 0);
 }
 
-static void mmc_pwrseq_simple_power_off(struct mmc_host *host)
+static void mmc_pwrseq_simple_power_off(struct pwrseq *_pwrseq)
 {
-	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(host->pwrseq);
+	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
 
 	mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
 
diff --git a/include/linux/pwrseq.h b/include/linux/pwrseq.h
index 6d2d2a44ad35..fcc8fd855d4c 100644
--- a/include/linux/pwrseq.h
+++ b/include/linux/pwrseq.h
@@ -11,9 +11,9 @@
 #include <linux/mmc/host.h>
 
 struct pwrseq_ops {
-	void (*pre_power_on)(struct mmc_host *host);
-	void (*post_power_on)(struct mmc_host *host);
-	void (*power_off)(struct mmc_host *host);
+	void (*pre_power_on)(struct pwrseq *pwrseq);
+	void (*post_power_on)(struct pwrseq *pwrseq);
+	void (*power_off)(struct pwrseq *pwrseq);
 };
 
 struct pwrseq {
@@ -28,10 +28,10 @@ struct pwrseq {
 int pwrseq_register(struct pwrseq *pwrseq);
 void pwrseq_unregister(struct pwrseq *pwrseq);
 
+void pwrseq_pre_power_on(struct pwrseq *pwrseq);
+void pwrseq_post_power_on(struct pwrseq *pwrseq);
+void pwrseq_power_off(struct pwrseq *pwrseq);
 int mmc_pwrseq_alloc(struct mmc_host *host);
-void mmc_pwrseq_pre_power_on(struct mmc_host *host);
-void mmc_pwrseq_post_power_on(struct mmc_host *host);
-void mmc_pwrseq_power_off(struct mmc_host *host);
 void mmc_pwrseq_free(struct mmc_host *host);
 
 #else /* CONFIG_POWER_SEQ */
@@ -41,10 +41,10 @@ static inline int pwrseq_register(struct pwrseq *pwrseq)
 	return -ENOSYS;
 }
 static inline void pwrseq_unregister(struct pwrseq *pwrseq) {}
+static inline void pwrseq_pre_power_on(struct pwrseq *pwrseq) {}
+static inline void pwrseq_post_power_on(struct pwrseq *pwrseq) {}
+static inline void pwrseq_power_off(struct pwrseq *pwrseq) {}
 static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; }
-static inline void mmc_pwrseq_pre_power_on(struct mmc_host *host) {}
-static inline void mmc_pwrseq_post_power_on(struct mmc_host *host) {}
-static inline void mmc_pwrseq_power_off(struct mmc_host *host) {}
 static inline void mmc_pwrseq_free(struct mmc_host *host) {}
 
 #endif /* CONFIG_POWER_SEQ */
-- 
1.9.1

Re: [RFC v2 06/13] power: pwrseq: Generalize mmc_pwrseq operations by removing mmc prefix

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> The power sequence hooks (mmc_pwrseq_pre_power_on(),
> mmc_pwrseq_post_power_on() and mmc_pwrseq_power_off()) can be made more
> generic to allow re-use in other subsystems. They do not need to take
> pointer to struct mmc_host but instead the struct pwrseq should be
> sufficient.
> 
> Remove the "mmc" prefix and use the pointer to struct pwrseq as
> argument.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---

Reviewed-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC v2 07/13] power: pwrseq: simple: Add support for toggling regulator

[Krzysztof Kozlowski]

Some devices need real hard-reset by cutting the power.  During power
sequence turn off and on the regulator, if it is provided.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 .../devicetree/bindings/mmc/mmc-pwrseq-simple.txt  |  2 +
 drivers/power/pwrseq/pwrseq_simple.c               | 50 ++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
index ce0e76749671..176ff831e7f1 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -16,6 +16,7 @@ Optional properties:
   See ../clocks/clock-bindings.txt for details.
 - clock-names : Must include the following entry:
   "ext_clock" (External clock provided to the card).
+- ext-supply : External regulator supply
 
 Example:
 
@@ -24,4 +25,5 @@ Example:
 		reset-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
 		clocks = <&clk_32768_ck>;
 		clock-names = "ext_clock";
+		ext-supply = <&buck8>;
 	}
diff --git a/drivers/power/pwrseq/pwrseq_simple.c b/drivers/power/pwrseq/pwrseq_simple.c
index ab0098412690..4d5ea53d3ead 100644
--- a/drivers/power/pwrseq/pwrseq_simple.c
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -16,7 +16,9 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
 #include <linux/pwrseq.h>
+#include <linux/delay.h>
 
 #include <linux/mmc/host.h>
 
@@ -25,6 +27,7 @@ struct mmc_pwrseq_simple {
 	bool clk_enabled;
 	struct clk *ext_clk;
 	struct gpio_descs *reset_gpios;
+	struct regulator *ext_reg;
 };
 
 #define to_pwrseq_simple(p) container_of(p, struct mmc_pwrseq_simple, pwrseq)
@@ -62,6 +65,13 @@ static void mmc_pwrseq_simple_post_power_on(struct pwrseq *_pwrseq)
 {
 	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
 
+	if (pwrseq->ext_reg) {
+		int err;
+
+		err = regulator_enable(pwrseq->ext_reg);
+		WARN_ON_ONCE(err);
+	}
+
 	mmc_pwrseq_simple_set_gpios_value(pwrseq, 0);
 }
 
@@ -75,6 +85,13 @@ static void mmc_pwrseq_simple_power_off(struct pwrseq *_pwrseq)
 		clk_disable_unprepare(pwrseq->ext_clk);
 		pwrseq->clk_enabled = false;
 	}
+
+	if (pwrseq->ext_reg) {
+		int err;
+
+		err = regulator_disable(pwrseq->ext_reg);
+		WARN_ON_ONCE(err);
+	}
 }
 
 static const struct pwrseq_ops mmc_pwrseq_simple_ops = {
@@ -102,6 +119,32 @@ static int mmc_pwrseq_simple_probe(struct platform_device *pdev)
 	if (IS_ERR(pwrseq->ext_clk) && PTR_ERR(pwrseq->ext_clk) != -ENOENT)
 		return PTR_ERR(pwrseq->ext_clk);
 
+	/* FIXME: regulator_get_exclusive? */
+	pwrseq->ext_reg = devm_regulator_get_optional(dev, "ext");
+	if (IS_ERR(pwrseq->ext_reg)) {
+		if (PTR_ERR(pwrseq->ext_reg) == -ENODEV)
+			pwrseq->ext_reg = NULL;
+		else
+			return PTR_ERR(pwrseq->ext_reg);
+	} else {
+		int err;
+		/*
+		 * Be sure that regulator is off, before the driver will start
+		 * power sequence. It is likely that regulator is on by default
+		 * and it without toggling it here, it would be disabled much
+		 * later by the core.
+		 */
+
+		err = regulator_enable(pwrseq->ext_reg);
+		WARN_ON_ONCE(err);
+
+		/* FIXME: handle this in a more sensible way */
+		mdelay(10);
+
+		err = regulator_disable(pwrseq->ext_reg);
+		WARN_ON_ONCE(err);
+	}
+
 	pwrseq->reset_gpios = devm_gpiod_get_array(dev, "reset",
 							GPIOD_OUT_HIGH);
 	if (IS_ERR(pwrseq->reset_gpios) &&
@@ -124,6 +167,13 @@ static int mmc_pwrseq_simple_remove(struct platform_device *pdev)
 
 	pwrseq_unregister(&pwrseq->pwrseq);
 
+	if (pwrseq->ext_reg) {
+		int err;
+
+		err = regulator_disable(pwrseq->ext_reg);
+		WARN_ON_ONCE(err);
+	}
+
 	return 0;
 }
 
-- 
1.9.1

Re: [RFC v2 07/13] power: pwrseq: simple: Add support for toggling regulator

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Some devices need real hard-reset by cutting the power.  During power
> sequence turn off and on the regulator, if it is provided.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---

[snip]

>  
>  #define to_pwrseq_simple(p) container_of(p, struct mmc_pwrseq_simple, pwrseq)
> @@ -62,6 +65,13 @@ static void mmc_pwrseq_simple_post_power_on(struct pwrseq *_pwrseq)
>  {
>  	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
>  
> +	if (pwrseq->ext_reg) {
> +		int err;
> +
> +		err = regulator_enable(pwrseq->ext_reg);
> +		WARN_ON_ONCE(err);
> +	}
> +

Shouldn't this be in mmc_pwrseq_simple_pre_power_on() instead?

For example, a chip may need to be powered on before attempting to
toggle its reset or power pins using some GPIO lines.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [RFC v2 07/13] power: pwrseq: simple: Add support for toggling regulator

[Krzysztof Kozlowski]

On 05/05/2016 09:31 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
>> Some devices need real hard-reset by cutting the power.  During power
>> sequence turn off and on the regulator, if it is provided.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> ---
> 
> [snip]
> 
>>  
>>  #define to_pwrseq_simple(p) container_of(p, struct mmc_pwrseq_simple, pwrseq)
>> @@ -62,6 +65,13 @@ static void mmc_pwrseq_simple_post_power_on(struct pwrseq *_pwrseq)
>>  {
>>  	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
>>  
>> +	if (pwrseq->ext_reg) {
>> +		int err;
>> +
>> +		err = regulator_enable(pwrseq->ext_reg);
>> +		WARN_ON_ONCE(err);
>> +	}
>> +
> 
> Shouldn't this be in mmc_pwrseq_simple_pre_power_on() instead?
> 
> For example, a chip may need to be powered on before attempting to
> toggle its reset or power pins using some GPIO lines.

Indeed this should be still sorted out but here the assumption is that
regulator is disabled (by probe()) so it should be turned on with
GPIO-reset set.

This can be done at the end of pre-power-on or here (beginning of
post-power-on). On the other hand I understand these pre/post callbacks
as one starting the reset sequence (pre) and second as finishing it
(post). In case of regulators, finishing power sequence is to turn the
regulator on.

Best regards,
Krzysztof

[RFC v2 08/13] usb: hub: Handle deferred probe

[Krzysztof Kozlowski]

Add support for deferred probing to the usb hub. Currently EPROBE_DEFER
does not exist in usb hub path but future patches will add it on the
port level.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/usb/core/hub.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 38cc4bae0a82..1c82fcc448f5 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1731,6 +1731,7 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	struct usb_endpoint_descriptor *endpoint;
 	struct usb_device *hdev;
 	struct usb_hub *hub;
+	int ret;
 
 	desc = intf->cur_altsetting;
 	hdev = interface_to_usbdev(intf);
@@ -1850,11 +1851,12 @@ descriptor_error:
 	if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
 		hub->quirk_check_port_auto_suspend = 1;
 
-	if (hub_configure(hub, endpoint) >= 0)
+	ret = hub_configure(hub, endpoint);
+	if (ret >= 0)
 		return 0;
 
 	hub_disconnect(intf);
-	return -ENODEV;
+	return ret;
 }
 
 static int
-- 
1.9.1

Re: [RFC v2 08/13] usb: hub: Handle deferred probe

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Add support for deferred probing to the usb hub. Currently EPROBE_DEFER
> does not exist in usb hub path but future patches will add it on the
> port level.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[RFC v2 09/13] power: pwrseq: Add support for USB hubs with external power

[Krzysztof Kozlowski]

Some USB devices on embedded boards have external power supply which has
to be reset in certain conditions. Add pwrseq interface for this.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/power/pwrseq/pwrseq.c | 44 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/pwrseq.h        |  8 ++++++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
index 495a19d3c30b..306265f55a10 100644
--- a/drivers/power/pwrseq/pwrseq.c
+++ b/drivers/power/pwrseq/pwrseq.c
@@ -52,6 +52,43 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
 }
 EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
 
+struct pwrseq *pwrseq_alloc(struct device *dev)
+{
+	struct device_node *np;
+	struct pwrseq *p, *ret = NULL;
+
+	np = of_parse_phandle(dev->of_node, "usb-pwrseq", 0);
+	if (!np)
+		return NULL;
+
+	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(dev,
+					"increasing module refcount failed\n");
+			else
+				ret = p;
+
+			break;
+		}
+	}
+
+	of_node_put(np);
+	mutex_unlock(&pwrseq_list_mutex);
+
+	/* FIXME: this path can be simplified... */
+	if (!ret) {
+		dev_info(dev, "usb-pwrseq defer\n");
+		return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	dev_info(dev, "allocated usb-pwrseq\n");
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pwrseq_alloc);
+
 void pwrseq_pre_power_on(struct pwrseq *pwrseq)
 {
 	if (pwrseq && pwrseq->ops->pre_power_on)
@@ -84,6 +121,13 @@ void mmc_pwrseq_free(struct mmc_host *host)
 }
 EXPORT_SYMBOL_GPL(mmc_pwrseq_free);
 
+void pwrseq_free(const struct pwrseq *pwrseq)
+{
+	if (pwrseq)
+		module_put(pwrseq->owner);
+}
+EXPORT_SYMBOL_GPL(pwrseq_free);
+
 int pwrseq_register(struct pwrseq *pwrseq)
 {
 	if (!pwrseq || !pwrseq->ops || !pwrseq->dev)
diff --git a/include/linux/pwrseq.h b/include/linux/pwrseq.h
index fcc8fd855d4c..c3c91f50e4cb 100644
--- a/include/linux/pwrseq.h
+++ b/include/linux/pwrseq.h
@@ -31,9 +31,13 @@ void pwrseq_unregister(struct pwrseq *pwrseq);
 void pwrseq_pre_power_on(struct pwrseq *pwrseq);
 void pwrseq_post_power_on(struct pwrseq *pwrseq);
 void pwrseq_power_off(struct pwrseq *pwrseq);
+
 int mmc_pwrseq_alloc(struct mmc_host *host);
 void mmc_pwrseq_free(struct mmc_host *host);
 
+struct pwrseq *pwrseq_alloc(struct device *dev);
+void pwrseq_free(const struct pwrseq *pwrseq);
+
 #else /* CONFIG_POWER_SEQ */
 
 static inline int pwrseq_register(struct pwrseq *pwrseq)
@@ -44,9 +48,13 @@ static inline void pwrseq_unregister(struct pwrseq *pwrseq) {}
 static inline void pwrseq_pre_power_on(struct pwrseq *pwrseq) {}
 static inline void pwrseq_post_power_on(struct pwrseq *pwrseq) {}
 static inline void pwrseq_power_off(struct pwrseq *pwrseq) {}
+
 static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; }
 static inline void mmc_pwrseq_free(struct mmc_host *host) {}
 
+static inline struct pwrseq *pwrseq_alloc(struct device *dev) { return NULL; }
+static inline void pwrseq_free(const struct pwrseq *pwrseq) {}
+
 #endif /* CONFIG_POWER_SEQ */
 
 #endif /* _LINUX_PWRSEQ_H */
-- 
1.9.1

Re: [RFC v2 09/13] power: pwrseq: Add support for USB hubs with external power

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Some USB devices on embedded boards have external power supply which has
> to be reset in certain conditions. Add pwrseq interface for this.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/power/pwrseq/pwrseq.c | 44 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pwrseq.h        |  8 ++++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
> index 495a19d3c30b..306265f55a10 100644
> --- a/drivers/power/pwrseq/pwrseq.c
> +++ b/drivers/power/pwrseq/pwrseq.c
> @@ -52,6 +52,43 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
>  }
>  EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
>  
> +struct pwrseq *pwrseq_alloc(struct device *dev)
> +{

This function is USB specific so better to call it usb_pwrseq_alloc() instead.

Although, this function has a lot of duplicated code from mmc_pwrseq_alloc()
so I think is better to keep the name generic and factorize the common code.

I expect other devices are also needing some kind of power seq in the future
so having a single alloc function instead of each for device type makes sense.

> +	struct device_node *np;
> +	struct pwrseq *p, *ret = NULL;
> +
> +	np = of_parse_phandle(dev->of_node, "usb-pwrseq", 0);

I know this is just an RFC but you should really add DT bindings for this.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RFC v2 09/13] power: pwrseq: Add support for USB hubs with external power

[Krzysztof Kozlowski]

On 05/05/2016 09:52 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
>> Some USB devices on embedded boards have external power supply which has
>> to be reset in certain conditions. Add pwrseq interface for this.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> ---
>>  drivers/power/pwrseq/pwrseq.c | 44 +++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/pwrseq.h        |  8 ++++++++
>>  2 files changed, 52 insertions(+)
>>
>> diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
>> index 495a19d3c30b..306265f55a10 100644
>> --- a/drivers/power/pwrseq/pwrseq.c
>> +++ b/drivers/power/pwrseq/pwrseq.c
>> @@ -52,6 +52,43 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
>>  }
>>  EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
>>  
>> +struct pwrseq *pwrseq_alloc(struct device *dev)
>> +{
> 
> This function is USB specific so better to call it usb_pwrseq_alloc() instead.

Indeed it is parsing USB specific bindings so such prefix is needed.

> Although, this function has a lot of duplicated code from mmc_pwrseq_alloc()
> so I think is better to keep the name generic and factorize the common code.
> 
> I expect other devices are also needing some kind of power seq in the future
> so having a single alloc function instead of each for device type makes sense.

Yes, this can be cleaned up and unified.

> 
>> +	struct device_node *np;
>> +	struct pwrseq *p, *ret = NULL;
>> +
>> +	np = of_parse_phandle(dev->of_node, "usb-pwrseq", 0);
> 
> I know this is just an RFC but you should really add DT bindings for this.

Yep, next step.

Best regards,
Krzysztof

[RFC v2 10/13] usb: hub: Power sequence the ports on activation

[Krzysztof Kozlowski]

The autodetection of attached USB device might not work on certain
boards where the power is delivered externally. These devices also might
require a hard reset. Use pwrseq for that in USB hub.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/usb/core/hub.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 1c82fcc448f5..0fddaacc62bf 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -26,6 +26,7 @@
 #include <linux/mutex.h>
 #include <linux/random.h>
 #include <linux/pm_qos.h>
+#include <linux/pwrseq.h>
 
 #include <asm/uaccess.h>
 #include <asm/byteorder.h>
@@ -1661,7 +1662,17 @@ static int hub_configure(struct usb_hub *hub,
 
 	usb_hub_adjust_deviceremovable(hdev, hub->descriptor);
 
+	/* FIXME: When do the pre-power-on? */
+	/*
+	for (i = 0; i < maxchild; i++)
+		pwrseq_pre_power_on(hub->ports[i]->pwrseq);
+	*/
+
+	for (i = 0; i < maxchild; i++)
+		pwrseq_post_power_on(hub->ports[i]->pwrseq);
+
 	hub_activate(hub, HUB_INIT);
+
 	return 0;
 
 fail:
-- 
1.9.1

Re: [RFC v2 10/13] usb: hub: Power sequence the ports on activation

[Alan Stern]

On Thu, 5 May 2016, Krzysztof Kozlowski wrote:

> The autodetection of attached USB device might not work on certain
> boards where the power is delivered externally. These devices also might
> require a hard reset. Use pwrseq for that in USB hub.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/usb/core/hub.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 1c82fcc448f5..0fddaacc62bf 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -26,6 +26,7 @@
>  #include <linux/mutex.h>
>  #include <linux/random.h>
>  #include <linux/pm_qos.h>
> +#include <linux/pwrseq.h>
>  
>  #include <asm/uaccess.h>
>  #include <asm/byteorder.h>
> @@ -1661,7 +1662,17 @@ static int hub_configure(struct usb_hub *hub,
>  
>  	usb_hub_adjust_deviceremovable(hdev, hub->descriptor);
>  
> +	/* FIXME: When do the pre-power-on? */

It's hard to answer this without knowing what pre-power-on involves.

Why not create a pwrseq_power_on() routine that does pre_power_on 
followed by post_power_on?

> +	/*
> +	for (i = 0; i < maxchild; i++)
> +		pwrseq_pre_power_on(hub->ports[i]->pwrseq);
> +	*/
> +
> +	for (i = 0; i < maxchild; i++)
> +		pwrseq_post_power_on(hub->ports[i]->pwrseq);

This is patch 10/13.  hub->ports[i]->pwrseq doesn't get added until 
11/13.  Obviously you never tried compiling each patch in the series.

> +
>  	hub_activate(hub, HUB_INIT);
> +

Unnecessary blank line added.

>  	return 0;
>  
>  fail:

Alan Stern

--
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: [RFC v2 10/13] usb: hub: Power sequence the ports on activation

[Krzysztof Kozlowski]

On Thu, May 05, 2016 at 10:09:47AM -0400, Alan Stern wrote:
> On Thu, 5 May 2016, Krzysztof Kozlowski wrote:
> 
> > The autodetection of attached USB device might not work on certain
> > boards where the power is delivered externally. These devices also might
> > require a hard reset. Use pwrseq for that in USB hub.
> > 
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > ---
> >  drivers/usb/core/hub.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 1c82fcc448f5..0fddaacc62bf 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/mutex.h>
> >  #include <linux/random.h>
> >  #include <linux/pm_qos.h>
> > +#include <linux/pwrseq.h>
> >  
> >  #include <asm/uaccess.h>
> >  #include <asm/byteorder.h>
> > @@ -1661,7 +1662,17 @@ static int hub_configure(struct usb_hub *hub,
> >  
> >  	usb_hub_adjust_deviceremovable(hdev, hub->descriptor);
> >  
> > +	/* FIXME: When do the pre-power-on? */
> 
> It's hard to answer this without knowing what pre-power-on involves.

In my particular case, I want to achieve a full reset through regulator (off
and on) because the bootloader left it in initialized state. I assume
that if bootloader did not configure the device, the reset won't be
harmful.

In a MMC case, this pre-power-on on is setting the 'reset' GPIO (thus
triggering the reset) and post-power-on is clearing the 'reset'.

> Why not create a pwrseq_power_on() routine that does pre_power_on 
> followed by post_power_on?

For my purpose it seems sensible.

> 
> > +	/*
> > +	for (i = 0; i < maxchild; i++)
> > +		pwrseq_pre_power_on(hub->ports[i]->pwrseq);
> > +	*/
> > +
> > +	for (i = 0; i < maxchild; i++)
> > +		pwrseq_post_power_on(hub->ports[i]->pwrseq);
> 
> This is patch 10/13.  hub->ports[i]->pwrseq doesn't get added until 
> 11/13.  Obviously you never tried compiling each patch in the series.

Ahh yes, I forgot to reorder them. Thanks for spotting this.

> 
> > +
> >  	hub_activate(hub, HUB_INIT);
> > +
> 
> Unnecessary blank line added.

Thanks for feedback,
Krzysztof

Re: [RFC v2 10/13] usb: hub: Power sequence the ports on activation

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> The autodetection of attached USB device might not work on certain
> boards where the power is delivered externally. These devices also might
> require a hard reset. Use pwrseq for that in USB hub.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---

Patch looks good to me. So after fixing the bisectability issue
pointed out by Alan Stern:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[RFC v2 04/13] power: pwrseq: Enable COMPILE_TEST for drivers

[Krzysztof Kozlowski]

Allow build testing for power sequence drivers.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/power/pwrseq/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
index b5d2d6c65f28..4731ba01a958 100644
--- a/drivers/power/pwrseq/Kconfig
+++ b/drivers/power/pwrseq/Kconfig
@@ -9,7 +9,7 @@ if POWER_SEQ
 config POWER_SEQ_EMMC
 	tristate "HW reset support for eMMC"
 	default y
-	depends on OF
+	depends on OF || COMPILE_TEST
 	help
 	  This selects Hardware reset support aka pwrseq-emmc for eMMC
 	  devices. By default this option is set to y.
@@ -20,7 +20,7 @@ config POWER_SEQ_EMMC
 config POWER_SEQ_SIMPLE
 	tristate "Simple HW reset support for MMC"
 	default y
-	depends on OF
+	depends on OF || COMPILE_TEST
 	help
 	  This selects simple hardware reset support aka pwrseq-simple for MMC
 	  devices. By default this option is set to y.
-- 
1.9.1

--
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: [RFC v2 04/13] power: pwrseq: Enable COMPILE_TEST for drivers

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Allow build testing for power sequence drivers.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---

Reviewed-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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

[RFC v2 05/13] power: pwrseq: Remove mmc prefix from mmc_pwrseq

[Krzysztof Kozlowski]

The "mmc" prefix is no longer needed after moving the pwrseq core code
from mmc/ to power/.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/power/pwrseq/pwrseq.c        | 18 +++++++++---------
 drivers/power/pwrseq/pwrseq_emmc.c   |  8 ++++----
 drivers/power/pwrseq/pwrseq_simple.c |  8 ++++----
 include/linux/mmc/host.h             |  4 ++--
 include/linux/pwrseq.h               | 20 ++++++++++----------
 5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
index 66310d7643cc..9c665821f890 100644
--- a/drivers/power/pwrseq/pwrseq.c
+++ b/drivers/power/pwrseq/pwrseq.c
@@ -21,7 +21,7 @@ static LIST_HEAD(pwrseq_list);
 int mmc_pwrseq_alloc(struct mmc_host *host)
 {
 	struct device_node *np;
-	struct mmc_pwrseq *p;
+	struct pwrseq *p;
 
 	np = of_parse_phandle(host->parent->of_node, "mmc-pwrseq", 0);
 	if (!np)
@@ -54,7 +54,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
 
 void mmc_pwrseq_pre_power_on(struct mmc_host *host)
 {
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
+	struct pwrseq *pwrseq = host->pwrseq;
 
 	if (pwrseq && pwrseq->ops->pre_power_on)
 		pwrseq->ops->pre_power_on(host);
@@ -63,7 +63,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_pre_power_on);
 
 void mmc_pwrseq_post_power_on(struct mmc_host *host)
 {
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
+	struct pwrseq *pwrseq = host->pwrseq;
 
 	if (pwrseq && pwrseq->ops->post_power_on)
 		pwrseq->ops->post_power_on(host);
@@ -72,7 +72,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_post_power_on);
 
 void mmc_pwrseq_power_off(struct mmc_host *host)
 {
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
+	struct pwrseq *pwrseq = host->pwrseq;
 
 	if (pwrseq && pwrseq->ops->power_off)
 		pwrseq->ops->power_off(host);
@@ -81,7 +81,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_power_off);
 
 void mmc_pwrseq_free(struct mmc_host *host)
 {
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
+	struct pwrseq *pwrseq = host->pwrseq;
 
 	if (pwrseq) {
 		module_put(pwrseq->owner);
@@ -90,7 +90,7 @@ void mmc_pwrseq_free(struct mmc_host *host)
 }
 EXPORT_SYMBOL_GPL(mmc_pwrseq_free);
 
-int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
+int pwrseq_register(struct pwrseq *pwrseq)
 {
 	if (!pwrseq || !pwrseq->ops || !pwrseq->dev)
 		return -EINVAL;
@@ -101,9 +101,9 @@ int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_register);
+EXPORT_SYMBOL_GPL(pwrseq_register);
 
-void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq)
+void pwrseq_unregister(struct pwrseq *pwrseq)
 {
 	if (pwrseq) {
 		mutex_lock(&pwrseq_list_mutex);
@@ -111,4 +111,4 @@ void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq)
 		mutex_unlock(&pwrseq_list_mutex);
 	}
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_unregister);
+EXPORT_SYMBOL_GPL(pwrseq_unregister);
diff --git a/drivers/power/pwrseq/pwrseq_emmc.c b/drivers/power/pwrseq/pwrseq_emmc.c
index a0583ed46d7f..a68ac9a68e04 100644
--- a/drivers/power/pwrseq/pwrseq_emmc.c
+++ b/drivers/power/pwrseq/pwrseq_emmc.c
@@ -22,7 +22,7 @@
 #include <linux/mmc/host.h>
 
 struct mmc_pwrseq_emmc {
-	struct mmc_pwrseq pwrseq;
+	struct pwrseq pwrseq;
 	struct notifier_block reset_nb;
 	struct gpio_desc *reset_gpio;
 };
@@ -54,7 +54,7 @@ static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
 	return NOTIFY_DONE;
 }
 
-static const struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
+static const struct pwrseq_ops mmc_pwrseq_emmc_ops = {
 	.post_power_on = mmc_pwrseq_emmc_reset,
 };
 
@@ -85,7 +85,7 @@ static int mmc_pwrseq_emmc_probe(struct platform_device *pdev)
 	pwrseq->pwrseq.owner = THIS_MODULE;
 	platform_set_drvdata(pdev, pwrseq);
 
-	return mmc_pwrseq_register(&pwrseq->pwrseq);
+	return pwrseq_register(&pwrseq->pwrseq);
 }
 
 static int mmc_pwrseq_emmc_remove(struct platform_device *pdev)
@@ -93,7 +93,7 @@ static int mmc_pwrseq_emmc_remove(struct platform_device *pdev)
 	struct mmc_pwrseq_emmc *pwrseq = platform_get_drvdata(pdev);
 
 	unregister_restart_handler(&pwrseq->reset_nb);
-	mmc_pwrseq_unregister(&pwrseq->pwrseq);
+	pwrseq_unregister(&pwrseq->pwrseq);
 
 	return 0;
 }
diff --git a/drivers/power/pwrseq/pwrseq_simple.c b/drivers/power/pwrseq/pwrseq_simple.c
index 786f1db53a3f..d5fbd653153e 100644
--- a/drivers/power/pwrseq/pwrseq_simple.c
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -21,7 +21,7 @@
 #include <linux/mmc/host.h>
 
 struct mmc_pwrseq_simple {
-	struct mmc_pwrseq pwrseq;
+	struct pwrseq pwrseq;
 	bool clk_enabled;
 	struct clk *ext_clk;
 	struct gpio_descs *reset_gpios;
@@ -77,7 +77,7 @@ static void mmc_pwrseq_simple_power_off(struct mmc_host *host)
 	}
 }
 
-static const struct mmc_pwrseq_ops mmc_pwrseq_simple_ops = {
+static const struct pwrseq_ops mmc_pwrseq_simple_ops = {
 	.pre_power_on = mmc_pwrseq_simple_pre_power_on,
 	.post_power_on = mmc_pwrseq_simple_post_power_on,
 	.power_off = mmc_pwrseq_simple_power_off,
@@ -115,14 +115,14 @@ static int mmc_pwrseq_simple_probe(struct platform_device *pdev)
 	pwrseq->pwrseq.owner = THIS_MODULE;
 	platform_set_drvdata(pdev, pwrseq);
 
-	return mmc_pwrseq_register(&pwrseq->pwrseq);
+	return pwrseq_register(&pwrseq->pwrseq);
 }
 
 static int mmc_pwrseq_simple_remove(struct platform_device *pdev)
 {
 	struct mmc_pwrseq_simple *pwrseq = platform_get_drvdata(pdev);
 
-	mmc_pwrseq_unregister(&pwrseq->pwrseq);
+	pwrseq_unregister(&pwrseq->pwrseq);
 
 	return 0;
 }
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 85800b48241f..ad281bb3475f 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -203,7 +203,7 @@ struct mmc_context_info {
 };
 
 struct regulator;
-struct mmc_pwrseq;
+struct pwrseq;
 
 struct mmc_supply {
 	struct regulator *vmmc;		/* Card power supply */
@@ -215,7 +215,7 @@ struct mmc_host {
 	struct device		class_dev;
 	int			index;
 	const struct mmc_host_ops *ops;
-	struct mmc_pwrseq	*pwrseq;
+	struct pwrseq		*pwrseq;
 	unsigned int		f_min;
 	unsigned int		f_max;
 	unsigned int		f_init;
diff --git a/include/linux/pwrseq.h b/include/linux/pwrseq.h
index e230670c1d8d..6d2d2a44ad35 100644
--- a/include/linux/pwrseq.h
+++ b/include/linux/pwrseq.h
@@ -5,19 +5,19 @@
  *
  * License terms: GNU General Public License (GPL) version 2
  */
-#ifndef _MMC_CORE_PWRSEQ_H
-#define _MMC_CORE_PWRSEQ_H
+#ifndef _LINUX_PWRSEQ_H
+#define _LINUX_PWRSEQ_H
 
 #include <linux/mmc/host.h>
 
-struct mmc_pwrseq_ops {
+struct pwrseq_ops {
 	void (*pre_power_on)(struct mmc_host *host);
 	void (*post_power_on)(struct mmc_host *host);
 	void (*power_off)(struct mmc_host *host);
 };
 
-struct mmc_pwrseq {
-	const struct mmc_pwrseq_ops *ops;
+struct pwrseq {
+	const struct pwrseq_ops *ops;
 	struct device *dev;
 	struct list_head pwrseq_node;
 	struct module *owner;
@@ -25,8 +25,8 @@ struct mmc_pwrseq {
 
 #ifdef CONFIG_POWER_SEQ
 
-int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq);
-void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq);
+int pwrseq_register(struct pwrseq *pwrseq);
+void pwrseq_unregister(struct pwrseq *pwrseq);
 
 int mmc_pwrseq_alloc(struct mmc_host *host);
 void mmc_pwrseq_pre_power_on(struct mmc_host *host);
@@ -36,11 +36,11 @@ void mmc_pwrseq_free(struct mmc_host *host);
 
 #else /* CONFIG_POWER_SEQ */
 
-static inline int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
+static inline int pwrseq_register(struct pwrseq *pwrseq)
 {
 	return -ENOSYS;
 }
-static inline void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq) {}
+static inline void pwrseq_unregister(struct pwrseq *pwrseq) {}
 static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; }
 static inline void mmc_pwrseq_pre_power_on(struct mmc_host *host) {}
 static inline void mmc_pwrseq_post_power_on(struct mmc_host *host) {}
@@ -49,4 +49,4 @@ static inline void mmc_pwrseq_free(struct mmc_host *host) {}
 
 #endif /* CONFIG_POWER_SEQ */
 
-#endif
+#endif /* _LINUX_PWRSEQ_H */
-- 
1.9.1

--
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: [RFC v2 05/13] power: pwrseq: Remove mmc prefix from mmc_pwrseq

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> The "mmc" prefix is no longer needed after moving the pwrseq core code
> from mmc/ to power/.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---

[snip]

> diff --git a/drivers/power/pwrseq/pwrseq_emmc.c b/drivers/power/pwrseq/pwrseq_emmc.c
> index a0583ed46d7f..a68ac9a68e04 100644
> --- a/drivers/power/pwrseq/pwrseq_emmc.c
> +++ b/drivers/power/pwrseq/pwrseq_emmc.c
> @@ -22,7 +22,7 @@
>  #include <linux/mmc/host.h>
> 

I don't think this header inclusion is needed. At least I didn't see anything
defined there that's used in this driver. This also applies to pwrseq_simple.

I think you could remove those in another preparatory patches before the move.

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [RFC v2 05/13] power: pwrseq: Remove mmc prefix from mmc_pwrseq

[Krzysztof Kozlowski]

On 05/05/2016 09:09 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
>> The "mmc" prefix is no longer needed after moving the pwrseq core code
>> from mmc/ to power/.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> ---
> 
> [snip]
> 
>> diff --git a/drivers/power/pwrseq/pwrseq_emmc.c b/drivers/power/pwrseq/pwrseq_emmc.c
>> index a0583ed46d7f..a68ac9a68e04 100644
>> --- a/drivers/power/pwrseq/pwrseq_emmc.c
>> +++ b/drivers/power/pwrseq/pwrseq_emmc.c
>> @@ -22,7 +22,7 @@
>>  #include <linux/mmc/host.h>
>>
> 
> I don't think this header inclusion is needed. At least I didn't see anything
> defined there that's used in this driver. This also applies to pwrseq_simple.
> 
> I think you could remove those in another preparatory patches before the move.
>
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Good catch, thanks.

BR,
Krzysztof

[RFC v2 11/13] usb: port: Parse pwrseq phandle from Device Tree

[Krzysztof Kozlowski]

Parse usb-pwrseq property from Device Tree to get the phandle to pwrseq
device. The pwrseq device will be used by USB hub to cycle the power
before activating ports.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/core/hub.h  |  3 +++
 drivers/usb/core/port.c | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 34c1a7e22aae..68ca89780d26 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -24,6 +24,8 @@
 #include <linux/usb/hcd.h>
 #include "usb.h"
 
+struct pwrseq;
+
 struct usb_hub {
 	struct device		*intfdev;	/* the "interface" device */
 	struct usb_device	*hdev;
@@ -101,6 +103,7 @@ struct usb_port {
 	struct usb_dev_state *port_owner;
 	struct usb_port *peer;
 	struct dev_pm_qos_request *req;
+	struct pwrseq *pwrseq;
 	enum usb_port_connect_type connect_type;
 	usb_port_location_t location;
 	struct mutex status_lock;
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 14718a9ffcfb..a875bd342452 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -18,6 +18,8 @@
 
 #include <linux/slab.h>
 #include <linux/pm_qos.h>
+#include <linux/pwrseq.h>
+#include <linux/usb/of.h>
 
 #include "hub.h"
 
@@ -532,6 +534,14 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
 		return retval;
 	}
 
+	port_dev->dev.of_node = usb_of_get_child_node(hub->hdev->dev.of_node, port1);
+	port_dev->pwrseq = pwrseq_alloc(&port_dev->dev);
+	if (IS_ERR(port_dev->pwrseq)) {
+		device_unregister(&port_dev->dev);
+		/* TODO: what about EPROBE_DEFER? */
+		return PTR_ERR(port_dev->pwrseq);
+	}
+
 	find_and_link_peer(hub, port1);
 
 	/*
@@ -573,8 +583,13 @@ void usb_hub_remove_port_device(struct usb_hub *hub, int port1)
 	struct usb_port *port_dev = hub->ports[port1 - 1];
 	struct usb_port *peer;
 
+	pwrseq_power_off(port_dev->pwrseq);
+
 	peer = port_dev->peer;
 	if (peer)
 		unlink_peers(port_dev, peer);
+
+	pwrseq_free(port_dev->pwrseq);
+	port_dev->pwrseq = NULL;
 	device_unregister(&port_dev->dev);
 }
-- 
1.9.1

--
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: [RFC v2 11/13] usb: port: Parse pwrseq phandle from Device Tree

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Parse usb-pwrseq property from Device Tree to get the phandle to pwrseq
> device. The pwrseq device will be used by USB hub to cycle the power
> before activating ports.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---

[snip]

>  
> @@ -532,6 +534,14 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
>  		return retval;
>  	}
>  
> +	port_dev->dev.of_node = usb_of_get_child_node(hub->hdev->dev.of_node, port1);
> +	port_dev->pwrseq = pwrseq_alloc(&port_dev->dev);
> +	if (IS_ERR(port_dev->pwrseq)) {
> +		device_unregister(&port_dev->dev);
> +		/* TODO: what about EPROBE_DEFER? */

I think it's OK since the call chain is:

hub_probe()
       hub_configure()
              usb_hub_create_port_device()

so the hub_probe() will be deferred if the usb-pwrseq was not registered yet.
Unless I misunderstood your question :)

Anyway, patch looks good to me:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
 
Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [RFC v2 11/13] usb: port: Parse pwrseq phandle from Device Tree

[Krzysztof Kozlowski]

On 05/05/2016 10:10 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
>> Parse usb-pwrseq property from Device Tree to get the phandle to pwrseq
>> device. The pwrseq device will be used by USB hub to cycle the power
>> before activating ports.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> ---
> 
> [snip]
> 
>>  
>> @@ -532,6 +534,14 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
>>  		return retval;
>>  	}
>>  
>> +	port_dev->dev.of_node = usb_of_get_child_node(hub->hdev->dev.of_node, port1);
>> +	port_dev->pwrseq = pwrseq_alloc(&port_dev->dev);
>> +	if (IS_ERR(port_dev->pwrseq)) {
>> +		device_unregister(&port_dev->dev);
>> +		/* TODO: what about EPROBE_DEFER? */
> 
> I think it's OK since the call chain is:
> 
> hub_probe()
>        hub_configure()
>               usb_hub_create_port_device()
> 
> so the hub_probe() will be deferred if the usb-pwrseq was not registered yet.
> Unless I misunderstood your question :)
> 
> Anyway, patch looks good to me:
> 
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Yes and deferred probing works in my case. That is an older comment
actually.

Thanks for review,
Krzysztof

[RFC v2 12/13] ARM: dts: exynos: Switch the buck8 to GPIO mode on Odroid U3

[Krzysztof Kozlowski]

Switch the control of buck8 to GPIO mode. It is faster than I2C/register
mode and it is the easiest way to disable it (regulator state is a
logical OR state of GPIO and register value).

Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 arch/arm/boot/dts/exynos4412-odroidu3.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts
index d73aa6c58fe3..31cdc036fda4 100644
--- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
@@ -74,6 +74,7 @@
 	regulator-name = "BUCK8_P3V3";
 	regulator-min-microvolt = <3300000>;
 	regulator-max-microvolt = <3300000>;
+	maxim,ena-gpios = <&gpa1 1 GPIO_ACTIVE_HIGH>;
 };
 
 /* VDDQ for MSHC (eMMC card) */
-- 
1.9.1

--
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: [RFC v2 12/13] ARM: dts: exynos: Switch the buck8 to GPIO mode on Odroid U3

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Switch the control of buck8 to GPIO mode. It is faster than I2C/register
> mode and it is the easiest way to disable it (regulator state is a
> logical OR state of GPIO and register value).
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Rob Herring]

On Thu, May 05, 2016 at 02:34:13PM +0200, Krzysztof Kozlowski wrote:
> Hi,
> 
> This is a different, second try to fix usb3503+lan on Odroid U3 board
> if it was initialized by bootloader (e.g. for TFTP boot).
> 
> First version:
> http://www.spinics.net/lists/linux-usb/msg140042.html
> 
> 
> Problem
> =======
> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
> is required, e.g. by suspend to RAM. The actual TFTP boot does
> not have to happen. Just "usb start" from U-Boot is sufficient.
> 
> From the schematics, the regulator is a supply only to LAN, however
> without toggling it off/on, the usb3503 hub won appear neither.
> 
> 
> Solution
> ========
> This is very similar to the MMC pwrseq behavior so the idea is to:
> 1. Move MMC pwrseq drivers to generic place,

You can do that, but I'm going to NAK any use of pwrseq bindings outside 
of MMC. I think it is the wrong way to do things. The DT should describe 
the devices. If they happen to be "simple" then the core can walk the 
tree and do any setup. For example, look for "reset-gpios" and toggle 
that GPIO. There is no need for a special node.

> 2. Extend the pwrseq-simple with regulator toggling,
> 3. Add support to USB hub and port core for pwrseq,

We discussed this for USB already[1] and is why we defined how to add 
USB child devices. The idea is not to add pwrseq to that.

Rob

[1] http://www.spinics.net/lists/linux-usb/msg134082.html
--
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: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Peter Chen]

On Thu, May 05, 2016 at 05:42:40PM -0500, Rob Herring wrote:
> On Thu, May 05, 2016 at 02:34:13PM +0200, Krzysztof Kozlowski wrote:
> > Hi,
> > 
> > This is a different, second try to fix usb3503+lan on Odroid U3 board
> > if it was initialized by bootloader (e.g. for TFTP boot).
> > 
> > First version:
> > http://www.spinics.net/lists/linux-usb/msg140042.html
> > 
> > 
> > Problem
> > =======
> > When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
> > the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
> > is required, e.g. by suspend to RAM. The actual TFTP boot does
> > not have to happen. Just "usb start" from U-Boot is sufficient.
> > 
> > From the schematics, the regulator is a supply only to LAN, however
> > without toggling it off/on, the usb3503 hub won appear neither.
> > 
> > 
> > Solution
> > ========
> > This is very similar to the MMC pwrseq behavior so the idea is to:
> > 1. Move MMC pwrseq drivers to generic place,
> 
> You can do that, but I'm going to NAK any use of pwrseq bindings outside 
> of MMC. I think it is the wrong way to do things. The DT should describe 
> the devices. If they happen to be "simple" then the core can walk the 
> tree and do any setup. For example, look for "reset-gpios" and toggle 
> that GPIO. There is no need for a special node.
> 

Oh, I am doing the same thing like this patch set doing. Then, how can
we let things be generic like you mention at [1] if without common
pwrseq driver/library? The properties like "reset-gpios" under
USB child node seems can only be handled by USB driver.

> > 2. Extend the pwrseq-simple with regulator toggling,
> > 3. Add support to USB hub and port core for pwrseq,
> 
> We discussed this for USB already[1] and is why we defined how to add 
> USB child devices. The idea is not to add pwrseq to that.
> 
> Rob
> 
> [1] http://www.spinics.net/lists/linux-usb/msg134082.html

[1] http://www.spinics.net/lists/linux-usb/msg137312.html

-- 

Best Regards,
Peter Chen

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Krzysztof Kozlowski]

On 05/06/2016 07:44 AM, Peter Chen wrote:
> On Thu, May 05, 2016 at 05:42:40PM -0500, Rob Herring wrote:
>> On Thu, May 05, 2016 at 02:34:13PM +0200, Krzysztof Kozlowski wrote:
>>> Hi,
>>>
>>> This is a different, second try to fix usb3503+lan on Odroid U3 board
>>> if it was initialized by bootloader (e.g. for TFTP boot).
>>>
>>> First version:
>>> http://www.spinics.net/lists/linux-usb/msg140042.html
>>>
>>>
>>> Problem
>>> =======
>>> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
>>> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
>>> is required, e.g. by suspend to RAM. The actual TFTP boot does
>>> not have to happen. Just "usb start" from U-Boot is sufficient.
>>>
>>> From the schematics, the regulator is a supply only to LAN, however
>>> without toggling it off/on, the usb3503 hub won appear neither.
>>>
>>>
>>> Solution
>>> ========
>>> This is very similar to the MMC pwrseq behavior so the idea is to:
>>> 1. Move MMC pwrseq drivers to generic place,
>>
>> You can do that, but I'm going to NAK any use of pwrseq bindings outside 
>> of MMC. I think it is the wrong way to do things. The DT should describe 
>> the devices. If they happen to be "simple" then the core can walk the 
>> tree and do any setup. For example, look for "reset-gpios" and toggle 
>> that GPIO. There is no need for a special node.
>>
> 
> Oh, I am doing the same thing like this patch set doing.

Shame on me that I did not use Google before starting the work. I could
just extend your patchset. I think we can combine our efforts.

Best regards,
Krzysztof

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Peter Chen]

On Fri, May 06, 2016 at 08:12:24AM +0200, Krzysztof Kozlowski wrote:
> On 05/06/2016 07:44 AM, Peter Chen wrote:
> > On Thu, May 05, 2016 at 05:42:40PM -0500, Rob Herring wrote:
> >> On Thu, May 05, 2016 at 02:34:13PM +0200, Krzysztof Kozlowski wrote:
> >>> Hi,
> >>>
> >>> This is a different, second try to fix usb3503+lan on Odroid U3 board
> >>> if it was initialized by bootloader (e.g. for TFTP boot).
> >>>
> >>> First version:
> >>> http://www.spinics.net/lists/linux-usb/msg140042.html
> >>>
> >>>
> >>> Problem
> >>> =======
> >>> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
> >>> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
> >>> is required, e.g. by suspend to RAM. The actual TFTP boot does
> >>> not have to happen. Just "usb start" from U-Boot is sufficient.
> >>>
> >>> From the schematics, the regulator is a supply only to LAN, however
> >>> without toggling it off/on, the usb3503 hub won appear neither.
> >>>
> >>>
> >>> Solution
> >>> ========
> >>> This is very similar to the MMC pwrseq behavior so the idea is to:
> >>> 1. Move MMC pwrseq drivers to generic place,
> >>
> >> You can do that, but I'm going to NAK any use of pwrseq bindings outside 
> >> of MMC. I think it is the wrong way to do things. The DT should describe 
> >> the devices. If they happen to be "simple" then the core can walk the 
> >> tree and do any setup. For example, look for "reset-gpios" and toggle 
> >> that GPIO. There is no need for a special node.
> >>
> > 
> > Oh, I am doing the same thing like this patch set doing.
> 
> Shame on me that I did not use Google before starting the work. I could
> just extend your patchset. I think we can combine our efforts.
> 

Sure :)

-- 

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

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Krzysztof Kozlowski]

On 05/06/2016 12:42 AM, Rob Herring wrote:
> On Thu, May 05, 2016 at 02:34:13PM +0200, Krzysztof Kozlowski wrote:
>> Hi,
>>
>> This is a different, second try to fix usb3503+lan on Odroid U3 board
>> if it was initialized by bootloader (e.g. for TFTP boot).
>>
>> First version:
>> http://www.spinics.net/lists/linux-usb/msg140042.html
>>
>>
>> Problem
>> =======
>> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
>> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
>> is required, e.g. by suspend to RAM. The actual TFTP boot does
>> not have to happen. Just "usb start" from U-Boot is sufficient.
>>
>> From the schematics, the regulator is a supply only to LAN, however
>> without toggling it off/on, the usb3503 hub won appear neither.
>>
>>
>> Solution
>> ========
>> This is very similar to the MMC pwrseq behavior so the idea is to:
>> 1. Move MMC pwrseq drivers to generic place,
> 
> You can do that, but I'm going to NAK any use of pwrseq bindings outside 
> of MMC. I think it is the wrong way to do things. The DT should describe 
> the devices. If they happen to be "simple" then the core can walk the 
> tree and do any setup. For example, look for "reset-gpios" and toggle 
> that GPIO. There is no need for a special node.

Okay, I got it, no node for pwrseq but parse device properties. In case
of reset-gpios it seems quite obvious but also actively used:
$ git grep reset-gpios arch/arm/boot/dts | wc -l
142

Definitely pwrseq shouldn't add itself to all of these devices.

My questions would be then:
1. An additional pwrseq compatible for device is acceptable?
2. How would you name the regulator? We shouldn't toggle off/on every
regulator but probably only some specific ones.

> 
>> 2. Extend the pwrseq-simple with regulator toggling,
>> 3. Add support to USB hub and port core for pwrseq,
> 
> We discussed this for USB already[1] and is why we defined how to add 
> USB child devices. The idea is not to add pwrseq to that.

Yes, I left it for next iteration because it would require much more
changes in USB core. As for now, these bindings are useless for USB
devices which are not yet enumerated (because power sequence has to be
done on them). Making use of these bindings would be a next step... Just
let me do it one step a time.

Best regards,
Krzysztof

> 
> Rob
> 
> [1] http://www.spinics.net/lists/linux-usb/msg134082.html
> 
> 

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Rob Herring]

On Fri, May 6, 2016 at 1:10 AM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> On 05/06/2016 12:42 AM, Rob Herring wrote:
>> On Thu, May 05, 2016 at 02:34:13PM +0200, Krzysztof Kozlowski wrote:
>>> Hi,
>>>
>>> This is a different, second try to fix usb3503+lan on Odroid U3 board
>>> if it was initialized by bootloader (e.g. for TFTP boot).
>>>
>>> First version:
>>> http://www.spinics.net/lists/linux-usb/msg140042.html
>>>
>>>
>>> Problem
>>> =======
>>> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
>>> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
>>> is required, e.g. by suspend to RAM. The actual TFTP boot does
>>> not have to happen. Just "usb start" from U-Boot is sufficient.
>>>
>>> From the schematics, the regulator is a supply only to LAN, however
>>> without toggling it off/on, the usb3503 hub won appear neither.
>>>
>>>
>>> Solution
>>> ========
>>> This is very similar to the MMC pwrseq behavior so the idea is to:
>>> 1. Move MMC pwrseq drivers to generic place,
>>
>> You can do that, but I'm going to NAK any use of pwrseq bindings outside
>> of MMC. I think it is the wrong way to do things. The DT should describe
>> the devices. If they happen to be "simple" then the core can walk the
>> tree and do any setup. For example, look for "reset-gpios" and toggle
>> that GPIO. There is no need for a special node.
>
> Okay, I got it, no node for pwrseq but parse device properties. In case
> of reset-gpios it seems quite obvious but also actively used:
> $ git grep reset-gpios arch/arm/boot/dts | wc -l
> 142
>
> Definitely pwrseq shouldn't add itself to all of these devices.
>
> My questions would be then:
> 1. An additional pwrseq compatible for device is acceptable?

Perhaps. The issue is whether common or driver specific code handles
this may change over time. In other words, it is purely a kernel
decision. It could move in either direction. It may be better to just
have a whitelist of devices (though it would need to be board specific
somehow).

> 2. How would you name the regulator? We shouldn't toggle off/on every
> regulator but probably only some specific ones.

I'd argue the generic case is just enable all the ones defined. If you
only need to deal with some of them or need a specific sequence, then
it is not generic.

Rob

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Ulf Hansson]

On 6 May 2016 at 00:42, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, May 05, 2016 at 02:34:13PM +0200, Krzysztof Kozlowski wrote:
>> Hi,
>>
>> This is a different, second try to fix usb3503+lan on Odroid U3 board
>> if it was initialized by bootloader (e.g. for TFTP boot).
>>
>> First version:
>> http://www.spinics.net/lists/linux-usb/msg140042.html
>>
>>
>> Problem
>> =======
>> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
>> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
>> is required, e.g. by suspend to RAM. The actual TFTP boot does
>> not have to happen. Just "usb start" from U-Boot is sufficient.
>>
>> From the schematics, the regulator is a supply only to LAN, however
>> without toggling it off/on, the usb3503 hub won appear neither.
>>
>>
>> Solution
>> ========
>> This is very similar to the MMC pwrseq behavior so the idea is to:
>> 1. Move MMC pwrseq drivers to generic place,
>
> You can do that, but I'm going to NAK any use of pwrseq bindings outside
> of MMC. I think it is the wrong way to do things. The DT should describe

Huh, I didn't know that was your view of the mmc pwrseq bindings. Why
didn't you NAK them before?

> the devices. If they happen to be "simple" then the core can walk the
> tree and do any setup. For example, look for "reset-gpios" and toggle
> that GPIO. There is no need for a special node.
>
>> 2. Extend the pwrseq-simple with regulator toggling,
>> 3. Add support to USB hub and port core for pwrseq,
>
> We discussed this for USB already[1] and is why we defined how to add
> USB child devices. The idea is not to add pwrseq to that.

I am not familiar with the USB discussion.

Still, let me give you some more background to the mmc pwrseq. The
idea from the mmc pwrseq bindings comes from the power-domain DT
bindings, as I thought these things were a bit related.
In both cases they are not directly a property of the device, but more
describing a HW dependency to allow the device to work.

One could probably use a child node instead of a phandle, but that
wasn't chosen back then. Of course you are the DT expert, but could
you perhaps tell me why a child node is better for cases like this?

>
> Rob
>
> [1] http://www.spinics.net/lists/linux-usb/msg134082.html

Kind regards
Uffe
--
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: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Rob Herring]

On Mon, May 09, 2016 at 09:46:40AM +0200, Ulf Hansson wrote:
> On 6 May 2016 at 00:42, Rob Herring <robh@kernel.org> wrote:
> > On Thu, May 05, 2016 at 02:34:13PM +0200, Krzysztof Kozlowski wrote:
> >> Hi,
> >>
> >> This is a different, second try to fix usb3503+lan on Odroid U3 board
> >> if it was initialized by bootloader (e.g. for TFTP boot).
> >>
> >> First version:
> >> http://www.spinics.net/lists/linux-usb/msg140042.html
> >>
> >>
> >> Problem
> >> =======
> >> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
> >> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
> >> is required, e.g. by suspend to RAM. The actual TFTP boot does
> >> not have to happen. Just "usb start" from U-Boot is sufficient.
> >>
> >> From the schematics, the regulator is a supply only to LAN, however
> >> without toggling it off/on, the usb3503 hub won appear neither.
> >>
> >>
> >> Solution
> >> ========
> >> This is very similar to the MMC pwrseq behavior so the idea is to:
> >> 1. Move MMC pwrseq drivers to generic place,
> >
> > You can do that, but I'm going to NAK any use of pwrseq bindings outside
> > of MMC. I think it is the wrong way to do things. The DT should describe
> 
> Huh, I didn't know that was your view of the mmc pwrseq bindings. Why
> didn't you NAK them before?

Unfortunately, either I missed it or it was a time I couldn't spend much 
time on reviews.

> > the devices. If they happen to be "simple" then the core can walk the
> > tree and do any setup. For example, look for "reset-gpios" and toggle
> > that GPIO. There is no need for a special node.
> >
> >> 2. Extend the pwrseq-simple with regulator toggling,
> >> 3. Add support to USB hub and port core for pwrseq,
> >
> > We discussed this for USB already[1] and is why we defined how to add
> > USB child devices. The idea is not to add pwrseq to that.
> 
> I am not familiar with the USB discussion.
> 
> Still, let me give you some more background to the mmc pwrseq. The
> idea from the mmc pwrseq bindings comes from the power-domain DT
> bindings, as I thought these things were a bit related.
> In both cases they are not directly a property of the device, but more
> describing a HW dependency to allow the device to work.

I could see this as a board level power domain. However the difference 
is we are not generally exposing internal SOC details the same way as 
board level components. Perhaps we could extend power domains to board 
level, but that is not what was done here.

> One could probably use a child node instead of a phandle, but that
> wasn't chosen back then. Of course you are the DT expert, but could
> you perhaps tell me why a child node is better for cases like this?

If there is a control path hierarchy, then we try to model that in DT 
with child nodes. In cases of SDIO and USB, there is a clear hierarchy. 
Ignoring the discovery ordering problem, we already have defined ways to 
describe GPIO connections, regulators, etc. to devices. Describing those 
things separately from the device to solve a particular issue that is 
really a kernel limitation is what I don't like.

Rob

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Ulf Hansson]

+ Arnd

[...]

>> >> Solution
>> >> ========
>> >> This is very similar to the MMC pwrseq behavior so the idea is to:
>> >> 1. Move MMC pwrseq drivers to generic place,
>> >
>> > You can do that, but I'm going to NAK any use of pwrseq bindings outside
>> > of MMC. I think it is the wrong way to do things. The DT should describe
>>
>> Huh, I didn't know that was your view of the mmc pwrseq bindings. Why
>> didn't you NAK them before?
>
> Unfortunately, either I missed it or it was a time I couldn't spend much
> time on reviews.

Okay, I guess it's common issue among maintainers. The problem with DT
is that it gets really hard to be fixed up later. :-)

>
>> > the devices. If they happen to be "simple" then the core can walk the
>> > tree and do any setup. For example, look for "reset-gpios" and toggle
>> > that GPIO. There is no need for a special node.
>> >
>> >> 2. Extend the pwrseq-simple with regulator toggling,
>> >> 3. Add support to USB hub and port core for pwrseq,
>> >
>> > We discussed this for USB already[1] and is why we defined how to add
>> > USB child devices. The idea is not to add pwrseq to that.
>>
>> I am not familiar with the USB discussion.
>>
>> Still, let me give you some more background to the mmc pwrseq. The
>> idea from the mmc pwrseq bindings comes from the power-domain DT
>> bindings, as I thought these things were a bit related.
>> In both cases they are not directly a property of the device, but more
>> describing a HW dependency to allow the device to work.
>
> I could see this as a board level power domain. However the difference
> is we are not generally exposing internal SOC details the same way as
> board level components. Perhaps we could extend power domains to board
> level, but that is not what was done here.
>
>> One could probably use a child node instead of a phandle, but that
>> wasn't chosen back then. Of course you are the DT expert, but could
>> you perhaps tell me why a child node is better for cases like this?
>
> If there is a control path hierarchy, then we try to model that in DT
> with child nodes. In cases of SDIO and USB, there is a clear hierarchy.
> Ignoring the discovery ordering problem, we already have defined ways to
> describe GPIO connections, regulators, etc. to devices. Describing those
> things separately from the device to solve a particular issue that is
> really a kernel limitation is what I don't like.

Okay, I see.

To move forward in trying to make mmc pwrseq a generic pwrseq, could
we perhaps allow both cases?

In the mmc case, there are already deployed bindings so we need to
cope with these by using the phandle option, but for USB etc we could
force the child node option.
As long as we agree that we keep using a compatible string for the
child node as well, both options should be able to co-exist and we
should probably be able to managed them both from a common pwrseq
driver framework.

Although, I do remember from an older conversations around some of
mine submission for the mmc pwrseq code, that some people (maybe
Arnd?) wasn't keen on adding a new framework for this. Perhaps that
has changed?

Kind regards
Uffe

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Peter Chen]

On Tue, May 10, 2016 at 01:02:08PM +0200, Ulf Hansson wrote:
> + Arnd
> 
> [...]
> 
> >> >> Solution
> >> >> ========
> >> >> This is very similar to the MMC pwrseq behavior so the idea is to:
> >> >> 1. Move MMC pwrseq drivers to generic place,
> >> >
> >> > You can do that, but I'm going to NAK any use of pwrseq bindings outside
> >> > of MMC. I think it is the wrong way to do things. The DT should describe
> >>
> >> Huh, I didn't know that was your view of the mmc pwrseq bindings. Why
> >> didn't you NAK them before?
> >
> > Unfortunately, either I missed it or it was a time I couldn't spend much
> > time on reviews.
> 
> Okay, I guess it's common issue among maintainers. The problem with DT
> is that it gets really hard to be fixed up later. :-)
> 
> >
> >> > the devices. If they happen to be "simple" then the core can walk the
> >> > tree and do any setup. For example, look for "reset-gpios" and toggle
> >> > that GPIO. There is no need for a special node.
> >> >
> >> >> 2. Extend the pwrseq-simple with regulator toggling,
> >> >> 3. Add support to USB hub and port core for pwrseq,
> >> >
> >> > We discussed this for USB already[1] and is why we defined how to add
> >> > USB child devices. The idea is not to add pwrseq to that.
> >>
> >> I am not familiar with the USB discussion.
> >>
> >> Still, let me give you some more background to the mmc pwrseq. The
> >> idea from the mmc pwrseq bindings comes from the power-domain DT
> >> bindings, as I thought these things were a bit related.
> >> In both cases they are not directly a property of the device, but more
> >> describing a HW dependency to allow the device to work.
> >
> > I could see this as a board level power domain. However the difference
> > is we are not generally exposing internal SOC details the same way as
> > board level components. Perhaps we could extend power domains to board
> > level, but that is not what was done here.
> >
> >> One could probably use a child node instead of a phandle, but that
> >> wasn't chosen back then. Of course you are the DT expert, but could
> >> you perhaps tell me why a child node is better for cases like this?
> >
> > If there is a control path hierarchy, then we try to model that in DT
> > with child nodes. In cases of SDIO and USB, there is a clear hierarchy.
> > Ignoring the discovery ordering problem, we already have defined ways to
> > describe GPIO connections, regulators, etc. to devices. Describing those
> > things separately from the device to solve a particular issue that is
> > really a kernel limitation is what I don't like.
> 
> Okay, I see.
> 
> To move forward in trying to make mmc pwrseq a generic pwrseq, could
> we perhaps allow both cases?
> 
> In the mmc case, there are already deployed bindings so we need to
> cope with these by using the phandle option, but for USB etc we could
> force the child node option.
> As long as we agree that we keep using a compatible string for the
> child node as well, both options should be able to co-exist and we
> should probably be able to managed them both from a common pwrseq
> driver framework.
> 
> Although, I do remember from an older conversations around some of
> mine submission for the mmc pwrseq code, that some people (maybe
> Arnd?) wasn't keen on adding a new framework for this. Perhaps that
> has changed?
> 

All, how we move on for this?

1. Using a generic driver to manage both mmc and USB (and further
subsystem), USB and further subsystem do not use pwrseq node in dts.
2. USB creates the similar driver under drivers/usb for its own use. 

Which one do you prefer, thanks.

-- 

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

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Peter Chen]

On Sat, May 28, 2016 at 11:36:13AM +0800, Peter Chen wrote:
> On Tue, May 10, 2016 at 01:02:08PM +0200, Ulf Hansson wrote:
> > + Arnd
> > 
> > [...]
> > 
> > >> >> Solution
> > >> >> ========
> > >> >> This is very similar to the MMC pwrseq behavior so the idea is to:
> > >> >> 1. Move MMC pwrseq drivers to generic place,
> > >> >
> > >> > You can do that, but I'm going to NAK any use of pwrseq bindings outside
> > >> > of MMC. I think it is the wrong way to do things. The DT should describe
> > >>
> > >> Huh, I didn't know that was your view of the mmc pwrseq bindings. Why
> > >> didn't you NAK them before?
> > >
> > > Unfortunately, either I missed it or it was a time I couldn't spend much
> > > time on reviews.
> > 
> > Okay, I guess it's common issue among maintainers. The problem with DT
> > is that it gets really hard to be fixed up later. :-)
> > 
> > >
> > >> > the devices. If they happen to be "simple" then the core can walk the
> > >> > tree and do any setup. For example, look for "reset-gpios" and toggle
> > >> > that GPIO. There is no need for a special node.
> > >> >
> > >> >> 2. Extend the pwrseq-simple with regulator toggling,
> > >> >> 3. Add support to USB hub and port core for pwrseq,
> > >> >
> > >> > We discussed this for USB already[1] and is why we defined how to add
> > >> > USB child devices. The idea is not to add pwrseq to that.
> > >>
> > >> I am not familiar with the USB discussion.
> > >>
> > >> Still, let me give you some more background to the mmc pwrseq. The
> > >> idea from the mmc pwrseq bindings comes from the power-domain DT
> > >> bindings, as I thought these things were a bit related.
> > >> In both cases they are not directly a property of the device, but more
> > >> describing a HW dependency to allow the device to work.
> > >
> > > I could see this as a board level power domain. However the difference
> > > is we are not generally exposing internal SOC details the same way as
> > > board level components. Perhaps we could extend power domains to board
> > > level, but that is not what was done here.
> > >
> > >> One could probably use a child node instead of a phandle, but that
> > >> wasn't chosen back then. Of course you are the DT expert, but could
> > >> you perhaps tell me why a child node is better for cases like this?
> > >
> > > If there is a control path hierarchy, then we try to model that in DT
> > > with child nodes. In cases of SDIO and USB, there is a clear hierarchy.
> > > Ignoring the discovery ordering problem, we already have defined ways to
> > > describe GPIO connections, regulators, etc. to devices. Describing those
> > > things separately from the device to solve a particular issue that is
> > > really a kernel limitation is what I don't like.
> > 
> > Okay, I see.
> > 
> > To move forward in trying to make mmc pwrseq a generic pwrseq, could
> > we perhaps allow both cases?
> > 
> > In the mmc case, there are already deployed bindings so we need to
> > cope with these by using the phandle option, but for USB etc we could
> > force the child node option.
> > As long as we agree that we keep using a compatible string for the
> > child node as well, both options should be able to co-exist and we
> > should probably be able to managed them both from a common pwrseq
> > driver framework.
> > 
> > Although, I do remember from an older conversations around some of
> > mine submission for the mmc pwrseq code, that some people (maybe
> > Arnd?) wasn't keen on adding a new framework for this. Perhaps that
> > has changed?
> > 
> 
> All, how we move on for this?
> 
> 1. Using a generic driver to manage both mmc and USB (and further
> subsystem), USB and further subsystem do not use pwrseq node in dts.
> 2. USB creates the similar driver under drivers/usb for its own use. 
> 
> Which one do you prefer, thanks.
> 

Hi Krzysztof Kozlowski,

I think option 1 may be better according to Rob and Ulf's comment.
Would you like going on your patch set? You can work on generic pwrseq
driver, I will do USB stuffs based on generic pwrseq driver?

-- 

Best Regards,
Peter Chen

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Krzysztof Kozlowski]

On 05/31/2016 02:58 AM, Peter Chen wrote:
> On Sat, May 28, 2016 at 11:36:13AM +0800, Peter Chen wrote:
>> All, how we move on for this?
>>
>> 1. Using a generic driver to manage both mmc and USB (and further
>> subsystem), USB and further subsystem do not use pwrseq node in dts.
>> 2. USB creates the similar driver under drivers/usb for its own use. 
>>
>> Which one do you prefer, thanks.
>>
> 
> Hi Krzysztof Kozlowski,
> 
> I think option 1 may be better according to Rob and Ulf's comment.

Hi,

I like the option #1 as well. The generic driver should rely and parse
existing bindings like 'reset-gpios'. Still additional property (e.g.
"do-the-power-sequence") seems needed as we do not want to play with all
reset-gpios in DTB.

> Would you like going on your patch set? You can work on generic pwrseq
> driver, I will do USB stuffs based on generic pwrseq driver?

Great, that sounds good! I'll start the work.

Best regards,
Krzysztof

[RFC v2 13/13] ARM: dts: exynos: Fix LAN and HUB after bootloader initialization on Odroid U3

[Krzysztof Kozlowski]

On Odroid U3 (Exynos4412-based) board if USB was initialized by
bootloader (in U-Boot "usb start" before tftpboot), the HUB (usb3503)
and LAN (smsc95xx) after after successful probing were not visible in the
system ("lsusb").

In such case the devices had to be fully reset before configuring.
Reset by GPIO (called RESET_N pin) and by RESET field in STCD register
in usb3503 HUB are not sufficient. Instead full reset has to be done by
disabling and enabling regulator.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 2 +-
 arch/arm/boot/dts/exynos4412-odroidu3.dts       | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index 14e653e32e0f..efa204a85c83 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -67,7 +67,7 @@
 		};
 	};
 
-	emmc_pwrseq: pwrseq {
+	emmc_pwrseq: pwrseq1 {
 		pinctrl-0 = <&sd1_cd>;
 		pinctrl-names = "default";
 		compatible = "mmc-pwrseq-emmc";
diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts
index 31cdc036fda4..3da0e6b3c32a 100644
--- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
@@ -41,6 +41,11 @@
 		cooling-levels = <0 102 170 230>;
 	};
 
+	lan_pwrseq: pwrseq2 {
+		compatible = "mmc-pwrseq-simple";
+		ext-supply = <&buck8_reg>;
+	};
+
 	thermal-zones {
 		cpu_thermal: cpu-thermal {
 			cooling-maps {
@@ -104,6 +109,7 @@
 &ehci {
 	port@1 {
 		status = "okay";
+		usb-pwrseq = <&lan_pwrseq>;
 	};
 	port@2 {
 		status = "okay";
-- 
1.9.1

Re: [RFC v2 13/13] ARM: dts: exynos: Fix LAN and HUB after bootloader initialization on Odroid U3

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> On Odroid U3 (Exynos4412-based) board if USB was initialized by
> bootloader (in U-Boot "usb start" before tftpboot), the HUB (usb3503)
> and LAN (smsc95xx) after after successful probing were not visible in the
> system ("lsusb").
> 
> In such case the devices had to be fully reset before configuring.
> Reset by GPIO (called RESET_N pin) and by RESET field in STCD register
> in usb3503 HUB are not sufficient. Instead full reset has to be done by
> disabling and enabling regulator.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---

[snip]

>  
> +	lan_pwrseq: pwrseq2 {
> +		compatible = "mmc-pwrseq-simple";

It feels strange to have a "mmc-pwrseq-simple" compatible for a USB power
sequence provider. As I mentioned in the other patch, I think there should
either be a DT binding for the USB pwrseq-simple with a "usb-pwrseq-simple"
compatible that binds to the same pwrseq-simple driver or maybe having a
generic DT binding for any device with a new "pwrseq-simple" compatible.

Patch looks good to me though, so after having a DT binding and changing
the compatible string:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [RFC v2 13/13] ARM: dts: exynos: Fix LAN and HUB after bootloader initialization on Odroid U3

[Krzysztof Kozlowski]

On 05/05/2016 10:16 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
>> On Odroid U3 (Exynos4412-based) board if USB was initialized by
>> bootloader (in U-Boot "usb start" before tftpboot), the HUB (usb3503)
>> and LAN (smsc95xx) after after successful probing were not visible in the
>> system ("lsusb").
>>
>> In such case the devices had to be fully reset before configuring.
>> Reset by GPIO (called RESET_N pin) and by RESET field in STCD register
>> in usb3503 HUB are not sufficient. Instead full reset has to be done by
>> disabling and enabling regulator.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> ---
> 
> [snip]
> 
>>  
>> +	lan_pwrseq: pwrseq2 {
>> +		compatible = "mmc-pwrseq-simple";
> 
> It feels strange to have a "mmc-pwrseq-simple" compatible for a USB power
> sequence provider. As I mentioned in the other patch, I think there should
> either be a DT binding for the USB pwrseq-simple with a "usb-pwrseq-simple"
> compatible that binds to the same pwrseq-simple driver or maybe having a
> generic DT binding for any device with a new "pwrseq-simple" compatible.
> 
> Patch looks good to me though, so after having a DT binding and changing
> the compatible string:
> 
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Probably this will change after Rob's feedback. :)

Best regards,
Krzysztof

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Hans Verkuil]

Hi Krzysztof,

This still seems to be broken with the latest 4.9 kernel, right?

Has there been any progress on this? Do you have an updated patch series
for me to use?

Regards,

	Hans

On 05/05/16 14:34, Krzysztof Kozlowski wrote:
> Hi,
>
> This is a different, second try to fix usb3503+lan on Odroid U3 board
> if it was initialized by bootloader (e.g. for TFTP boot).
>
> First version:
> http://www.spinics.net/lists/linux-usb/msg140042.html
>
>
> Problem
> =======
> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
> is required, e.g. by suspend to RAM. The actual TFTP boot does
> not have to happen. Just "usb start" from U-Boot is sufficient.
>

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Hans Verkuil]

Try again, this time with Krzysztof's new email address...

On 13/12/16 13:20, Hans Verkuil wrote:
> Hi Krzysztof,
>
> This still seems to be broken with the latest 4.9 kernel, right?
>
> Has there been any progress on this? Do you have an updated patch series
> for me to use?
>
> Regards,
>
>     Hans
>
> On 05/05/16 14:34, Krzysztof Kozlowski wrote:
>> Hi,
>>
>> This is a different, second try to fix usb3503+lan on Odroid U3 board
>> if it was initialized by bootloader (e.g. for TFTP boot).
>>
>> First version:
>> http://www.spinics.net/lists/linux-usb/msg140042.html
>>
>>
>> Problem
>> =======
>> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
>> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
>> is required, e.g. by suspend to RAM. The actual TFTP boot does
>> not have to happen. Just "usb start" from U-Boot is sufficient.
>>
>
>

Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Krzysztof Kozlowski]

On Tue, Dec 13, 2016 at 2:34 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Try again, this time with Krzysztof's new email address...
>
>
> On 13/12/16 13:20, Hans Verkuil wrote:
>>
>> Hi Krzysztof,
>>
>> This still seems to be broken with the latest 4.9 kernel, right?
>>
>> Has there been any progress on this? Do you have an updated patch series
>> for me to use?

Hi,

I think it is not fixed. Still. I left this work to others. AFAIK,
Peter Chen is working on a new generic approach:
https://lwn.net/Articles/703556/
On top of his patchset, Odroid would need some DTS code as well (and
maybe something in usb3503). However I do not plan to work on this
anymore as I do not have Odroid U3 anymore. Marek and Bartlomiej from
Samsung Poland are in CC-list so maybe they would like to continue the
work?

Best regards,
Krzysztof