diff mbox

[U-Boot,3/3] usb: host: xhci-dwc3: Add generic PHY support

Message ID 1495178132-32467-4-git-send-email-patrice.chotard@st.com
State Superseded
Delegated to: Marek Vasut
Headers show

Commit Message

Patrice CHOTARD May 19, 2017, 7:15 a.m. UTC 
From: Patrice Chotard <patrice.chotard@st.com>

Add support of generic PHY framework support

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 drivers/usb/host/xhci-dwc3.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

Comments

Marek Vasut May 19, 2017, 10:07 a.m. UTC | #1 
On 05/19/2017 09:15 AM, patrice.chotard@st.com wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
> 
> Add support of generic PHY framework support
> 
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>  drivers/usb/host/xhci-dwc3.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c
> index ea85834..70d3946 100644
> --- a/drivers/usb/host/xhci-dwc3.c
> +++ b/drivers/usb/host/xhci-dwc3.c
> @@ -10,6 +10,8 @@
>  
>  #include <common.h>
>  #include <dm.h>
> +#include <fdtdec.h>
> +#include <generic-phy.h>
>  #include <usb.h>
>  
>  #include "xhci.h"
> @@ -20,6 +22,7 @@
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  struct xhci_dwc3_platdata {
> +	struct phy usb_phy;
>  	phys_addr_t dwc3_regs;
>  };
>  
> @@ -136,11 +139,26 @@ static int xhci_dwc3_probe(struct udevice *dev)
>  	struct xhci_hccr *hccr;
>  	struct dwc3 *dwc3_reg;
>  	enum usb_dr_mode dr_mode;
> +	int ret;
>  
>  	hccr = (struct xhci_hccr *)plat->dwc3_regs;
>  	hcor = (struct xhci_hcor *)((phys_addr_t)hccr +
>  			HC_LENGTH(xhci_readl(&(hccr)->cr_capbase)));
>  
> +	ret = generic_phy_get_by_index(dev, 0, &plat->usb_phy);
> +	if (ret) {
> +		if (ret != -ENOENT) {
> +			error("Failed to get USB PHY for %s\n", dev->name);
> +			return ret;
> +		}
> +	} else {

You can drop the else and indent here by reordering the condition.

Otherwise OK.

> +		ret = generic_phy_init(&plat->usb_phy);
> +		if (ret) {
> +			error("Can't init USB PHY for %s\n", dev->name);
> +			return ret;
> +		}
> +	}
> +
>  	dwc3_reg = (struct dwc3 *)((char *)(hccr) + DWC3_REG_OFFSET);
>  
>  	dwc3_core_init(dwc3_reg);
> @@ -157,6 +175,17 @@ static int xhci_dwc3_probe(struct udevice *dev)
>  
>  static int xhci_dwc3_remove(struct udevice *dev)
>  {
> +	struct xhci_dwc3_platdata *plat = dev_get_platdata(dev);
> +	int ret;
> +
> +	if (plat->usb_phy.dev) {
> +		ret = generic_phy_exit(&plat->usb_phy);
> +		if (ret) {
> +			error("Can't deinit USB PHY for %s\n", dev->name);
> +			return ret;
> +		}
> +	}
> +
>  	return xhci_deregister(dev);
>  }
>  
>
Patrice CHOTARD May 19, 2017, 12:05 p.m. UTC | #2 
Hi Marek

On 05/19/2017 12:07 PM, Marek Vasut wrote:
> On 05/19/2017 09:15 AM, patrice.chotard@st.com wrote:

>> From: Patrice Chotard <patrice.chotard@st.com>

>>

>> Add support of generic PHY framework support

>>

>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

>> ---

>>   drivers/usb/host/xhci-dwc3.c | 29 +++++++++++++++++++++++++++++

>>   1 file changed, 29 insertions(+)

>>

>> diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c

>> index ea85834..70d3946 100644

>> --- a/drivers/usb/host/xhci-dwc3.c

>> +++ b/drivers/usb/host/xhci-dwc3.c

>> @@ -10,6 +10,8 @@

>>   

>>   #include <common.h>

>>   #include <dm.h>

>> +#include <fdtdec.h>

>> +#include <generic-phy.h>

>>   #include <usb.h>

>>   

>>   #include "xhci.h"

>> @@ -20,6 +22,7 @@

>>   DECLARE_GLOBAL_DATA_PTR;

>>   

>>   struct xhci_dwc3_platdata {

>> +	struct phy usb_phy;

>>   	phys_addr_t dwc3_regs;

>>   };

>>   

>> @@ -136,11 +139,26 @@ static int xhci_dwc3_probe(struct udevice *dev)

>>   	struct xhci_hccr *hccr;

>>   	struct dwc3 *dwc3_reg;

>>   	enum usb_dr_mode dr_mode;

>> +	int ret;

>>   

>>   	hccr = (struct xhci_hccr *)plat->dwc3_regs;

>>   	hcor = (struct xhci_hcor *)((phys_addr_t)hccr +

>>   			HC_LENGTH(xhci_readl(&(hccr)->cr_capbase)));

>>   

>> +	ret = generic_phy_get_by_index(dev, 0, &plat->usb_phy);

>> +	if (ret) {

>> +		if (ret != -ENOENT) {

>> +			error("Failed to get USB PHY for %s\n", dev->name);

>> +			return ret;

>> +		}

>> +	} else {

> 

> You can drop the else and indent here by reordering the condition.


I can't drop the "else", otherwise, in case there is no "phys" property 
in DT (by example for arch/arm/dts/zynqmp.dtsi), 
generic_phy_get_by_index() returns -ENOENT, we must continue without 
calling generic_phy_init().

Patrice
> 

> Otherwise OK.

> 

>> +		ret = generic_phy_init(&plat->usb_phy);

>> +		if (ret) {

>> +			error("Can't init USB PHY for %s\n", dev->name);

>> +			return ret;

>> +		}

>> +	}

>> +

>>   	dwc3_reg = (struct dwc3 *)((char *)(hccr) + DWC3_REG_OFFSET);

>>   

>>   	dwc3_core_init(dwc3_reg);

>> @@ -157,6 +175,17 @@ static int xhci_dwc3_probe(struct udevice *dev)

>>   

>>   static int xhci_dwc3_remove(struct udevice *dev)

>>   {

>> +	struct xhci_dwc3_platdata *plat = dev_get_platdata(dev);

>> +	int ret;

>> +

>> +	if (plat->usb_phy.dev) {

>> +		ret = generic_phy_exit(&plat->usb_phy);

>> +		if (ret) {

>> +			error("Can't deinit USB PHY for %s\n", dev->name);

>> +			return ret;

>> +		}

>> +	}

>> +

>>   	return xhci_deregister(dev);

>>   }

>>   

>>

> 

>
Simon Glass May 22, 2017, 8:26 p.m. UTC | #3 
On 19 May 2017 at 01:15,  <patrice.chotard@st.com> wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
>
> Add support of generic PHY framework support
>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>  drivers/usb/host/xhci-dwc3.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Please see below.

> diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c
> index ea85834..70d3946 100644
> --- a/drivers/usb/host/xhci-dwc3.c
> +++ b/drivers/usb/host/xhci-dwc3.c
> @@ -10,6 +10,8 @@
>
>  #include <common.h>
>  #include <dm.h>
> +#include <fdtdec.h>
> +#include <generic-phy.h>
>  #include <usb.h>
>
>  #include "xhci.h"
> @@ -20,6 +22,7 @@
>  DECLARE_GLOBAL_DATA_PTR;
>
>  struct xhci_dwc3_platdata {
> +       struct phy usb_phy;
>         phys_addr_t dwc3_regs;
>  };
>
> @@ -136,11 +139,26 @@ static int xhci_dwc3_probe(struct udevice *dev)
>         struct xhci_hccr *hccr;
>         struct dwc3 *dwc3_reg;
>         enum usb_dr_mode dr_mode;
> +       int ret;
>
>         hccr = (struct xhci_hccr *)plat->dwc3_regs;
>         hcor = (struct xhci_hcor *)((phys_addr_t)hccr +
>                         HC_LENGTH(xhci_readl(&(hccr)->cr_capbase)));
>
> +       ret = generic_phy_get_by_index(dev, 0, &plat->usb_phy);
> +       if (ret) {
> +               if (ret != -ENOENT) {
> +                       error("Failed to get USB PHY for %s\n", dev->name);
> +                       return ret;
> +               }
> +       } else {
> +               ret = generic_phy_init(&plat->usb_phy);
> +               if (ret) {
> +                       error("Can't init USB PHY for %s\n", dev->name);
> +                       return ret;
> +               }
> +       }
> +
>         dwc3_reg = (struct dwc3 *)((char *)(hccr) + DWC3_REG_OFFSET);
>
>         dwc3_core_init(dwc3_reg);
> @@ -157,6 +175,17 @@ static int xhci_dwc3_probe(struct udevice *dev)
>
>  static int xhci_dwc3_remove(struct udevice *dev)
>  {
> +       struct xhci_dwc3_platdata *plat = dev_get_platdata(dev);
> +       int ret;
> +
> +       if (plat->usb_phy.dev) {

I'd like to have something like generic_phy_valid() to avoid poking
inside this data structure. Could you send a patch to add this please?

> +               ret = generic_phy_exit(&plat->usb_phy);
> +               if (ret) {
> +                       error("Can't deinit USB PHY for %s\n", dev->name);
> +                       return ret;
> +               }
> +       }
> +
>         return xhci_deregister(dev);
>  }
>
> --
> 1.9.1
>

Regards,
Simon
Patrice CHOTARD May 23, 2017, 7:21 a.m. UTC | #4 
Hi Simon

On 05/22/2017 10:26 PM, Simon Glass wrote:
> On 19 May 2017 at 01:15,  <patrice.chotard@st.com> wrote:

>> From: Patrice Chotard <patrice.chotard@st.com>

>>

>> Add support of generic PHY framework support

>>

>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

>> ---

>>   drivers/usb/host/xhci-dwc3.c | 29 +++++++++++++++++++++++++++++

>>   1 file changed, 29 insertions(+)

>>

> 

> Reviewed-by: Simon Glass <sjg@chromium.org>

> 

> Please see below.

> 

>> diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c

>> index ea85834..70d3946 100644

>> --- a/drivers/usb/host/xhci-dwc3.c

>> +++ b/drivers/usb/host/xhci-dwc3.c

>> @@ -10,6 +10,8 @@

>>

>>   #include <common.h>

>>   #include <dm.h>

>> +#include <fdtdec.h>

>> +#include <generic-phy.h>

>>   #include <usb.h>

>>

>>   #include "xhci.h"

>> @@ -20,6 +22,7 @@

>>   DECLARE_GLOBAL_DATA_PTR;

>>

>>   struct xhci_dwc3_platdata {

>> +       struct phy usb_phy;

>>          phys_addr_t dwc3_regs;

>>   };

>>

>> @@ -136,11 +139,26 @@ static int xhci_dwc3_probe(struct udevice *dev)

>>          struct xhci_hccr *hccr;

>>          struct dwc3 *dwc3_reg;

>>          enum usb_dr_mode dr_mode;

>> +       int ret;

>>

>>          hccr = (struct xhci_hccr *)plat->dwc3_regs;

>>          hcor = (struct xhci_hcor *)((phys_addr_t)hccr +

>>                          HC_LENGTH(xhci_readl(&(hccr)->cr_capbase)));

>>

>> +       ret = generic_phy_get_by_index(dev, 0, &plat->usb_phy);

>> +       if (ret) {

>> +               if (ret != -ENOENT) {

>> +                       error("Failed to get USB PHY for %s\n", dev->name);

>> +                       return ret;

>> +               }

>> +       } else {

>> +               ret = generic_phy_init(&plat->usb_phy);

>> +               if (ret) {

>> +                       error("Can't init USB PHY for %s\n", dev->name);

>> +                       return ret;

>> +               }

>> +       }

>> +

>>          dwc3_reg = (struct dwc3 *)((char *)(hccr) + DWC3_REG_OFFSET);

>>

>>          dwc3_core_init(dwc3_reg);

>> @@ -157,6 +175,17 @@ static int xhci_dwc3_probe(struct udevice *dev)

>>

>>   static int xhci_dwc3_remove(struct udevice *dev)

>>   {

>> +       struct xhci_dwc3_platdata *plat = dev_get_platdata(dev);

>> +       int ret;

>> +

>> +       if (plat->usb_phy.dev) {

> 

> I'd like to have something like generic_phy_valid() to avoid poking

> inside this data structure. Could you send a patch to add this please?


Sure, i will add the generic_phy_valid() method.

As the v2 of this series has already been send and reviewed by Marek.
I will send the v3 including a patch with generic_phy_valid() and make 
usage of it.

Thanks

Patrice

> 

>> +               ret = generic_phy_exit(&plat->usb_phy);

>> +               if (ret) {

>> +                       error("Can't deinit USB PHY for %s\n", dev->name);

>> +                       return ret;

>> +               }

>> +       }

>> +

>>          return xhci_deregister(dev);

>>   }

>>

>> --

>> 1.9.1

>>

> 

> Regards,

> Simon

>
diff mbox

Patch

diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c
index ea85834..70d3946 100644
--- a/drivers/usb/host/xhci-dwc3.c
+++ b/drivers/usb/host/xhci-dwc3.c
@@ -10,6 +10,8 @@ 
 
 #include <common.h>
 #include <dm.h>
+#include <fdtdec.h>
+#include <generic-phy.h>
 #include <usb.h>
 
 #include "xhci.h"
@@ -20,6 +22,7 @@ 
 DECLARE_GLOBAL_DATA_PTR;
 
 struct xhci_dwc3_platdata {
+	struct phy usb_phy;
 	phys_addr_t dwc3_regs;
 };
 
@@ -136,11 +139,26 @@  static int xhci_dwc3_probe(struct udevice *dev)
 	struct xhci_hccr *hccr;
 	struct dwc3 *dwc3_reg;
 	enum usb_dr_mode dr_mode;
+	int ret;
 
 	hccr = (struct xhci_hccr *)plat->dwc3_regs;
 	hcor = (struct xhci_hcor *)((phys_addr_t)hccr +
 			HC_LENGTH(xhci_readl(&(hccr)->cr_capbase)));
 
+	ret = generic_phy_get_by_index(dev, 0, &plat->usb_phy);
+	if (ret) {
+		if (ret != -ENOENT) {
+			error("Failed to get USB PHY for %s\n", dev->name);
+			return ret;
+		}
+	} else {
+		ret = generic_phy_init(&plat->usb_phy);
+		if (ret) {
+			error("Can't init USB PHY for %s\n", dev->name);
+			return ret;
+		}
+	}
+
 	dwc3_reg = (struct dwc3 *)((char *)(hccr) + DWC3_REG_OFFSET);
 
 	dwc3_core_init(dwc3_reg);
@@ -157,6 +175,17 @@  static int xhci_dwc3_probe(struct udevice *dev)
 
 static int xhci_dwc3_remove(struct udevice *dev)
 {
+	struct xhci_dwc3_platdata *plat = dev_get_platdata(dev);
+	int ret;
+
+	if (plat->usb_phy.dev) {
+		ret = generic_phy_exit(&plat->usb_phy);
+		if (ret) {
+			error("Can't deinit USB PHY for %s\n", dev->name);
+			return ret;
+		}
+	}
+
 	return xhci_deregister(dev);
 }