diff mbox

[v2,05/13] usb: dwc3: change DWC3 core support code into a driver

Message ID 1463403086-25362-6-git-send-email-yamada.masahiro@socionext.com
State New
Headers show

Commit Message

Masahiro Yamada May 16, 2016, 12:51 p.m. UTC
Synopsys DWC3 IP generally works with an SoC-specific glue layer.
DT binding for that is like this:

  usb3_glue {
          compatible = "foo,dwc3";
          ...

          usb3 {
                  compatible = "snps,dwc3";
                  ...
          };
  };

The glue layer initializes some SoC-specific parts, then populates
the child DWC3 core.  To see how it works, refer to

  drivers/usb/dwc3/dwc3-exynos.c
  drivers/usb/dwc3/dwc3-keystone.c
  drivers/usb/dwc3/dwc3-omap.c
  drivers/usb/dwc3/dwc3-st.c

of Linux Kernel.

This commit implements a driver compatible with "snps,dwc3", allowing
to use the same binding in U-Boot.  The glue layer can be simply
implemented based on Simple Bus Uclass.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Marek Vasut <marex@denx.de>

---

Changes in v2: None

 drivers/usb/host/xhci-dwc3.c | 71 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

-- 
1.9.1

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Roger Quadros May 17, 2016, 8:20 a.m. UTC | #1
Hi Masahiro,

On 16/05/16 15:51, Masahiro Yamada wrote:
> Synopsys DWC3 IP generally works with an SoC-specific glue layer.

> DT binding for that is like this:

> 

>   usb3_glue {

>           compatible = "foo,dwc3";

>           ...

> 

>           usb3 {

>                   compatible = "snps,dwc3";

>                   ...

>           };

>   };

> 

> The glue layer initializes some SoC-specific parts, then populates

> the child DWC3 core.  To see how it works, refer to

> 

>   drivers/usb/dwc3/dwc3-exynos.c

>   drivers/usb/dwc3/dwc3-keystone.c

>   drivers/usb/dwc3/dwc3-omap.c

>   drivers/usb/dwc3/dwc3-st.c

> 

> of Linux Kernel.

> 

> This commit implements a driver compatible with "snps,dwc3", allowing

> to use the same binding in U-Boot.  The glue layer can be simply

> implemented based on Simple Bus Uclass.

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> Reviewed-by: Marek Vasut <marex@denx.de>

> ---

> 

> Changes in v2: None

> 

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


"snps,dwc3" compatible is for the dual-role controller that implements both
device and host controllers. So calling the driver xhci-dwc3.c is a bit misleading.

>  1 file changed, 70 insertions(+), 1 deletion(-)

> 

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

> index 33961cd..c7c8324 100644

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

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

> @@ -9,8 +9,13 @@

>   */

>  

>  #include <common.h>

> -#include <asm/io.h>

> +#include <dm/device.h>

> +#include <mapmem.h>

> +#include <linux/io.h>

>  #include <linux/usb/dwc3.h>

> +#include <linux/sizes.h>

> +

> +#include "xhci.h"

>  

>  void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode)

>  {

> @@ -97,3 +102,67 @@ void dwc3_set_fladj(struct dwc3 *dwc3_reg, u32 val)

>  	setbits_le32(&dwc3_reg->g_fladj, GFLADJ_30MHZ_REG_SEL |

>  			GFLADJ_30MHZ(val));

>  }

> +

> +struct dwc3_priv {

> +	struct xhci_ctrl ctrl;	/* should be the first member */

> +	void __iomem *regs;

> +};

> +

> +static int dwc3_probe(struct udevice *dev)

> +{

> +	struct dwc3_priv *priv = dev_get_priv(dev);

> +	struct xhci_hccr *hccr;

> +	struct xhci_hcor *hcor;

> +	fdt_addr_t base;

> +	int ret;

> +

> +	base = dev_get_addr(dev);

> +	if (base == FDT_ADDR_T_NONE)

> +		return -EINVAL;

> +

> +	priv->regs = map_sysmem(base, SZ_32K);

> +	if (!priv->regs)

> +		return -ENOMEM;

> +

> +	hccr = priv->regs;

> +

> +	hcor = priv->regs + HC_LENGTH(xhci_readl(&hccr->cr_capbase));

> +

> +	ret = dwc3_core_init(priv->regs + DWC3_REG_OFFSET);

> +	if (ret) {

> +		puts("XHCI: failed to initialize controller\n");

> +		return ret;

> +	}

> +

> +	/* We are hard-coding DWC3 core to Host Mode */

> +	dwc3_set_mode(priv->regs + DWC3_REG_OFFSET, DWC3_GCTL_PRTCAP_HOST);


Why are we hard-coding it to host mode? We need to take into account
the dr_mode DT property and the Kconfig options to decide which mode
we can operate in.

> +

> +	return xhci_register(dev, hccr, hcor);

> +}

> +

> +static int dwc3_remove(struct udevice *dev)

> +{

> +	struct dwc3_priv *priv = dev_get_priv(dev);

> +

> +	xhci_deregister(dev);

> +	unmap_sysmem(priv->regs);

> +

> +	return 0;

> +}

> +

> +static const struct udevice_id of_dwc3_match[] = {

> +	{ .compatible = "snps,dwc3" },

> +	{ .compatible = "synopsys,dwc3" },

> +	{ /* sentinel */ }

> +};

> +

> +U_BOOT_DRIVER(dwc3) = {

> +	.name = "dwc3",

> +	.id = UCLASS_USB,

> +	.of_match = of_dwc3_match,

> +	.probe = dwc3_probe,

> +	.remove = dwc3_remove,

> +	.ops = &xhci_usb_ops,

> +	.priv_auto_alloc_size = sizeof(struct dwc3_priv),

> +	.flags	= DM_FLAG_ALLOC_PRIV_DMA,

> +};

> 


cheers,
-roger
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Masahiro Yamada May 17, 2016, 8:57 a.m. UTC | #2
Hi Roger,


2016-05-17 17:20 GMT+09:00 Roger Quadros <rogerq@ti.com>:
> Hi Masahiro,

>

> On 16/05/16 15:51, Masahiro Yamada wrote:

>> Synopsys DWC3 IP generally works with an SoC-specific glue layer.

>> DT binding for that is like this:

>>

>>   usb3_glue {

>>           compatible = "foo,dwc3";

>>           ...

>>

>>           usb3 {

>>                   compatible = "snps,dwc3";

>>                   ...

>>           };

>>   };

>>

>> The glue layer initializes some SoC-specific parts, then populates

>> the child DWC3 core.  To see how it works, refer to

>>

>>   drivers/usb/dwc3/dwc3-exynos.c

>>   drivers/usb/dwc3/dwc3-keystone.c

>>   drivers/usb/dwc3/dwc3-omap.c

>>   drivers/usb/dwc3/dwc3-st.c

>>

>> of Linux Kernel.

>>

>> This commit implements a driver compatible with "snps,dwc3", allowing

>> to use the same binding in U-Boot.  The glue layer can be simply

>> implemented based on Simple Bus Uclass.

>>

>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

>> Reviewed-by: Marek Vasut <marex@denx.de>

>> ---

>>

>> Changes in v2: None

>>

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

>

> "snps,dwc3" compatible is for the dual-role controller that implements both

> device and host controllers.


I know this fact.
The "snps,dwc3" on my SoC only works in host mode, though.


> So calling the driver xhci-dwc3.c is a bit misleading.




I think we have separate work for
 - drivers/usb/host/xhci-dwc3.c   (only for host mode)
 - drivers/usb/dwc3/*             (currently, for gadget ??)

This is unfortunate, though.


>>  1 file changed, 70 insertions(+), 1 deletion(-)

>>

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

>> index 33961cd..c7c8324 100644

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

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

>> @@ -9,8 +9,13 @@

>>   */

>>

>>  #include <common.h>

>> -#include <asm/io.h>

>> +#include <dm/device.h>

>> +#include <mapmem.h>

>> +#include <linux/io.h>

>>  #include <linux/usb/dwc3.h>

>> +#include <linux/sizes.h>

>> +

>> +#include "xhci.h"

>>

>>  void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode)

>>  {

>> @@ -97,3 +102,67 @@ void dwc3_set_fladj(struct dwc3 *dwc3_reg, u32 val)

>>       setbits_le32(&dwc3_reg->g_fladj, GFLADJ_30MHZ_REG_SEL |

>>                       GFLADJ_30MHZ(val));

>>  }

>> +

>> +struct dwc3_priv {

>> +     struct xhci_ctrl ctrl;  /* should be the first member */

>> +     void __iomem *regs;

>> +};

>> +

>> +static int dwc3_probe(struct udevice *dev)

>> +{

>> +     struct dwc3_priv *priv = dev_get_priv(dev);

>> +     struct xhci_hccr *hccr;

>> +     struct xhci_hcor *hcor;

>> +     fdt_addr_t base;

>> +     int ret;

>> +

>> +     base = dev_get_addr(dev);

>> +     if (base == FDT_ADDR_T_NONE)

>> +             return -EINVAL;

>> +

>> +     priv->regs = map_sysmem(base, SZ_32K);

>> +     if (!priv->regs)

>> +             return -ENOMEM;

>> +

>> +     hccr = priv->regs;

>> +

>> +     hcor = priv->regs + HC_LENGTH(xhci_readl(&hccr->cr_capbase));

>> +

>> +     ret = dwc3_core_init(priv->regs + DWC3_REG_OFFSET);

>> +     if (ret) {

>> +             puts("XHCI: failed to initialize controller\n");

>> +             return ret;

>> +     }

>> +

>> +     /* We are hard-coding DWC3 core to Host Mode */

>> +     dwc3_set_mode(priv->regs + DWC3_REG_OFFSET, DWC3_GCTL_PRTCAP_HOST);

>

> Why are we hard-coding it to host mode?


Because this file is used for host mode.
It is enabled by CONFIG_USB_XHCI_DWC3, like follows.

obj-$(CONFIG_USB_XHCI_DWC3) += xhci-dwc3.o

Boards defining CONFIG_USB_XHCI_DWC3 expect
this driver works in the host mode, I guess.


> We need to take into account

> the dr_mode DT property and the Kconfig options to decide which mode

> we can operate in.



I thought drivers/usb/dwc3/* was working only for GADGET,
because currently USB_DWC3 depends on (USB && USB_GADGET).

As far as I understand, it will work for host mode with your series applied.
Correct?

If so, cool!
I am happy to move to drivers/usb/dwc3/*.

-- 
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Roger Quadros May 17, 2016, 10:22 a.m. UTC | #3
On 17/05/16 11:57, Masahiro Yamada wrote:
> Hi Roger,

> 

> 

> 2016-05-17 17:20 GMT+09:00 Roger Quadros <rogerq@ti.com>:

>> Hi Masahiro,

>>

>> On 16/05/16 15:51, Masahiro Yamada wrote:

>>> Synopsys DWC3 IP generally works with an SoC-specific glue layer.

>>> DT binding for that is like this:

>>>

>>>   usb3_glue {

>>>           compatible = "foo,dwc3";

>>>           ...

>>>

>>>           usb3 {

>>>                   compatible = "snps,dwc3";

>>>                   ...

>>>           };

>>>   };

>>>

>>> The glue layer initializes some SoC-specific parts, then populates

>>> the child DWC3 core.  To see how it works, refer to

>>>

>>>   drivers/usb/dwc3/dwc3-exynos.c

>>>   drivers/usb/dwc3/dwc3-keystone.c

>>>   drivers/usb/dwc3/dwc3-omap.c

>>>   drivers/usb/dwc3/dwc3-st.c

>>>

>>> of Linux Kernel.

>>>

>>> This commit implements a driver compatible with "snps,dwc3", allowing

>>> to use the same binding in U-Boot.  The glue layer can be simply

>>> implemented based on Simple Bus Uclass.

>>>

>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

>>> Reviewed-by: Marek Vasut <marex@denx.de>

>>> ---

>>>

>>> Changes in v2: None

>>>

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

>>

>> "snps,dwc3" compatible is for the dual-role controller that implements both

>> device and host controllers.

> 

> I know this fact.

> The "snps,dwc3" on my SoC only works in host mode, though.

> 

> 

>> So calling the driver xhci-dwc3.c is a bit misleading.

> 

> 

> 

> I think we have separate work for

>  - drivers/usb/host/xhci-dwc3.c   (only for host mode)

>  - drivers/usb/dwc3/*             (currently, for gadget ??)

> 

> This is unfortunate, though.


If we use the same compatible for both host and gadget, how do we specify
in the device tree which driver to probe?
> 

> 

>>>  1 file changed, 70 insertions(+), 1 deletion(-)

>>>

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

>>> index 33961cd..c7c8324 100644

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

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

>>> @@ -9,8 +9,13 @@

>>>   */

>>>

>>>  #include <common.h>

>>> -#include <asm/io.h>

>>> +#include <dm/device.h>

>>> +#include <mapmem.h>

>>> +#include <linux/io.h>

>>>  #include <linux/usb/dwc3.h>

>>> +#include <linux/sizes.h>

>>> +

>>> +#include "xhci.h"

>>>

>>>  void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode)

>>>  {

>>> @@ -97,3 +102,67 @@ void dwc3_set_fladj(struct dwc3 *dwc3_reg, u32 val)

>>>       setbits_le32(&dwc3_reg->g_fladj, GFLADJ_30MHZ_REG_SEL |

>>>                       GFLADJ_30MHZ(val));

>>>  }

>>> +

>>> +struct dwc3_priv {

>>> +     struct xhci_ctrl ctrl;  /* should be the first member */

>>> +     void __iomem *regs;

>>> +};

>>> +

>>> +static int dwc3_probe(struct udevice *dev)

>>> +{

>>> +     struct dwc3_priv *priv = dev_get_priv(dev);

>>> +     struct xhci_hccr *hccr;

>>> +     struct xhci_hcor *hcor;

>>> +     fdt_addr_t base;

>>> +     int ret;

>>> +

>>> +     base = dev_get_addr(dev);

>>> +     if (base == FDT_ADDR_T_NONE)

>>> +             return -EINVAL;

>>> +

>>> +     priv->regs = map_sysmem(base, SZ_32K);

>>> +     if (!priv->regs)

>>> +             return -ENOMEM;

>>> +

>>> +     hccr = priv->regs;

>>> +

>>> +     hcor = priv->regs + HC_LENGTH(xhci_readl(&hccr->cr_capbase));

>>> +

>>> +     ret = dwc3_core_init(priv->regs + DWC3_REG_OFFSET);

>>> +     if (ret) {

>>> +             puts("XHCI: failed to initialize controller\n");

>>> +             return ret;

>>> +     }

>>> +

>>> +     /* We are hard-coding DWC3 core to Host Mode */

>>> +     dwc3_set_mode(priv->regs + DWC3_REG_OFFSET, DWC3_GCTL_PRTCAP_HOST);

>>

>> Why are we hard-coding it to host mode?

> 

> Because this file is used for host mode.

> It is enabled by CONFIG_USB_XHCI_DWC3, like follows.

> 

> obj-$(CONFIG_USB_XHCI_DWC3) += xhci-dwc3.o

> 

> Boards defining CONFIG_USB_XHCI_DWC3 expect

> this driver works in the host mode, I guess.


OK as part of code will only be called for host mode.
> 

> 

>> We need to take into account

>> the dr_mode DT property and the Kconfig options to decide which mode

>> we can operate in.

> 

> 

> I thought drivers/usb/dwc3/* was working only for GADGET,

> because currently USB_DWC3 depends on (USB && USB_GADGET).

> 

> As far as I understand, it will work for host mode with your series applied.

> Correct?


I'm still unsure as to how all this fits in.

> 

> If so, cool!

> I am happy to move to drivers/usb/dwc3/*.

> 


Maybe Marek can share his view.

cheers,
-roger
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
diff mbox

Patch

diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c
index 33961cd..c7c8324 100644
--- a/drivers/usb/host/xhci-dwc3.c
+++ b/drivers/usb/host/xhci-dwc3.c
@@ -9,8 +9,13 @@ 
  */
 
 #include <common.h>
-#include <asm/io.h>
+#include <dm/device.h>
+#include <mapmem.h>
+#include <linux/io.h>
 #include <linux/usb/dwc3.h>
+#include <linux/sizes.h>
+
+#include "xhci.h"
 
 void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode)
 {
@@ -97,3 +102,67 @@  void dwc3_set_fladj(struct dwc3 *dwc3_reg, u32 val)
 	setbits_le32(&dwc3_reg->g_fladj, GFLADJ_30MHZ_REG_SEL |
 			GFLADJ_30MHZ(val));
 }
+
+struct dwc3_priv {
+	struct xhci_ctrl ctrl;	/* should be the first member */
+	void __iomem *regs;
+};
+
+static int dwc3_probe(struct udevice *dev)
+{
+	struct dwc3_priv *priv = dev_get_priv(dev);
+	struct xhci_hccr *hccr;
+	struct xhci_hcor *hcor;
+	fdt_addr_t base;
+	int ret;
+
+	base = dev_get_addr(dev);
+	if (base == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	priv->regs = map_sysmem(base, SZ_32K);
+	if (!priv->regs)
+		return -ENOMEM;
+
+	hccr = priv->regs;
+
+	hcor = priv->regs + HC_LENGTH(xhci_readl(&hccr->cr_capbase));
+
+	ret = dwc3_core_init(priv->regs + DWC3_REG_OFFSET);
+	if (ret) {
+		puts("XHCI: failed to initialize controller\n");
+		return ret;
+	}
+
+	/* We are hard-coding DWC3 core to Host Mode */
+	dwc3_set_mode(priv->regs + DWC3_REG_OFFSET, DWC3_GCTL_PRTCAP_HOST);
+
+	return xhci_register(dev, hccr, hcor);
+}
+
+static int dwc3_remove(struct udevice *dev)
+{
+	struct dwc3_priv *priv = dev_get_priv(dev);
+
+	xhci_deregister(dev);
+	unmap_sysmem(priv->regs);
+
+	return 0;
+}
+
+static const struct udevice_id of_dwc3_match[] = {
+	{ .compatible = "snps,dwc3" },
+	{ .compatible = "synopsys,dwc3" },
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(dwc3) = {
+	.name = "dwc3",
+	.id = UCLASS_USB,
+	.of_match = of_dwc3_match,
+	.probe = dwc3_probe,
+	.remove = dwc3_remove,
+	.ops = &xhci_usb_ops,
+	.priv_auto_alloc_size = sizeof(struct dwc3_priv),
+	.flags	= DM_FLAG_ALLOC_PRIV_DMA,
+};