[U-Boot] [PATCH 1/7] usb: xhci-rockchip: add rockchip dwc3 controller driver

[Kever Yang <kever.yang@rock-chips.com>]

Hi Marek,

On 08/17/2016 04:13 PM, Marek Vasut wrote:
> On 08/17/2016 09:42 AM, Kever Yang wrote:
>> From: MengDongyang <daniel.meng@rock-chips.com>
>>
>> This patch add support for rockchip dwc3 controller, which corresponding
>> to the two type-C port on rk3399 evb.
>> Only support usb2.0 currently for we have not enable the usb3.0 phy
>> driver and PD(fusb302) driver.
>>
>> Signed-off-by: MengDongyang <daniel.meng@rock-chips.com>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>
>>   drivers/usb/host/Makefile        |   3 +
>>   drivers/usb/host/xhci-rockchip.c | 226 +++++++++++++++++++++++++++++++++++++++
>>   include/linux/usb/dwc3.h         |   9 ++
>>   3 files changed, 238 insertions(+)
>>   create mode 100644 drivers/usb/host/xhci-rockchip.c
>>
>> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
>> index 620d114..b46e8df 100644
>> --- a/drivers/usb/host/Makefile
>> +++ b/drivers/usb/host/Makefile
>> @@ -54,8 +54,11 @@ obj-$(CONFIG_USB_EHCI_RMOBILE) += ehci-rmobile.o
>>   obj-$(CONFIG_USB_EHCI_ZYNQ) += ehci-zynq.o
>>   
>>   # xhci
>> +ifdef CONFIG_DM_USB
>>   obj-$(CONFIG_USB_XHCI_HCD) += xhci.o xhci-mem.o xhci-ring.o
>>   obj-$(CONFIG_USB_XHCI_DWC3) += xhci-dwc3.o
>> +obj-$(CONFIG_USB_XHCI_ROCKCHIP) += xhci-rockchip.o
>> +endif
> Please explain this (unrelated) ifdef .

This is not need, will remove.

>
>>   obj-$(CONFIG_USB_XHCI_ZYNQMP) += xhci-zynqmp.o
>>   obj-$(CONFIG_USB_XHCI_KEYSTONE) += xhci-keystone.o
>>   obj-$(CONFIG_USB_XHCI_EXYNOS) += xhci-exynos5.o
>> diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c
>> new file mode 100644
>> index 0000000..faefa7e
>> --- /dev/null
>> +++ b/drivers/usb/host/xhci-rockchip.c
>> @@ -0,0 +1,226 @@
>> +/*
>> + * Copyright (c) 2016 Rockchip, Inc.
>> + * Authors: Daniel Meng <daniel.meng@rock-chips.com>
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <fdtdec.h>
>> +#include <libfdt.h>
>> +#include <malloc.h>
>> +#include <usb.h>
>> +#include <watchdog.h>
>> +#include <asm/gpio.h>
>> +#include <asm-generic/errno.h>
>> +#include <linux/compat.h>
>> +#include <linux/usb/dwc3.h>
>> +
>> +#include "xhci.h"
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +struct rockchip_xhci_platdata {
>> +	fdt_addr_t hcd_base;
>> +	fdt_addr_t phy_base;
>> +	struct gpio_desc vbus_gpio;
>> +};
>> +
>> +/**
> Drop the other asterisk, this isn't javadoc.

Will remove in next version.

>
>> + * Contains pointers to register base addresses
>> + * for the usb controller.
>> + */
>> +struct rockchip_xhci {
>> +	struct usb_platdata usb_plat;
>> +	struct xhci_ctrl ctrl;
>> +	struct xhci_hccr *hcd;
>> +	struct dwc3 *dwc3_reg;
>> +	struct udevice *dev;
>> +};
>> +
>> +static int xhci_usb_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +	struct rockchip_xhci_platdata *plat = dev_get_platdata(dev);
>> +	struct udevice *child;
>> +
>> +	/*
>> +	 * Get the base address for XHCI controller from the device node
>> +	 */
>> +	plat->hcd_base = dev_get_addr(dev);
>> +	if (plat->hcd_base == FDT_ADDR_T_NONE) {
>> +		debug("Can't get the XHCI register base address\n");
>> +		return -ENXIO;
>> +	}
>> +
>> +	/*
>> +	 * Get the base address for usbphy from the device node
>> +	 */
>> +	for (device_find_first_child(dev, &child); child;
>> +	     device_find_next_child(&child)) {
>> +		if (!of_device_is_compatible(child, "rockchip,rk3399-usb3-phy"))
>> +			continue;
>> +		plat->phy_base = dev_get_addr(child);
>> +		break;
>> +	}
>> +
>> +	if (plat->phy_base == FDT_ADDR_T_NONE) {
>> +		debug("Can't get the usbphy register address\n");
>> +		return -ENXIO;
>> +	}
>> +
>> +	/* Vbus gpio */
>> +	gpio_request_by_name(dev, "rockchip,vbus-gpio", 0,
>> +			     &plat->vbus_gpio, GPIOD_IS_OUT);
> You should handle the return value here too

OK, will add in next version.

>
>> +	return 0;
>> +}
> [...]
>
>> +static int rockchip_xhci_core_init(struct rockchip_xhci *rockchip)
>> +{
>> +	int ret;
>> +
>> +	/*rockchip_usb3_phy_init(rockchip->usb3_phy);*/
> This should be removed or fixed.

Will remove in next version.
>
>> +	ret = dwc3_core_init(rockchip->dwc3_reg);
>> +	if (ret) {
>> +		debug("failed to initialize core\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	rockchip_dwc3_phy_setup(rockchip->dwc3_reg, rockchip);
>> +
>> +	/* We are hard-coding DWC3 core to Host Mode */
>> +	dwc3_set_mode(rockchip->dwc3_reg, DWC3_GCTL_PRTCAP_HOST);
>> +
>> +	return 0;
>> +}
> A general/conceptual question -- do we need yet another xhci-foo.c
> driver or could we unify them and have one single driver configured
> via DT which works for all platforms ?

Basically I don't like to add a new xhci-foo.c, and there are already 
five other xhch-xxx.c
in drivers/usb/host/ which based on dwc3 controller.

I think it's best to use one single driver configured via DT, maybe we 
can update source
code from kernel and all platforms use dwc3-of-simple.c. The kernel code 
has a lot of
update since this driver port from kernel to uboot.

Thanks,
- Kever