diff mbox

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

Message ID 1472696063-17470-3-git-send-email-kever.yang@rock-chips.com
State Changes Requested
Delegated to: Simon Glass
Headers show

Commit Message

Kever Yang Sept. 1, 2016, 2:14 a.m. UTC
This patch add board_usb_init() and interrupt callback
for dwc3 gadget.

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

Changes in v4:
- parse DT for quirk, base address and maximum speed

Changes in v3:
- remove utmi width DT parse from borad init

Changes in v2:
- parse dt for utmi width

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

Comments

Marek Vasut Sept. 1, 2016, 8:59 a.m. UTC | #1
On 09/01/2016 04:14 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>
> ---
> 
> Changes in v4:
> - parse DT for quirk, base address and maximum speed
> 
> Changes in v3:
> - remove utmi width DT parse from borad init
> 
> Changes in v2:
> - parse dt for utmi width
> 
>  board/rockchip/evb_rk3399/evb-rk3399.c | 51 ++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c b/board/rockchip/evb_rk3399/evb-rk3399.c
> index d394276..58bfa78 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,52 @@ 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 = {
> +	.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;
> +	const char *string;
> +
> +	node = fdt_node_offset_by_compatible(blob, -1,
> +			"rockchip,rk3399-xhci");
> +	if (node < 0) {
> +		debug("%s dwc3 node not found\n", __func__);
> +	} else {
> +		dwc3_device_data.base = fdtdec_get_addr(blob, node, "reg");
> +		dwc3_device_data.dis_u2_susphy_quirk =
> +			fdtdec_get_int(blob, node,
> +				       "snps,dis-u2-susphy-quirk", -1);
> +
> +		string = fdt_getprop(blob, node, "maximum-speed", NULL);
> +		if (string) {
> +			if (0 == strcmp(string, "super-speed"))
> +				dwc3_device_data.maximum_speed = USB_SPEED_SUPER;
> +			else if (0 == strcmp(string, "high-speed"))
> +				dwc3_device_data.maximum_speed = USB_SPEED_HIGH;
> +			else if (0 == strcmp(string, "full-speed"))
> +				dwc3_device_data.maximum_speed = USB_SPEED_FULL;
> +			else
> +				debug("%s: Cannot decode speed'%s'\n", __func__,
> +				      string);
> +		} else {
> +			dwc3_device_data.maximum_speed = USB_SPEED_HIGH;
> +		}
> +		/* Hardcode controller in peripheral mode */
> +		dwc3_device_data.dr_mode = USB_DR_MODE_PERIPHERAL;
> +	}
> +
> +	return dwc3_uboot_init(&dwc3_device_data);
> +}
> +#endif
> 
So will every board have a copy of this exact code, verbatim ?
Kever Yang Sept. 2, 2016, 9:58 a.m. UTC | #2
Hi Marek,

On 09/01/2016 04:59 PM, Marek Vasut wrote:
> On 09/01/2016 04:14 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>
>> ---
>>
>> Changes in v4:
>> - parse DT for quirk, base address and maximum speed
>>
>> Changes in v3:
>> - remove utmi width DT parse from borad init
>>
>> Changes in v2:
>> - parse dt for utmi width
>>
>>   board/rockchip/evb_rk3399/evb-rk3399.c | 51 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 51 insertions(+)
>>
>> diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c b/board/rockchip/evb_rk3399/evb-rk3399.c
>> index d394276..58bfa78 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,52 @@ 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 = {
>> +	.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;
>> +	const char *string;
>> +
>> +	node = fdt_node_offset_by_compatible(blob, -1,
>> +			"rockchip,rk3399-xhci");
>> +	if (node < 0) {
>> +		debug("%s dwc3 node not found\n", __func__);
>> +	} else {
>> +		dwc3_device_data.base = fdtdec_get_addr(blob, node, "reg");
>> +		dwc3_device_data.dis_u2_susphy_quirk =
>> +			fdtdec_get_int(blob, node,
>> +				       "snps,dis-u2-susphy-quirk", -1);
>> +
>> +		string = fdt_getprop(blob, node, "maximum-speed", NULL);
>> +		if (string) {
>> +			if (0 == strcmp(string, "super-speed"))
>> +				dwc3_device_data.maximum_speed = USB_SPEED_SUPER;
>> +			else if (0 == strcmp(string, "high-speed"))
>> +				dwc3_device_data.maximum_speed = USB_SPEED_HIGH;
>> +			else if (0 == strcmp(string, "full-speed"))
>> +				dwc3_device_data.maximum_speed = USB_SPEED_FULL;
>> +			else
>> +				debug("%s: Cannot decode speed'%s'\n", __func__,
>> +				      string);
>> +		} else {
>> +			dwc3_device_data.maximum_speed = USB_SPEED_HIGH;
>> +		}
>> +		/* Hardcode controller in peripheral mode */
>> +		dwc3_device_data.dr_mode = USB_DR_MODE_PERIPHERAL;
>> +	}
>> +
>> +	return dwc3_uboot_init(&dwc3_device_data);
>> +}
>> +#endif
>>
> So will every board have a copy of this exact code, verbatim ?
>
Yes for rk3399 based board.
Not sure why you ask this question, but I guess you still want these 
parse happen in driver instead of board_init()?
I didn't make these code in dwc3 driver because other SoC dts are 
different now, they are init in platdata data structure.

To be honest, I want to make the upstream code to support rk3399 dwc3, 
but I don't want to touch too much code in other SoC.

Thanks,
- Kever
Marek Vasut Sept. 2, 2016, 12:14 p.m. UTC | #3
On 09/02/2016 11:58 AM, Kever Yang wrote:
> Hi Marek,
> 
> On 09/01/2016 04:59 PM, Marek Vasut wrote:
>> On 09/01/2016 04:14 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>
>>> ---
>>>
>>> Changes in v4:
>>> - parse DT for quirk, base address and maximum speed
>>>
>>> Changes in v3:
>>> - remove utmi width DT parse from borad init
>>>
>>> Changes in v2:
>>> - parse dt for utmi width
>>>
>>>   board/rockchip/evb_rk3399/evb-rk3399.c | 51
>>> ++++++++++++++++++++++++++++++++++
>>>   1 file changed, 51 insertions(+)
>>>
>>> diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c
>>> b/board/rockchip/evb_rk3399/evb-rk3399.c
>>> index d394276..58bfa78 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,52 @@ 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 = {
>>> +    .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;
>>> +    const char *string;
>>> +
>>> +    node = fdt_node_offset_by_compatible(blob, -1,
>>> +            "rockchip,rk3399-xhci");
>>> +    if (node < 0) {
>>> +        debug("%s dwc3 node not found\n", __func__);
>>> +    } else {
>>> +        dwc3_device_data.base = fdtdec_get_addr(blob, node, "reg");
>>> +        dwc3_device_data.dis_u2_susphy_quirk =
>>> +            fdtdec_get_int(blob, node,
>>> +                       "snps,dis-u2-susphy-quirk", -1);
>>> +
>>> +        string = fdt_getprop(blob, node, "maximum-speed", NULL);
>>> +        if (string) {
>>> +            if (0 == strcmp(string, "super-speed"))
>>> +                dwc3_device_data.maximum_speed = USB_SPEED_SUPER;
>>> +            else if (0 == strcmp(string, "high-speed"))
>>> +                dwc3_device_data.maximum_speed = USB_SPEED_HIGH;
>>> +            else if (0 == strcmp(string, "full-speed"))
>>> +                dwc3_device_data.maximum_speed = USB_SPEED_FULL;
>>> +            else
>>> +                debug("%s: Cannot decode speed'%s'\n", __func__,
>>> +                      string);
>>> +        } else {
>>> +            dwc3_device_data.maximum_speed = USB_SPEED_HIGH;
>>> +        }
>>> +        /* Hardcode controller in peripheral mode */
>>> +        dwc3_device_data.dr_mode = USB_DR_MODE_PERIPHERAL;
>>> +    }
>>> +
>>> +    return dwc3_uboot_init(&dwc3_device_data);
>>> +}
>>> +#endif
>>>
>> So will every board have a copy of this exact code, verbatim ?
>>
> Yes for rk3399 based board.
> Not sure why you ask this question, but I guess you still want these
> parse happen in driver instead of board_init()?

Of course.

> I didn't make these code in dwc3 driver because other SoC dts are
> different now, they are init in platdata data structure.

You can retain this behavior while adding the support for OF probing,
see my reply to your v3 patchset.

> To be honest, I want to make the upstream code to support rk3399 dwc3,
> but I don't want to touch too much code in other SoC.

This would result in having many various vendor hacks in board files,
which would be almost identical but only slightly broken and nightmare
to maintain. We have been there are we're glad we are getting out of
there. So no, I will not allow this. Instead, do changes to common code
and let others review/test them for you, this is perfectly fine.

btw the way of parsing DT in the driver I outlined in the reply to v3
should be pretty unintrusive.

> Thanks,
> - Kever
>
diff mbox

Patch

diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c b/board/rockchip/evb_rk3399/evb-rk3399.c
index d394276..58bfa78 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,52 @@  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 = {
+	.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;
+	const char *string;
+
+	node = fdt_node_offset_by_compatible(blob, -1,
+			"rockchip,rk3399-xhci");
+	if (node < 0) {
+		debug("%s dwc3 node not found\n", __func__);
+	} else {
+		dwc3_device_data.base = fdtdec_get_addr(blob, node, "reg");
+		dwc3_device_data.dis_u2_susphy_quirk =
+			fdtdec_get_int(blob, node,
+				       "snps,dis-u2-susphy-quirk", -1);
+
+		string = fdt_getprop(blob, node, "maximum-speed", NULL);
+		if (string) {
+			if (0 == strcmp(string, "super-speed"))
+				dwc3_device_data.maximum_speed = USB_SPEED_SUPER;
+			else if (0 == strcmp(string, "high-speed"))
+				dwc3_device_data.maximum_speed = USB_SPEED_HIGH;
+			else if (0 == strcmp(string, "full-speed"))
+				dwc3_device_data.maximum_speed = USB_SPEED_FULL;
+			else
+				debug("%s: Cannot decode speed'%s'\n", __func__,
+				      string);
+		} else {
+			dwc3_device_data.maximum_speed = USB_SPEED_HIGH;
+		}
+		/* Hardcode controller in peripheral mode */
+		dwc3_device_data.dr_mode = USB_DR_MODE_PERIPHERAL;
+	}
+
+	return dwc3_uboot_init(&dwc3_device_data);
+}
+#endif