2014-12-16 02:11:11

by Sneeker Yeh

[permalink] [raw]
Subject: [PATCH 0/3] Add support for Fujitsu USB host controller

These patches add support for EHCI and XHCI compliant Host controller found
on Fujitsu Socs, and are based on
http://www.spinics.net/lists/arm-kernel/msg385573.html

First patch is EHCI platform glue driver. Patch 2 introduces Fujitsu
Specific Glue layer in Synopsis DesignWare USB3 IP driver. Patch 3
introduces a quirk with device disconnection management necessary for IPs
using the Synopsys Designware dwc3 core. It solves a problem where without
the quirk, that host controller will die after a usb device is disconnected
from port of root hub, which happened in Synopsis core with Fujitsu
config-free usb phy integrated.

Successfully tested on Fujitsu mb86s7x board.

Sneeker Yeh (3):
usb: host: f_usb20ho: add support for Fujitsu ehci/ohci USB 2.0 host
controller
usb: dwc3: add Fujitsu Specific Glue layer
usb: dwc3: add a quirk for device disconnection issue in Synopsis
dwc3 core

.../devicetree/bindings/usb/fujitsu-dwc3.txt | 25 ++
.../devicetree/bindings/usb/fujitsu-ehci.txt | 22 ++
drivers/usb/dwc3/Kconfig | 11 +
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/dwc3-mb86s70.c | 194 +++++++++++++
drivers/usb/host/Kconfig | 11 +
drivers/usb/host/Makefile | 1 +
drivers/usb/host/f_usb20ho_hcd.c | 306 ++++++++++++++++++++
drivers/usb/host/f_usb20ho_hcd.h | 35 +++
drivers/usb/host/xhci-hub.c | 4 +
drivers/usb/host/xhci-plat.c | 5 +
drivers/usb/host/xhci.c | 29 ++
drivers/usb/host/xhci.h | 7 +
13 files changed, 651 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-ehci.txt
create mode 100644 drivers/usb/dwc3/dwc3-mb86s70.c
create mode 100644 drivers/usb/host/f_usb20ho_hcd.c
create mode 100644 drivers/usb/host/f_usb20ho_hcd.h

--
1.7.9.5


2014-12-16 02:11:25

by Sneeker Yeh

[permalink] [raw]
Subject: [PATCH 1/3] usb: host: f_usb20ho: add support for Fujitsu ehci/ohci USB 2.0 host controller

This patch adds support for EHCI compliant Host controller found
on Fujitsu Socs.

Signed-off-by: Sneeker Yeh <[email protected]>
---
.../devicetree/bindings/usb/fujitsu-ehci.txt | 22 ++
drivers/usb/host/Kconfig | 11 +
drivers/usb/host/Makefile | 1 +
drivers/usb/host/f_usb20ho_hcd.c | 306 ++++++++++++++++++++
drivers/usb/host/f_usb20ho_hcd.h | 35 +++
5 files changed, 375 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-ehci.txt
create mode 100644 drivers/usb/host/f_usb20ho_hcd.c
create mode 100644 drivers/usb/host/f_usb20ho_hcd.h

diff --git a/Documentation/devicetree/bindings/usb/fujitsu-ehci.txt b/Documentation/devicetree/bindings/usb/fujitsu-ehci.txt
new file mode 100644
index 0000000..e180860
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/fujitsu-ehci.txt
@@ -0,0 +1,22 @@
+FUJITSU GLUE COMPONENTS
+
+MB86S7x EHCI GLUE
+ - compatible : Should be "fujitsu,f_usb20ho_hcd"
+ - reg : Address and length of the register set for the device.
+ - interrupts : The irq number of this device that is used to interrupt the
+ CPU
+ - clocks: from common clock binding, handle to usb clock.
+ - clock-names: from common clock binding.
+ - #stream-id-cells : handle to use "arm,mmu-400" ARM IOMMU driver
+ - fujitsu,power-domain : pd_usb2h node has to be builded, details can be
+ found in:
+ Documentation/devicetree/bindings/
+
+hcd21: f_usb20ho_hcd {
+ compatible = "fujitsu,f_usb20ho_hcd";
+ reg = <0 0x34200000 0x80000>;
+ interrupts = <0 419 0x4>;
+ clocks = <&clk_main_2_4>, <&clk_main_4_5>, <&clk_usb_0_0>;
+ clock-names = "h_clk", "p_clk", "p_cryclk";
+ #stream-id-cells = <0>;
+};
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index fafc628..9482140 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -786,6 +786,17 @@ config USB_HCD_SSB

If unsure, say N.

+config USB_F_USB20HO_HCD
+ tristate "F_USB20HO USB Host Controller"
+ depends on USB && ARCH_MB86S7X
+ default y
+ help
+ This driver enables support for USB20HO USB Host Controller,
+ the driver supports High, Full and Low Speed USB.
+
+ To compile this driver a module, choose M here: the module
+ will be called "f_usb20ho-hcd".
+
config USB_HCD_TEST_MODE
bool "HCD test mode support"
---help---
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index d6216a4..b89e536 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -78,3 +78,4 @@ obj-$(CONFIG_USB_HCD_SSB) += ssb-hcd.o
obj-$(CONFIG_USB_FUSBH200_HCD) += fusbh200-hcd.o
obj-$(CONFIG_USB_FOTG210_HCD) += fotg210-hcd.o
obj-$(CONFIG_USB_MAX3421_HCD) += max3421-hcd.o
+obj-$(CONFIG_USB_F_USB20HO_HCD)+= f_usb20ho_hcd.o
diff --git a/drivers/usb/host/f_usb20ho_hcd.c b/drivers/usb/host/f_usb20ho_hcd.c
new file mode 100644
index 0000000..d665487
--- /dev/null
+++ b/drivers/usb/host/f_usb20ho_hcd.c
@@ -0,0 +1,306 @@
+/**
+ * f_usb20ho_hcd.c - Fujitsu EHCI platform driver
+ *
+ * Copyright (c) 2013 - 2014 FUJITSU SEMICONDUCTOR LIMITED
+ * http://jp.fujitsu.com/group/fsl
+ *
+ * based on bcma-hcd.c
+ *
+ * Author: Sneeker Yeh <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <linux/usb/ehci_pdriver.h>
+#include <linux/usb/ohci_pdriver.h>
+#include <linux/dma-mapping.h>
+#include <linux/of.h>
+#include <linux/pm_runtime.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
+#include "f_usb20ho_hcd.h"
+
+static int f_usb20ho_clk_control(struct device *dev, bool on)
+{
+ int ret, i;
+ struct clk *clk;
+
+ if (!on)
+ goto clock_off;
+
+ for (i = 0;; i++) {
+ clk = of_clk_get(dev->of_node, i);
+ if (IS_ERR(clk))
+ break;
+
+ ret = clk_prepare_enable(clk);
+ if (ret) {
+ dev_err(dev, "failed to enable clock[%d]\n", i);
+ goto clock_off;
+ }
+ }
+
+ return 0;
+
+clock_off:
+ for (i = 0;; i++) {
+ clk = of_clk_get(dev->of_node, i);
+ if (IS_ERR(clk))
+ break;
+
+ clk_disable_unprepare(clk);
+ }
+
+ return on;
+}
+
+static struct platform_device *f_usb20ho_hcd_create_pdev(
+ struct platform_device *pdev, bool ohci)
+{
+ struct resource *resource;
+ struct platform_device *hci_dev;
+ struct resource hci_res[2];
+ int ret = -ENOMEM;
+ int irq;
+ resource_size_t resource_size;
+
+ memset(hci_res, 0, sizeof(hci_res));
+
+ resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!resource) {
+ dev_err(&pdev->dev, "%s() platform_get_resource() failed\n"
+ , __func__);
+ ret = -ENODEV;
+ goto err_res;
+ }
+ resource_size = resource->end - resource->start + 1;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev,
+ "%s() platform_get_irq() for F_USB20HO failed at %d\n",
+ __func__, irq);
+ ret = -ENODEV;
+ goto err_res;
+ }
+
+ hci_res[0].start = ohci ? resource->start + F_OHCI_OFFSET :
+ resource->start + F_EHCI_OFFSET;
+ hci_res[0].end = ohci ?
+ resource->start + F_OHCI_OFFSET + F_OHCI_SIZE - 1 :
+ resource->start + F_EHCI_OFFSET + F_EHCI_SIZE - 1;
+ hci_res[0].flags = IORESOURCE_MEM;
+
+ hci_res[1].start = irq;
+ hci_res[1].flags = IORESOURCE_IRQ;
+
+ hci_dev = platform_device_alloc(ohci ? "ohci-platform" :
+ "ehci-platform", 0);
+ if (!hci_dev) {
+ dev_err(&pdev->dev, "platform_device_alloc() failed\n");
+ ret = -ENODEV;
+ goto err_res;
+ }
+
+ dma_set_coherent_mask(&hci_dev->dev, pdev->dev.coherent_dma_mask);
+ hci_dev->dev.parent = &pdev->dev;
+ hci_dev->dev.dma_mask = pdev->dev.dma_mask;
+
+ ret = platform_device_add_resources(hci_dev, hci_res,
+ ARRAY_SIZE(hci_res));
+ if (ret) {
+ dev_err(&pdev->dev
+ , "platform_device_add_resources() failed\n");
+ goto err_alloc;
+ }
+
+ ret = platform_device_add(hci_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "platform_device_add() failed\n");
+ goto err_alloc;
+ }
+
+ return hci_dev;
+
+err_alloc:
+ platform_device_put(hci_dev);
+err_res:
+ return ERR_PTR(ret);
+}
+
+static u64 f_usb20ho_dma_mask = DMA_BIT_MASK(32);
+
+static int f_usb20ho_hcd_probe(struct platform_device *pdev)
+{
+ int err;
+ struct f_usb20ho_hcd *usb_dev;
+ struct device *dev = &pdev->dev;
+
+ dev->dma_mask = &f_usb20ho_dma_mask;
+ if (!dev->coherent_dma_mask)
+ dev->coherent_dma_mask = DMA_BIT_MASK(32);
+
+ usb_dev = kzalloc(sizeof(*usb_dev), GFP_KERNEL);
+ if (!usb_dev)
+ return -ENOMEM;
+
+ usb_dev->dev = &pdev->dev;
+ platform_set_drvdata(pdev, usb_dev);
+
+ usb_dev->irq = platform_get_irq(pdev, 0);
+ if (usb_dev->irq < 0) {
+ dev_err(&pdev->dev,
+ "%s() platform_get_irq() for F_USB20HO failed at %d\n",
+ __func__, usb_dev->irq);
+ err = -ENODEV;
+ goto err_free_usb_dev;
+ }
+ disable_irq(usb_dev->irq);
+
+ /* resume driver for clock, power, irq */
+ pm_runtime_enable(&pdev->dev);
+ err = pm_runtime_get_sync(&pdev->dev);
+ if (err < 0) {
+ dev_err(&pdev->dev, "get_sync failed with err %d\n", err);
+ goto err_unregister_ohci_dev;
+ }
+
+ usb_dev->ehci_dev = f_usb20ho_hcd_create_pdev(pdev, false);
+ if (IS_ERR(usb_dev->ehci_dev)) {
+ dev_err(&pdev->dev, "failed to create EHCI driver\n");
+ err = -ENODEV;
+ goto err_free_usb_dev;
+ }
+
+ usb_dev->ohci_dev = f_usb20ho_hcd_create_pdev(pdev, true);
+ if (IS_ERR(usb_dev->ohci_dev)) {
+ dev_err(&pdev->dev, "failed to create OHCI driver\n");
+ err = -ENODEV;
+ goto err_unregister_ehci_dev;
+ }
+
+ return 0;
+
+err_unregister_ohci_dev:
+ platform_device_unregister(usb_dev->ohci_dev);
+err_unregister_ehci_dev:
+ platform_device_unregister(usb_dev->ehci_dev);
+ pm_runtime_put_sync(&pdev->dev);
+err_free_usb_dev:
+ kfree(usb_dev);
+
+ return err;
+}
+
+static int f_usb20ho_hcd_remove(struct platform_device *pdev)
+{
+ struct f_usb20ho_hcd *usb_dev = platform_get_drvdata(pdev);
+ struct platform_device *ohci_dev = usb_dev->ohci_dev;
+ struct platform_device *ehci_dev = usb_dev->ehci_dev;
+
+ if (ohci_dev)
+ platform_device_unregister(ohci_dev);
+ if (ehci_dev)
+ platform_device_unregister(ehci_dev);
+
+ /* disable power,clock,irq */
+ pm_runtime_put_sync(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM
+#ifdef CONFIG_PM_RUNTIME
+static int f_usb20ho_runtime_suspend(struct device *dev)
+{
+ struct f_usb20ho_hcd *usb_dev = dev_get_drvdata(dev);
+
+ disable_irq(usb_dev->irq);
+ f_usb20ho_clk_control(dev, false);
+
+ return 0;
+}
+
+static int f_usb20ho_runtime_resume(struct device *dev)
+{
+ struct f_usb20ho_hcd *usb_dev = dev_get_drvdata(dev);
+
+ f_usb20ho_clk_control(dev, true);
+ enable_irq(usb_dev->irq);
+
+ return 0;
+}
+#endif /* CONFIG_PM_RUNTIME */
+
+static int f_usb20ho_hcd_suspend(struct device *dev)
+{
+ if (pm_runtime_status_suspended(dev))
+ return 0;
+
+ return f_usb20ho_runtime_suspend(dev);
+}
+
+static int f_usb20ho_hcd_resume(struct device *dev)
+{
+ if (pm_runtime_status_suspended(dev))
+ return 0;
+
+ return f_usb20ho_runtime_resume(dev);
+}
+
+static const struct dev_pm_ops f_usb20ho_hcd_ops = {
+ .suspend = f_usb20ho_hcd_suspend,
+ .resume = f_usb20ho_hcd_resume,
+ SET_RUNTIME_PM_OPS(f_usb20ho_runtime_suspend
+ , f_usb20ho_runtime_resume, NULL)
+};
+
+#define DEV_PM (&f_usb20ho_hcd_ops)
+#else /* !CONFIG_PM */
+#define DEV_PM NULL
+#endif /* CONFIG_PM */
+
+static void f_usb20ho_hcd_shutdown(struct platform_device *pdev)
+{
+#ifdef CONFIG_PM
+ f_usb20ho_hcd_suspend(&pdev->dev);
+#endif
+}
+
+static const struct of_device_id f_usb20ho_hcd_dt_ids[] = {
+ { .compatible = "fujitsu,f_usb20ho_hcd" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, f_usb20ho_hcd_dt_ids);
+
+static struct platform_driver f_usb20ho_hcd_driver = {
+ .probe = f_usb20ho_hcd_probe,
+ .remove = __exit_p(f_usb20ho_hcd_remove),
+ .shutdown = f_usb20ho_hcd_shutdown,
+ .driver = {
+ .name = "f_usb20ho_hcd",
+ .owner = THIS_MODULE,
+ .pm = DEV_PM,
+ .of_match_table = f_usb20ho_hcd_dt_ids,
+ },
+};
+
+module_platform_driver(f_usb20ho_hcd_driver);
+
+MODULE_ALIAS("platform:f_usb20ho_hcd");
+MODULE_AUTHOR("Sneeker Yeh <[email protected]>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("USB platform driver for f_usb20ho_lap IP ");
diff --git a/drivers/usb/host/f_usb20ho_hcd.h b/drivers/usb/host/f_usb20ho_hcd.h
new file mode 100644
index 0000000..046c636
--- /dev/null
+++ b/drivers/usb/host/f_usb20ho_hcd.h
@@ -0,0 +1,35 @@
+/*
+ * linux/drivers/usb/host/f_usb20ho_hcd.h - F_USB20HDC USB
+ * host controller driver
+ *
+ * Copyright (C) FUJITSU ELECTRONICS INC. 2011. All rights reserved.
+ * Copyright (C) 2013 - 2014 FUJITSU SEMICONDUCTOR LIMITED.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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.
+ */
+
+#ifndef _F_USB20HO_HCD_H
+#define _F_USB20HO_HCD_H
+
+#define F_EHCI_OFFSET 0x40000
+#define F_EHCI_SIZE 0x1000
+#define F_OHCI_OFFSET 0x41000
+#define F_OHCI_SIZE 0x1000
+#define F_OTHER_OFFSET 0x42000
+#define F_OTHER_SIZE 0x1000
+
+struct f_usb20ho_hcd {
+ struct device *dev;
+ struct platform_device *ehci_dev;
+ struct platform_device *ohci_dev;
+ int irq;
+};
+#endif
--
1.7.9.5

2014-12-16 02:11:32

by Sneeker Yeh

[permalink] [raw]
Subject: [PATCH 2/3] usb: dwc3: add Fujitsu Specific Glue layer

This patch adds support for Synopsis DesignWare USB3 IP Core found
on Fujitsu Socs.

Signed-off-by: Sneeker Yeh <[email protected]>
---
.../devicetree/bindings/usb/fujitsu-dwc3.txt | 25 +++
drivers/usb/dwc3/Kconfig | 11 ++
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/dwc3-mb86s70.c | 194 ++++++++++++++++++++
4 files changed, 231 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
create mode 100644 drivers/usb/dwc3/dwc3-mb86s70.c

diff --git a/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
new file mode 100644
index 0000000..df77f91
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
@@ -0,0 +1,25 @@
+FUJITSU GLUE COMPONENTS
+
+MB86S7x DWC3 GLUE
+ - compatible : Should be "fujitsu,mb86s70-dwc3"
+ - clocks: from common clock binding, handle to usb clock.
+ - clock-names: from common clock binding.
+ - #address-cells, #size-cells : Must be present if the device has sub-nodes
+ - ranges: the child address space are mapped 1:1 onto the parent address space
+ - #stream-id-cells : handle to use "arm,mmu-400" ARM IOMMU driver
+
+Sub-nodes:
+The dwc3 core should be added as subnode to MB86S7x dwc3 glue.
+- dwc3 :
+ The binding details of dwc3 can be found in:
+ Documentation/devicetree/bindings/usb/dwc3.txt
+
+usb3host: mb86s70_usb3host {
+ compatible = "fujitsu,mb86s70-dwc3";
+ clocks = <&clk_alw_1_1>;
+ clock-names = "bus_clk";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ ranges;
+ #stream-id-cells = <0>;
+};
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 58b5b2c..3390d42 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -61,6 +61,17 @@ config USB_DWC3_EXYNOS
Recent Exynos5 SoCs ship with one DesignWare Core USB3 IP inside,
say 'Y' or 'M' if you have one such device.

+config USB_DWC3_MB86S70
+ tristate "MB86S70 Designware USB3 Platform code"
+ default USB_DWC3
+ help
+ MB86S7X SOC ship with DesignWare Core USB3 IP inside,
+ this implementation also integrated Fujitsu USB PHY inside
+ this Core USB3 IP.
+
+ say 'Y' or 'M' if you have one such device.
+
+
config USB_DWC3_PCI
tristate "PCIe-based Platforms"
depends on PCI
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index bb34fbc..05d1de2 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -38,3 +38,4 @@ obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o
obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o
obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o
+obj-$(CONFIG_USB_DWC3_MB86S70) += dwc3-mb86s70.o
diff --git a/drivers/usb/dwc3/dwc3-mb86s70.c b/drivers/usb/dwc3/dwc3-mb86s70.c
new file mode 100644
index 0000000..241c5bd
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-mb86s70.c
@@ -0,0 +1,194 @@
+/**
+ * dwc3-mb86s70.c - Fujitsu mb86s70 DWC3 Specific Glue layer
+ *
+ * Copyright (c) 2013 - 2014 FUJITSU SEMICONDUCTOR LIMITED
+ * http://jp.fujitsu.com/group/fsl
+ *
+ * Author: Alice Chan <[email protected]>
+ * based on dwc3-exynos.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/clk.h>
+
+struct dwc3_mb86s70 {
+ struct device *dev;
+ struct clk **clk_table;
+};
+
+/* return 0 means successful */
+static int dwc3_mb86s70_clk_control(struct device *dev, bool on)
+{
+ int ret, i;
+ struct clk *clk;
+
+ if (!on)
+ goto clock_off;
+
+ for (i = 0;; i++) {
+ clk = of_clk_get(dev->of_node, i);
+ if (IS_ERR(clk))
+ break;
+
+ ret = clk_prepare_enable(clk);
+ if (ret) {
+ dev_err(dev, "failed to enable clock[%d]\n", i);
+ goto clock_off;
+ }
+ }
+
+ return 0;
+
+clock_off:
+ for (i = 0;; i++) {
+ clk = of_clk_get(dev->of_node, i);
+ if (IS_ERR(clk))
+ break;
+
+ clk_disable_unprepare(clk);
+ }
+
+ return on;
+}
+
+static int dwc3_mb86s70_remove_child(struct device *dev, void *unused)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+
+ of_device_unregister(pdev);
+
+ return 0;
+}
+
+static u64 dwc3_mb86s70_dma_mask = DMA_BIT_MASK(32);
+
+static int dwc3_mb86s70_probe(struct platform_device *pdev)
+{
+ struct dwc3_mb86s70 *mb86s70;
+ struct device *dev = &pdev->dev;
+ struct device_node *node = dev->of_node;
+ int ret;
+
+ mb86s70 = devm_kzalloc(dev, sizeof(*mb86s70), GFP_KERNEL);
+ if (!mb86s70) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ dev->dma_mask = &dwc3_mb86s70_dma_mask;
+
+ platform_set_drvdata(pdev, mb86s70);
+
+ mb86s70->dev = dev;
+
+ /* resume driver for clock, power */
+ pm_runtime_enable(&pdev->dev);
+ ret = pm_runtime_get_sync(&pdev->dev);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "get_sync failed with err %d\n", ret);
+ goto err;
+ }
+
+ if (node) {
+ ret = of_platform_populate(node, NULL, NULL, dev);
+ if (!ret)
+ return 0;
+ dev_err(dev, "failed to add dwc3 core\n");
+ }
+ dev_err(dev, "no device node, failed to add dwc3 core\n");
+ ret = -ENODEV;
+
+err:
+ return ret;
+}
+
+static int dwc3_mb86s70_remove(struct platform_device *pdev)
+{
+ device_for_each_child(&pdev->dev, NULL, dwc3_mb86s70_remove_child);
+
+ pm_runtime_put_sync(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+
+ return 0;
+}
+
+static const struct of_device_id mb86s70_dwc3_match[] = {
+ { .compatible = "fujitsu,mb86s70-dwc3" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mb86s70_dwc3_match);
+
+#ifdef CONFIG_PM
+#ifdef CONFIG_PM_RUNTIME
+static int dwc3_mb86s70_runtime_suspend(struct device *dev)
+{
+ dwc3_mb86s70_clk_control(dev, false);
+
+ return 0;
+}
+
+static int dwc3_mb86s70_runtime_resume(struct device *dev)
+{
+ dwc3_mb86s70_clk_control(dev, true);
+
+ return 0;
+}
+#endif /* CONFIG_PM_RUNTIME */
+static int dwc3_mb86s70_suspend(struct device *dev)
+{
+ if (pm_runtime_status_suspended(dev))
+ return 0;
+
+ return dwc3_mb86s70_runtime_suspend(dev);
+}
+
+static int dwc3_mb86s70_resume(struct device *dev)
+{
+ if (pm_runtime_status_suspended(dev))
+ return 0;
+
+ return dwc3_mb86s70_runtime_resume(dev);
+}
+
+static const struct dev_pm_ops dwc3_mb86s70_dev_pm_ops = {
+ .suspend = dwc3_mb86s70_suspend,
+ .resume = dwc3_mb86s70_resume,
+ SET_RUNTIME_PM_OPS(
+ dwc3_mb86s70_runtime_suspend,
+ dwc3_mb86s70_runtime_resume, NULL)
+};
+
+#define DEV_PM_OPS (&dwc3_mb86s70_dev_pm_ops)
+#else
+#define DEV_PM_OPS NULL
+#endif /* CONFIG_PM_SLEEP */
+
+static struct platform_driver dwc3_mb86s70_driver = {
+ .probe = dwc3_mb86s70_probe,
+ .remove = dwc3_mb86s70_remove,
+ .driver = {
+ .name = "mb86s70-dwc3",
+ .of_match_table = of_match_ptr(mb86s70_dwc3_match),
+ .pm = DEV_PM_OPS,
+ },
+};
+
+module_platform_driver(dwc3_mb86s70_driver);
+
+MODULE_ALIAS("platform:mb86s70-dwc3");
+MODULE_AUTHOR("Alice Chan <[email protected]>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("DesignWare USB3 mb86s70 Glue Layer");
--
1.7.9.5

2014-12-16 02:11:39

by Sneeker Yeh

[permalink] [raw]
Subject: [PATCH 3/3] usb: dwc3: add a quirk for device disconnection issue in Synopsis dwc3 core

Synopsis DesignWare USB3 IP Core integrated with a config-free
phy needs special handling during device disconnection to avoid
the host controller dying.

This quirk makes sure PORT_CSC is cleared after the disable slot
command when usb device is disconnected from internal root hub,
otherwise, Synopsis core would fall into a state that cannot use
any endpoint command. Consequently, device disconnection procedure
might not be finished because sometimes endpoint need to be stop
by endpoint stop command issuing.

Symptom usually happens when disconnected device is keyboard,
mouse, and hub.

Signed-off-by: Sneeker Yeh <[email protected]>
---
drivers/usb/host/xhci-hub.c | 4 ++++
drivers/usb/host/xhci-plat.c | 5 +++++
drivers/usb/host/xhci.c | 29 +++++++++++++++++++++++++++++
drivers/usb/host/xhci.h | 7 +++++++
4 files changed, 45 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a7865c4..3b8f7fc 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -368,6 +368,10 @@ static void xhci_clear_port_change_bit(struct xhci_hcd *xhci, u16 wValue,
port_change_bit = "warm(BH) reset";
break;
case USB_PORT_FEAT_C_CONNECTION:
+ if ((xhci->quirks & XHCI_DISCONNECT_QUIRK) &&
+ !(readl(addr) & PORT_CONNECT))
+ return;
+
status = PORT_CSC;
port_change_bit = "connect";
break;
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 08d402b..3ede6b4 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -33,6 +33,11 @@ static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
* dev struct in order to setup MSI
*/
xhci->quirks |= XHCI_PLAT;
+
+ if (dev->parent && dev->parent->parent &&
+ of_device_is_compatible(dev->parent->parent->of_node,
+ "fujitsu,mb86s70-dwc3"))
+ xhci->quirks |= XHCI_DISCONNECT_QUIRK;
}

/* called during probe() after chip reset completes */
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 01fcbb5..50d757b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3584,6 +3584,33 @@ command_cleanup:
return ret;
}

+static void xhci_try_to_clear_csc(struct usb_hcd *hcd, int dev_port_num)
+{
+ int max_ports;
+ struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ __le32 __iomem **port_array;
+ u32 status;
+
+ /* print debug info */
+ if (hcd->speed == HCD_USB3) {
+ max_ports = xhci->num_usb3_ports;
+ port_array = xhci->usb3_ports;
+ } else {
+ max_ports = xhci->num_usb2_ports;
+ port_array = xhci->usb2_ports;
+ }
+
+ if (dev_port_num > max_ports) {
+ xhci_err(xhci, "%s() port number invalid", __func__);
+ return;
+ }
+ status = readl(port_array[dev_port_num - 1]);
+
+ /* write 1 to clear */
+ if (!(status & PORT_CONNECT) && (status & PORT_CSC))
+ writel(status & PORT_CSC, port_array[0]);
+}
+
/*
* At this point, the struct usb_device is about to go away, the device has
* disconnected, and all traffic has been stopped and the endpoints have been
@@ -3649,6 +3676,8 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);

+ if (xhci->quirks & XHCI_DISCONNECT_QUIRK)
+ xhci_try_to_clear_csc(hcd, udev->portnum);
/*
* Event command completion handler will free any data structures
* associated with the slot. XXX Can free sleep?
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index cc7c5bb..e6c820c 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1560,6 +1560,13 @@ struct xhci_hcd {
#define XHCI_SPURIOUS_WAKEUP (1 << 18)
/* For controllers with a broken beyond repair streams implementation */
#define XHCI_BROKEN_STREAMS (1 << 19)
+/*
+ * Synopsis dwc3 core integrated with a config-free phy needs special
+ * handling during device disconnection to avoid the host controller
+ * dying. This quirk makes sure PORT_CSC is cleared after the disable
+ * slot command.
+ */
+#define XHCI_DISCONNECT_QUIRK (1 << 20)
unsigned int num_active_eps;
unsigned int limit_active_eps;
/* There are two roothubs to keep track of bus suspend info for */
--
1.7.9.5

2014-12-16 09:06:45

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 1/3] usb: host: f_usb20ho: add support for Fujitsu ehci/ohci USB 2.0 host controller

On Tuesday 16 December 2014 10:10:26 Sneeker Yeh wrote:
> This patch adds support for EHCI compliant Host controller found
> on Fujitsu Socs.
>
> Signed-off-by: Sneeker Yeh <[email protected]>
> ---
> .../devicetree/bindings/usb/fujitsu-ehci.txt | 22 ++
> drivers/usb/host/Kconfig | 11 +
> drivers/usb/host/Makefile | 1 +
> drivers/usb/host/f_usb20ho_hcd.c | 306 ++++++++++++++++++++
> drivers/usb/host/f_usb20ho_hcd.h | 35 +++
> 5 files changed, 375 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-ehci.txt
> create mode 100644 drivers/usb/host/f_usb20ho_hcd.c
> create mode 100644 drivers/usb/host/f_usb20ho_hcd.h
>
> diff --git a/Documentation/devicetree/bindings/usb/fujitsu-ehci.txt b/Documentation/devicetree/bindings/usb/fujitsu-ehci.txt
> new file mode 100644
> index 0000000..e180860
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/fujitsu-ehci.txt
> @@ -0,0 +1,22 @@
> +FUJITSU GLUE COMPONENTS
> +
> +MB86S7x EHCI GLUE
> + - compatible : Should be "fujitsu,f_usb20ho_hcd"

Please try to use the binding from

Documentation/devicetree/bindings/usb/usb-ehci.txt and the respective
ohci binding first, and use two separate devic enodes.

> + - reg : Address and length of the register set for the device.
> + - interrupts : The irq number of this device that is used to interrupt the
> + CPU
> + - clocks: from common clock binding, handle to usb clock.
> + - clock-names: from common clock binding.

You should always document the specific strings for a named property.

> + - #stream-id-cells : handle to use "arm,mmu-400" ARM IOMMU driver

Don't use that binding, we are trying to kill that off. Instead, use
an 'iommus' property.


> + hci_res[0].start = ohci ? resource->start + F_OHCI_OFFSET :
> + resource->start + F_EHCI_OFFSET;
> + hci_res[0].end = ohci ?
> + resource->start + F_OHCI_OFFSET + F_OHCI_SIZE - 1 :
> + resource->start + F_EHCI_OFFSET + F_EHCI_SIZE - 1;
> + hci_res[0].flags = IORESOURCE_MEM;
> +
> + hci_res[1].start = irq;
> + hci_res[1].flags = IORESOURCE_IRQ;
> +
> + hci_dev = platform_device_alloc(ohci ? "ohci-platform" :
> + "ehci-platform", 0);
> + if (!hci_dev) {
> + dev_err(&pdev->dev, "platform_device_alloc() failed\n");
> + ret = -ENODEV;
> + goto err_res;
> + }

No need for playing games with child devices, just see how the other
drivers do it.

> + ret = platform_device_add(hci_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "platform_device_add() failed\n");
> + goto err_alloc;
> + }
> +
> + return hci_dev;
> +
> +err_alloc:
> + platform_device_put(hci_dev);
> +err_res:
> + return ERR_PTR(ret);
> +}
> +
> +static u64 f_usb20ho_dma_mask = DMA_BIT_MASK(32);

The dma mask should come from the dma-ranges property of the parent bus,
as of_platform_populate now does.

Arnd

2014-12-17 15:44:49

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 1/3] usb: host: f_usb20ho: add support for Fujitsu ehci/ohci USB 2.0 host controller

On Wednesday 17 December 2014 23:33:02 Sneeker. Yeh wrote:
> >
> >No need for playing games with child devices, just see how the other drivers do
> >it.
>
> I am wondering if this is what you mentioned here? :
> Hcd21: f_usb20ho {
> compatible = "fujitsu,f_usb20ho";
> ...
> Ehci {
> compatible = "generic-ehci";
> reg = <>;
> }
> ohci {
> compatible = "generic-ohci";
> reg = <>;
> }
> }
> By doing that, I can just use of_platform_populate() to create platform device more gracefully,
> than using platform_device_xxx() those have been done in of_platform_populate().
> And also, managing register offset is totally done via dts description, not in the code.

What I mean is leaving out the fujitsu,f_usb20ho node entirely, just put the
two USB controllers on the bus directly and remove this driver.

Arnd

2014-12-22 15:38:41

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 3/3] usb: dwc3: add a quirk for device disconnection issue in Synopsis dwc3 core

On Tue, Dec 16, 2014 at 10:10:28AM +0800, Sneeker Yeh wrote:
> Synopsis DesignWare USB3 IP Core integrated with a config-free
> phy needs special handling during device disconnection to avoid
> the host controller dying.
>
> This quirk makes sure PORT_CSC is cleared after the disable slot
> command when usb device is disconnected from internal root hub,
> otherwise, Synopsis core would fall into a state that cannot use
> any endpoint command. Consequently, device disconnection procedure
> might not be finished because sometimes endpoint need to be stop
> by endpoint stop command issuing.
>
> Symptom usually happens when disconnected device is keyboard,
> mouse, and hub.

you need to point us to the synopsys STARS ticket number. That's the
only way to reference this specific quirk. Add it to a comment
somewhere.

--
balbi


Attachments:
(No filename) (836.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-12-22 16:00:39

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 2/3] usb: dwc3: add Fujitsu Specific Glue layer

Hi,

On Tue, Dec 16, 2014 at 10:10:27AM +0800, Sneeker Yeh wrote:
> This patch adds support for Synopsis DesignWare USB3 IP Core found
> on Fujitsu Socs.
>
> Signed-off-by: Sneeker Yeh <[email protected]>
> ---
> .../devicetree/bindings/usb/fujitsu-dwc3.txt | 25 +++
> drivers/usb/dwc3/Kconfig | 11 ++
> drivers/usb/dwc3/Makefile | 1 +
> drivers/usb/dwc3/dwc3-mb86s70.c | 194 ++++++++++++++++++++
> 4 files changed, 231 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
> create mode 100644 drivers/usb/dwc3/dwc3-mb86s70.c
>
> diff --git a/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
> new file mode 100644
> index 0000000..df77f91
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
> @@ -0,0 +1,25 @@
> +FUJITSU GLUE COMPONENTS
> +
> +MB86S7x DWC3 GLUE
> + - compatible : Should be "fujitsu,mb86s70-dwc3"
> + - clocks: from common clock binding, handle to usb clock.
> + - clock-names: from common clock binding.
> + - #address-cells, #size-cells : Must be present if the device has sub-nodes
> + - ranges: the child address space are mapped 1:1 onto the parent address space
> + - #stream-id-cells : handle to use "arm,mmu-400" ARM IOMMU driver
> +
> +Sub-nodes:
> +The dwc3 core should be added as subnode to MB86S7x dwc3 glue.
> +- dwc3 :
> + The binding details of dwc3 can be found in:
> + Documentation/devicetree/bindings/usb/dwc3.txt
> +
> +usb3host: mb86s70_usb3host {
> + compatible = "fujitsu,mb86s70-dwc3";
> + clocks = <&clk_alw_1_1>;
> + clock-names = "bus_clk";
> + #address-cells = <2>;
> + #size-cells = <1>;
> + ranges;
> + #stream-id-cells = <0>;
> +};
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 58b5b2c..3390d42 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -61,6 +61,17 @@ config USB_DWC3_EXYNOS
> Recent Exynos5 SoCs ship with one DesignWare Core USB3 IP inside,
> say 'Y' or 'M' if you have one such device.
>
> +config USB_DWC3_MB86S70
> + tristate "MB86S70 Designware USB3 Platform code"
> + default USB_DWC3
> + help
> + MB86S7X SOC ship with DesignWare Core USB3 IP inside,
> + this implementation also integrated Fujitsu USB PHY inside
> + this Core USB3 IP.
> +
> + say 'Y' or 'M' if you have one such device.
> +
> +
> config USB_DWC3_PCI
> tristate "PCIe-based Platforms"
> depends on PCI
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index bb34fbc..05d1de2 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -38,3 +38,4 @@ obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o
> obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o
> obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
> obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o
> +obj-$(CONFIG_USB_DWC3_MB86S70) += dwc3-mb86s70.o
> diff --git a/drivers/usb/dwc3/dwc3-mb86s70.c b/drivers/usb/dwc3/dwc3-mb86s70.c
> new file mode 100644
> index 0000000..241c5bd
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-mb86s70.c
> @@ -0,0 +1,194 @@
> +/**
> + * dwc3-mb86s70.c - Fujitsu mb86s70 DWC3 Specific Glue layer
> + *
> + * Copyright (c) 2013 - 2014 FUJITSU SEMICONDUCTOR LIMITED
> + * http://jp.fujitsu.com/group/fsl
> + *
> + * Author: Alice Chan <[email protected]>
> + * based on dwc3-exynos.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/clk.h>
> +
> +struct dwc3_mb86s70 {
> + struct device *dev;
> + struct clk **clk_table;

please align these.

> +};
> +
> +/* return 0 means successful */

this comment is unnecessary :-)

> +static int dwc3_mb86s70_clk_control(struct device *dev, bool on)
> +{
> + int ret, i;
> + struct clk *clk;
> +
> + if (!on)
> + goto clock_off;
> +
> + for (i = 0;; i++) {
> + clk = of_clk_get(dev->of_node, i);

you could count the number of properties first, then allocate clk_table
to the exact size.

> + if (IS_ERR(clk))
> + break;
> +
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_err(dev, "failed to enable clock[%d]\n", i);
> + goto clock_off;
> + }
> + }
> +
> + return 0;
> +
> +clock_off:
> + for (i = 0;; i++) {
> + clk = of_clk_get(dev->of_node, i);
> + if (IS_ERR(clk))
> + break;
> +
> + clk_disable_unprepare(clk);
> + }
> +
> + return on;
> +}
> +
> +static int dwc3_mb86s70_remove_child(struct device *dev, void *unused)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> +
> + of_device_unregister(pdev);
> +
> + return 0;
> +}
> +
> +static u64 dwc3_mb86s70_dma_mask = DMA_BIT_MASK(32);

why ? Use dma_coerce_mask_and_coherent().

> +
> +static int dwc3_mb86s70_probe(struct platform_device *pdev)
> +{
> + struct dwc3_mb86s70 *mb86s70;
> + struct device *dev = &pdev->dev;
> + struct device_node *node = dev->of_node;
> + int ret;
> +
> + mb86s70 = devm_kzalloc(dev, sizeof(*mb86s70), GFP_KERNEL);
> + if (!mb86s70) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + dev->dma_mask = &dwc3_mb86s70_dma_mask;
> +
> + platform_set_drvdata(pdev, mb86s70);
> +
> + mb86s70->dev = dev;
> +
> + /* resume driver for clock, power */
> + pm_runtime_enable(&pdev->dev);
> + ret = pm_runtime_get_sync(&pdev->dev);

here's a slightly better way (add error checking where necessary):

dwc3_mb86s70_clk_control(dev, true);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
pm_runtime_get(dev);


> + if (ret < 0) {
> + dev_err(&pdev->dev, "get_sync failed with err %d\n", ret);
> + goto err;

if get_sync() fails, you still need to call pm_runtime_put_sync(); In
this case, you also need to call pm_runtime_disable();

> + }
> +
> + if (node) {
> + ret = of_platform_populate(node, NULL, NULL, dev);
> + if (!ret)
> + return 0;
> + dev_err(dev, "failed to add dwc3 core\n");
> + }
> + dev_err(dev, "no device node, failed to add dwc3 core\n");
> + ret = -ENODEV;
> +
> +err:
> + return ret;
> +}
> +
> +static int dwc3_mb86s70_remove(struct platform_device *pdev)
> +{
> + device_for_each_child(&pdev->dev, NULL, dwc3_mb86s70_remove_child);
> +
> + pm_runtime_put_sync(&pdev->dev);
> + pm_runtime_disable(&pdev->dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mb86s70_dwc3_match[] = {
> + { .compatible = "fujitsu,mb86s70-dwc3" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mb86s70_dwc3_match);
> +
> +#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_RUNTIME
> +static int dwc3_mb86s70_runtime_suspend(struct device *dev)
> +{
> + dwc3_mb86s70_clk_control(dev, false);
> +
> + return 0;
> +}
> +
> +static int dwc3_mb86s70_runtime_resume(struct device *dev)
> +{
> + dwc3_mb86s70_clk_control(dev, true);
> +
> + return 0;
> +}
> +#endif /* CONFIG_PM_RUNTIME */
> +static int dwc3_mb86s70_suspend(struct device *dev)
> +{
> + if (pm_runtime_status_suspended(dev))
> + return 0;
> +
> + return dwc3_mb86s70_runtime_suspend(dev);

ok, this is a bit confusing and, in fact, wrong. You're not dealing with
wakeups (which are mandatory on runtime_pm and option on system sleep).

It's okay to have a common function called by both runtime_pm and
system_sleep methods, but calling your runtime_pm handler is a bit odd.

last but not least, I need to see results of this running on top of
v3.19-rc1. Are you using both host and peripheral modes or only one mode
for now ?

--
balbi


Attachments:
(No filename) (7.69 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-12-29 16:07:59

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 3/3] usb: dwc3: add a quirk for device disconnection issue in Synopsis dwc3 core

Hi,

On Mon, Dec 29, 2014 at 04:07:50PM +0800, Sneeker Yeh wrote:
> Hi,
>
> 2014-12-29 14:41 GMT+08:00 Sneeker Yeh <[email protected]>:
>
> > Hi,
> >
> > 2014-12-22 23:37 GMT+08:00 Felipe Balbi <[email protected]>:
> >
> >> On Tue, Dec 16, 2014 at 10:10:28AM +0800, Sneeker Yeh wrote:
> >> > Synopsis DesignWare USB3 IP Core integrated with a config-free
> >> > phy needs special handling during device disconnection to avoid
> >> > the host controller dying.
> >> >
> >> > This quirk makes sure PORT_CSC is cleared after the disable slot
> >> > command when usb device is disconnected from internal root hub,
> >> > otherwise, Synopsis core would fall into a state that cannot use
> >> > any endpoint command. Consequently, device disconnection procedure
> >> > might not be finished because sometimes endpoint need to be stop
> >> > by endpoint stop command issuing.
> >> >
> >> > Symptom usually happens when disconnected device is keyboard,
> >> > mouse, and hub.
> >>
> >> you need to point us to the synopsys STARS ticket number. That's the
> >> only way to reference this specific quirk. Add it to a comment
> >> somewhere.
> >>
> >
> > Thanks, we're still waiting for Synopsis STARS ticket number.
> > I'll add it to comment later.
> >
> >
> Sorry I didn't express myself clearly.
>
> So far Fujitsu Semiconductor got Synopsys internal case id , that is "
> Case: 8000679552".
> However the contents belongs this id cannot be referred except Fujitsu
> Semiconductor and Synopsys.
> Synopsis decide the official errata (STAR information) will be disclosed
> next February.
>
> Would you suggest if this quirk can be accepted with this case ID, or can
> only be accepted with STARS ticket number?

The thing is that without the STARS number I can't really verify which
versions of the IP would be affected. Clearly, it's not only your setup
and I think it's pretty unfair to have "is_compatible("fujitsu")" to
enable the quirk only for you. Isn't there a better way of enabling the
quirk based off of revision detection couple with a look on GHWPARAMS*
registers ?

What's tricking me is this claim that only config-free PHYs would be
affected, why ?

I still have many questions about this patch which are not answered by
commit log nor can I poke around on Synopsys solvnet for answers :-s

--
balbi


Attachments:
(No filename) (2.26 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-12-29 16:15:46

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 2/3] usb: dwc3: add Fujitsu Specific Glue layer

Hi,

On Mon, Dec 29, 2014 at 01:52:04AM +0800, Sneeker Yeh wrote:
> Hi,
>
> 2014-12-22 23:59 GMT+08:00 Felipe Balbi <[email protected]>:
>
> > Hi,
> >
> > On Tue, Dec 16, 2014 at 10:10:27AM +0800, Sneeker Yeh wrote:
> > > This patch adds support for Synopsis DesignWare USB3 IP Core found
> > > on Fujitsu Socs.
> > >
> > > Signed-off-by: Sneeker Yeh <[email protected]>
> > > ---
> > > .../devicetree/bindings/usb/fujitsu-dwc3.txt | 25 +++
> > > drivers/usb/dwc3/Kconfig | 11 ++
> > > drivers/usb/dwc3/Makefile | 1 +
> > > drivers/usb/dwc3/dwc3-mb86s70.c | 194
> > ++++++++++++++++++++
> > > 4 files changed, 231 insertions(+)
> > > create mode 100644
> > Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
> > > create mode 100644 drivers/usb/dwc3/dwc3-mb86s70.c
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
> > b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
> > > new file mode 100644
> > > index 0000000..df77f91
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
> > > @@ -0,0 +1,25 @@
> > > +FUJITSU GLUE COMPONENTS
> > > +
> > > +MB86S7x DWC3 GLUE
> > > + - compatible : Should be "fujitsu,mb86s70-dwc3"
> > > + - clocks: from common clock binding, handle to usb clock.
> > > + - clock-names: from common clock binding.
> > > + - #address-cells, #size-cells : Must be present if the device has
> > sub-nodes
> > > + - ranges: the child address space are mapped 1:1 onto the parent
> > address space
> > > + - #stream-id-cells : handle to use "arm,mmu-400" ARM IOMMU driver
> > > +
> > > +Sub-nodes:
> > > +The dwc3 core should be added as subnode to MB86S7x dwc3 glue.
> > > +- dwc3 :
> > > + The binding details of dwc3 can be found in:
> > > + Documentation/devicetree/bindings/usb/dwc3.txt
> > > +
> > > +usb3host: mb86s70_usb3host {
> > > + compatible = "fujitsu,mb86s70-dwc3";
> > > + clocks = <&clk_alw_1_1>;
> > > + clock-names = "bus_clk";
> > > + #address-cells = <2>;
> > > + #size-cells = <1>;
> > > + ranges;
> > > + #stream-id-cells = <0>;
> > > +};
> > > diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> > > index 58b5b2c..3390d42 100644
> > > --- a/drivers/usb/dwc3/Kconfig
> > > +++ b/drivers/usb/dwc3/Kconfig
> > > @@ -61,6 +61,17 @@ config USB_DWC3_EXYNOS
> > > Recent Exynos5 SoCs ship with one DesignWare Core USB3 IP inside,
> > > say 'Y' or 'M' if you have one such device.
> > >
> > > +config USB_DWC3_MB86S70
> > > + tristate "MB86S70 Designware USB3 Platform code"
> > > + default USB_DWC3
> > > + help
> > > + MB86S7X SOC ship with DesignWare Core USB3 IP inside,
> > > + this implementation also integrated Fujitsu USB PHY inside
> > > + this Core USB3 IP.
> > > +
> > > + say 'Y' or 'M' if you have one such device.
> > > +
> > > +
> > > config USB_DWC3_PCI
> > > tristate "PCIe-based Platforms"
> > > depends on PCI
> > > diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> > > index bb34fbc..05d1de2 100644
> > > --- a/drivers/usb/dwc3/Makefile
> > > +++ b/drivers/usb/dwc3/Makefile
> > > @@ -38,3 +38,4 @@ obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o
> > > obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o
> > > obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
> > > obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o
> > > +obj-$(CONFIG_USB_DWC3_MB86S70) += dwc3-mb86s70.o
> > > diff --git a/drivers/usb/dwc3/dwc3-mb86s70.c
> > b/drivers/usb/dwc3/dwc3-mb86s70.c
> > > new file mode 100644
> > > index 0000000..241c5bd
> > > --- /dev/null
> > > +++ b/drivers/usb/dwc3/dwc3-mb86s70.c
> > > @@ -0,0 +1,194 @@
> > > +/**
> > > + * dwc3-mb86s70.c - Fujitsu mb86s70 DWC3 Specific Glue layer
> > > + *
> > > + * Copyright (c) 2013 - 2014 FUJITSU SEMICONDUCTOR LIMITED
> > > + * http://jp.fujitsu.com/group/fsl
> > > + *
> > > + * Author: Alice Chan <[email protected]>
> > > + * based on dwc3-exynos.c
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + */
> > > +
> > > +#include <linux/module.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/dma-mapping.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/of.h>
> > > +#include <linux/of_platform.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/clk.h>
> > > +
> > > +struct dwc3_mb86s70 {
> > > + struct device *dev;
> > > + struct clk **clk_table;
> >
> > please align these.
>
>
> okay.
>
>
> >
>
>
> > > +};
> > > +
> > > +/* return 0 means successful */
> >
> > this comment is unnecessary :-)
> >
>
> okay
>
>
> >
> > > +static int dwc3_mb86s70_clk_control(struct device *dev, bool on)
> > > +{
> > > + int ret, i;
> > > + struct clk *clk;
> > > +
> > > + if (!on)
> > > + goto clock_off;
> > > +
> > > + for (i = 0;; i++) {
> > > + clk = of_clk_get(dev->of_node, i);
> >
> > you could count the number of properties first, then allocate clk_table
> > to the exact size.
> >
>
> okay, thanks.
>
>
> >
> > > + if (IS_ERR(clk))
> > > + break;
> > > +
> > > + ret = clk_prepare_enable(clk);
> > > + if (ret) {
> > > + dev_err(dev, "failed to enable clock[%d]\n", i);
> > > + goto clock_off;
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +clock_off:
> > > + for (i = 0;; i++) {
> > > + clk = of_clk_get(dev->of_node, i);
> > > + if (IS_ERR(clk))
> > > + break;
> > > +
> > > + clk_disable_unprepare(clk);
> > > + }
> > > +
> > > + return on;
> > > +}
> > > +
> > > +static int dwc3_mb86s70_remove_child(struct device *dev, void *unused)
> > > +{
> > > + struct platform_device *pdev = to_platform_device(dev);
> > > +
> > > + of_device_unregister(pdev);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static u64 dwc3_mb86s70_dma_mask = DMA_BIT_MASK(32);
> >
> > why ? Use dma_coerce_mask_and_coherent().
> >
>
> okay.
>
>
> >
> > > +
> > > +static int dwc3_mb86s70_probe(struct platform_device *pdev)
> > > +{
> > > + struct dwc3_mb86s70 *mb86s70;
> > > + struct device *dev = &pdev->dev;
> > > + struct device_node *node = dev->of_node;
> > > + int ret;
> > > +
> > > + mb86s70 = devm_kzalloc(dev, sizeof(*mb86s70), GFP_KERNEL);
> > > + if (!mb86s70) {
> > > + ret = -ENOMEM;
> > > + goto err;
> > > + }
> > > +
> > > + dev->dma_mask = &dwc3_mb86s70_dma_mask;
> > > +
> > > + platform_set_drvdata(pdev, mb86s70);
> > > +
> > > + mb86s70->dev = dev;
> > > +
> > > + /* resume driver for clock, power */
> > > + pm_runtime_enable(&pdev->dev);
> > > + ret = pm_runtime_get_sync(&pdev->dev);
> >
> > here's a slightly better way (add error checking where necessary):
> >
> > dwc3_mb86s70_clk_control(dev, true);
> > pm_runtime_set_active(dev);
> > pm_runtime_enable(dev);
> > pm_runtime_get(dev);
> >
> >
> okay, thanks,
> these seems much better and more correct.
>
>
>
> >
> > > + if (ret < 0) {
> > > + dev_err(&pdev->dev, "get_sync failed with err %d\n", ret);
> > > + goto err;
> >
> > if get_sync() fails, you still need to call pm_runtime_put_sync(); In
> > this case, you also need to call pm_runtime_disable();
> >
>
> okay, thanks.
>
>
> >
> > > + }
> > > +
> > > + if (node) {
> > > + ret = of_platform_populate(node, NULL, NULL, dev);
> > > + if (!ret)
> > > + return 0;
> > > + dev_err(dev, "failed to add dwc3 core\n");
> > > + }
> > > + dev_err(dev, "no device node, failed to add dwc3 core\n");
> > > + ret = -ENODEV;
> > > +
> > > +err:
> > > + return ret;
> > > +}
> > > +
> > > +static int dwc3_mb86s70_remove(struct platform_device *pdev)
> > > +{
> > > + device_for_each_child(&pdev->dev, NULL, dwc3_mb86s70_remove_child);
> > > +
> > > + pm_runtime_put_sync(&pdev->dev);
> > > + pm_runtime_disable(&pdev->dev);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id mb86s70_dwc3_match[] = {
> > > + { .compatible = "fujitsu,mb86s70-dwc3" },
> > > + {},
> > > +};
> > > +MODULE_DEVICE_TABLE(of, mb86s70_dwc3_match);
> > > +
> > > +#ifdef CONFIG_PM
> > > +#ifdef CONFIG_PM_RUNTIME
> > > +static int dwc3_mb86s70_runtime_suspend(struct device *dev)
> > > +{
> > > + dwc3_mb86s70_clk_control(dev, false);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int dwc3_mb86s70_runtime_resume(struct device *dev)
> > > +{
> > > + dwc3_mb86s70_clk_control(dev, true);
> > > +
> > > + return 0;
> > > +}
> > > +#endif /* CONFIG_PM_RUNTIME */
> > > +static int dwc3_mb86s70_suspend(struct device *dev)
> > > +{
> > > + if (pm_runtime_status_suspended(dev))
> > > + return 0;
> > > +
> > > + return dwc3_mb86s70_runtime_suspend(dev);
> >
> > ok, this is a bit confusing and, in fact, wrong. You're not dealing with
> > wakeups (which are mandatory on runtime_pm and option on system sleep).
> >
> > It's okay to have a common function called by both runtime_pm and
> > system_sleep methods, but calling your runtime_pm handler is a bit odd.
> >
>
> hm, sorry I might confuse people here.
> I'll change this to what you suggested.
>
>
> >
> > last but not least, I need to see results of this running on top of
> > v3.19-rc1. Are you using both host and peripheral modes or only one mode
> > for now ?
> >
>
> The dwc3 IP we adapted is host-only, and we run this platform glue driver
> in v3.19-rc1 for now.
> bellow log is about suspend and resume successfully, and dwc3 still works.
>
> root@linaro-nano:~# echo mem > /sys/power/state
> [ 56.713281] mb86s70-dwc3 mb86s70_usb3host1.5: dwc3_mb86s70_suspend() is
> started.
> [ 56.720721] mb86s70-dwc3 mb86s70_usb3host1.5:
> dwc3_mb86s70_runtime_suspend() is started.
> [ 56.729419] mb86s70-dwc3 mb86s70_usb3host1.5:
> dwc3_mb86s70_runtime_suspend() is ended.
> [ 56.737363] mb86s70-dwc3 mb86s70_usb3host1.5: dwc3_mb86s70_suspend() is
> ended.
> [ 56.744604] mb86s70-dwc3 mb86s70_usb3host0.4: dwc3_mb86s70_suspend() is
> started.
> [ 56.752022] mb86s70-dwc3 mb86s70_usb3host0.4:
> dwc3_mb86s70_runtime_suspend() is started.
> [ 56.760700] mb86s70-dwc3 mb86s70_usb3host0.4:
> dwc3_mb86s70_runtime_suspend() is ended.
> [ 56.768639] mb86s70-dwc3 mb86s70_usb3host0.4: dwc3_mb86s70_suspend() is
> ended.
> [ 57.107139] SUSPEND
>
> [ 74.564963] mmc0: execute_tuning --
> [ 74.877776] mb86s70-dwc3 mb86s70_usb3host0.4: dwc3_mb86s70_resume() is
> started.
> [ 74.885126] mb86s70-dwc3 mb86s70_usb3host0.4:
> dwc3_mb86s70_runtime_resume() is started.
> [ 74.893749] mb86s70-dwc3 mb86s70_usb3host0.4:
> dwc3_mb86s70_runtime_resume() is ended.
> [ 74.901606] mb86s70-dwc3 mb86s70_usb3host0.4: dwc3_mb86s70_resume() is
> ended.
> [ 74.908766] mb86s70-dwc3 mb86s70_usb3host1.5: dwc3_mb86s70_resume() is
> started.
> [ 74.916086] mb86s70-dwc3 mb86s70_usb3host1.5:
> dwc3_mb86s70_runtime_resume() is started.
> [ 74.924660] mb86s70-dwc3 mb86s70_usb3host1.5:
> dwc3_mb86s70_runtime_resume() is ended.
> [ 74.932513] mb86s70-dwc3 mb86s70_usb3host1.5: dwc3_mb86s70_resume() is
> ended.
> root@linaro-nano:~#
>
> But I realize something might confuse people, and even is wrong, through
> your comment.
> So I guess I should align these to a more clear form like dwc3-exynos.c :
>
> static int dwc3_exynos_suspend(struct device *dev)
> {
> struct dwc3_exynos *exynos = dev_get_drvdata(dev);
>
> clk_disable(exynos->clk);
>
> return 0;
> }
>
> static int dwc3_exynos_resume(struct device *dev)
> {
> struct dwc3_exynos *exynos = dev_get_drvdata(dev);
>
> clk_enable(exynos->clk);
>
> /* runtime set active to reflect active state. */
> pm_runtime_disable(dev);
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
>
> return 0;
> }
>
> Would it been better than how I do suspend/resume here?

sure, that would be better :-)

> however recently I got some thought inspired by Arnd and the commit
> 5f872175c867a839e2feeecfa59e3db38c88b561

where is that commit ?

$ git show 5f872175c867a839e2feeecfa59e3db38c88b561
fatal: bad object 5f872175c867a839e2feeecfa59e3db38c88b561

I have here linus and linux-next just fetched before trying to git show.

> For imitating that in my experiment recently, i only need to add clock
> on/off in dwc3 core.

Let's wait a little bit more before doing that. We have a ton of users
which don't have controllable clocks and, frankly speaking, if we hide
all clock control on glue layer (which is a parent device of core) it
will still work just fine because of the ordering of children vs parent
that device core gives you for free :-)

> Then dwc3 core won't be always created via sub node in platform glue node,
> and vendors like us can just drop platform glue which don't have any
> specific platform code to wrap dwc3 core, and just directly use dwc3 core
> via device tree.

If your core is really just xhci, why don't just use xhci-plat.c
directly ? You don't need dwc3 at all. In any case, I refrain from
allowing people to use dwc3.ko directly because that has bitten me in
the past with MUSB, so sorry but I'll always force people to use the
glue layer as that helps me keep dwc3 clean of platform-specific
details.

> Of course here dwc3 core won't process clock if he got null one, so this
> won't affect other vendor's platform which only pass clock phandle to their
> dwc3 platform glue driver.

right

> Just wondering if it's better to write a platform glue only did clock
> on/off, or to do the same improvement people ever did for
> ehci-platform driver and just drop this platform glue driver?

I'll always the sour taste in my mouth due to MUSB. The thing is a
complete mess of platform-specific hacks all over the place.

> Hope I didn't reply too late, or make any impolite move here,

merge window just closed, we've got time.

> Super thanks for every comment you made here.

hey, no problem.

--
balbi


Attachments:
(No filename) (14.26 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-12-30 10:13:50

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/3] usb: dwc3: add Fujitsu Specific Glue layer

On Monday 29 December 2014 01:52:04 Sneeker Yeh wrote:
> > > +static int dwc3_mb86s70_remove_child(struct device *dev, void *unused)
> > > +{
> > > + struct platform_device *pdev = to_platform_device(dev);
> > > +
> > > + of_device_unregister(pdev);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static u64 dwc3_mb86s70_dma_mask = DMA_BIT_MASK(32);
> >
> > why ? Use dma_coerce_mask_and_coherent().
> >
>
> okay.

Actually that is still wrong: we use dma_coerce_mask_and_coherent() to
annotate drivers that have traditionally been forcing their own dma mask
by some other means and that need to be changed to something proper (after
finding out why they did it in the first place).

Since this is about a child device, the correct interface is to use
platform_device_register_full().

Note that this will still only work on machines that have no dma offset,
iommu, swiotlb, special coherency requirements, or ones that need a
platform specific dma_map_ops. To make any of those work, the driver
would need to be changed to pass the correct device pointer into any
dma-mapping interfaces, so the settings can get set by the platform
code. We will need to do that for arm64 I assume.

Arnd