subject:"\[PATCH 7\/7\] usb\: dwc2\: refactor common low\-level hw code to platform.c"

Re: [PATCH 7/7] usb: dwc2: refactor common low-level hw code to platform.c

2015-09-02 Thread John Youn

On 8/21/2015 5:39 AM, Marek Szyprowski wrote:
> DWC2 module on some platforms needs three additional hardware
> resources: phy controller, clock and power supply. All of them must be
> enabled/activated to properly initialize and operate. This was initially
> handled in s3c-hsotg driver, which has been converted to 'gadget' part
> of dwc2 driver. Unfortunately, not all of this code got moved to common
> platform code, what resulted in accessing DWC2 registers without
> enabling low-level hardware resources. This fails for example on Exynos
> SoCs. This patch moves all the code for managing those resources to
> common platform.c file and provides convenient wrappers for controlling
> them.
> 
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/usb/dwc2/core.h |   4 +-
>  drivers/usb/dwc2/gadget.c   | 188 +---
>  drivers/usb/dwc2/platform.c | 207 
> 
>  3 files changed, 195 insertions(+), 204 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 0ed87620941b..ec820bdf98c0 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -690,6 +690,7 @@ struct dwc2_hsotg {
>   enum usb_dr_mode dr_mode;
>   unsigned int hcd_enabled:1;
>   unsigned int gadget_enabled:1;
> + unsigned int ll_hw_enabled:1;

It seems like this flag is used to restore the state of the
lowlevel_hw on suspend/resume?

I think it is clearer to put the setting/clearing of the flag
inside the ll_hw_enable/disable and then check and adjust on
suspend/resume. That way you don't have to manage this flag
everywhere you call enable/disable().

John



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 7/7] usb: dwc2: refactor common low-level hw code to platform.c

2015-09-02 Thread John Youn

On 8/21/2015 5:39 AM, Marek Szyprowski wrote:
> DWC2 module on some platforms needs three additional hardware
> resources: phy controller, clock and power supply. All of them must be
> enabled/activated to properly initialize and operate. This was initially
> handled in s3c-hsotg driver, which has been converted to 'gadget' part
> of dwc2 driver. Unfortunately, not all of this code got moved to common
> platform code, what resulted in accessing DWC2 registers without
> enabling low-level hardware resources. This fails for example on Exynos
> SoCs. This patch moves all the code for managing those resources to
> common platform.c file and provides convenient wrappers for controlling
> them.
> 
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/usb/dwc2/core.h |   4 +-
>  drivers/usb/dwc2/gadget.c   | 188 +---
>  drivers/usb/dwc2/platform.c | 207 
> 
>  3 files changed, 195 insertions(+), 204 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 0ed87620941b..ec820bdf98c0 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -690,6 +690,7 @@ struct dwc2_hsotg {
>   enum usb_dr_mode dr_mode;
>   unsigned int hcd_enabled:1;
>   unsigned int gadget_enabled:1;
> + unsigned int ll_hw_enabled:1;

It seems like this flag is used to restore the state of the
lowlevel_hw on suspend/resume?

I think it is clearer to put the setting/clearing of the flag
inside the ll_hw_enable/disable and then check and adjust on
suspend/resume. That way you don't have to manage this flag
everywhere you call enable/disable().

John



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 7/7] usb: dwc2: refactor common low-level hw code to platform.c

2015-08-21 Thread Marek Szyprowski

DWC2 module on some platforms needs three additional hardware
resources: phy controller, clock and power supply. All of them must be
enabled/activated to properly initialize and operate. This was initially
handled in s3c-hsotg driver, which has been converted to 'gadget' part
of dwc2 driver. Unfortunately, not all of this code got moved to common
platform code, what resulted in accessing DWC2 registers without
enabling low-level hardware resources. This fails for example on Exynos
SoCs. This patch moves all the code for managing those resources to
common platform.c file and provides convenient wrappers for controlling
them.

Signed-off-by: Marek Szyprowski 
---
 drivers/usb/dwc2/core.h |   4 +-
 drivers/usb/dwc2/gadget.c   | 188 +---
 drivers/usb/dwc2/platform.c | 207 
 3 files changed, 195 insertions(+), 204 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 0ed87620941b..ec820bdf98c0 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -690,6 +690,7 @@ struct dwc2_hsotg {
enum usb_dr_mode dr_mode;
unsigned int hcd_enabled:1;
unsigned int gadget_enabled:1;
+   unsigned int ll_hw_enabled:1;
 
struct phy *phy;
struct usb_phy *uphy;
@@ -1088,7 +1089,8 @@ extern void dwc2_set_all_params(struct dwc2_core_params 
*params, int value);
 
 extern int dwc2_get_hwparams(struct dwc2_hsotg *hsotg);
 
-
+extern int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg);
+extern int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg);
 
 /*
  * Dump core registers and SPRAM
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index bccb60fcdd70..98f139be8c7f 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -25,15 +25,11 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
-#include 
 
 #include 
 #include 
 #include 
-#include 
 
 #include "core.h"
 #include "hw.h"
@@ -2944,50 +2940,6 @@ static struct usb_ep_ops s3c_hsotg_ep_ops = {
 };
 
 /**
- * s3c_hsotg_phy_enable - enable platform phy dev
- * @hsotg: The driver state
- *
- * A wrapper for platform code responsible for controlling
- * low-level USB code
- */
-static void s3c_hsotg_phy_enable(struct dwc2_hsotg *hsotg)
-{
-   struct platform_device *pdev = to_platform_device(hsotg->dev);
-
-   dev_dbg(hsotg->dev, "pdev 0x%p\n", pdev);
-
-   if (hsotg->uphy)
-   usb_phy_init(hsotg->uphy);
-   else if (hsotg->plat && hsotg->plat->phy_init)
-   hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
-   else {
-   phy_init(hsotg->phy);
-   phy_power_on(hsotg->phy);
-   }
-}
-
-/**
- * s3c_hsotg_phy_disable - disable platform phy dev
- * @hsotg: The driver state
- *
- * A wrapper for platform code responsible for controlling
- * low-level USB code
- */
-static void s3c_hsotg_phy_disable(struct dwc2_hsotg *hsotg)
-{
-   struct platform_device *pdev = to_platform_device(hsotg->dev);
-
-   if (hsotg->uphy)
-   usb_phy_shutdown(hsotg->uphy);
-   else if (hsotg->plat && hsotg->plat->phy_exit)
-   hsotg->plat->phy_exit(pdev, hsotg->plat->phy_type);
-   else {
-   phy_power_off(hsotg->phy);
-   phy_exit(hsotg->phy);
-   }
-}
-
-/**
  * s3c_hsotg_init - initalize the usb core
  * @hsotg: The driver state
  */
@@ -3068,14 +3020,13 @@ static int s3c_hsotg_udc_start(struct usb_gadget 
*gadget,
hsotg->gadget.dev.of_node = hsotg->dev->of_node;
hsotg->gadget.speed = USB_SPEED_UNKNOWN;
 
-   ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
-   hsotg->supplies);
-   if (ret) {
-   dev_err(hsotg->dev, "failed to enable supplies: %d\n", ret);
-   goto err;
+   if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) {
+   ret = dwc2_lowlevel_hw_enable(hsotg);
+   if (ret)
+   goto err;
+   hsotg->ll_hw_enabled = true;
}
 
-   s3c_hsotg_phy_enable(hsotg);
if (!IS_ERR_OR_NULL(hsotg->uphy))
otg_set_peripheral(hsotg->uphy->otg, >gadget);
 
@@ -3133,9 +3084,11 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget)
 
if (!IS_ERR_OR_NULL(hsotg->uphy))
otg_set_peripheral(hsotg->uphy->otg, NULL);
-   s3c_hsotg_phy_disable(hsotg);
 
-   regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
+   if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) {
+   dwc2_lowlevel_hw_disable(hsotg);
+   hsotg->ll_hw_enabled = false;
+   }
 
mutex_unlock(>init_mutex);
 
@@ -3473,7 +3426,6 @@ static inline void s3c_hsotg_of_probe(struct dwc2_hsotg 
*hsotg) { }
 int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
 {
struct device *dev = hsotg->dev;
-   struct s3c_hsotg_plat *plat =

[PATCH 7/7] usb: dwc2: refactor common low-level hw code to platform.c

2015-08-21 Thread Marek Szyprowski

DWC2 module on some platforms needs three additional hardware
resources: phy controller, clock and power supply. All of them must be
enabled/activated to properly initialize and operate. This was initially
handled in s3c-hsotg driver, which has been converted to 'gadget' part
of dwc2 driver. Unfortunately, not all of this code got moved to common
platform code, what resulted in accessing DWC2 registers without
enabling low-level hardware resources. This fails for example on Exynos
SoCs. This patch moves all the code for managing those resources to
common platform.c file and provides convenient wrappers for controlling
them.

Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
 drivers/usb/dwc2/core.h |   4 +-
 drivers/usb/dwc2/gadget.c   | 188 +---
 drivers/usb/dwc2/platform.c | 207 
 3 files changed, 195 insertions(+), 204 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 0ed87620941b..ec820bdf98c0 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -690,6 +690,7 @@ struct dwc2_hsotg {
enum usb_dr_mode dr_mode;
unsigned int hcd_enabled:1;
unsigned int gadget_enabled:1;
+   unsigned int ll_hw_enabled:1;
 
struct phy *phy;
struct usb_phy *uphy;
@@ -1088,7 +1089,8 @@ extern void dwc2_set_all_params(struct dwc2_core_params 
*params, int value);
 
 extern int dwc2_get_hwparams(struct dwc2_hsotg *hsotg);
 
-
+extern int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg);
+extern int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg);
 
 /*
  * Dump core registers and SPRAM
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index bccb60fcdd70..98f139be8c7f 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -25,15 +25,11 @@
 #include linux/delay.h
 #include linux/io.h
 #include linux/slab.h
-#include linux/clk.h
-#include linux/regulator/consumer.h
 #include linux/of_platform.h
-#include linux/phy/phy.h
 
 #include linux/usb/ch9.h
 #include linux/usb/gadget.h
 #include linux/usb/phy.h
-#include linux/platform_data/s3c-hsotg.h
 
 #include core.h
 #include hw.h
@@ -2944,50 +2940,6 @@ static struct usb_ep_ops s3c_hsotg_ep_ops = {
 };
 
 /**
- * s3c_hsotg_phy_enable - enable platform phy dev
- * @hsotg: The driver state
- *
- * A wrapper for platform code responsible for controlling
- * low-level USB code
- */
-static void s3c_hsotg_phy_enable(struct dwc2_hsotg *hsotg)
-{
-   struct platform_device *pdev = to_platform_device(hsotg-dev);
-
-   dev_dbg(hsotg-dev, pdev 0x%p\n, pdev);
-
-   if (hsotg-uphy)
-   usb_phy_init(hsotg-uphy);
-   else if (hsotg-plat  hsotg-plat-phy_init)
-   hsotg-plat-phy_init(pdev, hsotg-plat-phy_type);
-   else {
-   phy_init(hsotg-phy);
-   phy_power_on(hsotg-phy);
-   }
-}
-
-/**
- * s3c_hsotg_phy_disable - disable platform phy dev
- * @hsotg: The driver state
- *
- * A wrapper for platform code responsible for controlling
- * low-level USB code
- */
-static void s3c_hsotg_phy_disable(struct dwc2_hsotg *hsotg)
-{
-   struct platform_device *pdev = to_platform_device(hsotg-dev);
-
-   if (hsotg-uphy)
-   usb_phy_shutdown(hsotg-uphy);
-   else if (hsotg-plat  hsotg-plat-phy_exit)
-   hsotg-plat-phy_exit(pdev, hsotg-plat-phy_type);
-   else {
-   phy_power_off(hsotg-phy);
-   phy_exit(hsotg-phy);
-   }
-}
-
-/**
  * s3c_hsotg_init - initalize the usb core
  * @hsotg: The driver state
  */
@@ -3068,14 +3020,13 @@ static int s3c_hsotg_udc_start(struct usb_gadget 
*gadget,
hsotg-gadget.dev.of_node = hsotg-dev-of_node;
hsotg-gadget.speed = USB_SPEED_UNKNOWN;
 
-   ret = regulator_bulk_enable(ARRAY_SIZE(hsotg-supplies),
-   hsotg-supplies);
-   if (ret) {
-   dev_err(hsotg-dev, failed to enable supplies: %d\n, ret);
-   goto err;
+   if (hsotg-dr_mode == USB_DR_MODE_PERIPHERAL) {
+   ret = dwc2_lowlevel_hw_enable(hsotg);
+   if (ret)
+   goto err;
+   hsotg-ll_hw_enabled = true;
}
 
-   s3c_hsotg_phy_enable(hsotg);
if (!IS_ERR_OR_NULL(hsotg-uphy))
otg_set_peripheral(hsotg-uphy-otg, hsotg-gadget);
 
@@ -3133,9 +3084,11 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget)
 
if (!IS_ERR_OR_NULL(hsotg-uphy))
otg_set_peripheral(hsotg-uphy-otg, NULL);
-   s3c_hsotg_phy_disable(hsotg);
 
-   regulator_bulk_disable(ARRAY_SIZE(hsotg-supplies), hsotg-supplies);
+   if (hsotg-dr_mode == USB_DR_MODE_PERIPHERAL) {
+   dwc2_lowlevel_hw_disable(hsotg);
+   hsotg-ll_hw_enabled = false;
+   }
 
mutex_unlock(hsotg-init_mutex);
 
@@ -3473,7 +3426,6 @@ static inline void s3c_hsotg_of_probe(struct dwc2_hsotg

Re: [PATCH 7/7] usb: dwc2: refactor common low-level hw code to platform.c

Re: [PATCH 7/7] usb: dwc2: refactor common low-level hw code to platform.c

[PATCH 7/7] usb: dwc2: refactor common low-level hw code to platform.c

[PATCH 7/7] usb: dwc2: refactor common low-level hw code to platform.c

4 matches

Site Navigation

Mail list logo

Footer information