[U-Boot] [PATCH v2 0/4] rk3399: enable dwc3 gadget and fastboot

[U-Boot] [PATCH v2 0/4] rk3399: enable dwc3 gadget and fastboot

[Kever Yang]

This patch set enable rk3399 dwc3 controller and gadget driver
for fastboot.


Changes in v2:
- parse dt for utmi width
- use a variable to identify utmi+ bus width instead of CONFIG MACRO
- remove config for USB2PHY UTMI BITS

Kever Yang (4):
  rk3399: add a empty "sys_proto.h" header file
  board: evb-rk3399: add api to support dwc3 gadget
  usb: dwc3: add support for 16 bit UTMI+ interface
  config: rk3399: add support for dwc3 gadget

 arch/arm/include/asm/arch-rockchip/sys_proto.h | 10 ++++++++
 board/rockchip/evb_rk3399/evb-rk3399.c         | 33 ++++++++++++++++++++++++++
 drivers/usb/dwc3/core.c                        |  6 +++++
 drivers/usb/dwc3/core.h                        | 12 ++++++++++
 include/configs/rk3399_common.h                | 20 ++++++++++++++++
 include/dwc3-uboot.h                           |  1 +
 6 files changed, 82 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-rockchip/sys_proto.h

-- 
1.9.1

[U-Boot] [PATCH v2 1/4] rk3399: add a empty "sys_proto.h" header file

[Kever Yang]

driver/usb/dwc3/gadget.c need a "sys_proto.h" header file, add a
empty one to make compile success.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/arm/include/asm/arch-rockchip/sys_proto.h | 10 ++++++++++
 1 file changed, 10 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-rockchip/sys_proto.h

diff --git a/arch/arm/include/asm/arch-rockchip/sys_proto.h b/arch/arm/include/asm/arch-rockchip/sys_proto.h
new file mode 100644
index 0000000..35423e1
--- /dev/null
+++ b/arch/arm/include/asm/arch-rockchip/sys_proto.h
@@ -0,0 +1,10 @@
+/*
+ * (C) Copyright 2016 Rockchip Electronics Co.,Ltd
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _ASM_ARCH_SYS_PROTO_H
+#define _ASM_ARCH_SYS_PROTO_H
+
+#endif /* _ASM_ARCH_SYS_PROTO_H */
-- 
1.9.1

[U-Boot] [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget

[Kever Yang]

This patch add board_usb_init() and interrupt callback
for dwc3 gadget.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- parse dt for utmi width

 board/rockchip/evb_rk3399/evb-rk3399.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c b/board/rockchip/evb_rk3399/evb-rk3399.c
index d394276..5d3a09a 100644
--- a/board/rockchip/evb_rk3399/evb-rk3399.c
+++ b/board/rockchip/evb_rk3399/evb-rk3399.c
@@ -7,6 +7,8 @@
 #include <dm.h>
 #include <dm/pinctrl.h>
 #include <asm/arch/periph.h>
+#include <usb.h>
+#include <dwc3-uboot.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -54,3 +56,34 @@ void dram_init_banksize(void)
 	gd->bd->bi_dram[0].start = 0x200000;
 	gd->bd->bi_dram[0].size = 0x80000000;
 }
+
+#ifdef CONFIG_USB_DWC3
+static struct dwc3_device dwc3_device_data = {
+	.maximum_speed = USB_SPEED_HIGH,
+	.base = 0xfe800000,
+	.dr_mode = USB_DR_MODE_PERIPHERAL,
+	.index = 0,
+};
+
+int usb_gadget_handle_interrupts(void)
+{
+	dwc3_uboot_handle_interrupt(0);
+	return 0;
+}
+
+int board_usb_init(int index, enum usb_init_type init)
+{
+	const void *blob = gd->fdt_blob;
+	int node;
+
+	node = fdt_node_offset_by_compatible(blob, -1,
+					"rockchip,rk3399-xhci");
+	if (node < 0)
+		debug("%s node not found\n", __func__);
+	else
+		dwc3_device_data.usb2_phyif_utmi_width =
+			fdtdec_get_int(blob, node, "snps,phyif-utmi-bits", -1);
+
+	return dwc3_uboot_init(&dwc3_device_data);
+}
+#endif
-- 
1.9.1

[U-Boot] [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget

[Kever Yang]

+ Marek

On 08/24/2016 11:46 AM, Kever Yang wrote:
> This patch add board_usb_init() and interrupt callback
> for dwc3 gadget.
>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - parse dt for utmi width
>
>   board/rockchip/evb_rk3399/evb-rk3399.c | 33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
>
> diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c b/board/rockchip/evb_rk3399/evb-rk3399.c
> index d394276..5d3a09a 100644
> --- a/board/rockchip/evb_rk3399/evb-rk3399.c
> +++ b/board/rockchip/evb_rk3399/evb-rk3399.c
> @@ -7,6 +7,8 @@
>   #include <dm.h>
>   #include <dm/pinctrl.h>
>   #include <asm/arch/periph.h>
> +#include <usb.h>
> +#include <dwc3-uboot.h>
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> @@ -54,3 +56,34 @@ void dram_init_banksize(void)
>   	gd->bd->bi_dram[0].start = 0x200000;
>   	gd->bd->bi_dram[0].size = 0x80000000;
>   }
> +
> +#ifdef CONFIG_USB_DWC3
> +static struct dwc3_device dwc3_device_data = {
> +	.maximum_speed = USB_SPEED_HIGH,
> +	.base = 0xfe800000,
> +	.dr_mode = USB_DR_MODE_PERIPHERAL,
> +	.index = 0,
> +};
> +
> +int usb_gadget_handle_interrupts(void)
> +{
> +	dwc3_uboot_handle_interrupt(0);
> +	return 0;
> +}
> +
> +int board_usb_init(int index, enum usb_init_type init)
> +{
> +	const void *blob = gd->fdt_blob;
> +	int node;
> +
> +	node = fdt_node_offset_by_compatible(blob, -1,
> +					"rockchip,rk3399-xhci");
> +	if (node < 0)
> +		debug("%s node not found\n", __func__);
> +	else
> +		dwc3_device_data.usb2_phyif_utmi_width =
> +			fdtdec_get_int(blob, node, "snps,phyif-utmi-bits", -1);
> +
> +	return dwc3_uboot_init(&dwc3_device_data);
> +}
> +#endif

[U-Boot] [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget

[Marek Vasut]

On 08/25/2016 03:07 AM, Kever Yang wrote:
> + Marek
> 
> On 08/24/2016 11:46 AM, Kever Yang wrote:
>> This patch add board_usb_init() and interrupt callback
>> for dwc3 gadget.
>>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v2:
>> - parse dt for utmi width
>>
>>   board/rockchip/evb_rk3399/evb-rk3399.c | 33
>> +++++++++++++++++++++++++++++++++
>>   1 file changed, 33 insertions(+)
>>
>> diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c
>> b/board/rockchip/evb_rk3399/evb-rk3399.c
>> index d394276..5d3a09a 100644
>> --- a/board/rockchip/evb_rk3399/evb-rk3399.c
>> +++ b/board/rockchip/evb_rk3399/evb-rk3399.c
>> @@ -7,6 +7,8 @@
>>   #include <dm.h>
>>   #include <dm/pinctrl.h>
>>   #include <asm/arch/periph.h>
>> +#include <usb.h>
>> +#include <dwc3-uboot.h>
>>     DECLARE_GLOBAL_DATA_PTR;
>>   @@ -54,3 +56,34 @@ void dram_init_banksize(void)
>>       gd->bd->bi_dram[0].start = 0x200000;
>>       gd->bd->bi_dram[0].size = 0x80000000;
>>   }
>> +
>> +#ifdef CONFIG_USB_DWC3
>> +static struct dwc3_device dwc3_device_data = {
>> +    .maximum_speed = USB_SPEED_HIGH,
>> +    .base = 0xfe800000,
>> +    .dr_mode = USB_DR_MODE_PERIPHERAL,

Why don't you parse all of this from DT ? You already parse DT below.

>> +    .index = 0,
>> +};
>> +
>> +int usb_gadget_handle_interrupts(void)
>> +{
>> +    dwc3_uboot_handle_interrupt(0);
>> +    return 0;
>> +}
>> +
>> +int board_usb_init(int index, enum usb_init_type init)
>> +{
>> +    const void *blob = gd->fdt_blob;
>> +    int node;
>> +
>> +    node = fdt_node_offset_by_compatible(blob, -1,
>> +                    "rockchip,rk3399-xhci");
>> +    if (node < 0)
>> +        debug("%s node not found\n", __func__);
>> +    else
>> +        dwc3_device_data.usb2_phyif_utmi_width =
>> +            fdtdec_get_int(blob, node, "snps,phyif-utmi-bits", -1);
>> +
>> +    return dwc3_uboot_init(&dwc3_device_data);
>> +}
>> +#endif
> 
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget

[Kever Yang]

Hi Marek,

On 08/26/2016 05:11 PM, Marek Vasut wrote:
> On 08/25/2016 03:07 AM, Kever Yang wrote:
>> + Marek
>>
>> On 08/24/2016 11:46 AM, Kever Yang wrote:
>>> This patch add board_usb_init() and interrupt callback
>>> for dwc3 gadget.
>>>
>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>> Changes in v2:
>>> - parse dt for utmi width
>>>
>>>    board/rockchip/evb_rk3399/evb-rk3399.c | 33
>>> +++++++++++++++++++++++++++++++++
>>>    1 file changed, 33 insertions(+)
>>>
>>> diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c
>>> b/board/rockchip/evb_rk3399/evb-rk3399.c
>>> index d394276..5d3a09a 100644
>>> --- a/board/rockchip/evb_rk3399/evb-rk3399.c
>>> +++ b/board/rockchip/evb_rk3399/evb-rk3399.c
>>> @@ -7,6 +7,8 @@
>>>    #include <dm.h>
>>>    #include <dm/pinctrl.h>
>>>    #include <asm/arch/periph.h>
>>> +#include <usb.h>
>>> +#include <dwc3-uboot.h>
>>>      DECLARE_GLOBAL_DATA_PTR;
>>>    @@ -54,3 +56,34 @@ void dram_init_banksize(void)
>>>        gd->bd->bi_dram[0].start = 0x200000;
>>>        gd->bd->bi_dram[0].size = 0x80000000;
>>>    }
>>> +
>>> +#ifdef CONFIG_USB_DWC3
>>> +static struct dwc3_device dwc3_device_data = {
>>> +    .maximum_speed = USB_SPEED_HIGH,
>>> +    .base = 0xfe800000,
>>> +    .dr_mode = USB_DR_MODE_PERIPHERAL,
> Why don't you parse all of this from DT ? You already parse DT below.

I add these code by reference to exynos5 and zynqmp platform, I can parse
all information from DT which including in DT, but maximum_speed still 
defined
here, because it's not in DT node, is that OK?

Thanks,
- Kever
>
>>> +    .index = 0,
>>> +};
>>> +
>>> +int usb_gadget_handle_interrupts(void)
>>> +{
>>> +    dwc3_uboot_handle_interrupt(0);
>>> +    return 0;
>>> +}
>>> +
>>> +int board_usb_init(int index, enum usb_init_type init)
>>> +{
>>> +    const void *blob = gd->fdt_blob;
>>> +    int node;
>>> +
>>> +    node = fdt_node_offset_by_compatible(blob, -1,
>>> +                    "rockchip,rk3399-xhci");
>>> +    if (node < 0)
>>> +        debug("%s node not found\n", __func__);
>>> +    else
>>> +        dwc3_device_data.usb2_phyif_utmi_width =
>>> +            fdtdec_get_int(blob, node, "snps,phyif-utmi-bits", -1);
>>> +
>>> +    return dwc3_uboot_init(&dwc3_device_data);
>>> +}
>>> +#endif
>>
>

[U-Boot] [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget

[Marek Vasut]

On 08/29/2016 02:49 AM, Kever Yang wrote:
> Hi Marek,
> 
> On 08/26/2016 05:11 PM, Marek Vasut wrote:
>> On 08/25/2016 03:07 AM, Kever Yang wrote:
>>> + Marek
>>>
>>> On 08/24/2016 11:46 AM, Kever Yang wrote:
>>>> This patch add board_usb_init() and interrupt callback
>>>> for dwc3 gadget.
>>>>
>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - parse dt for utmi width
>>>>
>>>>    board/rockchip/evb_rk3399/evb-rk3399.c | 33
>>>> +++++++++++++++++++++++++++++++++
>>>>    1 file changed, 33 insertions(+)
>>>>
>>>> diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c
>>>> b/board/rockchip/evb_rk3399/evb-rk3399.c
>>>> index d394276..5d3a09a 100644
>>>> --- a/board/rockchip/evb_rk3399/evb-rk3399.c
>>>> +++ b/board/rockchip/evb_rk3399/evb-rk3399.c
>>>> @@ -7,6 +7,8 @@
>>>>    #include <dm.h>
>>>>    #include <dm/pinctrl.h>
>>>>    #include <asm/arch/periph.h>
>>>> +#include <usb.h>
>>>> +#include <dwc3-uboot.h>
>>>>      DECLARE_GLOBAL_DATA_PTR;
>>>>    @@ -54,3 +56,34 @@ void dram_init_banksize(void)
>>>>        gd->bd->bi_dram[0].start = 0x200000;
>>>>        gd->bd->bi_dram[0].size = 0x80000000;
>>>>    }
>>>> +
>>>> +#ifdef CONFIG_USB_DWC3
>>>> +static struct dwc3_device dwc3_device_data = {
>>>> +    .maximum_speed = USB_SPEED_HIGH,
>>>> +    .base = 0xfe800000,
>>>> +    .dr_mode = USB_DR_MODE_PERIPHERAL,
>> Why don't you parse all of this from DT ? You already parse DT below.
> 
> I add these code by reference to exynos5 and zynqmp platform, I can parse
> all information from DT which including in DT, but maximum_speed still
> defined
> here, because it's not in DT node, is that OK?

Sorry, can you please rephrase ?

Linux already has maximum-speed binding, see
Documentation/devicetree/bindings/usb/generic.txt

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 3/4] usb: dwc3: add support for 16 bit UTMI+ interface

[Kever Yang]

The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY,
add one variable in dwc3/dwc3_device struct to support 16 bit
UTMI+ interface on some SoCs like Rockchip rk3399.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v2:
- use a variable to identify utmi+ bus width instead of CONFIG MACRO

 drivers/usb/dwc3/core.c |  6 ++++++
 drivers/usb/dwc3/core.h | 12 ++++++++++++
 include/dwc3-uboot.h    |  1 +
 3 files changed, 19 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 85cc96a..0613508 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc)
 	if (dwc->dis_u2_susphy_quirk)
 		reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
 
+	if (dwc->usb2_phyif_utmi_width == 16) {
+		reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
+		reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT;
+		reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT;
+	}
 	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
 
 	mdelay(100);
@@ -676,6 +681,7 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev)
 
 	dwc->lpm_nyet_threshold = lpm_nyet_threshold;
 	dwc->tx_de_emphasis = tx_de_emphasis;
+	dwc->usb2_phyif_utmi_width = dwc3_dev->usb2_phyif_utmi_width;
 
 	dwc->hird_threshold = hird_threshold
 		| (dwc->is_utmi_l1_suspend << 4);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 72d2fcd..d5bdf97 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -74,6 +74,7 @@
 #define DWC3_GCTL		0xc110
 #define DWC3_GEVTEN		0xc114
 #define DWC3_GSTS		0xc118
+#define DWC3_GUCTL1		0xc11c
 #define DWC3_GSNPSID		0xc120
 #define DWC3_GGPIO		0xc124
 #define DWC3_GUID		0xc128
@@ -162,7 +163,17 @@
 
 /* Global USB2 PHY Configuration Register */
 #define DWC3_GUSB2PHYCFG_PHYSOFTRST	(1 << 31)
+#define DWC3_GUSB2PHYCFG_ENBLSLPM   (1 << 8)
 #define DWC3_GUSB2PHYCFG_SUSPHY		(1 << 6)
+#define DWC3_GUSB2PHYCFG_PHYIF_8BIT	(0 << 3)
+#define DWC3_GUSB2PHYCFG_PHYIF_16BIT	(1 << 3)
+#define DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT	(10)
+#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK	(0xf << \
+		DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
+#define DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT (0x5 << \
+		DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
+#define DWC3_GUSB2PHYCFG_USBTRDTIM_8BIT (0x9 << \
+		DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
 
 /* Global USB3 PIPE Control Register */
 #define DWC3_GUSB3PIPECTL_PHYSOFTRST	(1 << 31)
@@ -813,6 +824,7 @@ struct dwc3 {
 
 	unsigned		tx_de_emphasis_quirk:1;
 	unsigned		tx_de_emphasis:2;
+	unsigned		usb2_phyif_utmi_width;
 	int			index;
 	struct list_head        list;
 };
diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h
index 7af2ad1..cc9ffe8 100644
--- a/include/dwc3-uboot.h
+++ b/include/dwc3-uboot.h
@@ -33,6 +33,7 @@ struct dwc3_device {
 	unsigned dis_u2_susphy_quirk;
 	unsigned tx_de_emphasis_quirk;
 	unsigned tx_de_emphasis;
+	unsigned usb2_phyif_utmi_width;
 	int index;
 };
 
-- 
1.9.1

[U-Boot] [PATCH v2 3/4] usb: dwc3: add support for 16 bit UTMI+ interface

[Marek Vasut]

On 08/24/2016 05:46 AM, Kever Yang wrote:
> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY,
> add one variable in dwc3/dwc3_device struct to support 16 bit
> UTMI+ interface on some SoCs like Rockchip rk3399.
> 
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
> 
> Changes in v2:
> - use a variable to identify utmi+ bus width instead of CONFIG MACRO
> 
>  drivers/usb/dwc3/core.c |  6 ++++++
>  drivers/usb/dwc3/core.h | 12 ++++++++++++
>  include/dwc3-uboot.h    |  1 +
>  3 files changed, 19 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 85cc96a..0613508 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc)
>  	if (dwc->dis_u2_susphy_quirk)
>  		reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>  
> +	if (dwc->usb2_phyif_utmi_width == 16) {
> +		reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
> +		reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT;
> +		reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT;
> +	}

Didn't we agree to pull this info from OF ?

>  	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>  
>  	mdelay(100);
> @@ -676,6 +681,7 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev)
>  
>  	dwc->lpm_nyet_threshold = lpm_nyet_threshold;
>  	dwc->tx_de_emphasis = tx_de_emphasis;
> +	dwc->usb2_phyif_utmi_width = dwc3_dev->usb2_phyif_utmi_width;
>  
>  	dwc->hird_threshold = hird_threshold
>  		| (dwc->is_utmi_l1_suspend << 4);
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 72d2fcd..d5bdf97 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -74,6 +74,7 @@
>  #define DWC3_GCTL		0xc110
>  #define DWC3_GEVTEN		0xc114
>  #define DWC3_GSTS		0xc118
> +#define DWC3_GUCTL1		0xc11c
>  #define DWC3_GSNPSID		0xc120
>  #define DWC3_GGPIO		0xc124
>  #define DWC3_GUID		0xc128
> @@ -162,7 +163,17 @@
>  
>  /* Global USB2 PHY Configuration Register */
>  #define DWC3_GUSB2PHYCFG_PHYSOFTRST	(1 << 31)
> +#define DWC3_GUSB2PHYCFG_ENBLSLPM   (1 << 8)
>  #define DWC3_GUSB2PHYCFG_SUSPHY		(1 << 6)
> +#define DWC3_GUSB2PHYCFG_PHYIF_8BIT	(0 << 3)
> +#define DWC3_GUSB2PHYCFG_PHYIF_16BIT	(1 << 3)
> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT	(10)
> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK	(0xf << \
> +		DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT (0x5 << \
> +		DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_8BIT (0x9 << \
> +		DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>  
>  /* Global USB3 PIPE Control Register */
>  #define DWC3_GUSB3PIPECTL_PHYSOFTRST	(1 << 31)
> @@ -813,6 +824,7 @@ struct dwc3 {
>  
>  	unsigned		tx_de_emphasis_quirk:1;
>  	unsigned		tx_de_emphasis:2;
> +	unsigned		usb2_phyif_utmi_width;
>  	int			index;
>  	struct list_head        list;
>  };
> diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h
> index 7af2ad1..cc9ffe8 100644
> --- a/include/dwc3-uboot.h
> +++ b/include/dwc3-uboot.h
> @@ -33,6 +33,7 @@ struct dwc3_device {
>  	unsigned dis_u2_susphy_quirk;
>  	unsigned tx_de_emphasis_quirk;
>  	unsigned tx_de_emphasis;
> +	unsigned usb2_phyif_utmi_width;

The utmi width is either 8 or 16, so you can continue the bitfield
instead of wasting 32 bits.

>  	int index;
>  };
>  
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 3/4] usb: dwc3: add support for 16 bit UTMI+ interface

[Kever Yang]

Hi Marek,

On 08/24/2016 07:38 PM, Marek Vasut wrote:
> On 08/24/2016 05:46 AM, Kever Yang wrote:
>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY,
>> add one variable in dwc3/dwc3_device struct to support 16 bit
>> UTMI+ interface on some SoCs like Rockchip rk3399.
>>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>
>> Changes in v2:
>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO
>>
>>   drivers/usb/dwc3/core.c |  6 ++++++
>>   drivers/usb/dwc3/core.h | 12 ++++++++++++
>>   include/dwc3-uboot.h    |  1 +
>>   3 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 85cc96a..0613508 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc)
>>   	if (dwc->dis_u2_susphy_quirk)
>>   		reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>   
>> +	if (dwc->usb2_phyif_utmi_width == 16) {
>> +		reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
>> +		reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT;
>> +		reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT;
>> +	}
> Didn't we agree to pull this info from OF ?

Yes, the dwc->usb2_phyif_utmi_width is from OF,  but I make the DT parse
in board file instead of in the dwc3 driver,  see my patch:
[PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget

I think implement in this way won't affect other soc also using dwc3 
controller,
the DT parse would cost some time which may not necessary for other soc.

>
>>   	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>   
>>   	mdelay(100);
>> @@ -676,6 +681,7 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev)
>>   
>>   	dwc->lpm_nyet_threshold = lpm_nyet_threshold;
>>   	dwc->tx_de_emphasis = tx_de_emphasis;
>> +	dwc->usb2_phyif_utmi_width = dwc3_dev->usb2_phyif_utmi_width;
>>   
>>   	dwc->hird_threshold = hird_threshold
>>   		| (dwc->is_utmi_l1_suspend << 4);
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 72d2fcd..d5bdf97 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -74,6 +74,7 @@
>>   #define DWC3_GCTL		0xc110
>>   #define DWC3_GEVTEN		0xc114
>>   #define DWC3_GSTS		0xc118
>> +#define DWC3_GUCTL1		0xc11c
>>   #define DWC3_GSNPSID		0xc120
>>   #define DWC3_GGPIO		0xc124
>>   #define DWC3_GUID		0xc128
>> @@ -162,7 +163,17 @@
>>   
>>   /* Global USB2 PHY Configuration Register */
>>   #define DWC3_GUSB2PHYCFG_PHYSOFTRST	(1 << 31)
>> +#define DWC3_GUSB2PHYCFG_ENBLSLPM   (1 << 8)
>>   #define DWC3_GUSB2PHYCFG_SUSPHY		(1 << 6)
>> +#define DWC3_GUSB2PHYCFG_PHYIF_8BIT	(0 << 3)
>> +#define DWC3_GUSB2PHYCFG_PHYIF_16BIT	(1 << 3)
>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT	(10)
>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK	(0xf << \
>> +		DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT (0x5 << \
>> +		DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_8BIT (0x9 << \
>> +		DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>>   
>>   /* Global USB3 PIPE Control Register */
>>   #define DWC3_GUSB3PIPECTL_PHYSOFTRST	(1 << 31)
>> @@ -813,6 +824,7 @@ struct dwc3 {
>>   
>>   	unsigned		tx_de_emphasis_quirk:1;
>>   	unsigned		tx_de_emphasis:2;
>> +	unsigned		usb2_phyif_utmi_width;
>>   	int			index;
>>   	struct list_head        list;
>>   };
>> diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h
>> index 7af2ad1..cc9ffe8 100644
>> --- a/include/dwc3-uboot.h
>> +++ b/include/dwc3-uboot.h
>> @@ -33,6 +33,7 @@ struct dwc3_device {
>>   	unsigned dis_u2_susphy_quirk;
>>   	unsigned tx_de_emphasis_quirk;
>>   	unsigned tx_de_emphasis;
>> +	unsigned usb2_phyif_utmi_width;
> The utmi width is either 8 or 16, so you can continue the bitfield
> instead of wasting 32 bits.

I think you mean the usb2_phyif_utmi_width in struct dwc3, but not 
struct dwc3_device, right?

Thanks,
-Kever
>
>>   	int index;
>>   };
>>   
>>
>

[U-Boot] [PATCH v2 3/4] usb: dwc3: add support for 16 bit UTMI+ interface

[Marek Vasut]

On 08/25/2016 03:17 AM, Kever Yang wrote:
> Hi Marek,
> 
> On 08/24/2016 07:38 PM, Marek Vasut wrote:
>> On 08/24/2016 05:46 AM, Kever Yang wrote:
>>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY,
>>> add one variable in dwc3/dwc3_device struct to support 16 bit
>>> UTMI+ interface on some SoCs like Rockchip rk3399.
>>>
>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>> ---
>>>
>>> Changes in v2:
>>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO
>>>
>>>   drivers/usb/dwc3/core.c |  6 ++++++
>>>   drivers/usb/dwc3/core.h | 12 ++++++++++++
>>>   include/dwc3-uboot.h    |  1 +
>>>   3 files changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 85cc96a..0613508 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc)
>>>       if (dwc->dis_u2_susphy_quirk)
>>>           reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>   +    if (dwc->usb2_phyif_utmi_width == 16) {
>>> +        reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
>>> +        reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT;
>>> +        reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT;
>>> +    }
>> Didn't we agree to pull this info from OF ?
> 
> Yes, the dwc->usb2_phyif_utmi_width is from OF,  but I make the DT parse
> in board file instead of in the dwc3 driver,  see my patch:
> [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget

So this is basically a passing of platform data ?

> I think implement in this way won't affect other soc also using dwc3
> controller,
> the DT parse would cost some time which may not necessary for other soc.

The time needed to run the DT parsing is completely insignificant.
This sounds like a premature optimization.

>>>       dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>         mdelay(100);
>>> @@ -676,6 +681,7 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev)
>>>         dwc->lpm_nyet_threshold = lpm_nyet_threshold;
>>>       dwc->tx_de_emphasis = tx_de_emphasis;
>>> +    dwc->usb2_phyif_utmi_width = dwc3_dev->usb2_phyif_utmi_width;
>>>         dwc->hird_threshold = hird_threshold
>>>           | (dwc->is_utmi_l1_suspend << 4);
>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>> index 72d2fcd..d5bdf97 100644
>>> --- a/drivers/usb/dwc3/core.h
>>> +++ b/drivers/usb/dwc3/core.h
>>> @@ -74,6 +74,7 @@
>>>   #define DWC3_GCTL        0xc110
>>>   #define DWC3_GEVTEN        0xc114
>>>   #define DWC3_GSTS        0xc118
>>> +#define DWC3_GUCTL1        0xc11c
>>>   #define DWC3_GSNPSID        0xc120
>>>   #define DWC3_GGPIO        0xc124
>>>   #define DWC3_GUID        0xc128
>>> @@ -162,7 +163,17 @@
>>>     /* Global USB2 PHY Configuration Register */
>>>   #define DWC3_GUSB2PHYCFG_PHYSOFTRST    (1 << 31)
>>> +#define DWC3_GUSB2PHYCFG_ENBLSLPM   (1 << 8)
>>>   #define DWC3_GUSB2PHYCFG_SUSPHY        (1 << 6)
>>> +#define DWC3_GUSB2PHYCFG_PHYIF_8BIT    (0 << 3)
>>> +#define DWC3_GUSB2PHYCFG_PHYIF_16BIT    (1 << 3)
>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT    (10)
>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK    (0xf << \
>>> +        DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT (0x5 << \
>>> +        DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_8BIT (0x9 << \
>>> +        DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>>>     /* Global USB3 PIPE Control Register */
>>>   #define DWC3_GUSB3PIPECTL_PHYSOFTRST    (1 << 31)
>>> @@ -813,6 +824,7 @@ struct dwc3 {
>>>         unsigned        tx_de_emphasis_quirk:1;
>>>       unsigned        tx_de_emphasis:2;
>>> +    unsigned        usb2_phyif_utmi_width;
>>>       int            index;
>>>       struct list_head        list;
>>>   };
>>> diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h
>>> index 7af2ad1..cc9ffe8 100644
>>> --- a/include/dwc3-uboot.h
>>> +++ b/include/dwc3-uboot.h
>>> @@ -33,6 +33,7 @@ struct dwc3_device {
>>>       unsigned dis_u2_susphy_quirk;
>>>       unsigned tx_de_emphasis_quirk;
>>>       unsigned tx_de_emphasis;
>>> +    unsigned usb2_phyif_utmi_width;
>> The utmi width is either 8 or 16, so you can continue the bitfield
>> instead of wasting 32 bits.
> 
> I think you mean the usb2_phyif_utmi_width in struct dwc3, but not
> struct dwc3_device, right?

Yes, sorry.

> Thanks,
> -Kever
>>
>>>       int index;
>>>   };
>>>  
>>
> 
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 3/4] usb: dwc3: add support for 16 bit UTMI+ interface

[Kever Yang]

Hi Marek,

On 08/26/2016 05:09 PM, Marek Vasut wrote:
> On 08/25/2016 03:17 AM, Kever Yang wrote:
>> Hi Marek,
>>
>> On 08/24/2016 07:38 PM, Marek Vasut wrote:
>>> On 08/24/2016 05:46 AM, Kever Yang wrote:
>>>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY,
>>>> add one variable in dwc3/dwc3_device struct to support 16 bit
>>>> UTMI+ interface on some SoCs like Rockchip rk3399.
>>>>
>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO
>>>>
>>>>    drivers/usb/dwc3/core.c |  6 ++++++
>>>>    drivers/usb/dwc3/core.h | 12 ++++++++++++
>>>>    include/dwc3-uboot.h    |  1 +
>>>>    3 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>> index 85cc96a..0613508 100644
>>>> --- a/drivers/usb/dwc3/core.c
>>>> +++ b/drivers/usb/dwc3/core.c
>>>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc)
>>>>        if (dwc->dis_u2_susphy_quirk)
>>>>            reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>>    +    if (dwc->usb2_phyif_utmi_width == 16) {
>>>> +        reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
>>>> +        reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT;
>>>> +        reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT;
>>>> +    }
>>> Didn't we agree to pull this info from OF ?
>> Yes, the dwc->usb2_phyif_utmi_width is from OF,  but I make the DT parse
>> in board file instead of in the dwc3 driver,  see my patch:
>> [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget
> So this is basically a passing of platform data ?

Yes, this is what other platform do, I just extend one more setting.

>
>> I think implement in this way won't affect other soc also using dwc3
>> controller,
>> the DT parse would cost some time which may not necessary for other soc.
> The time needed to run the DT parsing is completely insignificant.
> This sounds like a premature optimization.

Which way of implement this DT parse do you prefer and more reasonable,
in dwc3 driver or in board init and dwc3 driver use it as platform data?

Thanks,
- Kever

>
>>>>        dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>>          mdelay(100);
>>>> @@ -676,6 +681,7 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev)
>>>>          dwc->lpm_nyet_threshold = lpm_nyet_threshold;
>>>>        dwc->tx_de_emphasis = tx_de_emphasis;
>>>> +    dwc->usb2_phyif_utmi_width = dwc3_dev->usb2_phyif_utmi_width;
>>>>          dwc->hird_threshold = hird_threshold
>>>>            | (dwc->is_utmi_l1_suspend << 4);
>>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>>> index 72d2fcd..d5bdf97 100644
>>>> --- a/drivers/usb/dwc3/core.h
>>>> +++ b/drivers/usb/dwc3/core.h
>>>> @@ -74,6 +74,7 @@
>>>>    #define DWC3_GCTL        0xc110
>>>>    #define DWC3_GEVTEN        0xc114
>>>>    #define DWC3_GSTS        0xc118
>>>> +#define DWC3_GUCTL1        0xc11c
>>>>    #define DWC3_GSNPSID        0xc120
>>>>    #define DWC3_GGPIO        0xc124
>>>>    #define DWC3_GUID        0xc128
>>>> @@ -162,7 +163,17 @@
>>>>      /* Global USB2 PHY Configuration Register */
>>>>    #define DWC3_GUSB2PHYCFG_PHYSOFTRST    (1 << 31)
>>>> +#define DWC3_GUSB2PHYCFG_ENBLSLPM   (1 << 8)
>>>>    #define DWC3_GUSB2PHYCFG_SUSPHY        (1 << 6)
>>>> +#define DWC3_GUSB2PHYCFG_PHYIF_8BIT    (0 << 3)
>>>> +#define DWC3_GUSB2PHYCFG_PHYIF_16BIT    (1 << 3)
>>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT    (10)
>>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK    (0xf << \
>>>> +        DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT (0x5 << \
>>>> +        DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_8BIT (0x9 << \
>>>> +        DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT)
>>>>      /* Global USB3 PIPE Control Register */
>>>>    #define DWC3_GUSB3PIPECTL_PHYSOFTRST    (1 << 31)
>>>> @@ -813,6 +824,7 @@ struct dwc3 {
>>>>          unsigned        tx_de_emphasis_quirk:1;
>>>>        unsigned        tx_de_emphasis:2;
>>>> +    unsigned        usb2_phyif_utmi_width;
>>>>        int            index;
>>>>        struct list_head        list;
>>>>    };
>>>> diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h
>>>> index 7af2ad1..cc9ffe8 100644
>>>> --- a/include/dwc3-uboot.h
>>>> +++ b/include/dwc3-uboot.h
>>>> @@ -33,6 +33,7 @@ struct dwc3_device {
>>>>        unsigned dis_u2_susphy_quirk;
>>>>        unsigned tx_de_emphasis_quirk;
>>>>        unsigned tx_de_emphasis;
>>>> +    unsigned usb2_phyif_utmi_width;
>>> The utmi width is either 8 or 16, so you can continue the bitfield
>>> instead of wasting 32 bits.
>> I think you mean the usb2_phyif_utmi_width in struct dwc3, but not
>> struct dwc3_device, right?
> Yes, sorry.
>
>> Thanks,
>> -Kever
>>>>        int index;
>>>>    };
>>>>   
>>
>

[U-Boot] [PATCH v2 3/4] usb: dwc3: add support for 16 bit UTMI+ interface

[Marek Vasut]

On 08/29/2016 02:55 AM, Kever Yang wrote:
> Hi Marek,
> 
> On 08/26/2016 05:09 PM, Marek Vasut wrote:
>> On 08/25/2016 03:17 AM, Kever Yang wrote:
>>> Hi Marek,
>>>
>>> On 08/24/2016 07:38 PM, Marek Vasut wrote:
>>>> On 08/24/2016 05:46 AM, Kever Yang wrote:
>>>>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY,
>>>>> add one variable in dwc3/dwc3_device struct to support 16 bit
>>>>> UTMI+ interface on some SoCs like Rockchip rk3399.
>>>>>
>>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO
>>>>>
>>>>>    drivers/usb/dwc3/core.c |  6 ++++++
>>>>>    drivers/usb/dwc3/core.h | 12 ++++++++++++
>>>>>    include/dwc3-uboot.h    |  1 +
>>>>>    3 files changed, 19 insertions(+)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>> index 85cc96a..0613508 100644
>>>>> --- a/drivers/usb/dwc3/core.c
>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc)
>>>>>        if (dwc->dis_u2_susphy_quirk)
>>>>>            reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>    +    if (dwc->usb2_phyif_utmi_width == 16) {
>>>>> +        reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
>>>>> +        reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT;
>>>>> +        reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT;
>>>>> +    }
>>>> Didn't we agree to pull this info from OF ?
>>> Yes, the dwc->usb2_phyif_utmi_width is from OF,  but I make the DT parse
>>> in board file instead of in the dwc3 driver,  see my patch:
>>> [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget
>> So this is basically a passing of platform data ?
> 
> Yes, this is what other platform do, I just extend one more setting.

Platform data should go away in favor of the DT probing.

>>> I think implement in this way won't affect other soc also using dwc3
>>> controller,
>>> the DT parse would cost some time which may not necessary for other soc.
>> The time needed to run the DT parsing is completely insignificant.
>> This sounds like a premature optimization.
> 
> Which way of implement this DT parse do you prefer and more reasonable,
> in dwc3 driver or in board init and dwc3 driver use it as platform data?

Driver should parse the DT.

> Thanks,
> - Kever

[...]

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 3/4] usb: dwc3: add support for 16 bit UTMI+ interface

[Kever Yang]

Hi Marek,

On 08/29/2016 09:01 AM, Marek Vasut wrote:
> On 08/29/2016 02:55 AM, Kever Yang wrote:
>> Hi Marek,
>>
>> On 08/26/2016 05:09 PM, Marek Vasut wrote:
>>> On 08/25/2016 03:17 AM, Kever Yang wrote:
>>>> Hi Marek,
>>>>
>>>> On 08/24/2016 07:38 PM, Marek Vasut wrote:
>>>>> On 08/24/2016 05:46 AM, Kever Yang wrote:
>>>>>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY,
>>>>>> add one variable in dwc3/dwc3_device struct to support 16 bit
>>>>>> UTMI+ interface on some SoCs like Rockchip rk3399.
>>>>>>
>>>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>>>>> ---
>>>>>>
>>>>>> Changes in v2:
>>>>>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO
>>>>>>
>>>>>>     drivers/usb/dwc3/core.c |  6 ++++++
>>>>>>     drivers/usb/dwc3/core.h | 12 ++++++++++++
>>>>>>     include/dwc3-uboot.h    |  1 +
>>>>>>     3 files changed, 19 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>>> index 85cc96a..0613508 100644
>>>>>> --- a/drivers/usb/dwc3/core.c
>>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc)
>>>>>>         if (dwc->dis_u2_susphy_quirk)
>>>>>>             reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>>     +    if (dwc->usb2_phyif_utmi_width == 16) {
>>>>>> +        reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
>>>>>> +        reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT;
>>>>>> +        reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT;
>>>>>> +    }
>>>>> Didn't we agree to pull this info from OF ?
>>>> Yes, the dwc->usb2_phyif_utmi_width is from OF,  but I make the DT parse
>>>> in board file instead of in the dwc3 driver,  see my patch:
>>>> [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget
>>> So this is basically a passing of platform data ?
>> Yes, this is what other platform do, I just extend one more setting.
> Platform data should go away in favor of the DT probing.
>
>>>> I think implement in this way won't affect other soc also using dwc3
>>>> controller,
>>>> the DT parse would cost some time which may not necessary for other soc.
>>> The time needed to run the DT parsing is completely insignificant.
>>> This sounds like a premature optimization.
>> Which way of implement this DT parse do you prefer and more reasonable,
>> in dwc3 driver or in board init and dwc3 driver use it as platform data?
> Driver should parse the DT.

I'm going to parse the DT for this new 16bit UTMI+ interface, and keep 
other part "as is"
because the dwc3 device node for different SoC are different now.

Does this make sense to you?

Thanks,
- Kever
>
>> Thanks,
>> - Kever
> [...]
>

[U-Boot] [PATCH v2 3/4] usb: dwc3: add support for 16 bit UTMI+ interface

[Marek Vasut]

On 08/30/2016 05:21 AM, Kever Yang wrote:
> Hi Marek,
> 
> On 08/29/2016 09:01 AM, Marek Vasut wrote:
>> On 08/29/2016 02:55 AM, Kever Yang wrote:
>>> Hi Marek,
>>>
>>> On 08/26/2016 05:09 PM, Marek Vasut wrote:
>>>> On 08/25/2016 03:17 AM, Kever Yang wrote:
>>>>> Hi Marek,
>>>>>
>>>>> On 08/24/2016 07:38 PM, Marek Vasut wrote:
>>>>>> On 08/24/2016 05:46 AM, Kever Yang wrote:
>>>>>>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY,
>>>>>>> add one variable in dwc3/dwc3_device struct to support 16 bit
>>>>>>> UTMI+ interface on some SoCs like Rockchip rk3399.
>>>>>>>
>>>>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Changes in v2:
>>>>>>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO
>>>>>>>
>>>>>>>     drivers/usb/dwc3/core.c |  6 ++++++
>>>>>>>     drivers/usb/dwc3/core.h | 12 ++++++++++++
>>>>>>>     include/dwc3-uboot.h    |  1 +
>>>>>>>     3 files changed, 19 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>>>> index 85cc96a..0613508 100644
>>>>>>> --- a/drivers/usb/dwc3/core.c
>>>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>>>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc)
>>>>>>>         if (dwc->dis_u2_susphy_quirk)
>>>>>>>             reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>>>>>     +    if (dwc->usb2_phyif_utmi_width == 16) {
>>>>>>> +        reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK;
>>>>>>> +        reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT;
>>>>>>> +        reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT;
>>>>>>> +    }
>>>>>> Didn't we agree to pull this info from OF ?
>>>>> Yes, the dwc->usb2_phyif_utmi_width is from OF,  but I make the DT
>>>>> parse
>>>>> in board file instead of in the dwc3 driver,  see my patch:
>>>>> [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget
>>>> So this is basically a passing of platform data ?
>>> Yes, this is what other platform do, I just extend one more setting.
>> Platform data should go away in favor of the DT probing.
>>
>>>>> I think implement in this way won't affect other soc also using dwc3
>>>>> controller,
>>>>> the DT parse would cost some time which may not necessary for other
>>>>> soc.
>>>> The time needed to run the DT parsing is completely insignificant.
>>>> This sounds like a premature optimization.
>>> Which way of implement this DT parse do you prefer and more reasonable,
>>> in dwc3 driver or in board init and dwc3 driver use it as platform data?
>> Driver should parse the DT.
> 
> I'm going to parse the DT for this new 16bit UTMI+ interface, and keep
> other part "as is"
> because the dwc3 device node for different SoC are different now.
> 
> Does this make sense to you?

That's fine, I'll complain if the code looks crappy tho ;-)

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 4/4] config: rk3399: add support for dwc3 gadget

[Kever Yang]

To support fastboot, we need to enable the controller first.
rk3399 is using dwc3 as usb device controller, this patch enable
the configs for dwc3 gadget.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- remove config for USB2PHY UTMI BITS

 include/configs/rk3399_common.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/configs/rk3399_common.h b/include/configs/rk3399_common.h
index 6875308..fcd1700 100644
--- a/include/configs/rk3399_common.h
+++ b/include/configs/rk3399_common.h
@@ -27,6 +27,26 @@
 
 #define CONFIG_SYS_BOOTM_LEN	(64 << 20)	/* 64M */
 
+/*  usb device */
+#define CONFIG_USB_DWC3
+#define CONFIG_USB_DWC3_GADGET
+#define CONFIG_USB_GADGET
+#define CONFIG_USB_GADGET_DUALSPEED
+#define CONFIG_USB_GADGET_VBUS_DRAW	0
+
+/*  fastboot  */
+#define CONFIG_CMD_FASTBOOT
+#define CONFIG_USB_FUNCTION_FASTBOOT
+#define CONFIG_FASTBOOT_FLASH
+#define CONFIG_FASTBOOT_FLASH_MMC_DEV	1	/* eMMC */
+#define CONFIG_FASTBOOT_BUF_ADDR	CONFIG_SYS_LOAD_ADDR
+#define CONFIG_FASTBOOT_BUF_SIZE	0x08000000
+
+#define CONFIG_USB_GADGET_DOWNLOAD
+#define CONFIG_G_DNL_MANUFACTURER	"Rockchip"
+#define CONFIG_G_DNL_VENDOR_NUM		0x2207
+#define CONFIG_G_DNL_PRODUCT_NUM	0x330a
+
 /* MMC/SD IP block */
 #define CONFIG_MMC
 #define CONFIG_GENERIC_MMC
-- 
1.9.1

[U-Boot] [PATCH v2 4/4] config: rk3399: add support for dwc3 gadget

[Steve Rae]

Hi Kever,

On Aug 23, 2016 20:48, "Kever Yang" <kever.yang@rock-chips.com> wrote:
>
> To support fastboot, we need to enable the controller first.
> rk3399 is using dwc3 as usb device controller, this patch enable
> the configs for dwc3 gadget.
>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - remove config for USB2PHY UTMI BITS
>
>  include/configs/rk3399_common.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/include/configs/rk3399_common.h
b/include/configs/rk3399_common.h
> index 6875308..fcd1700 100644
> --- a/include/configs/rk3399_common.h
> +++ b/include/configs/rk3399_common.h
> @@ -27,6 +27,26 @@
>
>  #define CONFIG_SYS_BOOTM_LEN   (64 << 20)      /* 64M */
>
> +/*  usb device */
> +#define CONFIG_USB_DWC3
> +#define CONFIG_USB_DWC3_GADGET
> +#define CONFIG_USB_GADGET
> +#define CONFIG_USB_GADGET_DUALSPEED
> +#define CONFIG_USB_GADGET_VBUS_DRAW    0
> +
> +/*  fastboot  */
> +#define CONFIG_CMD_FASTBOOT
> +#define CONFIG_USB_FUNCTION_FASTBOOT
> +#define CONFIG_FASTBOOT_FLASH
> +#define CONFIG_FASTBOOT_FLASH_MMC_DEV  1       /* eMMC */
> +#define CONFIG_FASTBOOT_BUF_ADDR       CONFIG_SYS_LOAD_ADDR
> +#define CONFIG_FASTBOOT_BUF_SIZE       0x08000000
> +
> +#define CONFIG_USB_GADGET_DOWNLOAD
> +#define CONFIG_G_DNL_MANUFACTURER      "Rockchip"
> +#define CONFIG_G_DNL_VENDOR_NUM                0x2207
> +#define CONFIG_G_DNL_PRODUCT_NUM       0x330a
> +
FYI - All of these new defines are now in Kconfig....
Thanks, Steve

>  /* MMC/SD IP block */
>  #define CONFIG_MMC
>  #define CONFIG_GENERIC_MMC
> --
> 1.9.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 4/4] config: rk3399: add support for dwc3 gadget

[Kever Yang]

Hi Steve,


On 08/28/2016 06:22 AM, Steve Rae wrote:
>
> Hi Kever,
>
> On Aug 23, 2016 20:48, "Kever Yang" <kever.yang@rock-chips.com 
> <mailto:kever.yang@rock-chips.com>> wrote:
> >
> > To support fastboot, we need to enable the controller first.
> > rk3399 is using dwc3 as usb device controller, this patch enable
> > the configs for dwc3 gadget.
> >
> > Signed-off-by: Kever Yang <kever.yang@rock-chips.com 
> <mailto:kever.yang@rock-chips.com>>
> > Reviewed-by: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org>>
> > ---
> >
> > Changes in v2:
> > - remove config for USB2PHY UTMI BITS
> >
> >  include/configs/rk3399_common.h | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/include/configs/rk3399_common.h 
> b/include/configs/rk3399_common.h
> > index 6875308..fcd1700 100644
> > --- a/include/configs/rk3399_common.h
> > +++ b/include/configs/rk3399_common.h
> > @@ -27,6 +27,26 @@
> >
> >  #define CONFIG_SYS_BOOTM_LEN   (64 << 20)      /* 64M */
> >
> > +/*  usb device */
> > +#define CONFIG_USB_DWC3
> > +#define CONFIG_USB_DWC3_GADGET
> > +#define CONFIG_USB_GADGET
> > +#define CONFIG_USB_GADGET_DUALSPEED
> > +#define CONFIG_USB_GADGET_VBUS_DRAW    0
> > +
> > +/*  fastboot  */
> > +#define CONFIG_CMD_FASTBOOT
> > +#define CONFIG_USB_FUNCTION_FASTBOOT
> > +#define CONFIG_FASTBOOT_FLASH
> > +#define CONFIG_FASTBOOT_FLASH_MMC_DEV  1       /* eMMC */
> > +#define CONFIG_FASTBOOT_BUF_ADDR  CONFIG_SYS_LOAD_ADDR
> > +#define CONFIG_FASTBOOT_BUF_SIZE       0x08000000
> > +
> > +#define CONFIG_USB_GADGET_DOWNLOAD
> > +#define CONFIG_G_DNL_MANUFACTURER      "Rockchip"
> > +#define CONFIG_G_DNL_VENDOR_NUM                0x2207
> > +#define CONFIG_G_DNL_PRODUCT_NUM       0x330a
> > +
> FYI - All of these new defines are now in Kconfig....
>

Thanks for point out, I will rebase this patch and move these defines 
into Kconfig.

Regards,
- Kever
>
> Thanks, Steve
>
> >  /* MMC/SD IP block */
> >  #define CONFIG_MMC
> >  #define CONFIG_GENERIC_MMC
> > --
> > 1.9.1
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de <mailto:U-Boot@lists.denx.de>
> > http://lists.denx.de/mailman/listinfo/u-boot
>