| Message ID | 1471251284-1804-1-git-send-email-william.wu@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi William,
[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.8-rc2 next-20160815]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/William-Wu/support-rockchip-dwc3-driver/20160815-165927
base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: m68k-allyesconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k
All errors (new ones prefixed by >>):
drivers/usb/dwc3/dwc3-rockchip.c: In function 'dwc3_rockchip_probe':
>> drivers/usb/dwc3/dwc3-rockchip.c:239:2: error: implicit declaration of function 'of_clk_get_parent_count' [-Werror=implicit-function-declaration]
count = of_clk_get_parent_count(np);
^
cc1: some warnings being treated as errors
vim +/of_clk_get_parent_count +239 drivers/usb/dwc3/dwc3-rockchip.c
233
234 rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL);
235
236 if (!rockchip)
237 return -ENOMEM;
238
> 239 count = of_clk_get_parent_count(np);
240 if (!count)
241 return -ENOENT;
242
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi William,
[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.8-rc2 next-20160815]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/William-Wu/support-rockchip-dwc3-driver/20160815-165927
base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: arm-arm67 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
>> ERROR: "dwc3_trace" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!
>> ERROR: "dwc3_phy_setup" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!
>> ERROR: "dwc3_set_mode" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!
>> ERROR: "__tracepoint_dwc3_readl" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi, William Wu <william.wu@rock-chips.com> writes: > Add rockchip specific glue layer to support USB3 Peripheral mode > and Host mode on rockchip platforms (e.g. rk3399). > > The DesignWare USB3 integrated in rockchip SoCs is a configurable > IP Core which can be instantiated as Dual-Role Device (DRD), Host > Only (XHCI) and Peripheral Only configurations. > > We use extcon notifier to manage usb cable detection and mode switch. > Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend > if USB cable is dettached. For host mode, it requires to keep whole > DWC3 controller in reset state to hold pipe power state in P2 before > initializing PHY. And it need to reconfigure USB PHY interface of DWC3 > core after deassert DWC3 controller reset. > > The current driver supports Host only and Peripheral Only well, for > now, we will add support for OTG after we have it all stabilized. > > Signed-off-by: William Wu <william.wu@rock-chips.com> > --- > Changes in v10: > - fix building error > > Changes in v9: > - Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c > > drivers/usb/dwc3/Kconfig | 9 + > drivers/usb/dwc3/Makefile | 1 + > drivers/usb/dwc3/core.c | 2 +- > drivers/usb/dwc3/core.h | 1 + > drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++ William, if you need to touch core dwc3 to introduce a glue layer, you're doing it wrong. > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index e887b38..3da6215 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc) > * initialized. The PHY interfaces and the PHYs get initialized together with > * the core in dwc3_core_init. > */ > -static int dwc3_phy_setup(struct dwc3 *dwc) > +int dwc3_phy_setup(struct dwc3 *dwc) there's no way I'll let this slip through the cracks :-) > diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c > new file mode 100644 > index 0000000..eeae1a9 > --- /dev/null > +++ b/drivers/usb/dwc3/dwc3-rockchip.c > @@ -0,0 +1,441 @@ [...] > +static int dwc3_rockchip_probe(struct platform_device *pdev) > +{ > + struct dwc3_rockchip *rockchip; > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node, *child; > + struct platform_device *child_pdev; > + > + unsigned int count; > + int ret; > + int i; > + > + rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL); > + > + if (!rockchip) > + return -ENOMEM; > + > + count = of_clk_get_parent_count(np); > + if (!count) > + return -ENOENT; > + > + rockchip->num_clocks = count; > + > + rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks, > + sizeof(struct clk *), GFP_KERNEL); > + if (!rockchip->clks) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, rockchip); > + > + rockchip->dev = dev; > + rockchip->edev = NULL; > + > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev); > + ret = pm_runtime_get_sync(dev); > + if (ret < 0) { > + dev_err(dev, "get_sync failed with err %d\n", ret); > + goto err1; > + } > + > + for (i = 0; i < rockchip->num_clocks; i++) { > + struct clk *clk; > + > + clk = of_clk_get(np, i); > + if (IS_ERR(clk)) { > + while (--i >= 0) > + clk_put(rockchip->clks[i]); > + ret = PTR_ERR(clk); > + > + goto err1; > + } > + > + ret = clk_prepare_enable(clk); > + if (ret < 0) { > + while (--i >= 0) { > + clk_disable_unprepare(rockchip->clks[i]); > + clk_put(rockchip->clks[i]); > + } > + clk_put(clk); > + > + goto err1; > + } > + > + rockchip->clks[i] = clk; > + } > + > + rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg"); > + if (IS_ERR(rockchip->otg_rst)) { > + dev_err(dev, "could not get reset controller\n"); > + ret = PTR_ERR(rockchip->otg_rst); > + goto err2; > + } > + > + ret = dwc3_rockchip_extcon_register(rockchip); > + if (ret < 0) > + goto err2; > + > + child = of_get_child_by_name(np, "dwc3"); > + if (!child) { > + dev_err(dev, "failed to find dwc3 core node\n"); > + ret = -ENODEV; > + goto err3; > + } > + > + /* Allocate and initialize the core */ > + ret = of_platform_populate(np, NULL, NULL, dev); > + if (ret) { > + dev_err(dev, "failed to create dwc3 core\n"); > + goto err3; > + } > + > + child_pdev = of_find_device_by_node(child); > + if (!child_pdev) { > + dev_err(dev, "failed to find dwc3 core device\n"); > + ret = -ENODEV; > + goto err4; > + } > + > + rockchip->dwc = platform_get_drvdata(child_pdev); No! You will *NOT* the core struct device. Don't even try to come up with tricks like this. Let's do this: introduce a glue layer that gets peripheral-only working. Remove PM for now, too. Start with something simple, get the bare minimum working upstream and add more stuff as you go. Trying to do everything in one patch just makes it much more likely for your patch to be NAKed. What you're doing here is bypassing all the layering we've built. That won't work very well. The only thing you'll get is for your patches to continue to be NAKed. Avoid the tricks and abuses. Just because you _can_ do it somehow, it doesn't mean you _should_ do it :-) Your best option right now, is to remove PM and dual-role support and a minimal glue layer supported. In fact, all you *really* need is to add a compatible to dwc3-of-simple.c. That should be enough to get your dwc3 working. Don't do anything more than that. For dual-role and PM, we can add it generically to dwc3-of-simple.c when all pieces fall into place.
Dear Balbi， On 2016/8/16 15:19, Felipe Balbi wrote: > Hi, > > William Wu <william.wu@rock-chips.com> writes: >> Add rockchip specific glue layer to support USB3 Peripheral mode >> and Host mode on rockchip platforms (e.g. rk3399). >> >> The DesignWare USB3 integrated in rockchip SoCs is a configurable >> IP Core which can be instantiated as Dual-Role Device (DRD), Host >> Only (XHCI) and Peripheral Only configurations. >> >> We use extcon notifier to manage usb cable detection and mode switch. >> Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend >> if USB cable is dettached. For host mode, it requires to keep whole >> DWC3 controller in reset state to hold pipe power state in P2 before >> initializing PHY. And it need to reconfigure USB PHY interface of DWC3 >> core after deassert DWC3 controller reset. >> >> The current driver supports Host only and Peripheral Only well, for >> now, we will add support for OTG after we have it all stabilized. >> >> Signed-off-by: William Wu <william.wu@rock-chips.com> >> --- >> Changes in v10: >> - fix building error >> >> Changes in v9: >> - Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c >> >> drivers/usb/dwc3/Kconfig | 9 + >> drivers/usb/dwc3/Makefile | 1 + >> drivers/usb/dwc3/core.c | 2 +- >> drivers/usb/dwc3/core.h | 1 + >> drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++ > William, if you need to touch core dwc3 to introduce a glue layer, > you're doing it wrong. Sorry， I realized that it's not better to touch core dwc3 in a specific glue layer. I touched dwc3 in glue layer, because I want to support dual-role mode， and according to our dwc3 IP and usb3 PHY IP design， it need to reinit dwc3 core whenever usb cable attached. Anyway, it's wrong to do that.:-[ >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index e887b38..3da6215 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc) >> * initialized. The PHY interfaces and the PHYs get initialized together with >> * the core in dwc3_core_init. >> */ >> -static int dwc3_phy_setup(struct dwc3 *dwc) >> +int dwc3_phy_setup(struct dwc3 *dwc) > there's no way I'll let this slip through the cracks :-) Why I need dwc3_phy_setup in glue layer is because usb3 IP design in rockchip platform. If dwc3 works on host mode, it requires to put dwc3 controller in reset state before usb3 phy initializing，and after deassert reset, we need to reconfigure UTMI+ PHY interface because our dwc3 core can't configure PHY interface correctly. Thank you for give me a chance to explain it.:-) > >> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c >> new file mode 100644 >> index 0000000..eeae1a9 >> --- /dev/null >> +++ b/drivers/usb/dwc3/dwc3-rockchip.c >> @@ -0,0 +1,441 @@ > [...] > >> +static int dwc3_rockchip_probe(struct platform_device *pdev) >> +{ >> + struct dwc3_rockchip *rockchip; >> + struct device *dev = &pdev->dev; >> + struct device_node *np = dev->of_node, *child; >> + struct platform_device *child_pdev; >> + >> + unsigned int count; >> + int ret; >> + int i; >> + >> + rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL); >> + >> + if (!rockchip) >> + return -ENOMEM; >> + >> + count = of_clk_get_parent_count(np); >> + if (!count) >> + return -ENOENT; >> + >> + rockchip->num_clocks = count; >> + >> + rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks, >> + sizeof(struct clk *), GFP_KERNEL); >> + if (!rockchip->clks) >> + return -ENOMEM; >> + >> + platform_set_drvdata(pdev, rockchip); >> + >> + rockchip->dev = dev; >> + rockchip->edev = NULL; >> + >> + pm_runtime_set_active(dev); >> + pm_runtime_enable(dev); >> + ret = pm_runtime_get_sync(dev); >> + if (ret < 0) { >> + dev_err(dev, "get_sync failed with err %d\n", ret); >> + goto err1; >> + } >> + >> + for (i = 0; i < rockchip->num_clocks; i++) { >> + struct clk *clk; >> + >> + clk = of_clk_get(np, i); >> + if (IS_ERR(clk)) { >> + while (--i >= 0) >> + clk_put(rockchip->clks[i]); >> + ret = PTR_ERR(clk); >> + >> + goto err1; >> + } >> + >> + ret = clk_prepare_enable(clk); >> + if (ret < 0) { >> + while (--i >= 0) { >> + clk_disable_unprepare(rockchip->clks[i]); >> + clk_put(rockchip->clks[i]); >> + } >> + clk_put(clk); >> + >> + goto err1; >> + } >> + >> + rockchip->clks[i] = clk; >> + } >> + >> + rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg"); >> + if (IS_ERR(rockchip->otg_rst)) { >> + dev_err(dev, "could not get reset controller\n"); >> + ret = PTR_ERR(rockchip->otg_rst); >> + goto err2; >> + } >> + >> + ret = dwc3_rockchip_extcon_register(rockchip); >> + if (ret < 0) >> + goto err2; >> + >> + child = of_get_child_by_name(np, "dwc3"); >> + if (!child) { >> + dev_err(dev, "failed to find dwc3 core node\n"); >> + ret = -ENODEV; >> + goto err3; >> + } >> + >> + /* Allocate and initialize the core */ >> + ret = of_platform_populate(np, NULL, NULL, dev); >> + if (ret) { >> + dev_err(dev, "failed to create dwc3 core\n"); >> + goto err3; >> + } >> + >> + child_pdev = of_find_device_by_node(child); >> + if (!child_pdev) { >> + dev_err(dev, "failed to find dwc3 core device\n"); >> + ret = -ENODEV; >> + goto err4; >> + } >> + >> + rockchip->dwc = platform_get_drvdata(child_pdev); > No! You will *NOT* the core struct device. Don't even try to come up > with tricks like this. > > Let's do this: introduce a glue layer that gets peripheral-only > working. Remove PM for now, too. Start with something simple, get the > bare minimum working upstream and add more stuff as you go. > > Trying to do everything in one patch just makes it much more likely for > your patch to be NAKed. What you're doing here is bypassing all the > layering we've built. That won't work very well. The only thing you'll > get is for your patches to continue to be NAKed. > > Avoid the tricks and abuses. Just because you _can_ do it somehow, it > doesn't mean you _should_ do it :-) > > Your best option right now, is to remove PM and dual-role support and a > minimal glue layer supported. > > In fact, all you *really* need is to add a compatible to > dwc3-of-simple.c. That should be enough to get your dwc3 working. Don't > do anything more than that. For dual-role and PM, we can add it > generically to dwc3-of-simple.c when all pieces fall into place. > Ah, thanks very much for your kind explanation. I think I just only need to add a compatible to dwc3-of-simple.c，for now， and I have tested my dwc3， it worked well on peripheral only mode and host only mode without PM. Further, if dwc3-of-simple.c adds generic handling of dual-role and PM, I can improve our dwc3 feature.:-) Best regards， William.wu
Hi, "William.wu" <William.wu@rock-chips.com> writes: >> William Wu <william.wu@rock-chips.com> writes: >>> Add rockchip specific glue layer to support USB3 Peripheral mode >>> and Host mode on rockchip platforms (e.g. rk3399). >>> >>> The DesignWare USB3 integrated in rockchip SoCs is a configurable >>> IP Core which can be instantiated as Dual-Role Device (DRD), Host >>> Only (XHCI) and Peripheral Only configurations. >>> >>> We use extcon notifier to manage usb cable detection and mode switch. >>> Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend >>> if USB cable is dettached. For host mode, it requires to keep whole >>> DWC3 controller in reset state to hold pipe power state in P2 before >>> initializing PHY. And it need to reconfigure USB PHY interface of DWC3 >>> core after deassert DWC3 controller reset. >>> >>> The current driver supports Host only and Peripheral Only well, for >>> now, we will add support for OTG after we have it all stabilized. >>> >>> Signed-off-by: William Wu <william.wu@rock-chips.com> >>> --- >>> Changes in v10: >>> - fix building error >>> >>> Changes in v9: >>> - Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c >>> >>> drivers/usb/dwc3/Kconfig | 9 + >>> drivers/usb/dwc3/Makefile | 1 + >>> drivers/usb/dwc3/core.c | 2 +- >>> drivers/usb/dwc3/core.h | 1 + >>> drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++ >> William, if you need to touch core dwc3 to introduce a glue layer, >> you're doing it wrong. > > Sorry， I realized that it's not better to touch core dwc3 in a specific > glue layer. > I touched dwc3 in glue layer, because I want to support dual-role mode， and > according to our dwc3 IP and usb3 PHY IP design， it need to reinit dwc3 > core > whenever usb cable attached. > > Anyway, it's wrong to do that.:-[ > >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>> index e887b38..3da6215 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc) >>> * initialized. The PHY interfaces and the PHYs get initialized together with >>> * the core in dwc3_core_init. >>> */ >>> -static int dwc3_phy_setup(struct dwc3 *dwc) >>> +int dwc3_phy_setup(struct dwc3 *dwc) >> there's no way I'll let this slip through the cracks :-) > > Why I need dwc3_phy_setup in glue layer is because usb3 IP design > in rockchip platform. If dwc3 works on host mode, it requires to put > dwc3 controller in reset state before usb3 phy initializing，and after > deassert reset, we need to reconfigure UTMI+ PHY interface because > our dwc3 core can't configure PHY interface correctly. > > Thank you for give me a chance to explain it.:-) > >> >>> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c >>> new file mode 100644 >>> index 0000000..eeae1a9 >>> --- /dev/null >>> +++ b/drivers/usb/dwc3/dwc3-rockchip.c >>> @@ -0,0 +1,441 @@ >> [...] >> >>> +static int dwc3_rockchip_probe(struct platform_device *pdev) >>> +{ >>> + struct dwc3_rockchip *rockchip; >>> + struct device *dev = &pdev->dev; >>> + struct device_node *np = dev->of_node, *child; >>> + struct platform_device *child_pdev; >>> + >>> + unsigned int count; >>> + int ret; >>> + int i; >>> + >>> + rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL); >>> + >>> + if (!rockchip) >>> + return -ENOMEM; >>> + >>> + count = of_clk_get_parent_count(np); >>> + if (!count) >>> + return -ENOENT; >>> + >>> + rockchip->num_clocks = count; >>> + >>> + rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks, >>> + sizeof(struct clk *), GFP_KERNEL); >>> + if (!rockchip->clks) >>> + return -ENOMEM; >>> + >>> + platform_set_drvdata(pdev, rockchip); >>> + >>> + rockchip->dev = dev; >>> + rockchip->edev = NULL; >>> + >>> + pm_runtime_set_active(dev); >>> + pm_runtime_enable(dev); >>> + ret = pm_runtime_get_sync(dev); >>> + if (ret < 0) { >>> + dev_err(dev, "get_sync failed with err %d\n", ret); >>> + goto err1; >>> + } >>> + >>> + for (i = 0; i < rockchip->num_clocks; i++) { >>> + struct clk *clk; >>> + >>> + clk = of_clk_get(np, i); >>> + if (IS_ERR(clk)) { >>> + while (--i >= 0) >>> + clk_put(rockchip->clks[i]); >>> + ret = PTR_ERR(clk); >>> + >>> + goto err1; >>> + } >>> + >>> + ret = clk_prepare_enable(clk); >>> + if (ret < 0) { >>> + while (--i >= 0) { >>> + clk_disable_unprepare(rockchip->clks[i]); >>> + clk_put(rockchip->clks[i]); >>> + } >>> + clk_put(clk); >>> + >>> + goto err1; >>> + } >>> + >>> + rockchip->clks[i] = clk; >>> + } >>> + >>> + rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg"); >>> + if (IS_ERR(rockchip->otg_rst)) { >>> + dev_err(dev, "could not get reset controller\n"); >>> + ret = PTR_ERR(rockchip->otg_rst); >>> + goto err2; >>> + } >>> + >>> + ret = dwc3_rockchip_extcon_register(rockchip); >>> + if (ret < 0) >>> + goto err2; >>> + >>> + child = of_get_child_by_name(np, "dwc3"); >>> + if (!child) { >>> + dev_err(dev, "failed to find dwc3 core node\n"); >>> + ret = -ENODEV; >>> + goto err3; >>> + } >>> + >>> + /* Allocate and initialize the core */ >>> + ret = of_platform_populate(np, NULL, NULL, dev); >>> + if (ret) { >>> + dev_err(dev, "failed to create dwc3 core\n"); >>> + goto err3; >>> + } >>> + >>> + child_pdev = of_find_device_by_node(child); >>> + if (!child_pdev) { >>> + dev_err(dev, "failed to find dwc3 core device\n"); >>> + ret = -ENODEV; >>> + goto err4; >>> + } >>> + >>> + rockchip->dwc = platform_get_drvdata(child_pdev); >> No! You will *NOT* the core struct device. Don't even try to come up >> with tricks like this. >> >> Let's do this: introduce a glue layer that gets peripheral-only >> working. Remove PM for now, too. Start with something simple, get the >> bare minimum working upstream and add more stuff as you go. >> >> Trying to do everything in one patch just makes it much more likely for >> your patch to be NAKed. What you're doing here is bypassing all the >> layering we've built. That won't work very well. The only thing you'll >> get is for your patches to continue to be NAKed. >> >> Avoid the tricks and abuses. Just because you _can_ do it somehow, it >> doesn't mean you _should_ do it :-) >> >> Your best option right now, is to remove PM and dual-role support and a >> minimal glue layer supported. >> >> In fact, all you *really* need is to add a compatible to >> dwc3-of-simple.c. That should be enough to get your dwc3 working. Don't >> do anything more than that. For dual-role and PM, we can add it >> generically to dwc3-of-simple.c when all pieces fall into place. >> > Ah, thanks very much for your kind explanation. I think I just only need > to add a compatible to dwc3-of-simple.c，for now， and I have tested > my dwc3， it worked well on peripheral only mode and host only mode > without PM. Further, if dwc3-of-simple.c adds generic handling of dual-role > and PM, I can improve our dwc3 feature.:-) that's my point exactly. We can add more support generically so that other platforms can benefit from the work. PM should be simple for dwc3-of-simple.c. Dual-role will take a little more effort. In almost there actually. There are a few missing pieces but it should be doable (hopefully) within the next two major releases. Your integration is no different than other companies' using DWC3 in dual-role setup. For example TI's DWC3 have the same requirements as you do, so it makes sense to add it straight to dwc3-core. Roger Quadros (now in Cc) has been working on dual-role for TI's platforms and we've been discussing about how to add missing pieces generically. Perhaps you'd want to join the discussion.
Dear Balbi, On 2016/8/16 18:43, Felipe Balbi wrote: > Hi, > > "William.wu" <William.wu@rock-chips.com> writes: >>> William Wu <william.wu@rock-chips.com> writes: >>>> Add rockchip specific glue layer to support USB3 Peripheral mode >>>> and Host mode on rockchip platforms (e.g. rk3399). >>>> >>>> The DesignWare USB3 integrated in rockchip SoCs is a configurable >>>> IP Core which can be instantiated as Dual-Role Device (DRD), Host >>>> Only (XHCI) and Peripheral Only configurations. >>>> >>>> We use extcon notifier to manage usb cable detection and mode switch. >>>> Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend >>>> if USB cable is dettached. For host mode, it requires to keep whole >>>> DWC3 controller in reset state to hold pipe power state in P2 before >>>> initializing PHY. And it need to reconfigure USB PHY interface of DWC3 >>>> core after deassert DWC3 controller reset. >>>> >>>> The current driver supports Host only and Peripheral Only well, for >>>> now, we will add support for OTG after we have it all stabilized. >>>> >>>> Signed-off-by: William Wu <william.wu@rock-chips.com> >>>> --- >>>> Changes in v10: >>>> - fix building error >>>> >>>> Changes in v9: >>>> - Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c >>>> >>>> drivers/usb/dwc3/Kconfig | 9 + >>>> drivers/usb/dwc3/Makefile | 1 + >>>> drivers/usb/dwc3/core.c | 2 +- >>>> drivers/usb/dwc3/core.h | 1 + >>>> drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++ >>> William, if you need to touch core dwc3 to introduce a glue layer, >>> you're doing it wrong. >> Sorry， I realized that it's not better to touch core dwc3 in a specific >> glue layer. >> I touched dwc3 in glue layer, because I want to support dual-role mode， and >> according to our dwc3 IP and usb3 PHY IP design， it need to reinit dwc3 >> core >> whenever usb cable attached. >> >> Anyway, it's wrong to do that.:-[ >> >>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>> index e887b38..3da6215 100644 >>>> --- a/drivers/usb/dwc3/core.c >>>> +++ b/drivers/usb/dwc3/core.c >>>> @@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc) >>>> * initialized. The PHY interfaces and the PHYs get initialized together with >>>> * the core in dwc3_core_init. >>>> */ >>>> -static int dwc3_phy_setup(struct dwc3 *dwc) >>>> +int dwc3_phy_setup(struct dwc3 *dwc) >>> there's no way I'll let this slip through the cracks :-) >> Why I need dwc3_phy_setup in glue layer is because usb3 IP design >> in rockchip platform. If dwc3 works on host mode, it requires to put >> dwc3 controller in reset state before usb3 phy initializing，and after >> deassert reset, we need to reconfigure UTMI+ PHY interface because >> our dwc3 core can't configure PHY interface correctly. >> >> Thank you for give me a chance to explain it.:-) >> >>>> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c >>>> new file mode 100644 >>>> index 0000000..eeae1a9 >>>> --- /dev/null >>>> +++ b/drivers/usb/dwc3/dwc3-rockchip.c >>>> @@ -0,0 +1,441 @@ >>> [...] >>> >>>> +static int dwc3_rockchip_probe(struct platform_device *pdev) >>>> +{ >>>> + struct dwc3_rockchip *rockchip; >>>> + struct device *dev = &pdev->dev; >>>> + struct device_node *np = dev->of_node, *child; >>>> + struct platform_device *child_pdev; >>>> + >>>> + unsigned int count; >>>> + int ret; >>>> + int i; >>>> + >>>> + rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL); >>>> + >>>> + if (!rockchip) >>>> + return -ENOMEM; >>>> + >>>> + count = of_clk_get_parent_count(np); >>>> + if (!count) >>>> + return -ENOENT; >>>> + >>>> + rockchip->num_clocks = count; >>>> + >>>> + rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks, >>>> + sizeof(struct clk *), GFP_KERNEL); >>>> + if (!rockchip->clks) >>>> + return -ENOMEM; >>>> + >>>> + platform_set_drvdata(pdev, rockchip); >>>> + >>>> + rockchip->dev = dev; >>>> + rockchip->edev = NULL; >>>> + >>>> + pm_runtime_set_active(dev); >>>> + pm_runtime_enable(dev); >>>> + ret = pm_runtime_get_sync(dev); >>>> + if (ret < 0) { >>>> + dev_err(dev, "get_sync failed with err %d\n", ret); >>>> + goto err1; >>>> + } >>>> + >>>> + for (i = 0; i < rockchip->num_clocks; i++) { >>>> + struct clk *clk; >>>> + >>>> + clk = of_clk_get(np, i); >>>> + if (IS_ERR(clk)) { >>>> + while (--i >= 0) >>>> + clk_put(rockchip->clks[i]); >>>> + ret = PTR_ERR(clk); >>>> + >>>> + goto err1; >>>> + } >>>> + >>>> + ret = clk_prepare_enable(clk); >>>> + if (ret < 0) { >>>> + while (--i >= 0) { >>>> + clk_disable_unprepare(rockchip->clks[i]); >>>> + clk_put(rockchip->clks[i]); >>>> + } >>>> + clk_put(clk); >>>> + >>>> + goto err1; >>>> + } >>>> + >>>> + rockchip->clks[i] = clk; >>>> + } >>>> + >>>> + rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg"); >>>> + if (IS_ERR(rockchip->otg_rst)) { >>>> + dev_err(dev, "could not get reset controller\n"); >>>> + ret = PTR_ERR(rockchip->otg_rst); >>>> + goto err2; >>>> + } >>>> + >>>> + ret = dwc3_rockchip_extcon_register(rockchip); >>>> + if (ret < 0) >>>> + goto err2; >>>> + >>>> + child = of_get_child_by_name(np, "dwc3"); >>>> + if (!child) { >>>> + dev_err(dev, "failed to find dwc3 core node\n"); >>>> + ret = -ENODEV; >>>> + goto err3; >>>> + } >>>> + >>>> + /* Allocate and initialize the core */ >>>> + ret = of_platform_populate(np, NULL, NULL, dev); >>>> + if (ret) { >>>> + dev_err(dev, "failed to create dwc3 core\n"); >>>> + goto err3; >>>> + } >>>> + >>>> + child_pdev = of_find_device_by_node(child); >>>> + if (!child_pdev) { >>>> + dev_err(dev, "failed to find dwc3 core device\n"); >>>> + ret = -ENODEV; >>>> + goto err4; >>>> + } >>>> + >>>> + rockchip->dwc = platform_get_drvdata(child_pdev); >>> No! You will *NOT* the core struct device. Don't even try to come up >>> with tricks like this. >>> >>> Let's do this: introduce a glue layer that gets peripheral-only >>> working. Remove PM for now, too. Start with something simple, get the >>> bare minimum working upstream and add more stuff as you go. >>> >>> Trying to do everything in one patch just makes it much more likely for >>> your patch to be NAKed. What you're doing here is bypassing all the >>> layering we've built. That won't work very well. The only thing you'll >>> get is for your patches to continue to be NAKed. >>> >>> Avoid the tricks and abuses. Just because you _can_ do it somehow, it >>> doesn't mean you _should_ do it :-) >>> >>> Your best option right now, is to remove PM and dual-role support and a >>> minimal glue layer supported. >>> >>> In fact, all you *really* need is to add a compatible to >>> dwc3-of-simple.c. That should be enough to get your dwc3 working. Don't >>> do anything more than that. For dual-role and PM, we can add it >>> generically to dwc3-of-simple.c when all pieces fall into place. >>> >> Ah, thanks very much for your kind explanation. I think I just only need >> to add a compatible to dwc3-of-simple.c，for now， and I have tested >> my dwc3， it worked well on peripheral only mode and host only mode >> without PM. Further, if dwc3-of-simple.c adds generic handling of dual-role >> and PM, I can improve our dwc3 feature.:-) > that's my point exactly. We can add more support generically so that > other platforms can benefit from the work. PM should be simple for > dwc3-of-simple.c. Dual-role will take a little more effort. In almost > there actually. There are a few missing pieces but it should be doable > (hopefully) within the next two major releases. > > Your integration is no different than other companies' using DWC3 in > dual-role setup. For example TI's DWC3 have the same requirements as you > do, so it makes sense to add it straight to dwc3-core. Roger Quadros > (now in Cc) has been working on dual-role for TI's platforms and we've > been discussing about how to add missing pieces generically. Perhaps > you'd want to join the discussion. Thanks！ I'll upload a new patch later. And I have seen the dual-role patch uploaded by Roger Quadros, it's helpful for me. I'm interested in the patch， but I need to understand the patch first， hope to be able to join the discussion.:-)
Hi, "William.wu" <William.wu@rock-chips.com> writes: >> "William.wu" <William.wu@rock-chips.com> writes: >>>> William Wu <william.wu@rock-chips.com> writes: >>>>> Add rockchip specific glue layer to support USB3 Peripheral mode >>>>> and Host mode on rockchip platforms (e.g. rk3399). >>>>> >>>>> The DesignWare USB3 integrated in rockchip SoCs is a configurable >>>>> IP Core which can be instantiated as Dual-Role Device (DRD), Host >>>>> Only (XHCI) and Peripheral Only configurations. >>>>> >>>>> We use extcon notifier to manage usb cable detection and mode switch. >>>>> Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend >>>>> if USB cable is dettached. For host mode, it requires to keep whole >>>>> DWC3 controller in reset state to hold pipe power state in P2 before >>>>> initializing PHY. And it need to reconfigure USB PHY interface of DWC3 >>>>> core after deassert DWC3 controller reset. >>>>> >>>>> The current driver supports Host only and Peripheral Only well, for >>>>> now, we will add support for OTG after we have it all stabilized. >>>>> >>>>> Signed-off-by: William Wu <william.wu@rock-chips.com> >>>>> --- >>>>> Changes in v10: >>>>> - fix building error >>>>> >>>>> Changes in v9: >>>>> - Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c >>>>> >>>>> drivers/usb/dwc3/Kconfig | 9 + >>>>> drivers/usb/dwc3/Makefile | 1 + >>>>> drivers/usb/dwc3/core.c | 2 +- >>>>> drivers/usb/dwc3/core.h | 1 + >>>>> drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++ >>>> William, if you need to touch core dwc3 to introduce a glue layer, >>>> you're doing it wrong. >>> Sorry， I realized that it's not better to touch core dwc3 in a specific >>> glue layer. >>> I touched dwc3 in glue layer, because I want to support dual-role mode， and >>> according to our dwc3 IP and usb3 PHY IP design， it need to reinit dwc3 >>> core >>> whenever usb cable attached. >>> >>> Anyway, it's wrong to do that.:-[ >>> >>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>>> index e887b38..3da6215 100644 >>>>> --- a/drivers/usb/dwc3/core.c >>>>> +++ b/drivers/usb/dwc3/core.c >>>>> @@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc) >>>>> * initialized. The PHY interfaces and the PHYs get initialized together with >>>>> * the core in dwc3_core_init. >>>>> */ >>>>> -static int dwc3_phy_setup(struct dwc3 *dwc) >>>>> +int dwc3_phy_setup(struct dwc3 *dwc) >>>> there's no way I'll let this slip through the cracks :-) >>> Why I need dwc3_phy_setup in glue layer is because usb3 IP design >>> in rockchip platform. If dwc3 works on host mode, it requires to put >>> dwc3 controller in reset state before usb3 phy initializing，and after >>> deassert reset, we need to reconfigure UTMI+ PHY interface because >>> our dwc3 core can't configure PHY interface correctly. >>> >>> Thank you for give me a chance to explain it.:-) >>> >>>>> diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c >>>>> new file mode 100644 >>>>> index 0000000..eeae1a9 >>>>> --- /dev/null >>>>> +++ b/drivers/usb/dwc3/dwc3-rockchip.c >>>>> @@ -0,0 +1,441 @@ >>>> [...] >>>> >>>>> +static int dwc3_rockchip_probe(struct platform_device *pdev) >>>>> +{ >>>>> + struct dwc3_rockchip *rockchip; >>>>> + struct device *dev = &pdev->dev; >>>>> + struct device_node *np = dev->of_node, *child; >>>>> + struct platform_device *child_pdev; >>>>> + >>>>> + unsigned int count; >>>>> + int ret; >>>>> + int i; >>>>> + >>>>> + rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL); >>>>> + >>>>> + if (!rockchip) >>>>> + return -ENOMEM; >>>>> + >>>>> + count = of_clk_get_parent_count(np); >>>>> + if (!count) >>>>> + return -ENOENT; >>>>> + >>>>> + rockchip->num_clocks = count; >>>>> + >>>>> + rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks, >>>>> + sizeof(struct clk *), GFP_KERNEL); >>>>> + if (!rockchip->clks) >>>>> + return -ENOMEM; >>>>> + >>>>> + platform_set_drvdata(pdev, rockchip); >>>>> + >>>>> + rockchip->dev = dev; >>>>> + rockchip->edev = NULL; >>>>> + >>>>> + pm_runtime_set_active(dev); >>>>> + pm_runtime_enable(dev); >>>>> + ret = pm_runtime_get_sync(dev); >>>>> + if (ret < 0) { >>>>> + dev_err(dev, "get_sync failed with err %d\n", ret); >>>>> + goto err1; >>>>> + } >>>>> + >>>>> + for (i = 0; i < rockchip->num_clocks; i++) { >>>>> + struct clk *clk; >>>>> + >>>>> + clk = of_clk_get(np, i); >>>>> + if (IS_ERR(clk)) { >>>>> + while (--i >= 0) >>>>> + clk_put(rockchip->clks[i]); >>>>> + ret = PTR_ERR(clk); >>>>> + >>>>> + goto err1; >>>>> + } >>>>> + >>>>> + ret = clk_prepare_enable(clk); >>>>> + if (ret < 0) { >>>>> + while (--i >= 0) { >>>>> + clk_disable_unprepare(rockchip->clks[i]); >>>>> + clk_put(rockchip->clks[i]); >>>>> + } >>>>> + clk_put(clk); >>>>> + >>>>> + goto err1; >>>>> + } >>>>> + >>>>> + rockchip->clks[i] = clk; >>>>> + } >>>>> + >>>>> + rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg"); >>>>> + if (IS_ERR(rockchip->otg_rst)) { >>>>> + dev_err(dev, "could not get reset controller\n"); >>>>> + ret = PTR_ERR(rockchip->otg_rst); >>>>> + goto err2; >>>>> + } >>>>> + >>>>> + ret = dwc3_rockchip_extcon_register(rockchip); >>>>> + if (ret < 0) >>>>> + goto err2; >>>>> + >>>>> + child = of_get_child_by_name(np, "dwc3"); >>>>> + if (!child) { >>>>> + dev_err(dev, "failed to find dwc3 core node\n"); >>>>> + ret = -ENODEV; >>>>> + goto err3; >>>>> + } >>>>> + >>>>> + /* Allocate and initialize the core */ >>>>> + ret = of_platform_populate(np, NULL, NULL, dev); >>>>> + if (ret) { >>>>> + dev_err(dev, "failed to create dwc3 core\n"); >>>>> + goto err3; >>>>> + } >>>>> + >>>>> + child_pdev = of_find_device_by_node(child); >>>>> + if (!child_pdev) { >>>>> + dev_err(dev, "failed to find dwc3 core device\n"); >>>>> + ret = -ENODEV; >>>>> + goto err4; >>>>> + } >>>>> + >>>>> + rockchip->dwc = platform_get_drvdata(child_pdev); >>>> No! You will *NOT* the core struct device. Don't even try to come up >>>> with tricks like this. >>>> >>>> Let's do this: introduce a glue layer that gets peripheral-only >>>> working. Remove PM for now, too. Start with something simple, get the >>>> bare minimum working upstream and add more stuff as you go. >>>> >>>> Trying to do everything in one patch just makes it much more likely for >>>> your patch to be NAKed. What you're doing here is bypassing all the >>>> layering we've built. That won't work very well. The only thing you'll >>>> get is for your patches to continue to be NAKed. >>>> >>>> Avoid the tricks and abuses. Just because you _can_ do it somehow, it >>>> doesn't mean you _should_ do it :-) >>>> >>>> Your best option right now, is to remove PM and dual-role support and a >>>> minimal glue layer supported. >>>> >>>> In fact, all you *really* need is to add a compatible to >>>> dwc3-of-simple.c. That should be enough to get your dwc3 working. Don't >>>> do anything more than that. For dual-role and PM, we can add it >>>> generically to dwc3-of-simple.c when all pieces fall into place. >>>> >>> Ah, thanks very much for your kind explanation. I think I just only need >>> to add a compatible to dwc3-of-simple.c，for now， and I have tested >>> my dwc3， it worked well on peripheral only mode and host only mode >>> without PM. Further, if dwc3-of-simple.c adds generic handling of dual-role >>> and PM, I can improve our dwc3 feature.:-) >> that's my point exactly. We can add more support generically so that >> other platforms can benefit from the work. PM should be simple for >> dwc3-of-simple.c. Dual-role will take a little more effort. In almost >> there actually. There are a few missing pieces but it should be doable >> (hopefully) within the next two major releases. >> >> Your integration is no different than other companies' using DWC3 in >> dual-role setup. For example TI's DWC3 have the same requirements as you >> do, so it makes sense to add it straight to dwc3-core. Roger Quadros >> (now in Cc) has been working on dual-role for TI's platforms and we've >> been discussing about how to add missing pieces generically. Perhaps >> you'd want to join the discussion. > Thanks！ I'll upload a new patch later. And I have seen the dual-role patch > uploaded by Roger Quadros, it's helpful for me. I'm interested in the > patch， > but I need to understand the patch first， hope to be able to join the > discussion.:-) cool, thanks. More users means we're more likely to make a trully generic layer. We're discussing some simplification of that layer, however, so that it doesn't take as much code to get DRD working.
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index a64ce1c..3d5ec30 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -96,6 +96,15 @@ config USB_DWC3_OF_SIMPLE Currently supports Xilinx and Qualcomm DWC USB3 IP. Say 'Y' or 'M' if you have one such device. +config USB_DWC3_ROCKCHIP + tristate "Rockchip Platforms" + depends on EXTCON && (ARCH_ROCKCHIP || COMPILE_TEST) + depends on OF + default USB_DWC3 + help + Support of USB2/3 functionality in Rockchip platforms. + say 'Y' or 'M' if you have one such device. + config USB_DWC3_ST tristate "STMicroelectronics Platforms" depends on ARCH_STI && OF diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile index 22420e1..86fc4fd 100644 --- a/drivers/usb/dwc3/Makefile +++ b/drivers/usb/dwc3/Makefile @@ -38,4 +38,5 @@ obj-$(CONFIG_USB_DWC3_EXYNOS) += dwc3-exynos.o obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o +obj-$(CONFIG_USB_DWC3_ROCKCHIP) += dwc3-rockchip.o obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index e887b38..3da6215 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -405,7 +405,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc) * initialized. The PHY interfaces and the PHYs get initialized together with * the core in dwc3_core_init. */ -static int dwc3_phy_setup(struct dwc3 *dwc) +int dwc3_phy_setup(struct dwc3 *dwc) { u32 reg; int ret; diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 3d94acd..79403ff 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -1123,6 +1123,7 @@ struct dwc3_gadget_ep_cmd_params { /* prototypes */ void dwc3_set_mode(struct dwc3 *dwc, u32 mode); u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type); +int dwc3_phy_setup(struct dwc3 *dwc); /* check whether we are on the DWC_usb31 core */ static inline bool dwc3_is_usb31(struct dwc3 *dwc) diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c new file mode 100644 index 0000000..eeae1a9 --- /dev/null +++ b/drivers/usb/dwc3/dwc3-rockchip.c @@ -0,0 +1,441 @@ +/** + * dwc3-rockchip.c - Rockchip Specific Glue layer + * + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd + * + * Authors: William Wu <william.wu@rock-chips.com> + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 of + * the License as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/platform_device.h> +#include <linux/dma-mapping.h> +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/of.h> +#include <linux/of_platform.h> +#include <linux/pm_runtime.h> +#include <linux/extcon.h> +#include <linux/reset.h> +#include <linux/usb.h> +#include <linux/usb/hcd.h> + +#include "core.h" +#include "io.h" + +#define DWC3_ROCKCHIP_AUTOSUSPEND_DELAY 500 /* ms */ + +struct dwc3_rockchip { + int num_clocks; + struct device *dev; + struct clk **clks; + struct dwc3 *dwc; + struct reset_control *otg_rst; + struct extcon_dev *edev; + struct notifier_block device_nb; + struct notifier_block host_nb; + struct work_struct otg_work; +}; + +static int dwc3_rockchip_device_notifier(struct notifier_block *nb, + unsigned long event, void *ptr) +{ + struct dwc3_rockchip *rockchip = + container_of(nb, struct dwc3_rockchip, device_nb); + + schedule_work(&rockchip->otg_work); + + return NOTIFY_DONE; +} + +static int dwc3_rockchip_host_notifier(struct notifier_block *nb, + unsigned long event, void *ptr) +{ + struct dwc3_rockchip *rockchip = + container_of(nb, struct dwc3_rockchip, host_nb); + + schedule_work(&rockchip->otg_work); + + return NOTIFY_DONE; +} + +static void dwc3_rockchip_otg_extcon_evt_work(struct work_struct *work) +{ + struct dwc3_rockchip *rockchip = + container_of(work, struct dwc3_rockchip, otg_work); + struct dwc3 *dwc = rockchip->dwc; + struct extcon_dev *edev = rockchip->edev; + struct usb_hcd *hcd; + unsigned long flags; + int ret; + u32 reg; + + if (extcon_get_cable_state_(edev, EXTCON_USB) > 0) { + if (dwc->connected) + return; + + /* + * If dr_mode is host only, never to set + * the mode to the peripheral mode. + */ + if (WARN_ON(dwc->dr_mode == USB_DR_MODE_HOST)) + return; + + pm_runtime_get_sync(dwc->dev); + + spin_lock_irqsave(&dwc->lock, flags); + dwc->connected = true; + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE); + spin_unlock_irqrestore(&dwc->lock, flags); + + pm_runtime_put_sync(dwc->dev); + + dev_info(rockchip->dev, "USB peripheral connected\n"); + } else if (extcon_get_cable_state_(edev, EXTCON_USB_HOST) > 0) { + if (dwc->connected) + return; + + /* + * If dr_mode is device only, never to + * set the mode to the host mode. + */ + if (WARN_ON(dwc->dr_mode == USB_DR_MODE_PERIPHERAL)) + return; + + reset_control_assert(rockchip->otg_rst); + + ret = phy_power_on(dwc->usb2_generic_phy); + if (ret < 0) + return; + + ret = phy_power_on(dwc->usb3_generic_phy); + if (ret < 0) + return; + + reset_control_deassert(rockchip->otg_rst); + + dwc3_phy_setup(dwc); + + hcd = dev_get_drvdata(&dwc->xhci->dev); + + spin_lock_irqsave(&dwc->lock, flags); + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST); + dwc->connected = true; + spin_unlock_irqrestore(&dwc->lock, flags); + + if (hcd->state == HC_STATE_HALT) { + usb_add_hcd(hcd, hcd->irq, IRQF_SHARED); + usb_add_hcd(hcd->shared_hcd, hcd->irq, IRQF_SHARED); + } + + dev_info(rockchip->dev, "USB HOST connected\n"); + } else { + if (!dwc->connected) + return; + + reg = dwc3_readl(dwc->regs, DWC3_GCTL); + + if (DWC3_GCTL_PRTCAP(reg) == DWC3_GCTL_PRTCAP_HOST || + DWC3_GCTL_PRTCAP(reg) == DWC3_GCTL_PRTCAP_OTG) { + hcd = dev_get_drvdata(&dwc->xhci->dev); + + if (hcd->state != HC_STATE_HALT) { + usb_remove_hcd(hcd->shared_hcd); + usb_remove_hcd(hcd); + } + + phy_power_off(dwc->usb2_generic_phy); + phy_power_off(dwc->usb3_generic_phy); + } + + spin_lock_irqsave(&dwc->lock, flags); + dwc->connected = false; + spin_unlock_irqrestore(&dwc->lock, flags); + + dev_info(rockchip->dev, "USB unconnected\n"); + } +} + +static int dwc3_rockchip_extcon_register(struct dwc3_rockchip *rockchip) +{ + int ret; + struct device *dev = rockchip->dev; + struct extcon_dev *edev; + + if (device_property_read_bool(dev, "extcon")) { + edev = extcon_get_edev_by_phandle(dev, 0); + if (IS_ERR(edev)) { + if (PTR_ERR(edev) != -EPROBE_DEFER) + dev_err(dev, "couldn't get extcon device\n"); + return PTR_ERR(edev); + } + + INIT_WORK(&rockchip->otg_work, + dwc3_rockchip_otg_extcon_evt_work); + + rockchip->device_nb.notifier_call = + dwc3_rockchip_device_notifier; + ret = extcon_register_notifier(edev, EXTCON_USB, + &rockchip->device_nb); + if (ret < 0) { + dev_err(dev, "failed to register notifier for USB\n"); + return ret; + } + + rockchip->host_nb.notifier_call = + dwc3_rockchip_host_notifier; + ret = extcon_register_notifier(edev, EXTCON_USB_HOST, + &rockchip->host_nb); + if (ret < 0) { + dev_err(dev, "failed to register notifier for USB HOST\n"); + extcon_unregister_notifier(edev, EXTCON_USB, + &rockchip->device_nb); + return ret; + } + + rockchip->edev = edev; + } + + return 0; +} + +static void dwc3_rockchip_extcon_unregister(struct dwc3_rockchip *rockchip) +{ + if (!rockchip->edev) + return; + + extcon_unregister_notifier(rockchip->edev, EXTCON_USB, + &rockchip->device_nb); + extcon_unregister_notifier(rockchip->edev, EXTCON_USB_HOST, + &rockchip->host_nb); +} + +static int dwc3_rockchip_probe(struct platform_device *pdev) +{ + struct dwc3_rockchip *rockchip; + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node, *child; + struct platform_device *child_pdev; + + unsigned int count; + int ret; + int i; + + rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL); + + if (!rockchip) + return -ENOMEM; + + count = of_clk_get_parent_count(np); + if (!count) + return -ENOENT; + + rockchip->num_clocks = count; + + rockchip->clks = devm_kcalloc(dev, rockchip->num_clocks, + sizeof(struct clk *), GFP_KERNEL); + if (!rockchip->clks) + return -ENOMEM; + + platform_set_drvdata(pdev, rockchip); + + rockchip->dev = dev; + rockchip->edev = NULL; + + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + dev_err(dev, "get_sync failed with err %d\n", ret); + goto err1; + } + + for (i = 0; i < rockchip->num_clocks; i++) { + struct clk *clk; + + clk = of_clk_get(np, i); + if (IS_ERR(clk)) { + while (--i >= 0) + clk_put(rockchip->clks[i]); + ret = PTR_ERR(clk); + + goto err1; + } + + ret = clk_prepare_enable(clk); + if (ret < 0) { + while (--i >= 0) { + clk_disable_unprepare(rockchip->clks[i]); + clk_put(rockchip->clks[i]); + } + clk_put(clk); + + goto err1; + } + + rockchip->clks[i] = clk; + } + + rockchip->otg_rst = devm_reset_control_get(dev, "usb3-otg"); + if (IS_ERR(rockchip->otg_rst)) { + dev_err(dev, "could not get reset controller\n"); + ret = PTR_ERR(rockchip->otg_rst); + goto err2; + } + + ret = dwc3_rockchip_extcon_register(rockchip); + if (ret < 0) + goto err2; + + child = of_get_child_by_name(np, "dwc3"); + if (!child) { + dev_err(dev, "failed to find dwc3 core node\n"); + ret = -ENODEV; + goto err3; + } + + /* Allocate and initialize the core */ + ret = of_platform_populate(np, NULL, NULL, dev); + if (ret) { + dev_err(dev, "failed to create dwc3 core\n"); + goto err3; + } + + child_pdev = of_find_device_by_node(child); + if (!child_pdev) { + dev_err(dev, "failed to find dwc3 core device\n"); + ret = -ENODEV; + goto err4; + } + + rockchip->dwc = platform_get_drvdata(child_pdev); + + if (rockchip->edev) { + pm_runtime_set_autosuspend_delay(&child_pdev->dev, + DWC3_ROCKCHIP_AUTOSUSPEND_DELAY); + pm_runtime_allow(&child_pdev->dev); + + if (rockchip->dwc->dr_mode == USB_DR_MODE_HOST || + rockchip->dwc->dr_mode == USB_DR_MODE_OTG) { + struct usb_hcd *hcd = + dev_get_drvdata(&rockchip->dwc->xhci->dev); + + if (hcd->state != HC_STATE_HALT) { + usb_remove_hcd(hcd->shared_hcd); + usb_remove_hcd(hcd); + } + } + } + + return ret; + +err4: + of_platform_depopulate(dev); + +err3: + dwc3_rockchip_extcon_unregister(rockchip); + +err2: + for (i = 0; i < rockchip->num_clocks; i++) { + clk_disable_unprepare(rockchip->clks[i]); + clk_put(rockchip->clks[i]); + } + +err1: + pm_runtime_put_sync(dev); + pm_runtime_disable(dev); + + return ret; +} + +static int dwc3_rockchip_remove(struct platform_device *pdev) +{ + struct dwc3_rockchip *rockchip = platform_get_drvdata(pdev); + struct device *dev = &pdev->dev; + int i; + + for (i = 0; i < rockchip->num_clocks; i++) { + clk_disable_unprepare(rockchip->clks[i]); + clk_put(rockchip->clks[i]); + } + + dwc3_rockchip_extcon_unregister(rockchip); + + of_platform_depopulate(dev); + + pm_runtime_put_sync(dev); + pm_runtime_disable(dev); + + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int dwc3_rockchip_suspend(struct device *dev) +{ + struct dwc3_rockchip *rockchip = dev_get_drvdata(dev); + int i; + + for (i = 0; i < rockchip->num_clocks; i++) + clk_disable(rockchip->clks[i]); + + return 0; +} + +static int dwc3_rockchip_resume(struct device *dev) +{ + struct dwc3_rockchip *rockchip = dev_get_drvdata(dev); + int i; + + for (i = 0; i < rockchip->num_clocks; i++) + clk_enable(rockchip->clks[i]); + + /* runtime set active to reflect active state. */ + pm_runtime_disable(dev); + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + + return 0; +} + +static const struct dev_pm_ops dwc3_rockchip_dev_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(dwc3_rockchip_suspend, dwc3_rockchip_resume) +}; + +#define DEV_PM_OPS (&dwc3_rockchip_dev_pm_ops) +#else +#define DEV_PM_OPS NULL +#endif /* CONFIG_PM_SLEEP */ + +static const struct of_device_id rockchip_dwc3_match[] = { + { .compatible = "rockchip,rk3399-dwc3" }, + { /* Sentinel */ } +}; + +MODULE_DEVICE_TABLE(of, rockchip_dwc3_match); + +static struct platform_driver dwc3_rockchip_driver = { + .probe = dwc3_rockchip_probe, + .remove = dwc3_rockchip_remove, + .driver = { + .name = "rockchip-dwc3", + .of_match_table = rockchip_dwc3_match, + .pm = DEV_PM_OPS, + }, +}; + +module_platform_driver(dwc3_rockchip_driver); + +MODULE_ALIAS("platform:rockchip-dwc3"); +MODULE_AUTHOR("William Wu <william.wu@rock-chips.com>"); +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("DesignWare USB3 ROCKCHIP Glue Layer");
Add rockchip specific glue layer to support USB3 Peripheral mode and Host mode on rockchip platforms (e.g. rk3399). The DesignWare USB3 integrated in rockchip SoCs is a configurable IP Core which can be instantiated as Dual-Role Device (DRD), Host Only (XHCI) and Peripheral Only configurations. We use extcon notifier to manage usb cable detection and mode switch. Enable DWC3 PM runtime auto suspend to allow core enter runtime_suspend if USB cable is dettached. For host mode, it requires to keep whole DWC3 controller in reset state to hold pipe power state in P2 before initializing PHY. And it need to reconfigure USB PHY interface of DWC3 core after deassert DWC3 controller reset. The current driver supports Host only and Peripheral Only well, for now, we will add support for OTG after we have it all stabilized. Signed-off-by: William Wu <william.wu@rock-chips.com> --- Changes in v10: - fix building error Changes in v9: - Add a new dwc3-rockchip.c driver for rk3399, rather than use dwc3-of-simple.c drivers/usb/dwc3/Kconfig | 9 + drivers/usb/dwc3/Makefile | 1 + drivers/usb/dwc3/core.c | 2 +- drivers/usb/dwc3/core.h | 1 + drivers/usb/dwc3/dwc3-rockchip.c | 441 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 453 insertions(+), 1 deletion(-) create mode 100644 drivers/usb/dwc3/dwc3-rockchip.c