[U-Boot] [PATCH v2 05/13] usb: dwc3: change DWC3 core support code into a driver

Roger Quadros
2016-05-17T10:22:13Z

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