[PATCH v4 0/3] Add ST dwc3 glue layer driver

[PATCH v4 0/3] Add ST dwc3 glue layer driver

[Peter Griffin]

This series adds support for the ST glue logic which wraps the DWC3 controller
on STiH407 SoC family chipsets.

Changes since v3
 - Various formating nits

Changes since v2
 - Use dr_mode for host/device static configuration
 - Manage shared reset signal to usbss to avoid hang if probing before usb3 phy
 - Remove DT checks and make driver depend on OF
 - Change some #define to use BIT macro
 - Make some comments clearer
 - Use kerneldoc for struct documentation
 - Remove udelay
 - Let DT create platform_devices, and remove legacy alloc
 - Change some logging levels to dev_dbg
 - Various whitespace and formatting cleanup
 - Use SIMPLE_DEV_PM_OPS()
 - Add const to of_device struct
 - Reorder header files alphabetically
 - Use devm_ioremap_resource instead of devm_request_and_ioremap
 - Remove of_match_ptr()

Changes since v1
 - Fix Kconfig mistake

Peter Griffin (3):
  usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC
  usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation
  MAINTAINERS: Add dwc3-st.c file to ARCH/STI architecture

 Documentation/devicetree/bindings/usb/dwc3-st.txt |  69 +++++
 MAINTAINERS                                       |   1 +
 drivers/usb/dwc3/Kconfig                          |   9 +
 drivers/usb/dwc3/Makefile                         |   1 +
 drivers/usb/dwc3/dwc3-st.c                        | 336 ++++++++++++++++++++++
 5 files changed, 416 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/dwc3-st.txt
 create mode 100644 drivers/usb/dwc3/dwc3-st.c

-- 
1.9.1

[PATCH v4 0/3] Add ST dwc3 glue layer driver

[Peter Griffin]

This series adds support for the ST glue logic which wraps the DWC3 controller
on STiH407 SoC family chipsets.

Changes since v3
 - Various formating nits

Changes since v2
 - Use dr_mode for host/device static configuration
 - Manage shared reset signal to usbss to avoid hang if probing before usb3 phy
 - Remove DT checks and make driver depend on OF
 - Change some #define to use BIT macro
 - Make some comments clearer
 - Use kerneldoc for struct documentation
 - Remove udelay
 - Let DT create platform_devices, and remove legacy alloc
 - Change some logging levels to dev_dbg
 - Various whitespace and formatting cleanup
 - Use SIMPLE_DEV_PM_OPS()
 - Add const to of_device struct
 - Reorder header files alphabetically
 - Use devm_ioremap_resource instead of devm_request_and_ioremap
 - Remove of_match_ptr()

Changes since v1
 - Fix Kconfig mistake

Peter Griffin (3):
  usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC
  usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation
  MAINTAINERS: Add dwc3-st.c file to ARCH/STI architecture

 Documentation/devicetree/bindings/usb/dwc3-st.txt |  69 +++++
 MAINTAINERS                                       |   1 +
 drivers/usb/dwc3/Kconfig                          |   9 +
 drivers/usb/dwc3/Makefile                         |   1 +
 drivers/usb/dwc3/dwc3-st.c                        | 336 ++++++++++++++++++++++
 5 files changed, 416 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/dwc3-st.txt
 create mode 100644 drivers/usb/dwc3/dwc3-st.c

-- 
1.9.1

[PATCH v4 1/3] usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC

[Peter Griffin]

This patch adds the ST glue logic to manage the DWC3 HC
on STiH407 SoC family. It manages the powerdown signal,
and configures the internal glue logic and syscfg registers.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/usb/dwc3/Kconfig   |   9 ++
 drivers/usb/dwc3/Makefile  |   1 +
 drivers/usb/dwc3/dwc3-st.c | 336 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 346 insertions(+)
 create mode 100644 drivers/usb/dwc3/dwc3-st.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 8eb996e..6c85c43 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -79,6 +79,15 @@ config USB_DWC3_KEYSTONE
 	  Support of USB2/3 functionality in TI Keystone2 platforms.
 	  Say 'Y' or 'M' here if you have one such device
 
+config USB_DWC3_ST
+	tristate "STMicroelectronics Platforms"
+	depends on ARCH_STI && OF
+	default USB_DWC3_HOST
+	help
+	  STMicroelectronics SoCs with one DesignWare Core USB3 IP
+	  inside (i.e. STiH407).
+	  Say 'Y' or 'M' if you have one such device.
+
 comment "Debugging features"
 
 config USB_DWC3_DEBUG
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 10ac3e7..11c9f54 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -33,3 +33,4 @@ obj-$(CONFIG_USB_DWC3_OMAP)		+= dwc3-omap.o
 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_ST)		+= dwc3-st.o
diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
new file mode 100644
index 0000000..227698f
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -0,0 +1,336 @@
+/**
+ * dwc3-st.c Support for dwc3 platform devices on ST Microelectronics platforms
+ *
+ * This is a small driver for the dwc3 to provide the glue logic
+ * to configure the controller. Tested on STi platforms.
+ *
+ * Copyright (C) 2014 Stmicroelectronics
+ *
+ * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ * Contributors: Aymen Bouattay <aymen.bouattay@st.com>
+ *               Peter Griffin <peter.griffin@linaro.org>
+ *
+ * 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.
+ *
+ * Inspired by dwc3-omap.c and dwc3-exynos.c.
+ */
+
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/usb/of.h>
+
+#include "core.h"
+#include "io.h"
+
+/* glue registers */
+#define CLKRST_CTRL		0x00
+#define AUX_CLK_EN		BIT(0)
+#define SW_PIPEW_RESET_N	BIT(4)
+#define EXT_CFG_RESET_N		BIT(8)
+/*
+ * 1'b0 : The host controller complies with the xHCI revision 0.96
+ * 1'b1 : The host controller complies with the xHCI revision 1.0
+ */
+#define XHCI_REVISION		BIT(12)
+
+#define USB2_VBUS_MNGMNT_SEL1	0x2C
+/*
+ * For all fields in USB2_VBUS_MNGMNT_SEL1
+ * 2’b00 : Override value from Reg 0x30 is selected
+ * 2’b01 : utmiotg_<signal_name> from usb3_top is selected
+ * 2’b10 : pipew_<signal_name> from PIPEW instance is selected
+ * 2’b11 : value is 1'b0
+ */
+#define USB2_VBUS_REG30		0x0
+#define USB2_VBUS_UTMIOTG	0x1
+#define USB2_VBUS_PIPEW		0x2
+#define USB2_VBUS_ZERO		0x3
+
+#define SEL_OVERRIDE_VBUSVALID(n)	(n << 0)
+#define SEL_OVERRIDE_POWERPRESENT(n)	(n << 4)
+#define SEL_OVERRIDE_BVALID(n)		(n << 8)
+
+/* Static DRD configuration */
+#define USB_HOST_DEFAULT_MASK	0xffe
+#define USB_SET_PORT_DEVICE	0x1
+
+/**
+ * struct st_dwc3 - dwc3-st driver private structure
+ * @dev:		device pointer
+ * @glue_base:		ioaddr for the glue registers
+ * @regmap:		regmap pointer for getting syscfg
+ * @syscfg_reg_off:	usb syscfg control offset
+ * @dr_mode:		drd static host/device config
+ * @rstc_pwrdn:		rest controller for powerdown signal
+ * @rstc_rst:		reset controller for softreset signal
+ */
+
+struct st_dwc3 {
+	struct device *dev;
+	void __iomem *glue_base;
+	struct regmap *regmap;
+	int syscfg_reg_off;
+	enum usb_dr_mode dr_mode;
+	struct reset_control *rstc_pwrdn;
+	struct reset_control *rstc_rst;
+};
+
+static inline u32 st_dwc3_readl(void __iomem *base, u32 offset)
+{
+	return readl_relaxed(base + offset);
+}
+
+static inline void st_dwc3_writel(void __iomem *base, u32 offset, u32 value)
+{
+	writel_relaxed(value, base + offset);
+}
+
+/**
+ * st_dwc3_drd_init: program the port
+ * @dwc3_data: driver private structure
+ * Description: this function is to program the port as either host or device
+ * according to the static configuration passed from devicetree.
+ * OTG and dual role are not yet supported!
+ */
+static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
+{
+	u32 val;
+	int err;
+
+	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
+	if (err)
+		return err;
+
+	switch (dwc3_data->dr_mode) {
+	case USB_DR_MODE_PERIPHERAL:
+		val |= USB_SET_PORT_DEVICE;
+		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
+		break;
+
+	case USB_DR_MODE_HOST:
+		val &= USB_HOST_DEFAULT_MASK;
+		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
+		break;
+
+	default:
+		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n",
+			dwc3_data->dr_mode);
+		return -EINVAL;
+	}
+
+	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
+}
+
+/**
+ * st_dwc3_init: init the controller via glue logic
+ * @dwc3_data: driver private structure
+ */
+static void st_dwc3_init(struct st_dwc3 *dwc3_data)
+{
+
+	u32 reg = st_dwc3_readl(dwc3_data->glue_base, CLKRST_CTRL);
+
+	reg |= AUX_CLK_EN | EXT_CFG_RESET_N | XHCI_REVISION;
+	reg &= ~SW_PIPEW_RESET_N;
+	st_dwc3_writel(dwc3_data->glue_base, CLKRST_CTRL, reg);
+
+	/* configure mux for vbus, powerpresent and bvalid signals */
+	reg = st_dwc3_readl(dwc3_data->glue_base, USB2_VBUS_MNGMNT_SEL1);
+
+	reg |= SEL_OVERRIDE_VBUSVALID(USB2_VBUS_UTMIOTG) |
+		SEL_OVERRIDE_POWERPRESENT(USB2_VBUS_UTMIOTG) |
+		SEL_OVERRIDE_BVALID(USB2_VBUS_UTMIOTG);
+
+	st_dwc3_writel(dwc3_data->glue_base, USB2_VBUS_MNGMNT_SEL1, reg);
+
+	reg = st_dwc3_readl(dwc3_data->glue_base, CLKRST_CTRL);
+	reg |= SW_PIPEW_RESET_N;
+	st_dwc3_writel(dwc3_data->glue_base, CLKRST_CTRL, reg);
+}
+
+static int st_dwc3_probe(struct platform_device *pdev)
+{
+	struct st_dwc3 *dwc3_data;
+	struct resource *res;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node, *child;
+	struct regmap *regmap;
+	int ret;
+
+	dwc3_data = devm_kzalloc(dev, sizeof(*dwc3_data), GFP_KERNEL);
+	if (!dwc3_data)
+		return -ENOMEM;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg-glue");
+	dwc3_data->glue_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(dwc3_data->glue_base))
+		return PTR_ERR(dwc3_data->glue_base);
+
+	regmap = syscon_regmap_lookup_by_phandle(node, "st,syscfg");
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	dma_set_coherent_mask(dev, dev->coherent_dma_mask);
+	dwc3_data->dev = dev;
+	dwc3_data->regmap = regmap;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
+	if (!res) {
+		ret = -ENXIO;
+		goto undo_platform_dev_alloc;
+	}
+
+	dwc3_data->syscfg_reg_off = res->start;
+
+	dev_dbg(&pdev->dev, "glue-logic addr 0x%p, syscfg-reg offset 0x%x\n",
+		 dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
+
+	dwc3_data->rstc_pwrdn = devm_reset_control_get(dev, "powerdown");
+	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
+		dev_err(&pdev->dev, "could not get power controller\n");
+		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
+		goto undo_platform_dev_alloc;
+	}
+
+	/* Manage PowerDown */
+	reset_control_deassert(dwc3_data->rstc_pwrdn);
+
+	dwc3_data->rstc_rst = devm_reset_control_get(dev, "softreset");
+	if (IS_ERR(dwc3_data->rstc_rst)) {
+		dev_err(&pdev->dev, "could not get reset controller\n");
+		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
+		goto undo_powerdown;
+	}
+
+	/* Manage SoftReset */
+	reset_control_deassert(dwc3_data->rstc_rst);
+
+	child = of_get_child_by_name(node, "dwc3");
+	if (!child) {
+		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
+		ret = -ENODEV;
+		goto undo_softreset;
+	}
+
+	dwc3_data->dr_mode = of_usb_get_dr_mode(child);
+
+	/* Allocate and initialize the core */
+	ret = of_platform_populate(node, NULL, NULL, dev);
+	if (ret) {
+		dev_err(dev, "failed to add dwc3 core\n");
+		goto undo_softreset;
+	}
+
+	/*
+	 * Configure the USB port as device or host according to the static
+	 * configuration passed from DT.
+	 * DRD is the only mode currently supported so this will be enhanced
+	 * as soon as OTG is available.
+	 */
+	ret = st_dwc3_drd_init(dwc3_data);
+	if (ret) {
+		dev_err(dev, "drd initialisation failed\n");
+		goto undo_softreset;
+	}
+
+	/* ST glue logic init */
+	st_dwc3_init(dwc3_data);
+
+	platform_set_drvdata(pdev, dwc3_data);
+	return 0;
+
+undo_softreset:
+	reset_control_assert(dwc3_data->rstc_rst);
+undo_powerdown:
+	reset_control_assert(dwc3_data->rstc_pwrdn);
+undo_platform_dev_alloc:
+	platform_device_put(pdev);
+	return ret;
+}
+
+
+static int st_dwc3_remove_child(struct device *dev, void *c)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+
+	of_device_unregister(pdev);
+
+	return 0;
+}
+
+static int st_dwc3_remove(struct platform_device *pdev)
+{
+	struct st_dwc3 *dwc3_data = platform_get_drvdata(pdev);
+
+	device_for_each_child(&pdev->dev, NULL, st_dwc3_remove_child);
+
+	reset_control_assert(dwc3_data->rstc_pwrdn);
+	reset_control_assert(dwc3_data->rstc_rst);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int st_dwc3_suspend(struct device *dev)
+{
+	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
+
+	reset_control_assert(dwc3_data->rstc_pwrdn);
+	reset_control_assert(dwc3_data->rstc_rst);
+
+	pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
+}
+
+static int st_dwc3_resume(struct device *dev)
+{
+	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
+
+	pinctrl_pm_select_default_state(dev);
+
+	reset_control_deassert(dwc3_data->rstc_pwrdn);
+	reset_control_deassert(dwc3_data->rstc_rst);
+
+	return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(st_dwc3_dev_pm_ops, st_dwc3_suspend, st_dwc3_resume);
+
+static const struct of_device_id st_dwc3_match[] = {
+	{ .compatible = "st,stih407-dwc3" },
+	{ /* sentinel */ },
+};
+
+MODULE_DEVICE_TABLE(of, st_dwc3_match);
+
+static struct platform_driver st_dwc3_driver = {
+	.probe = st_dwc3_probe,
+	.remove = st_dwc3_remove,
+	.driver = {
+		.name = "usb-st-dwc3",
+		.of_match_table = st_dwc3_match,
+		.pm = &st_dwc3_dev_pm_ops,
+	},
+};
+
+module_platform_driver(st_dwc3_driver);
+
+MODULE_AUTHOR("Giuseppe Cavallaro <peppe.cavallaro@st.com>");
+MODULE_DESCRIPTION("DesignWare USB3 STi Glue Layer");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH v4 1/3] usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC

[Peter Griffin]

This patch adds the ST glue logic to manage the DWC3 HC
on STiH407 SoC family. It manages the powerdown signal,
and configures the internal glue logic and syscfg registers.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/usb/dwc3/Kconfig   |   9 ++
 drivers/usb/dwc3/Makefile  |   1 +
 drivers/usb/dwc3/dwc3-st.c | 336 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 346 insertions(+)
 create mode 100644 drivers/usb/dwc3/dwc3-st.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 8eb996e..6c85c43 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -79,6 +79,15 @@ config USB_DWC3_KEYSTONE
 	  Support of USB2/3 functionality in TI Keystone2 platforms.
 	  Say 'Y' or 'M' here if you have one such device
 
+config USB_DWC3_ST
+	tristate "STMicroelectronics Platforms"
+	depends on ARCH_STI && OF
+	default USB_DWC3_HOST
+	help
+	  STMicroelectronics SoCs with one DesignWare Core USB3 IP
+	  inside (i.e. STiH407).
+	  Say 'Y' or 'M' if you have one such device.
+
 comment "Debugging features"
 
 config USB_DWC3_DEBUG
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 10ac3e7..11c9f54 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -33,3 +33,4 @@ obj-$(CONFIG_USB_DWC3_OMAP)		+= dwc3-omap.o
 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_ST)		+= dwc3-st.o
diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
new file mode 100644
index 0000000..227698f
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -0,0 +1,336 @@
+/**
+ * dwc3-st.c Support for dwc3 platform devices on ST Microelectronics platforms
+ *
+ * This is a small driver for the dwc3 to provide the glue logic
+ * to configure the controller. Tested on STi platforms.
+ *
+ * Copyright (C) 2014 Stmicroelectronics
+ *
+ * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ * Contributors: Aymen Bouattay <aymen.bouattay@st.com>
+ *               Peter Griffin <peter.griffin@linaro.org>
+ *
+ * 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.
+ *
+ * Inspired by dwc3-omap.c and dwc3-exynos.c.
+ */
+
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/usb/of.h>
+
+#include "core.h"
+#include "io.h"
+
+/* glue registers */
+#define CLKRST_CTRL		0x00
+#define AUX_CLK_EN		BIT(0)
+#define SW_PIPEW_RESET_N	BIT(4)
+#define EXT_CFG_RESET_N		BIT(8)
+/*
+ * 1'b0 : The host controller complies with the xHCI revision 0.96
+ * 1'b1 : The host controller complies with the xHCI revision 1.0
+ */
+#define XHCI_REVISION		BIT(12)
+
+#define USB2_VBUS_MNGMNT_SEL1	0x2C
+/*
+ * For all fields in USB2_VBUS_MNGMNT_SEL1
+ * 2?b00 : Override value from Reg 0x30 is selected
+ * 2?b01 : utmiotg_<signal_name> from usb3_top is selected
+ * 2?b10 : pipew_<signal_name> from PIPEW instance is selected
+ * 2?b11 : value is 1'b0
+ */
+#define USB2_VBUS_REG30		0x0
+#define USB2_VBUS_UTMIOTG	0x1
+#define USB2_VBUS_PIPEW		0x2
+#define USB2_VBUS_ZERO		0x3
+
+#define SEL_OVERRIDE_VBUSVALID(n)	(n << 0)
+#define SEL_OVERRIDE_POWERPRESENT(n)	(n << 4)
+#define SEL_OVERRIDE_BVALID(n)		(n << 8)
+
+/* Static DRD configuration */
+#define USB_HOST_DEFAULT_MASK	0xffe
+#define USB_SET_PORT_DEVICE	0x1
+
+/**
+ * struct st_dwc3 - dwc3-st driver private structure
+ * @dev:		device pointer
+ * @glue_base:		ioaddr for the glue registers
+ * @regmap:		regmap pointer for getting syscfg
+ * @syscfg_reg_off:	usb syscfg control offset
+ * @dr_mode:		drd static host/device config
+ * @rstc_pwrdn:		rest controller for powerdown signal
+ * @rstc_rst:		reset controller for softreset signal
+ */
+
+struct st_dwc3 {
+	struct device *dev;
+	void __iomem *glue_base;
+	struct regmap *regmap;
+	int syscfg_reg_off;
+	enum usb_dr_mode dr_mode;
+	struct reset_control *rstc_pwrdn;
+	struct reset_control *rstc_rst;
+};
+
+static inline u32 st_dwc3_readl(void __iomem *base, u32 offset)
+{
+	return readl_relaxed(base + offset);
+}
+
+static inline void st_dwc3_writel(void __iomem *base, u32 offset, u32 value)
+{
+	writel_relaxed(value, base + offset);
+}
+
+/**
+ * st_dwc3_drd_init: program the port
+ * @dwc3_data: driver private structure
+ * Description: this function is to program the port as either host or device
+ * according to the static configuration passed from devicetree.
+ * OTG and dual role are not yet supported!
+ */
+static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
+{
+	u32 val;
+	int err;
+
+	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
+	if (err)
+		return err;
+
+	switch (dwc3_data->dr_mode) {
+	case USB_DR_MODE_PERIPHERAL:
+		val |= USB_SET_PORT_DEVICE;
+		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
+		break;
+
+	case USB_DR_MODE_HOST:
+		val &= USB_HOST_DEFAULT_MASK;
+		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
+		break;
+
+	default:
+		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n",
+			dwc3_data->dr_mode);
+		return -EINVAL;
+	}
+
+	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
+}
+
+/**
+ * st_dwc3_init: init the controller via glue logic
+ * @dwc3_data: driver private structure
+ */
+static void st_dwc3_init(struct st_dwc3 *dwc3_data)
+{
+
+	u32 reg = st_dwc3_readl(dwc3_data->glue_base, CLKRST_CTRL);
+
+	reg |= AUX_CLK_EN | EXT_CFG_RESET_N | XHCI_REVISION;
+	reg &= ~SW_PIPEW_RESET_N;
+	st_dwc3_writel(dwc3_data->glue_base, CLKRST_CTRL, reg);
+
+	/* configure mux for vbus, powerpresent and bvalid signals */
+	reg = st_dwc3_readl(dwc3_data->glue_base, USB2_VBUS_MNGMNT_SEL1);
+
+	reg |= SEL_OVERRIDE_VBUSVALID(USB2_VBUS_UTMIOTG) |
+		SEL_OVERRIDE_POWERPRESENT(USB2_VBUS_UTMIOTG) |
+		SEL_OVERRIDE_BVALID(USB2_VBUS_UTMIOTG);
+
+	st_dwc3_writel(dwc3_data->glue_base, USB2_VBUS_MNGMNT_SEL1, reg);
+
+	reg = st_dwc3_readl(dwc3_data->glue_base, CLKRST_CTRL);
+	reg |= SW_PIPEW_RESET_N;
+	st_dwc3_writel(dwc3_data->glue_base, CLKRST_CTRL, reg);
+}
+
+static int st_dwc3_probe(struct platform_device *pdev)
+{
+	struct st_dwc3 *dwc3_data;
+	struct resource *res;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node, *child;
+	struct regmap *regmap;
+	int ret;
+
+	dwc3_data = devm_kzalloc(dev, sizeof(*dwc3_data), GFP_KERNEL);
+	if (!dwc3_data)
+		return -ENOMEM;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg-glue");
+	dwc3_data->glue_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(dwc3_data->glue_base))
+		return PTR_ERR(dwc3_data->glue_base);
+
+	regmap = syscon_regmap_lookup_by_phandle(node, "st,syscfg");
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	dma_set_coherent_mask(dev, dev->coherent_dma_mask);
+	dwc3_data->dev = dev;
+	dwc3_data->regmap = regmap;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
+	if (!res) {
+		ret = -ENXIO;
+		goto undo_platform_dev_alloc;
+	}
+
+	dwc3_data->syscfg_reg_off = res->start;
+
+	dev_dbg(&pdev->dev, "glue-logic addr 0x%p, syscfg-reg offset 0x%x\n",
+		 dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
+
+	dwc3_data->rstc_pwrdn = devm_reset_control_get(dev, "powerdown");
+	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
+		dev_err(&pdev->dev, "could not get power controller\n");
+		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
+		goto undo_platform_dev_alloc;
+	}
+
+	/* Manage PowerDown */
+	reset_control_deassert(dwc3_data->rstc_pwrdn);
+
+	dwc3_data->rstc_rst = devm_reset_control_get(dev, "softreset");
+	if (IS_ERR(dwc3_data->rstc_rst)) {
+		dev_err(&pdev->dev, "could not get reset controller\n");
+		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
+		goto undo_powerdown;
+	}
+
+	/* Manage SoftReset */
+	reset_control_deassert(dwc3_data->rstc_rst);
+
+	child = of_get_child_by_name(node, "dwc3");
+	if (!child) {
+		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
+		ret = -ENODEV;
+		goto undo_softreset;
+	}
+
+	dwc3_data->dr_mode = of_usb_get_dr_mode(child);
+
+	/* Allocate and initialize the core */
+	ret = of_platform_populate(node, NULL, NULL, dev);
+	if (ret) {
+		dev_err(dev, "failed to add dwc3 core\n");
+		goto undo_softreset;
+	}
+
+	/*
+	 * Configure the USB port as device or host according to the static
+	 * configuration passed from DT.
+	 * DRD is the only mode currently supported so this will be enhanced
+	 * as soon as OTG is available.
+	 */
+	ret = st_dwc3_drd_init(dwc3_data);
+	if (ret) {
+		dev_err(dev, "drd initialisation failed\n");
+		goto undo_softreset;
+	}
+
+	/* ST glue logic init */
+	st_dwc3_init(dwc3_data);
+
+	platform_set_drvdata(pdev, dwc3_data);
+	return 0;
+
+undo_softreset:
+	reset_control_assert(dwc3_data->rstc_rst);
+undo_powerdown:
+	reset_control_assert(dwc3_data->rstc_pwrdn);
+undo_platform_dev_alloc:
+	platform_device_put(pdev);
+	return ret;
+}
+
+
+static int st_dwc3_remove_child(struct device *dev, void *c)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+
+	of_device_unregister(pdev);
+
+	return 0;
+}
+
+static int st_dwc3_remove(struct platform_device *pdev)
+{
+	struct st_dwc3 *dwc3_data = platform_get_drvdata(pdev);
+
+	device_for_each_child(&pdev->dev, NULL, st_dwc3_remove_child);
+
+	reset_control_assert(dwc3_data->rstc_pwrdn);
+	reset_control_assert(dwc3_data->rstc_rst);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int st_dwc3_suspend(struct device *dev)
+{
+	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
+
+	reset_control_assert(dwc3_data->rstc_pwrdn);
+	reset_control_assert(dwc3_data->rstc_rst);
+
+	pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
+}
+
+static int st_dwc3_resume(struct device *dev)
+{
+	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
+
+	pinctrl_pm_select_default_state(dev);
+
+	reset_control_deassert(dwc3_data->rstc_pwrdn);
+	reset_control_deassert(dwc3_data->rstc_rst);
+
+	return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(st_dwc3_dev_pm_ops, st_dwc3_suspend, st_dwc3_resume);
+
+static const struct of_device_id st_dwc3_match[] = {
+	{ .compatible = "st,stih407-dwc3" },
+	{ /* sentinel */ },
+};
+
+MODULE_DEVICE_TABLE(of, st_dwc3_match);
+
+static struct platform_driver st_dwc3_driver = {
+	.probe = st_dwc3_probe,
+	.remove = st_dwc3_remove,
+	.driver = {
+		.name = "usb-st-dwc3",
+		.of_match_table = st_dwc3_match,
+		.pm = &st_dwc3_dev_pm_ops,
+	},
+};
+
+module_platform_driver(st_dwc3_driver);
+
+MODULE_AUTHOR("Giuseppe Cavallaro <peppe.cavallaro@st.com>");
+MODULE_DESCRIPTION("DesignWare USB3 STi Glue Layer");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [PATCH v4 1/3] usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 11754 bytes --]

Hi,

On Wed, Jul 30, 2014 at 04:28:09PM +0100, Peter Griffin wrote:
> This patch adds the ST glue logic to manage the DWC3 HC
> on STiH407 SoC family. It manages the powerdown signal,
> and configures the internal glue logic and syscfg registers.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/usb/dwc3/Kconfig   |   9 ++
>  drivers/usb/dwc3/Makefile  |   1 +
>  drivers/usb/dwc3/dwc3-st.c | 336 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 346 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-st.c
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 8eb996e..6c85c43 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -79,6 +79,15 @@ config USB_DWC3_KEYSTONE
>  	  Support of USB2/3 functionality in TI Keystone2 platforms.
>  	  Say 'Y' or 'M' here if you have one such device
>  
> +config USB_DWC3_ST
> +	tristate "STMicroelectronics Platforms"
> +	depends on ARCH_STI && OF
> +	default USB_DWC3_HOST

this seems wrong as USB_DWC3_{HOST,GADGET,DUAL_ROLE} are booleans and
USB_DWC3_ST is a tristate. Better to stick with defaulting to USB_DWC3
instead like all the others.

> +	help
> +	  STMicroelectronics SoCs with one DesignWare Core USB3 IP
> +	  inside (i.e. STiH407).
> +	  Say 'Y' or 'M' if you have one such device.
> +
>  comment "Debugging features"
>  
>  config USB_DWC3_DEBUG
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index 10ac3e7..11c9f54 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -33,3 +33,4 @@ obj-$(CONFIG_USB_DWC3_OMAP)		+= dwc3-omap.o
>  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_ST)		+= dwc3-st.o
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> new file mode 100644
> index 0000000..227698f
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -0,0 +1,336 @@
> +/**
> + * dwc3-st.c Support for dwc3 platform devices on ST Microelectronics platforms
> + *
> + * This is a small driver for the dwc3 to provide the glue logic
> + * to configure the controller. Tested on STi platforms.
> + *
> + * Copyright (C) 2014 Stmicroelectronics
> + *
> + * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> + * Contributors: Aymen Bouattay <aymen.bouattay@st.com>
> + *               Peter Griffin <peter.griffin@linaro.org>
> + *
> + * 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.
> + *
> + * Inspired by dwc3-omap.c and dwc3-exynos.c.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/usb/of.h>
> +
> +#include "core.h"
> +#include "io.h"
> +
> +/* glue registers */
> +#define CLKRST_CTRL		0x00
> +#define AUX_CLK_EN		BIT(0)
> +#define SW_PIPEW_RESET_N	BIT(4)
> +#define EXT_CFG_RESET_N		BIT(8)
> +/*
> + * 1'b0 : The host controller complies with the xHCI revision 0.96
> + * 1'b1 : The host controller complies with the xHCI revision 1.0
> + */
> +#define XHCI_REVISION		BIT(12)
> +
> +#define USB2_VBUS_MNGMNT_SEL1	0x2C
> +/*
> + * For all fields in USB2_VBUS_MNGMNT_SEL1
> + * 2’b00 : Override value from Reg 0x30 is selected
> + * 2’b01 : utmiotg_<signal_name> from usb3_top is selected
> + * 2’b10 : pipew_<signal_name> from PIPEW instance is selected
> + * 2’b11 : value is 1'b0
> + */
> +#define USB2_VBUS_REG30		0x0
> +#define USB2_VBUS_UTMIOTG	0x1
> +#define USB2_VBUS_PIPEW		0x2
> +#define USB2_VBUS_ZERO		0x3
> +
> +#define SEL_OVERRIDE_VBUSVALID(n)	(n << 0)
> +#define SEL_OVERRIDE_POWERPRESENT(n)	(n << 4)
> +#define SEL_OVERRIDE_BVALID(n)		(n << 8)
> +
> +/* Static DRD configuration */
> +#define USB_HOST_DEFAULT_MASK	0xffe
> +#define USB_SET_PORT_DEVICE	0x1
> +
> +/**
> + * struct st_dwc3 - dwc3-st driver private structure
> + * @dev:		device pointer
> + * @glue_base:		ioaddr for the glue registers
> + * @regmap:		regmap pointer for getting syscfg
> + * @syscfg_reg_off:	usb syscfg control offset
> + * @dr_mode:		drd static host/device config
> + * @rstc_pwrdn:		rest controller for powerdown signal
> + * @rstc_rst:		reset controller for softreset signal
> + */
> +
> +struct st_dwc3 {
> +	struct device *dev;
> +	void __iomem *glue_base;
> +	struct regmap *regmap;
> +	int syscfg_reg_off;
> +	enum usb_dr_mode dr_mode;
> +	struct reset_control *rstc_pwrdn;
> +	struct reset_control *rstc_rst;
> +};
> +
> +static inline u32 st_dwc3_readl(void __iomem *base, u32 offset)
> +{
> +	return readl_relaxed(base + offset);
> +}
> +
> +static inline void st_dwc3_writel(void __iomem *base, u32 offset, u32 value)
> +{
> +	writel_relaxed(value, base + offset);

why relaxed ?

> +}
> +
> +/**
> + * st_dwc3_drd_init: program the port
> + * @dwc3_data: driver private structure
> + * Description: this function is to program the port as either host or device
> + * according to the static configuration passed from devicetree.
> + * OTG and dual role are not yet supported!
> + */
> +static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
> +{
> +	u32 val;
> +	int err;
> +
> +	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
> +	if (err)
> +		return err;
> +
> +	switch (dwc3_data->dr_mode) {
> +	case USB_DR_MODE_PERIPHERAL:
> +		val |= USB_SET_PORT_DEVICE;
> +		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
> +		break;
> +
> +	case USB_DR_MODE_HOST:
> +		val &= USB_HOST_DEFAULT_MASK;

are you missing a ~ here ? Also, shouldn't you mask off the bits before
this switch ?

> +		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
> +		break;
> +
> +	default:
> +		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n",
> +			dwc3_data->dr_mode);
> +		return -EINVAL;
> +	}
> +
> +	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
> +}
> +
> +/**
> + * st_dwc3_init: init the controller via glue logic
> + * @dwc3_data: driver private structure
> + */
> +static void st_dwc3_init(struct st_dwc3 *dwc3_data)
> +{
> +

this blank line isn't necessary.

> +	u32 reg = st_dwc3_readl(dwc3_data->glue_base, CLKRST_CTRL);
> +
> +	reg |= AUX_CLK_EN | EXT_CFG_RESET_N | XHCI_REVISION;
> +	reg &= ~SW_PIPEW_RESET_N;
> +	st_dwc3_writel(dwc3_data->glue_base, CLKRST_CTRL, reg);
> +
> +	/* configure mux for vbus, powerpresent and bvalid signals */
> +	reg = st_dwc3_readl(dwc3_data->glue_base, USB2_VBUS_MNGMNT_SEL1);
> +
> +	reg |= SEL_OVERRIDE_VBUSVALID(USB2_VBUS_UTMIOTG) |
> +		SEL_OVERRIDE_POWERPRESENT(USB2_VBUS_UTMIOTG) |
> +		SEL_OVERRIDE_BVALID(USB2_VBUS_UTMIOTG);
> +
> +	st_dwc3_writel(dwc3_data->glue_base, USB2_VBUS_MNGMNT_SEL1, reg);
> +
> +	reg = st_dwc3_readl(dwc3_data->glue_base, CLKRST_CTRL);
> +	reg |= SW_PIPEW_RESET_N;
> +	st_dwc3_writel(dwc3_data->glue_base, CLKRST_CTRL, reg);
> +}
> +
> +static int st_dwc3_probe(struct platform_device *pdev)
> +{
> +	struct st_dwc3 *dwc3_data;
> +	struct resource *res;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node, *child;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	dwc3_data = devm_kzalloc(dev, sizeof(*dwc3_data), GFP_KERNEL);
> +	if (!dwc3_data)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg-glue");
> +	dwc3_data->glue_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(dwc3_data->glue_base))
> +		return PTR_ERR(dwc3_data->glue_base);
> +
> +	regmap = syscon_regmap_lookup_by_phandle(node, "st,syscfg");
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	dma_set_coherent_mask(dev, dev->coherent_dma_mask);
> +	dwc3_data->dev = dev;
> +	dwc3_data->regmap = regmap;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
> +	if (!res) {
> +		ret = -ENXIO;
> +		goto undo_platform_dev_alloc;
> +	}
> +
> +	dwc3_data->syscfg_reg_off = res->start;
> +
> +	dev_dbg(&pdev->dev, "glue-logic addr 0x%p, syscfg-reg offset 0x%x\n",
> +		dwc3_data->glue_base, dwc3_data->syscfg_reg_off);

looks like this message would be more of a dev_vdbg().

> +	dwc3_data->rstc_pwrdn = devm_reset_control_get(dev, "powerdown");
> +	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
> +		dev_err(&pdev->dev, "could not get power controller\n");
> +		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
> +		goto undo_platform_dev_alloc;
> +	}
> +
> +	/* Manage PowerDown */
> +	reset_control_deassert(dwc3_data->rstc_pwrdn);
> +
> +	dwc3_data->rstc_rst = devm_reset_control_get(dev, "softreset");
> +	if (IS_ERR(dwc3_data->rstc_rst)) {
> +		dev_err(&pdev->dev, "could not get reset controller\n");
> +		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
> +		goto undo_powerdown;
> +	}
> +
> +	/* Manage SoftReset */
> +	reset_control_deassert(dwc3_data->rstc_rst);
> +
> +	child = of_get_child_by_name(node, "dwc3");
> +	if (!child) {
> +		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
> +		ret = -ENODEV;
> +		goto undo_softreset;
> +	}
> +
> +	dwc3_data->dr_mode = of_usb_get_dr_mode(child);
> +
> +	/* Allocate and initialize the core */
> +	ret = of_platform_populate(node, NULL, NULL, dev);
> +	if (ret) {
> +		dev_err(dev, "failed to add dwc3 core\n");
> +		goto undo_softreset;
> +	}
> +
> +	/*
> +	 * Configure the USB port as device or host according to the static
> +	 * configuration passed from DT.
> +	 * DRD is the only mode currently supported so this will be enhanced
> +	 * as soon as OTG is available.
> +	 */
> +	ret = st_dwc3_drd_init(dwc3_data);
> +	if (ret) {
> +		dev_err(dev, "drd initialisation failed\n");
> +		goto undo_softreset;
> +	}
> +
> +	/* ST glue logic init */
> +	st_dwc3_init(dwc3_data);
> +
> +	platform_set_drvdata(pdev, dwc3_data);
> +	return 0;
> +
> +undo_softreset:
> +	reset_control_assert(dwc3_data->rstc_rst);
> +undo_powerdown:
> +	reset_control_assert(dwc3_data->rstc_pwrdn);
> +undo_platform_dev_alloc:
> +	platform_device_put(pdev);
> +	return ret;
> +}
> +
> +
> +static int st_dwc3_remove_child(struct device *dev, void *c)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +
> +	of_device_unregister(pdev);
> +
> +	return 0;
> +}
> +
> +static int st_dwc3_remove(struct platform_device *pdev)
> +{
> +	struct st_dwc3 *dwc3_data = platform_get_drvdata(pdev);
> +
> +	device_for_each_child(&pdev->dev, NULL, st_dwc3_remove_child);
> +
> +	reset_control_assert(dwc3_data->rstc_pwrdn);
> +	reset_control_assert(dwc3_data->rstc_rst);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int st_dwc3_suspend(struct device *dev)
> +{
> +	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
> +
> +	reset_control_assert(dwc3_data->rstc_pwrdn);
> +	reset_control_assert(dwc3_data->rstc_rst);

Two questions:

1) how would you handle the case when this device is a wakeup source ?
2) when resuming, wouldn't you have to reinitialize the entire core ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 1/3] usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC

[Felipe Balbi]

Hi,

On Wed, Jul 30, 2014 at 04:28:09PM +0100, Peter Griffin wrote:
> This patch adds the ST glue logic to manage the DWC3 HC
> on STiH407 SoC family. It manages the powerdown signal,
> and configures the internal glue logic and syscfg registers.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/usb/dwc3/Kconfig   |   9 ++
>  drivers/usb/dwc3/Makefile  |   1 +
>  drivers/usb/dwc3/dwc3-st.c | 336 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 346 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-st.c
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 8eb996e..6c85c43 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -79,6 +79,15 @@ config USB_DWC3_KEYSTONE
>  	  Support of USB2/3 functionality in TI Keystone2 platforms.
>  	  Say 'Y' or 'M' here if you have one such device
>  
> +config USB_DWC3_ST
> +	tristate "STMicroelectronics Platforms"
> +	depends on ARCH_STI && OF
> +	default USB_DWC3_HOST

this seems wrong as USB_DWC3_{HOST,GADGET,DUAL_ROLE} are booleans and
USB_DWC3_ST is a tristate. Better to stick with defaulting to USB_DWC3
instead like all the others.

> +	help
> +	  STMicroelectronics SoCs with one DesignWare Core USB3 IP
> +	  inside (i.e. STiH407).
> +	  Say 'Y' or 'M' if you have one such device.
> +
>  comment "Debugging features"
>  
>  config USB_DWC3_DEBUG
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index 10ac3e7..11c9f54 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -33,3 +33,4 @@ obj-$(CONFIG_USB_DWC3_OMAP)		+= dwc3-omap.o
>  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_ST)		+= dwc3-st.o
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> new file mode 100644
> index 0000000..227698f
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -0,0 +1,336 @@
> +/**
> + * dwc3-st.c Support for dwc3 platform devices on ST Microelectronics platforms
> + *
> + * This is a small driver for the dwc3 to provide the glue logic
> + * to configure the controller. Tested on STi platforms.
> + *
> + * Copyright (C) 2014 Stmicroelectronics
> + *
> + * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> + * Contributors: Aymen Bouattay <aymen.bouattay@st.com>
> + *               Peter Griffin <peter.griffin@linaro.org>
> + *
> + * 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.
> + *
> + * Inspired by dwc3-omap.c and dwc3-exynos.c.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/usb/of.h>
> +
> +#include "core.h"
> +#include "io.h"
> +
> +/* glue registers */
> +#define CLKRST_CTRL		0x00
> +#define AUX_CLK_EN		BIT(0)
> +#define SW_PIPEW_RESET_N	BIT(4)
> +#define EXT_CFG_RESET_N		BIT(8)
> +/*
> + * 1'b0 : The host controller complies with the xHCI revision 0.96
> + * 1'b1 : The host controller complies with the xHCI revision 1.0
> + */
> +#define XHCI_REVISION		BIT(12)
> +
> +#define USB2_VBUS_MNGMNT_SEL1	0x2C
> +/*
> + * For all fields in USB2_VBUS_MNGMNT_SEL1
> + * 2?b00 : Override value from Reg 0x30 is selected
> + * 2?b01 : utmiotg_<signal_name> from usb3_top is selected
> + * 2?b10 : pipew_<signal_name> from PIPEW instance is selected
> + * 2?b11 : value is 1'b0
> + */
> +#define USB2_VBUS_REG30		0x0
> +#define USB2_VBUS_UTMIOTG	0x1
> +#define USB2_VBUS_PIPEW		0x2
> +#define USB2_VBUS_ZERO		0x3
> +
> +#define SEL_OVERRIDE_VBUSVALID(n)	(n << 0)
> +#define SEL_OVERRIDE_POWERPRESENT(n)	(n << 4)
> +#define SEL_OVERRIDE_BVALID(n)		(n << 8)
> +
> +/* Static DRD configuration */
> +#define USB_HOST_DEFAULT_MASK	0xffe
> +#define USB_SET_PORT_DEVICE	0x1
> +
> +/**
> + * struct st_dwc3 - dwc3-st driver private structure
> + * @dev:		device pointer
> + * @glue_base:		ioaddr for the glue registers
> + * @regmap:		regmap pointer for getting syscfg
> + * @syscfg_reg_off:	usb syscfg control offset
> + * @dr_mode:		drd static host/device config
> + * @rstc_pwrdn:		rest controller for powerdown signal
> + * @rstc_rst:		reset controller for softreset signal
> + */
> +
> +struct st_dwc3 {
> +	struct device *dev;
> +	void __iomem *glue_base;
> +	struct regmap *regmap;
> +	int syscfg_reg_off;
> +	enum usb_dr_mode dr_mode;
> +	struct reset_control *rstc_pwrdn;
> +	struct reset_control *rstc_rst;
> +};
> +
> +static inline u32 st_dwc3_readl(void __iomem *base, u32 offset)
> +{
> +	return readl_relaxed(base + offset);
> +}
> +
> +static inline void st_dwc3_writel(void __iomem *base, u32 offset, u32 value)
> +{
> +	writel_relaxed(value, base + offset);

why relaxed ?

> +}
> +
> +/**
> + * st_dwc3_drd_init: program the port
> + * @dwc3_data: driver private structure
> + * Description: this function is to program the port as either host or device
> + * according to the static configuration passed from devicetree.
> + * OTG and dual role are not yet supported!
> + */
> +static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
> +{
> +	u32 val;
> +	int err;
> +
> +	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
> +	if (err)
> +		return err;
> +
> +	switch (dwc3_data->dr_mode) {
> +	case USB_DR_MODE_PERIPHERAL:
> +		val |= USB_SET_PORT_DEVICE;
> +		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
> +		break;
> +
> +	case USB_DR_MODE_HOST:
> +		val &= USB_HOST_DEFAULT_MASK;

are you missing a ~ here ? Also, shouldn't you mask off the bits before
this switch ?

> +		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
> +		break;
> +
> +	default:
> +		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n",
> +			dwc3_data->dr_mode);
> +		return -EINVAL;
> +	}
> +
> +	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
> +}
> +
> +/**
> + * st_dwc3_init: init the controller via glue logic
> + * @dwc3_data: driver private structure
> + */
> +static void st_dwc3_init(struct st_dwc3 *dwc3_data)
> +{
> +

this blank line isn't necessary.

> +	u32 reg = st_dwc3_readl(dwc3_data->glue_base, CLKRST_CTRL);
> +
> +	reg |= AUX_CLK_EN | EXT_CFG_RESET_N | XHCI_REVISION;
> +	reg &= ~SW_PIPEW_RESET_N;
> +	st_dwc3_writel(dwc3_data->glue_base, CLKRST_CTRL, reg);
> +
> +	/* configure mux for vbus, powerpresent and bvalid signals */
> +	reg = st_dwc3_readl(dwc3_data->glue_base, USB2_VBUS_MNGMNT_SEL1);
> +
> +	reg |= SEL_OVERRIDE_VBUSVALID(USB2_VBUS_UTMIOTG) |
> +		SEL_OVERRIDE_POWERPRESENT(USB2_VBUS_UTMIOTG) |
> +		SEL_OVERRIDE_BVALID(USB2_VBUS_UTMIOTG);
> +
> +	st_dwc3_writel(dwc3_data->glue_base, USB2_VBUS_MNGMNT_SEL1, reg);
> +
> +	reg = st_dwc3_readl(dwc3_data->glue_base, CLKRST_CTRL);
> +	reg |= SW_PIPEW_RESET_N;
> +	st_dwc3_writel(dwc3_data->glue_base, CLKRST_CTRL, reg);
> +}
> +
> +static int st_dwc3_probe(struct platform_device *pdev)
> +{
> +	struct st_dwc3 *dwc3_data;
> +	struct resource *res;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node, *child;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	dwc3_data = devm_kzalloc(dev, sizeof(*dwc3_data), GFP_KERNEL);
> +	if (!dwc3_data)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg-glue");
> +	dwc3_data->glue_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(dwc3_data->glue_base))
> +		return PTR_ERR(dwc3_data->glue_base);
> +
> +	regmap = syscon_regmap_lookup_by_phandle(node, "st,syscfg");
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	dma_set_coherent_mask(dev, dev->coherent_dma_mask);
> +	dwc3_data->dev = dev;
> +	dwc3_data->regmap = regmap;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
> +	if (!res) {
> +		ret = -ENXIO;
> +		goto undo_platform_dev_alloc;
> +	}
> +
> +	dwc3_data->syscfg_reg_off = res->start;
> +
> +	dev_dbg(&pdev->dev, "glue-logic addr 0x%p, syscfg-reg offset 0x%x\n",
> +		dwc3_data->glue_base, dwc3_data->syscfg_reg_off);

looks like this message would be more of a dev_vdbg().

> +	dwc3_data->rstc_pwrdn = devm_reset_control_get(dev, "powerdown");
> +	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
> +		dev_err(&pdev->dev, "could not get power controller\n");
> +		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
> +		goto undo_platform_dev_alloc;
> +	}
> +
> +	/* Manage PowerDown */
> +	reset_control_deassert(dwc3_data->rstc_pwrdn);
> +
> +	dwc3_data->rstc_rst = devm_reset_control_get(dev, "softreset");
> +	if (IS_ERR(dwc3_data->rstc_rst)) {
> +		dev_err(&pdev->dev, "could not get reset controller\n");
> +		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
> +		goto undo_powerdown;
> +	}
> +
> +	/* Manage SoftReset */
> +	reset_control_deassert(dwc3_data->rstc_rst);
> +
> +	child = of_get_child_by_name(node, "dwc3");
> +	if (!child) {
> +		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
> +		ret = -ENODEV;
> +		goto undo_softreset;
> +	}
> +
> +	dwc3_data->dr_mode = of_usb_get_dr_mode(child);
> +
> +	/* Allocate and initialize the core */
> +	ret = of_platform_populate(node, NULL, NULL, dev);
> +	if (ret) {
> +		dev_err(dev, "failed to add dwc3 core\n");
> +		goto undo_softreset;
> +	}
> +
> +	/*
> +	 * Configure the USB port as device or host according to the static
> +	 * configuration passed from DT.
> +	 * DRD is the only mode currently supported so this will be enhanced
> +	 * as soon as OTG is available.
> +	 */
> +	ret = st_dwc3_drd_init(dwc3_data);
> +	if (ret) {
> +		dev_err(dev, "drd initialisation failed\n");
> +		goto undo_softreset;
> +	}
> +
> +	/* ST glue logic init */
> +	st_dwc3_init(dwc3_data);
> +
> +	platform_set_drvdata(pdev, dwc3_data);
> +	return 0;
> +
> +undo_softreset:
> +	reset_control_assert(dwc3_data->rstc_rst);
> +undo_powerdown:
> +	reset_control_assert(dwc3_data->rstc_pwrdn);
> +undo_platform_dev_alloc:
> +	platform_device_put(pdev);
> +	return ret;
> +}
> +
> +
> +static int st_dwc3_remove_child(struct device *dev, void *c)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +
> +	of_device_unregister(pdev);
> +
> +	return 0;
> +}
> +
> +static int st_dwc3_remove(struct platform_device *pdev)
> +{
> +	struct st_dwc3 *dwc3_data = platform_get_drvdata(pdev);
> +
> +	device_for_each_child(&pdev->dev, NULL, st_dwc3_remove_child);
> +
> +	reset_control_assert(dwc3_data->rstc_pwrdn);
> +	reset_control_assert(dwc3_data->rstc_rst);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int st_dwc3_suspend(struct device *dev)
> +{
> +	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
> +
> +	reset_control_assert(dwc3_data->rstc_pwrdn);
> +	reset_control_assert(dwc3_data->rstc_rst);

Two questions:

1) how would you handle the case when this device is a wakeup source ?
2) when resuming, wouldn't you have to reinitialize the entire core ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140820/d2dfdd1e/attachment-0001.sig>

Re: [PATCH v4 1/3] usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 11754 bytes --]

Hi,

On Wed, Jul 30, 2014 at 04:28:09PM +0100, Peter Griffin wrote:
> This patch adds the ST glue logic to manage the DWC3 HC
> on STiH407 SoC family. It manages the powerdown signal,
> and configures the internal glue logic and syscfg registers.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/usb/dwc3/Kconfig   |   9 ++
>  drivers/usb/dwc3/Makefile  |   1 +
>  drivers/usb/dwc3/dwc3-st.c | 336 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 346 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-st.c
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 8eb996e..6c85c43 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -79,6 +79,15 @@ config USB_DWC3_KEYSTONE
>  	  Support of USB2/3 functionality in TI Keystone2 platforms.
>  	  Say 'Y' or 'M' here if you have one such device
>  
> +config USB_DWC3_ST
> +	tristate "STMicroelectronics Platforms"
> +	depends on ARCH_STI && OF
> +	default USB_DWC3_HOST

this seems wrong as USB_DWC3_{HOST,GADGET,DUAL_ROLE} are booleans and
USB_DWC3_ST is a tristate. Better to stick with defaulting to USB_DWC3
instead like all the others.

> +	help
> +	  STMicroelectronics SoCs with one DesignWare Core USB3 IP
> +	  inside (i.e. STiH407).
> +	  Say 'Y' or 'M' if you have one such device.
> +
>  comment "Debugging features"
>  
>  config USB_DWC3_DEBUG
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index 10ac3e7..11c9f54 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -33,3 +33,4 @@ obj-$(CONFIG_USB_DWC3_OMAP)		+= dwc3-omap.o
>  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_ST)		+= dwc3-st.o
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> new file mode 100644
> index 0000000..227698f
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -0,0 +1,336 @@
> +/**
> + * dwc3-st.c Support for dwc3 platform devices on ST Microelectronics platforms
> + *
> + * This is a small driver for the dwc3 to provide the glue logic
> + * to configure the controller. Tested on STi platforms.
> + *
> + * Copyright (C) 2014 Stmicroelectronics
> + *
> + * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> + * Contributors: Aymen Bouattay <aymen.bouattay@st.com>
> + *               Peter Griffin <peter.griffin@linaro.org>
> + *
> + * 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.
> + *
> + * Inspired by dwc3-omap.c and dwc3-exynos.c.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/usb/of.h>
> +
> +#include "core.h"
> +#include "io.h"
> +
> +/* glue registers */
> +#define CLKRST_CTRL		0x00
> +#define AUX_CLK_EN		BIT(0)
> +#define SW_PIPEW_RESET_N	BIT(4)
> +#define EXT_CFG_RESET_N		BIT(8)
> +/*
> + * 1'b0 : The host controller complies with the xHCI revision 0.96
> + * 1'b1 : The host controller complies with the xHCI revision 1.0
> + */
> +#define XHCI_REVISION		BIT(12)
> +
> +#define USB2_VBUS_MNGMNT_SEL1	0x2C
> +/*
> + * For all fields in USB2_VBUS_MNGMNT_SEL1
> + * 2’b00 : Override value from Reg 0x30 is selected
> + * 2’b01 : utmiotg_<signal_name> from usb3_top is selected
> + * 2’b10 : pipew_<signal_name> from PIPEW instance is selected
> + * 2’b11 : value is 1'b0
> + */
> +#define USB2_VBUS_REG30		0x0
> +#define USB2_VBUS_UTMIOTG	0x1
> +#define USB2_VBUS_PIPEW		0x2
> +#define USB2_VBUS_ZERO		0x3
> +
> +#define SEL_OVERRIDE_VBUSVALID(n)	(n << 0)
> +#define SEL_OVERRIDE_POWERPRESENT(n)	(n << 4)
> +#define SEL_OVERRIDE_BVALID(n)		(n << 8)
> +
> +/* Static DRD configuration */
> +#define USB_HOST_DEFAULT_MASK	0xffe
> +#define USB_SET_PORT_DEVICE	0x1
> +
> +/**
> + * struct st_dwc3 - dwc3-st driver private structure
> + * @dev:		device pointer
> + * @glue_base:		ioaddr for the glue registers
> + * @regmap:		regmap pointer for getting syscfg
> + * @syscfg_reg_off:	usb syscfg control offset
> + * @dr_mode:		drd static host/device config
> + * @rstc_pwrdn:		rest controller for powerdown signal
> + * @rstc_rst:		reset controller for softreset signal
> + */
> +
> +struct st_dwc3 {
> +	struct device *dev;
> +	void __iomem *glue_base;
> +	struct regmap *regmap;
> +	int syscfg_reg_off;
> +	enum usb_dr_mode dr_mode;
> +	struct reset_control *rstc_pwrdn;
> +	struct reset_control *rstc_rst;
> +};
> +
> +static inline u32 st_dwc3_readl(void __iomem *base, u32 offset)
> +{
> +	return readl_relaxed(base + offset);
> +}
> +
> +static inline void st_dwc3_writel(void __iomem *base, u32 offset, u32 value)
> +{
> +	writel_relaxed(value, base + offset);

why relaxed ?

> +}
> +
> +/**
> + * st_dwc3_drd_init: program the port
> + * @dwc3_data: driver private structure
> + * Description: this function is to program the port as either host or device
> + * according to the static configuration passed from devicetree.
> + * OTG and dual role are not yet supported!
> + */
> +static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
> +{
> +	u32 val;
> +	int err;
> +
> +	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
> +	if (err)
> +		return err;
> +
> +	switch (dwc3_data->dr_mode) {
> +	case USB_DR_MODE_PERIPHERAL:
> +		val |= USB_SET_PORT_DEVICE;
> +		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
> +		break;
> +
> +	case USB_DR_MODE_HOST:
> +		val &= USB_HOST_DEFAULT_MASK;

are you missing a ~ here ? Also, shouldn't you mask off the bits before
this switch ?

> +		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
> +		break;
> +
> +	default:
> +		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n",
> +			dwc3_data->dr_mode);
> +		return -EINVAL;
> +	}
> +
> +	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
> +}
> +
> +/**
> + * st_dwc3_init: init the controller via glue logic
> + * @dwc3_data: driver private structure
> + */
> +static void st_dwc3_init(struct st_dwc3 *dwc3_data)
> +{
> +

this blank line isn't necessary.

> +	u32 reg = st_dwc3_readl(dwc3_data->glue_base, CLKRST_CTRL);
> +
> +	reg |= AUX_CLK_EN | EXT_CFG_RESET_N | XHCI_REVISION;
> +	reg &= ~SW_PIPEW_RESET_N;
> +	st_dwc3_writel(dwc3_data->glue_base, CLKRST_CTRL, reg);
> +
> +	/* configure mux for vbus, powerpresent and bvalid signals */
> +	reg = st_dwc3_readl(dwc3_data->glue_base, USB2_VBUS_MNGMNT_SEL1);
> +
> +	reg |= SEL_OVERRIDE_VBUSVALID(USB2_VBUS_UTMIOTG) |
> +		SEL_OVERRIDE_POWERPRESENT(USB2_VBUS_UTMIOTG) |
> +		SEL_OVERRIDE_BVALID(USB2_VBUS_UTMIOTG);
> +
> +	st_dwc3_writel(dwc3_data->glue_base, USB2_VBUS_MNGMNT_SEL1, reg);
> +
> +	reg = st_dwc3_readl(dwc3_data->glue_base, CLKRST_CTRL);
> +	reg |= SW_PIPEW_RESET_N;
> +	st_dwc3_writel(dwc3_data->glue_base, CLKRST_CTRL, reg);
> +}
> +
> +static int st_dwc3_probe(struct platform_device *pdev)
> +{
> +	struct st_dwc3 *dwc3_data;
> +	struct resource *res;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node, *child;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	dwc3_data = devm_kzalloc(dev, sizeof(*dwc3_data), GFP_KERNEL);
> +	if (!dwc3_data)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg-glue");
> +	dwc3_data->glue_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(dwc3_data->glue_base))
> +		return PTR_ERR(dwc3_data->glue_base);
> +
> +	regmap = syscon_regmap_lookup_by_phandle(node, "st,syscfg");
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	dma_set_coherent_mask(dev, dev->coherent_dma_mask);
> +	dwc3_data->dev = dev;
> +	dwc3_data->regmap = regmap;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
> +	if (!res) {
> +		ret = -ENXIO;
> +		goto undo_platform_dev_alloc;
> +	}
> +
> +	dwc3_data->syscfg_reg_off = res->start;
> +
> +	dev_dbg(&pdev->dev, "glue-logic addr 0x%p, syscfg-reg offset 0x%x\n",
> +		dwc3_data->glue_base, dwc3_data->syscfg_reg_off);

looks like this message would be more of a dev_vdbg().

> +	dwc3_data->rstc_pwrdn = devm_reset_control_get(dev, "powerdown");
> +	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
> +		dev_err(&pdev->dev, "could not get power controller\n");
> +		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
> +		goto undo_platform_dev_alloc;
> +	}
> +
> +	/* Manage PowerDown */
> +	reset_control_deassert(dwc3_data->rstc_pwrdn);
> +
> +	dwc3_data->rstc_rst = devm_reset_control_get(dev, "softreset");
> +	if (IS_ERR(dwc3_data->rstc_rst)) {
> +		dev_err(&pdev->dev, "could not get reset controller\n");
> +		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
> +		goto undo_powerdown;
> +	}
> +
> +	/* Manage SoftReset */
> +	reset_control_deassert(dwc3_data->rstc_rst);
> +
> +	child = of_get_child_by_name(node, "dwc3");
> +	if (!child) {
> +		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
> +		ret = -ENODEV;
> +		goto undo_softreset;
> +	}
> +
> +	dwc3_data->dr_mode = of_usb_get_dr_mode(child);
> +
> +	/* Allocate and initialize the core */
> +	ret = of_platform_populate(node, NULL, NULL, dev);
> +	if (ret) {
> +		dev_err(dev, "failed to add dwc3 core\n");
> +		goto undo_softreset;
> +	}
> +
> +	/*
> +	 * Configure the USB port as device or host according to the static
> +	 * configuration passed from DT.
> +	 * DRD is the only mode currently supported so this will be enhanced
> +	 * as soon as OTG is available.
> +	 */
> +	ret = st_dwc3_drd_init(dwc3_data);
> +	if (ret) {
> +		dev_err(dev, "drd initialisation failed\n");
> +		goto undo_softreset;
> +	}
> +
> +	/* ST glue logic init */
> +	st_dwc3_init(dwc3_data);
> +
> +	platform_set_drvdata(pdev, dwc3_data);
> +	return 0;
> +
> +undo_softreset:
> +	reset_control_assert(dwc3_data->rstc_rst);
> +undo_powerdown:
> +	reset_control_assert(dwc3_data->rstc_pwrdn);
> +undo_platform_dev_alloc:
> +	platform_device_put(pdev);
> +	return ret;
> +}
> +
> +
> +static int st_dwc3_remove_child(struct device *dev, void *c)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +
> +	of_device_unregister(pdev);
> +
> +	return 0;
> +}
> +
> +static int st_dwc3_remove(struct platform_device *pdev)
> +{
> +	struct st_dwc3 *dwc3_data = platform_get_drvdata(pdev);
> +
> +	device_for_each_child(&pdev->dev, NULL, st_dwc3_remove_child);
> +
> +	reset_control_assert(dwc3_data->rstc_pwrdn);
> +	reset_control_assert(dwc3_data->rstc_rst);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int st_dwc3_suspend(struct device *dev)
> +{
> +	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
> +
> +	reset_control_assert(dwc3_data->rstc_pwrdn);
> +	reset_control_assert(dwc3_data->rstc_rst);

Two questions:

1) how would you handle the case when this device is a wakeup source ?
2) when resuming, wouldn't you have to reinitialize the entire core ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v4 1/3] usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC

[Peter Griffin]

Hi Felipe,

Sorry for the delay in replying to this mail, I've been trying to get answers to the suspend/resume
questions you had.

> > +config USB_DWC3_ST
> > +	tristate "STMicroelectronics Platforms"
> > +	depends on ARCH_STI && OF
> > +	default USB_DWC3_HOST
> 
> this seems wrong as USB_DWC3_{HOST,GADGET,DUAL_ROLE} are booleans and
> USB_DWC3_ST is a tristate. Better to stick with defaulting to USB_DWC3
> instead like all the others.

Ok will fix.

> > +static inline void st_dwc3_writel(void __iomem *base, u32 offset, u32 value)
> > +{
> > +	writel_relaxed(value, base + offset);
> 
> why relaxed ?

The writel and readl implementations on ARM are quite expensive.

The writel, does a memory barrier, and also a outer_sync(), which involves taking a spinlock, and draining the cache store buffers.
The readl also does a memory barrier.

These barriers / cache operations are unnecessary here as the peripheral memory has been ioremap'ed as device memory, and
it is only one device we are writing to, so the writel/readl_relaxed variants are good enough as the ARM arch guarentees they
will arrive in program order.

There is some more info about this here http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117658

Note: It's only possible when we know the driver is not being used on other architectures which may have different constraints.
However for this driver, we know this IP (st glue logic) has only been used on ARM based SoC's.

> 
> > +}
> > +
> > +/**
> > + * st_dwc3_drd_init: program the port
> > + * @dwc3_data: driver private structure
> > + * Description: this function is to program the port as either host or device
> > + * according to the static configuration passed from devicetree.
> > + * OTG and dual role are not yet supported!
> > + */
> > +static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
> > +{
> > +	u32 val;
> > +	int err;
> > +
> > +	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
> > +	if (err)
> > +		return err;
> > +
> > +	switch (dwc3_data->dr_mode) {
> > +	case USB_DR_MODE_PERIPHERAL:
> > +		val |= USB_SET_PORT_DEVICE;
> > +		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
> > +		break;
> > +
> > +	case USB_DR_MODE_HOST:
> > +		val &= USB_HOST_DEFAULT_MASK;
> 
> are you missing a ~ here ? Also, shouldn't you mask off the bits before
> this switch ?

Yes your right, good spot! In the next iteration I've defined macros for the bits in
this control register and explitcitly set/clear them for both cases, also adding a comment regarding
the USB3_DELAY_VBUSVALID bit.

By chance host mode still worked with this bug present as the reset value of the register on 
this SoC is OK to have working host mode.

> 
> > +		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
> > +		break;
> > +
> > +	default:
> > +		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n",
> > +			dwc3_data->dr_mode);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
> > +}
> > +
> > +/**
> > + * st_dwc3_init: init the controller via glue logic
> > + * @dwc3_data: driver private structure
> > + */
> > +static void st_dwc3_init(struct st_dwc3 *dwc3_data)
> > +{
> > +
> 
> this blank line isn't necessary.

Ok, removed in next iteration

<snip>

> > +
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
> > +	if (!res) {
> > +		ret = -ENXIO;
> > +		goto undo_platform_dev_alloc;
> > +	}
> > +
> > +	dwc3_data->syscfg_reg_off = res->start;
> > +
> > +	dev_dbg(&pdev->dev, "glue-logic addr 0x%p, syscfg-reg offset 0x%x\n",
> > +		dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
> 
> looks like this message would be more of a dev_vdbg().

Ok, changed to dev_vdbg in next iteration

<snip>

> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int st_dwc3_suspend(struct device *dev)
> > +{
> > +	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
> > +
> > +	reset_control_assert(dwc3_data->rstc_pwrdn);
> > +	reset_control_assert(dwc3_data->rstc_rst);
> 
> Two questions:
> 
> 1) how would you handle the case when this device is a wakeup source ?

I've confirmed with ST the usb3 IP can't be a wakeup source on this SoC.

> 2) when resuming, wouldn't you have to reinitialize the entire core ?

I asked ST to test this, as a full working suspend / resume setup involves some firmware for the
standby controller which I don't currently have access to (and it is only with the SBC running that all
power will be removed from this part of the SoC). They have confirmed that the usb3 port works after a suspend / resume
and devices are correctly enumerated etc after a resume with the code as it was submitted.

What I did notice though after re-reading it, is that we are not re-configuring the ST glue logic registers
on a resume. So the controller could end up with the vbus mux configured differently. So in the next iteration I've
fixed this, and call st_dwc3_drd_init and st_dwc3_init in the resume path.

Although ST confirmed that suspend / resume works with or without this change applied.

regards,

Peter.

[PATCH v4 1/3] usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC

[Peter Griffin]

Hi Felipe,

Sorry for the delay in replying to this mail, I've been trying to get answers to the suspend/resume
questions you had.

> > +config USB_DWC3_ST
> > +	tristate "STMicroelectronics Platforms"
> > +	depends on ARCH_STI && OF
> > +	default USB_DWC3_HOST
> 
> this seems wrong as USB_DWC3_{HOST,GADGET,DUAL_ROLE} are booleans and
> USB_DWC3_ST is a tristate. Better to stick with defaulting to USB_DWC3
> instead like all the others.

Ok will fix.

> > +static inline void st_dwc3_writel(void __iomem *base, u32 offset, u32 value)
> > +{
> > +	writel_relaxed(value, base + offset);
> 
> why relaxed ?

The writel and readl implementations on ARM are quite expensive.

The writel, does a memory barrier, and also a outer_sync(), which involves taking a spinlock, and draining the cache store buffers.
The readl also does a memory barrier.

These barriers / cache operations are unnecessary here as the peripheral memory has been ioremap'ed as device memory, and
it is only one device we are writing to, so the writel/readl_relaxed variants are good enough as the ARM arch guarentees they
will arrive in program order.

There is some more info about this here http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117658

Note: It's only possible when we know the driver is not being used on other architectures which may have different constraints.
However for this driver, we know this IP (st glue logic) has only been used on ARM based SoC's.

> 
> > +}
> > +
> > +/**
> > + * st_dwc3_drd_init: program the port
> > + * @dwc3_data: driver private structure
> > + * Description: this function is to program the port as either host or device
> > + * according to the static configuration passed from devicetree.
> > + * OTG and dual role are not yet supported!
> > + */
> > +static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
> > +{
> > +	u32 val;
> > +	int err;
> > +
> > +	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
> > +	if (err)
> > +		return err;
> > +
> > +	switch (dwc3_data->dr_mode) {
> > +	case USB_DR_MODE_PERIPHERAL:
> > +		val |= USB_SET_PORT_DEVICE;
> > +		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
> > +		break;
> > +
> > +	case USB_DR_MODE_HOST:
> > +		val &= USB_HOST_DEFAULT_MASK;
> 
> are you missing a ~ here ? Also, shouldn't you mask off the bits before
> this switch ?

Yes your right, good spot! In the next iteration I've defined macros for the bits in
this control register and explitcitly set/clear them for both cases, also adding a comment regarding
the USB3_DELAY_VBUSVALID bit.

By chance host mode still worked with this bug present as the reset value of the register on 
this SoC is OK to have working host mode.

> 
> > +		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
> > +		break;
> > +
> > +	default:
> > +		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n",
> > +			dwc3_data->dr_mode);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
> > +}
> > +
> > +/**
> > + * st_dwc3_init: init the controller via glue logic
> > + * @dwc3_data: driver private structure
> > + */
> > +static void st_dwc3_init(struct st_dwc3 *dwc3_data)
> > +{
> > +
> 
> this blank line isn't necessary.

Ok, removed in next iteration

<snip>

> > +
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
> > +	if (!res) {
> > +		ret = -ENXIO;
> > +		goto undo_platform_dev_alloc;
> > +	}
> > +
> > +	dwc3_data->syscfg_reg_off = res->start;
> > +
> > +	dev_dbg(&pdev->dev, "glue-logic addr 0x%p, syscfg-reg offset 0x%x\n",
> > +		dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
> 
> looks like this message would be more of a dev_vdbg().

Ok, changed to dev_vdbg in next iteration

<snip>

> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int st_dwc3_suspend(struct device *dev)
> > +{
> > +	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
> > +
> > +	reset_control_assert(dwc3_data->rstc_pwrdn);
> > +	reset_control_assert(dwc3_data->rstc_rst);
> 
> Two questions:
> 
> 1) how would you handle the case when this device is a wakeup source ?

I've confirmed with ST the usb3 IP can't be a wakeup source on this SoC.

> 2) when resuming, wouldn't you have to reinitialize the entire core ?

I asked ST to test this, as a full working suspend / resume setup involves some firmware for the
standby controller which I don't currently have access to (and it is only with the SBC running that all
power will be removed from this part of the SoC). They have confirmed that the usb3 port works after a suspend / resume
and devices are correctly enumerated etc after a resume with the code as it was submitted.

What I did notice though after re-reading it, is that we are not re-configuring the ST glue logic registers
on a resume. So the controller could end up with the vbus mux configured differently. So in the next iteration I've
fixed this, and call st_dwc3_drd_init and st_dwc3_init in the resume path.

Although ST confirmed that suspend / resume works with or without this change applied.

regards,

Peter.

Re: [PATCH v4 1/3] usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC

[Peter Griffin]

Hi Felipe,

Sorry for the delay in replying to this mail, I've been trying to get answers to the suspend/resume
questions you had.

> > +config USB_DWC3_ST
> > +	tristate "STMicroelectronics Platforms"
> > +	depends on ARCH_STI && OF
> > +	default USB_DWC3_HOST
> 
> this seems wrong as USB_DWC3_{HOST,GADGET,DUAL_ROLE} are booleans and
> USB_DWC3_ST is a tristate. Better to stick with defaulting to USB_DWC3
> instead like all the others.

Ok will fix.

> > +static inline void st_dwc3_writel(void __iomem *base, u32 offset, u32 value)
> > +{
> > +	writel_relaxed(value, base + offset);
> 
> why relaxed ?

The writel and readl implementations on ARM are quite expensive.

The writel, does a memory barrier, and also a outer_sync(), which involves taking a spinlock, and draining the cache store buffers.
The readl also does a memory barrier.

These barriers / cache operations are unnecessary here as the peripheral memory has been ioremap'ed as device memory, and
it is only one device we are writing to, so the writel/readl_relaxed variants are good enough as the ARM arch guarentees they
will arrive in program order.

There is some more info about this here http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117658

Note: It's only possible when we know the driver is not being used on other architectures which may have different constraints.
However for this driver, we know this IP (st glue logic) has only been used on ARM based SoC's.

> 
> > +}
> > +
> > +/**
> > + * st_dwc3_drd_init: program the port
> > + * @dwc3_data: driver private structure
> > + * Description: this function is to program the port as either host or device
> > + * according to the static configuration passed from devicetree.
> > + * OTG and dual role are not yet supported!
> > + */
> > +static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
> > +{
> > +	u32 val;
> > +	int err;
> > +
> > +	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
> > +	if (err)
> > +		return err;
> > +
> > +	switch (dwc3_data->dr_mode) {
> > +	case USB_DR_MODE_PERIPHERAL:
> > +		val |= USB_SET_PORT_DEVICE;
> > +		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
> > +		break;
> > +
> > +	case USB_DR_MODE_HOST:
> > +		val &= USB_HOST_DEFAULT_MASK;
> 
> are you missing a ~ here ? Also, shouldn't you mask off the bits before
> this switch ?

Yes your right, good spot! In the next iteration I've defined macros for the bits in
this control register and explitcitly set/clear them for both cases, also adding a comment regarding
the USB3_DELAY_VBUSVALID bit.

By chance host mode still worked with this bug present as the reset value of the register on 
this SoC is OK to have working host mode.

> 
> > +		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
> > +		break;
> > +
> > +	default:
> > +		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n",
> > +			dwc3_data->dr_mode);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
> > +}
> > +
> > +/**
> > + * st_dwc3_init: init the controller via glue logic
> > + * @dwc3_data: driver private structure
> > + */
> > +static void st_dwc3_init(struct st_dwc3 *dwc3_data)
> > +{
> > +
> 
> this blank line isn't necessary.

Ok, removed in next iteration

<snip>

> > +
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
> > +	if (!res) {
> > +		ret = -ENXIO;
> > +		goto undo_platform_dev_alloc;
> > +	}
> > +
> > +	dwc3_data->syscfg_reg_off = res->start;
> > +
> > +	dev_dbg(&pdev->dev, "glue-logic addr 0x%p, syscfg-reg offset 0x%x\n",
> > +		dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
> 
> looks like this message would be more of a dev_vdbg().

Ok, changed to dev_vdbg in next iteration

<snip>

> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int st_dwc3_suspend(struct device *dev)
> > +{
> > +	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
> > +
> > +	reset_control_assert(dwc3_data->rstc_pwrdn);
> > +	reset_control_assert(dwc3_data->rstc_rst);
> 
> Two questions:
> 
> 1) how would you handle the case when this device is a wakeup source ?

I've confirmed with ST the usb3 IP can't be a wakeup source on this SoC.

> 2) when resuming, wouldn't you have to reinitialize the entire core ?

I asked ST to test this, as a full working suspend / resume setup involves some firmware for the
standby controller which I don't currently have access to (and it is only with the SBC running that all
power will be removed from this part of the SoC). They have confirmed that the usb3 port works after a suspend / resume
and devices are correctly enumerated etc after a resume with the code as it was submitted.

What I did notice though after re-reading it, is that we are not re-configuring the ST glue logic registers
on a resume. So the controller could end up with the vbus mux configured differently. So in the next iteration I've
fixed this, and call st_dwc3_drd_init and st_dwc3_init in the resume path.

Although ST confirmed that suspend / resume works with or without this change applied.

regards,

Peter.



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/3] usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 5704 bytes --]

Hi,

On Tue, Sep 02, 2014 at 12:18:12PM +0100, Peter Griffin wrote:
> Hi Felipe,
> 
> Sorry for the delay in replying to this mail, I've been trying to get
> answers to the suspend/resume questions you had.

np

> > > +config USB_DWC3_ST
> > > +	tristate "STMicroelectronics Platforms"
> > > +	depends on ARCH_STI && OF
> > > +	default USB_DWC3_HOST
> > 
> > this seems wrong as USB_DWC3_{HOST,GADGET,DUAL_ROLE} are booleans and
> > USB_DWC3_ST is a tristate. Better to stick with defaulting to USB_DWC3
> > instead like all the others.
> 
> Ok will fix.

tks

> > > +static inline void st_dwc3_writel(void __iomem *base, u32 offset, u32 value)
> > > +{
> > > +	writel_relaxed(value, base + offset);
> > 
> > why relaxed ?
> 
> The writel and readl implementations on ARM are quite expensive.
> 
> The writel, does a memory barrier, and also a outer_sync(), which
> involves taking a spinlock, and draining the cache store buffers.
> The readl also does a memory barrier.
> 
> These barriers / cache operations are unnecessary here as the
> peripheral memory has been ioremap'ed as device memory, and it is only
> one device we are writing to, so the writel/readl_relaxed variants are
> good enough as the ARM arch guarentees they will arrive in program
> order.

good point :-)

> There is some more info about this here
> http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117658
> 
> Note: It's only possible when we know the driver is not being used on
> other architectures which may have different constraints.
> However for this driver, we know this IP (st glue logic) has only been
> used on ARM based SoC's.

alright :-)

> > > +}
> > > +
> > > +/**
> > > + * st_dwc3_drd_init: program the port
> > > + * @dwc3_data: driver private structure
> > > + * Description: this function is to program the port as either host or device
> > > + * according to the static configuration passed from devicetree.
> > > + * OTG and dual role are not yet supported!
> > > + */
> > > +static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
> > > +{
> > > +	u32 val;
> > > +	int err;
> > > +
> > > +	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
> > > +	if (err)
> > > +		return err;
> > > +
> > > +	switch (dwc3_data->dr_mode) {
> > > +	case USB_DR_MODE_PERIPHERAL:
> > > +		val |= USB_SET_PORT_DEVICE;
> > > +		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
> > > +		break;
> > > +
> > > +	case USB_DR_MODE_HOST:
> > > +		val &= USB_HOST_DEFAULT_MASK;
> > 
> > are you missing a ~ here ? Also, shouldn't you mask off the bits before
> > this switch ?
> 
> Yes your right, good spot! In the next iteration I've defined macros
> for the bits in this control register and explitcitly set/clear them
> for both cases, also adding a comment regarding the
> USB3_DELAY_VBUSVALID bit.

ok, cool.

> By chance host mode still worked with this bug present as the reset
> value of the register on this SoC is OK to have working host mode.

heh :-)

> > > +		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
> > > +		break;
> > > +
> > > +	default:
> > > +		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n",
> > > +			dwc3_data->dr_mode);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
> > > +}
> > > +
> > > +/**
> > > + * st_dwc3_init: init the controller via glue logic
> > > + * @dwc3_data: driver private structure
> > > + */
> > > +static void st_dwc3_init(struct st_dwc3 *dwc3_data)
> > > +{
> > > +
> > 
> > this blank line isn't necessary.
> 
> Ok, removed in next iteration
> 
> <snip>
> 
> > > +
> > > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
> > > +	if (!res) {
> > > +		ret = -ENXIO;
> > > +		goto undo_platform_dev_alloc;
> > > +	}
> > > +
> > > +	dwc3_data->syscfg_reg_off = res->start;
> > > +
> > > +	dev_dbg(&pdev->dev, "glue-logic addr 0x%p, syscfg-reg offset 0x%x\n",
> > > +		dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
> > 
> > looks like this message would be more of a dev_vdbg().
> 
> Ok, changed to dev_vdbg in next iteration
> 
> <snip>
> 
> > > +
> > > +#ifdef CONFIG_PM_SLEEP
> > > +static int st_dwc3_suspend(struct device *dev)
> > > +{
> > > +	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
> > > +
> > > +	reset_control_assert(dwc3_data->rstc_pwrdn);
> > > +	reset_control_assert(dwc3_data->rstc_rst);
> > 
> > Two questions:
> > 
> > 1) how would you handle the case when this device is a wakeup source ?
> 
> I've confirmed with ST the usb3 IP can't be a wakeup source on this SoC.
> 
> > 2) when resuming, wouldn't you have to reinitialize the entire core ?
> 
> I asked ST to test this, as a full working suspend / resume setup
> involves some firmware for the standby controller which I don't
> currently have access to (and it is only with the SBC running that all
> power will be removed from this part of the SoC). They have confirmed
> that the usb3 port works after a suspend / resume and devices are
> correctly enumerated etc after a resume with the code as it was
> submitted.
> 
> What I did notice though after re-reading it, is that we are not
> re-configuring the ST glue logic registers on a resume. So the
> controller could end up with the vbus mux configured differently. So
> in the next iteration I've fixed this, and call st_dwc3_drd_init and
> st_dwc3_init in the resume path.
> 
> Although ST confirmed that suspend / resume works with or without this
> change applied.

alright, thanks a lot for confirming.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 1/3] usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC

[Felipe Balbi]

Hi,

On Tue, Sep 02, 2014 at 12:18:12PM +0100, Peter Griffin wrote:
> Hi Felipe,
> 
> Sorry for the delay in replying to this mail, I've been trying to get
> answers to the suspend/resume questions you had.

np

> > > +config USB_DWC3_ST
> > > +	tristate "STMicroelectronics Platforms"
> > > +	depends on ARCH_STI && OF
> > > +	default USB_DWC3_HOST
> > 
> > this seems wrong as USB_DWC3_{HOST,GADGET,DUAL_ROLE} are booleans and
> > USB_DWC3_ST is a tristate. Better to stick with defaulting to USB_DWC3
> > instead like all the others.
> 
> Ok will fix.

tks

> > > +static inline void st_dwc3_writel(void __iomem *base, u32 offset, u32 value)
> > > +{
> > > +	writel_relaxed(value, base + offset);
> > 
> > why relaxed ?
> 
> The writel and readl implementations on ARM are quite expensive.
> 
> The writel, does a memory barrier, and also a outer_sync(), which
> involves taking a spinlock, and draining the cache store buffers.
> The readl also does a memory barrier.
> 
> These barriers / cache operations are unnecessary here as the
> peripheral memory has been ioremap'ed as device memory, and it is only
> one device we are writing to, so the writel/readl_relaxed variants are
> good enough as the ARM arch guarentees they will arrive in program
> order.

good point :-)

> There is some more info about this here
> http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117658
> 
> Note: It's only possible when we know the driver is not being used on
> other architectures which may have different constraints.
> However for this driver, we know this IP (st glue logic) has only been
> used on ARM based SoC's.

alright :-)

> > > +}
> > > +
> > > +/**
> > > + * st_dwc3_drd_init: program the port
> > > + * @dwc3_data: driver private structure
> > > + * Description: this function is to program the port as either host or device
> > > + * according to the static configuration passed from devicetree.
> > > + * OTG and dual role are not yet supported!
> > > + */
> > > +static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
> > > +{
> > > +	u32 val;
> > > +	int err;
> > > +
> > > +	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
> > > +	if (err)
> > > +		return err;
> > > +
> > > +	switch (dwc3_data->dr_mode) {
> > > +	case USB_DR_MODE_PERIPHERAL:
> > > +		val |= USB_SET_PORT_DEVICE;
> > > +		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
> > > +		break;
> > > +
> > > +	case USB_DR_MODE_HOST:
> > > +		val &= USB_HOST_DEFAULT_MASK;
> > 
> > are you missing a ~ here ? Also, shouldn't you mask off the bits before
> > this switch ?
> 
> Yes your right, good spot! In the next iteration I've defined macros
> for the bits in this control register and explitcitly set/clear them
> for both cases, also adding a comment regarding the
> USB3_DELAY_VBUSVALID bit.

ok, cool.

> By chance host mode still worked with this bug present as the reset
> value of the register on this SoC is OK to have working host mode.

heh :-)

> > > +		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
> > > +		break;
> > > +
> > > +	default:
> > > +		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n",
> > > +			dwc3_data->dr_mode);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
> > > +}
> > > +
> > > +/**
> > > + * st_dwc3_init: init the controller via glue logic
> > > + * @dwc3_data: driver private structure
> > > + */
> > > +static void st_dwc3_init(struct st_dwc3 *dwc3_data)
> > > +{
> > > +
> > 
> > this blank line isn't necessary.
> 
> Ok, removed in next iteration
> 
> <snip>
> 
> > > +
> > > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
> > > +	if (!res) {
> > > +		ret = -ENXIO;
> > > +		goto undo_platform_dev_alloc;
> > > +	}
> > > +
> > > +	dwc3_data->syscfg_reg_off = res->start;
> > > +
> > > +	dev_dbg(&pdev->dev, "glue-logic addr 0x%p, syscfg-reg offset 0x%x\n",
> > > +		dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
> > 
> > looks like this message would be more of a dev_vdbg().
> 
> Ok, changed to dev_vdbg in next iteration
> 
> <snip>
> 
> > > +
> > > +#ifdef CONFIG_PM_SLEEP
> > > +static int st_dwc3_suspend(struct device *dev)
> > > +{
> > > +	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
> > > +
> > > +	reset_control_assert(dwc3_data->rstc_pwrdn);
> > > +	reset_control_assert(dwc3_data->rstc_rst);
> > 
> > Two questions:
> > 
> > 1) how would you handle the case when this device is a wakeup source ?
> 
> I've confirmed with ST the usb3 IP can't be a wakeup source on this SoC.
> 
> > 2) when resuming, wouldn't you have to reinitialize the entire core ?
> 
> I asked ST to test this, as a full working suspend / resume setup
> involves some firmware for the standby controller which I don't
> currently have access to (and it is only with the SBC running that all
> power will be removed from this part of the SoC). They have confirmed
> that the usb3 port works after a suspend / resume and devices are
> correctly enumerated etc after a resume with the code as it was
> submitted.
> 
> What I did notice though after re-reading it, is that we are not
> re-configuring the ST glue logic registers on a resume. So the
> controller could end up with the vbus mux configured differently. So
> in the next iteration I've fixed this, and call st_dwc3_drd_init and
> st_dwc3_init in the resume path.
> 
> Although ST confirmed that suspend / resume works with or without this
> change applied.

alright, thanks a lot for confirming.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140902/538b0d81/attachment.sig>

Re: [PATCH v4 1/3] usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 5704 bytes --]

Hi,

On Tue, Sep 02, 2014 at 12:18:12PM +0100, Peter Griffin wrote:
> Hi Felipe,
> 
> Sorry for the delay in replying to this mail, I've been trying to get
> answers to the suspend/resume questions you had.

np

> > > +config USB_DWC3_ST
> > > +	tristate "STMicroelectronics Platforms"
> > > +	depends on ARCH_STI && OF
> > > +	default USB_DWC3_HOST
> > 
> > this seems wrong as USB_DWC3_{HOST,GADGET,DUAL_ROLE} are booleans and
> > USB_DWC3_ST is a tristate. Better to stick with defaulting to USB_DWC3
> > instead like all the others.
> 
> Ok will fix.

tks

> > > +static inline void st_dwc3_writel(void __iomem *base, u32 offset, u32 value)
> > > +{
> > > +	writel_relaxed(value, base + offset);
> > 
> > why relaxed ?
> 
> The writel and readl implementations on ARM are quite expensive.
> 
> The writel, does a memory barrier, and also a outer_sync(), which
> involves taking a spinlock, and draining the cache store buffers.
> The readl also does a memory barrier.
> 
> These barriers / cache operations are unnecessary here as the
> peripheral memory has been ioremap'ed as device memory, and it is only
> one device we are writing to, so the writel/readl_relaxed variants are
> good enough as the ARM arch guarentees they will arrive in program
> order.

good point :-)

> There is some more info about this here
> http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117658
> 
> Note: It's only possible when we know the driver is not being used on
> other architectures which may have different constraints.
> However for this driver, we know this IP (st glue logic) has only been
> used on ARM based SoC's.

alright :-)

> > > +}
> > > +
> > > +/**
> > > + * st_dwc3_drd_init: program the port
> > > + * @dwc3_data: driver private structure
> > > + * Description: this function is to program the port as either host or device
> > > + * according to the static configuration passed from devicetree.
> > > + * OTG and dual role are not yet supported!
> > > + */
> > > +static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
> > > +{
> > > +	u32 val;
> > > +	int err;
> > > +
> > > +	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
> > > +	if (err)
> > > +		return err;
> > > +
> > > +	switch (dwc3_data->dr_mode) {
> > > +	case USB_DR_MODE_PERIPHERAL:
> > > +		val |= USB_SET_PORT_DEVICE;
> > > +		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
> > > +		break;
> > > +
> > > +	case USB_DR_MODE_HOST:
> > > +		val &= USB_HOST_DEFAULT_MASK;
> > 
> > are you missing a ~ here ? Also, shouldn't you mask off the bits before
> > this switch ?
> 
> Yes your right, good spot! In the next iteration I've defined macros
> for the bits in this control register and explitcitly set/clear them
> for both cases, also adding a comment regarding the
> USB3_DELAY_VBUSVALID bit.

ok, cool.

> By chance host mode still worked with this bug present as the reset
> value of the register on this SoC is OK to have working host mode.

heh :-)

> > > +		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
> > > +		break;
> > > +
> > > +	default:
> > > +		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n",
> > > +			dwc3_data->dr_mode);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
> > > +}
> > > +
> > > +/**
> > > + * st_dwc3_init: init the controller via glue logic
> > > + * @dwc3_data: driver private structure
> > > + */
> > > +static void st_dwc3_init(struct st_dwc3 *dwc3_data)
> > > +{
> > > +
> > 
> > this blank line isn't necessary.
> 
> Ok, removed in next iteration
> 
> <snip>
> 
> > > +
> > > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
> > > +	if (!res) {
> > > +		ret = -ENXIO;
> > > +		goto undo_platform_dev_alloc;
> > > +	}
> > > +
> > > +	dwc3_data->syscfg_reg_off = res->start;
> > > +
> > > +	dev_dbg(&pdev->dev, "glue-logic addr 0x%p, syscfg-reg offset 0x%x\n",
> > > +		dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
> > 
> > looks like this message would be more of a dev_vdbg().
> 
> Ok, changed to dev_vdbg in next iteration
> 
> <snip>
> 
> > > +
> > > +#ifdef CONFIG_PM_SLEEP
> > > +static int st_dwc3_suspend(struct device *dev)
> > > +{
> > > +	struct st_dwc3 *dwc3_data = dev_get_drvdata(dev);
> > > +
> > > +	reset_control_assert(dwc3_data->rstc_pwrdn);
> > > +	reset_control_assert(dwc3_data->rstc_rst);
> > 
> > Two questions:
> > 
> > 1) how would you handle the case when this device is a wakeup source ?
> 
> I've confirmed with ST the usb3 IP can't be a wakeup source on this SoC.
> 
> > 2) when resuming, wouldn't you have to reinitialize the entire core ?
> 
> I asked ST to test this, as a full working suspend / resume setup
> involves some firmware for the standby controller which I don't
> currently have access to (and it is only with the SBC running that all
> power will be removed from this part of the SoC). They have confirmed
> that the usb3 port works after a suspend / resume and devices are
> correctly enumerated etc after a resume with the code as it was
> submitted.
> 
> What I did notice though after re-reading it, is that we are not
> re-configuring the ST glue logic registers on a resume. So the
> controller could end up with the vbus mux configured differently. So
> in the next iteration I've fixed this, and call st_dwc3_drd_init and
> st_dwc3_init in the resume path.
> 
> Although ST confirmed that suspend / resume works with or without this
> change applied.

alright, thanks a lot for confirming.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Peter Griffin]

This patch documents the device tree documentation required for
the ST usb3 controller glue layer found in STiH407 devices.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 Documentation/devicetree/bindings/usb/dwc3-st.txt | 69 +++++++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/dwc3-st.txt

diff --git a/Documentation/devicetree/bindings/usb/dwc3-st.txt b/Documentation/devicetree/bindings/usb/dwc3-st.txt
new file mode 100644
index 0000000..de3fea3
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/dwc3-st.txt
@@ -0,0 +1,69 @@
+ST DWC3 glue logic
+
+This file documents the parameters for the dwc3-st driver.
+This driver controls the glue logic used to configure the dwc3 core on
+STiH407 based platforms.
+
+Required properties:
+ - compatible	: must be "st,stih407-dwc3"
+ - reg		: glue logic base address and USB syscfg ctrl register offset
+ - reg-names	: should be "reg-glue" and "syscfg-reg"
+ - st,syscon	: should be phandle to system configuration node which
+		  encompasses the glue registers
+ - resets	: list of phandle and reset specifier pairs. There should be two entries, one
+		  for the powerdown and softreset lines of the usb3 IP
+ - reset-names	: list of reset signal names. Names should be "powerdown" and "softreset"
+See: Documentation/devicetree/bindings/reset/st,sti-powerdown.txt
+See: Documentation/devicetree/bindings/reset/reset.txt
+
+ - #address-cells, #size-cells : should be '1' if the device has sub-nodes
+                                 with 'reg' property
+
+ - pinctl-names	: A pinctrl state named "default" must be defined
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+
+ - pinctrl-0	: Pin control group
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+
+ - ranges	: allows valid 1:1 translation between child's address space and
+		  parent's address space
+
+Sub-nodes:
+The dwc3 core should be added as subnode to ST DWC3 glue as shown in the
+example below. The DT binding details of dwc3 can be found in:
+Documentation/devicetree/bindings/usb/dwc3.txt
+
+NB: The dr_mode property described in [1] is NOT optional for this driver, as the default value
+is "otg", which isn't supported by this SoC. Valid dr_mode values for dwc3-st are either "host"
+or "device".
+
+[1] Documentation/devicetree/bindings/usb/generic.txt
+
+Example:
+
+st_dwc3: dwc3@8f94000 {
+	status		= "disabled";
+	compatible	= "st,stih407-dwc3";
+	reg		= <0x08f94000 0x1000>, <0x110 0x4>;
+	reg-names	= "reg-glue", "syscfg-reg";
+	st,syscfg	= <&syscfg_core>;
+	resets		= <&powerdown STIH407_USB3_POWERDOWN>;
+			  <&softreset STIH407_MIPHY2_SOFTRESET>;
+	reset-names	= "powerdown",
+			  "softreset";
+	#address-cells	= <1>;
+	#size-cells	= <1>;
+	pinctrl-names	= "default";
+	pinctrl-0	= <&pinctrl_usb3>;
+	ranges;
+
+	dwc3: dwc3@9900000 {
+		compatible	= "snps,dwc3";
+		reg		= <0x09900000 0x100000>;
+		interrupts	= <GIC_SPI 155 IRQ_TYPE_NONE>;
+		dr_mode		= "host"
+		usb-phy		= <&usb3_phy>;
+		phy-names	= "usb2-phy";
+		phys		= <&usb2_picophy2>;
+	};
+};
-- 
1.9.1

[PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Peter Griffin]

This patch documents the device tree documentation required for
the ST usb3 controller glue layer found in STiH407 devices.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 Documentation/devicetree/bindings/usb/dwc3-st.txt | 69 +++++++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/dwc3-st.txt

diff --git a/Documentation/devicetree/bindings/usb/dwc3-st.txt b/Documentation/devicetree/bindings/usb/dwc3-st.txt
new file mode 100644
index 0000000..de3fea3
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/dwc3-st.txt
@@ -0,0 +1,69 @@
+ST DWC3 glue logic
+
+This file documents the parameters for the dwc3-st driver.
+This driver controls the glue logic used to configure the dwc3 core on
+STiH407 based platforms.
+
+Required properties:
+ - compatible	: must be "st,stih407-dwc3"
+ - reg		: glue logic base address and USB syscfg ctrl register offset
+ - reg-names	: should be "reg-glue" and "syscfg-reg"
+ - st,syscon	: should be phandle to system configuration node which
+		  encompasses the glue registers
+ - resets	: list of phandle and reset specifier pairs. There should be two entries, one
+		  for the powerdown and softreset lines of the usb3 IP
+ - reset-names	: list of reset signal names. Names should be "powerdown" and "softreset"
+See: Documentation/devicetree/bindings/reset/st,sti-powerdown.txt
+See: Documentation/devicetree/bindings/reset/reset.txt
+
+ - #address-cells, #size-cells : should be '1' if the device has sub-nodes
+                                 with 'reg' property
+
+ - pinctl-names	: A pinctrl state named "default" must be defined
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+
+ - pinctrl-0	: Pin control group
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+
+ - ranges	: allows valid 1:1 translation between child's address space and
+		  parent's address space
+
+Sub-nodes:
+The dwc3 core should be added as subnode to ST DWC3 glue as shown in the
+example below. The DT binding details of dwc3 can be found in:
+Documentation/devicetree/bindings/usb/dwc3.txt
+
+NB: The dr_mode property described in [1] is NOT optional for this driver, as the default value
+is "otg", which isn't supported by this SoC. Valid dr_mode values for dwc3-st are either "host"
+or "device".
+
+[1] Documentation/devicetree/bindings/usb/generic.txt
+
+Example:
+
+st_dwc3: dwc3 at 8f94000 {
+	status		= "disabled";
+	compatible	= "st,stih407-dwc3";
+	reg		= <0x08f94000 0x1000>, <0x110 0x4>;
+	reg-names	= "reg-glue", "syscfg-reg";
+	st,syscfg	= <&syscfg_core>;
+	resets		= <&powerdown STIH407_USB3_POWERDOWN>;
+			  <&softreset STIH407_MIPHY2_SOFTRESET>;
+	reset-names	= "powerdown",
+			  "softreset";
+	#address-cells	= <1>;
+	#size-cells	= <1>;
+	pinctrl-names	= "default";
+	pinctrl-0	= <&pinctrl_usb3>;
+	ranges;
+
+	dwc3: dwc3 at 9900000 {
+		compatible	= "snps,dwc3";
+		reg		= <0x09900000 0x100000>;
+		interrupts	= <GIC_SPI 155 IRQ_TYPE_NONE>;
+		dr_mode		= "host"
+		usb-phy		= <&usb3_phy>;
+		phy-names	= "usb2-phy";
+		phys		= <&usb2_picophy2>;
+	};
+};
-- 
1.9.1

[PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Peter Griffin]

This patch documents the device tree documentation required for
the ST usb3 controller glue layer found in STiH407 devices.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro-qxv4g6HH51o@public.gmane.org>
Signed-off-by: Peter Griffin <peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/dwc3-st.txt | 69 +++++++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/dwc3-st.txt

diff --git a/Documentation/devicetree/bindings/usb/dwc3-st.txt b/Documentation/devicetree/bindings/usb/dwc3-st.txt
new file mode 100644
index 0000000..de3fea3
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/dwc3-st.txt
@@ -0,0 +1,69 @@
+ST DWC3 glue logic
+
+This file documents the parameters for the dwc3-st driver.
+This driver controls the glue logic used to configure the dwc3 core on
+STiH407 based platforms.
+
+Required properties:
+ - compatible	: must be "st,stih407-dwc3"
+ - reg		: glue logic base address and USB syscfg ctrl register offset
+ - reg-names	: should be "reg-glue" and "syscfg-reg"
+ - st,syscon	: should be phandle to system configuration node which
+		  encompasses the glue registers
+ - resets	: list of phandle and reset specifier pairs. There should be two entries, one
+		  for the powerdown and softreset lines of the usb3 IP
+ - reset-names	: list of reset signal names. Names should be "powerdown" and "softreset"
+See: Documentation/devicetree/bindings/reset/st,sti-powerdown.txt
+See: Documentation/devicetree/bindings/reset/reset.txt
+
+ - #address-cells, #size-cells : should be '1' if the device has sub-nodes
+                                 with 'reg' property
+
+ - pinctl-names	: A pinctrl state named "default" must be defined
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+
+ - pinctrl-0	: Pin control group
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+
+ - ranges	: allows valid 1:1 translation between child's address space and
+		  parent's address space
+
+Sub-nodes:
+The dwc3 core should be added as subnode to ST DWC3 glue as shown in the
+example below. The DT binding details of dwc3 can be found in:
+Documentation/devicetree/bindings/usb/dwc3.txt
+
+NB: The dr_mode property described in [1] is NOT optional for this driver, as the default value
+is "otg", which isn't supported by this SoC. Valid dr_mode values for dwc3-st are either "host"
+or "device".
+
+[1] Documentation/devicetree/bindings/usb/generic.txt
+
+Example:
+
+st_dwc3: dwc3@8f94000 {
+	status		= "disabled";
+	compatible	= "st,stih407-dwc3";
+	reg		= <0x08f94000 0x1000>, <0x110 0x4>;
+	reg-names	= "reg-glue", "syscfg-reg";
+	st,syscfg	= <&syscfg_core>;
+	resets		= <&powerdown STIH407_USB3_POWERDOWN>;
+			  <&softreset STIH407_MIPHY2_SOFTRESET>;
+	reset-names	= "powerdown",
+			  "softreset";
+	#address-cells	= <1>;
+	#size-cells	= <1>;
+	pinctrl-names	= "default";
+	pinctrl-0	= <&pinctrl_usb3>;
+	ranges;
+
+	dwc3: dwc3@9900000 {
+		compatible	= "snps,dwc3";
+		reg		= <0x09900000 0x100000>;
+		interrupts	= <GIC_SPI 155 IRQ_TYPE_NONE>;
+		dr_mode		= "host"
+		usb-phy		= <&usb3_phy>;
+		phy-names	= "usb2-phy";
+		phys		= <&usb2_picophy2>;
+	};
+};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3707 bytes --]

On Wed, Jul 30, 2014 at 04:28:10PM +0100, Peter Griffin wrote:
> This patch documents the device tree documentation required for
> the ST usb3 controller glue layer found in STiH407 devices.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  Documentation/devicetree/bindings/usb/dwc3-st.txt | 69 +++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/dwc3-st.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/dwc3-st.txt b/Documentation/devicetree/bindings/usb/dwc3-st.txt
> new file mode 100644
> index 0000000..de3fea3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/dwc3-st.txt
> @@ -0,0 +1,69 @@
> +ST DWC3 glue logic
> +
> +This file documents the parameters for the dwc3-st driver.
> +This driver controls the glue logic used to configure the dwc3 core on
> +STiH407 based platforms.
> +
> +Required properties:
> + - compatible	: must be "st,stih407-dwc3"
> + - reg		: glue logic base address and USB syscfg ctrl register offset
> + - reg-names	: should be "reg-glue" and "syscfg-reg"
> + - st,syscon	: should be phandle to system configuration node which
> +		  encompasses the glue registers
> + - resets	: list of phandle and reset specifier pairs. There should be two entries, one
> +		  for the powerdown and softreset lines of the usb3 IP
> + - reset-names	: list of reset signal names. Names should be "powerdown" and "softreset"
> +See: Documentation/devicetree/bindings/reset/st,sti-powerdown.txt
> +See: Documentation/devicetree/bindings/reset/reset.txt
> +
> + - #address-cells, #size-cells : should be '1' if the device has sub-nodes
> +                                 with 'reg' property
> +
> + - pinctl-names	: A pinctrl state named "default" must be defined
> +See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
> +
> + - pinctrl-0	: Pin control group
> +See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
> +
> + - ranges	: allows valid 1:1 translation between child's address space and
> +		  parent's address space
> +
> +Sub-nodes:
> +The dwc3 core should be added as subnode to ST DWC3 glue as shown in the
> +example below. The DT binding details of dwc3 can be found in:
> +Documentation/devicetree/bindings/usb/dwc3.txt
> +
> +NB: The dr_mode property described in [1] is NOT optional for this driver, as the default value
> +is "otg", which isn't supported by this SoC. Valid dr_mode values for dwc3-st are either "host"
> +or "device".
> +
> +[1] Documentation/devicetree/bindings/usb/generic.txt
> +
> +Example:
> +
> +st_dwc3: dwc3@8f94000 {
> +	status		= "disabled";
> +	compatible	= "st,stih407-dwc3";
> +	reg		= <0x08f94000 0x1000>, <0x110 0x4>;
> +	reg-names	= "reg-glue", "syscfg-reg";
> +	st,syscfg	= <&syscfg_core>;
> +	resets		= <&powerdown STIH407_USB3_POWERDOWN>;
> +			  <&softreset STIH407_MIPHY2_SOFTRESET>;
> +	reset-names	= "powerdown",
> +			  "softreset";
> +	#address-cells	= <1>;
> +	#size-cells	= <1>;
> +	pinctrl-names	= "default";
> +	pinctrl-0	= <&pinctrl_usb3>;
> +	ranges;
> +
> +	dwc3: dwc3@9900000 {
> +		compatible	= "snps,dwc3";
> +		reg		= <0x09900000 0x100000>;
> +		interrupts	= <GIC_SPI 155 IRQ_TYPE_NONE>;
> +		dr_mode		= "host"
> +		usb-phy		= <&usb3_phy>;
> +		phy-names	= "usb2-phy";
> +		phys		= <&usb2_picophy2>;

why are you using different binding for usb2 and usb3 phys ? Why can't
you just:

	phys-names	= "usb2-phy", "usb3-phy";
	phys		= <&usb2_picophy2>, <&usb3_phy>;

??

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Felipe Balbi]

On Wed, Jul 30, 2014 at 04:28:10PM +0100, Peter Griffin wrote:
> This patch documents the device tree documentation required for
> the ST usb3 controller glue layer found in STiH407 devices.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  Documentation/devicetree/bindings/usb/dwc3-st.txt | 69 +++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/dwc3-st.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/dwc3-st.txt b/Documentation/devicetree/bindings/usb/dwc3-st.txt
> new file mode 100644
> index 0000000..de3fea3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/dwc3-st.txt
> @@ -0,0 +1,69 @@
> +ST DWC3 glue logic
> +
> +This file documents the parameters for the dwc3-st driver.
> +This driver controls the glue logic used to configure the dwc3 core on
> +STiH407 based platforms.
> +
> +Required properties:
> + - compatible	: must be "st,stih407-dwc3"
> + - reg		: glue logic base address and USB syscfg ctrl register offset
> + - reg-names	: should be "reg-glue" and "syscfg-reg"
> + - st,syscon	: should be phandle to system configuration node which
> +		  encompasses the glue registers
> + - resets	: list of phandle and reset specifier pairs. There should be two entries, one
> +		  for the powerdown and softreset lines of the usb3 IP
> + - reset-names	: list of reset signal names. Names should be "powerdown" and "softreset"
> +See: Documentation/devicetree/bindings/reset/st,sti-powerdown.txt
> +See: Documentation/devicetree/bindings/reset/reset.txt
> +
> + - #address-cells, #size-cells : should be '1' if the device has sub-nodes
> +                                 with 'reg' property
> +
> + - pinctl-names	: A pinctrl state named "default" must be defined
> +See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
> +
> + - pinctrl-0	: Pin control group
> +See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
> +
> + - ranges	: allows valid 1:1 translation between child's address space and
> +		  parent's address space
> +
> +Sub-nodes:
> +The dwc3 core should be added as subnode to ST DWC3 glue as shown in the
> +example below. The DT binding details of dwc3 can be found in:
> +Documentation/devicetree/bindings/usb/dwc3.txt
> +
> +NB: The dr_mode property described in [1] is NOT optional for this driver, as the default value
> +is "otg", which isn't supported by this SoC. Valid dr_mode values for dwc3-st are either "host"
> +or "device".
> +
> +[1] Documentation/devicetree/bindings/usb/generic.txt
> +
> +Example:
> +
> +st_dwc3: dwc3 at 8f94000 {
> +	status		= "disabled";
> +	compatible	= "st,stih407-dwc3";
> +	reg		= <0x08f94000 0x1000>, <0x110 0x4>;
> +	reg-names	= "reg-glue", "syscfg-reg";
> +	st,syscfg	= <&syscfg_core>;
> +	resets		= <&powerdown STIH407_USB3_POWERDOWN>;
> +			  <&softreset STIH407_MIPHY2_SOFTRESET>;
> +	reset-names	= "powerdown",
> +			  "softreset";
> +	#address-cells	= <1>;
> +	#size-cells	= <1>;
> +	pinctrl-names	= "default";
> +	pinctrl-0	= <&pinctrl_usb3>;
> +	ranges;
> +
> +	dwc3: dwc3 at 9900000 {
> +		compatible	= "snps,dwc3";
> +		reg		= <0x09900000 0x100000>;
> +		interrupts	= <GIC_SPI 155 IRQ_TYPE_NONE>;
> +		dr_mode		= "host"
> +		usb-phy		= <&usb3_phy>;
> +		phy-names	= "usb2-phy";
> +		phys		= <&usb2_picophy2>;

why are you using different binding for usb2 and usb3 phys ? Why can't
you just:

	phys-names	= "usb2-phy", "usb3-phy";
	phys		= <&usb2_picophy2>, <&usb3_phy>;

??

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140820/4887fdca/attachment.sig>

Re: [PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3787 bytes --]

On Wed, Jul 30, 2014 at 04:28:10PM +0100, Peter Griffin wrote:
> This patch documents the device tree documentation required for
> the ST usb3 controller glue layer found in STiH407 devices.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro-qxv4g6HH51o@public.gmane.org>
> Signed-off-by: Peter Griffin <peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/usb/dwc3-st.txt | 69 +++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/dwc3-st.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/dwc3-st.txt b/Documentation/devicetree/bindings/usb/dwc3-st.txt
> new file mode 100644
> index 0000000..de3fea3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/dwc3-st.txt
> @@ -0,0 +1,69 @@
> +ST DWC3 glue logic
> +
> +This file documents the parameters for the dwc3-st driver.
> +This driver controls the glue logic used to configure the dwc3 core on
> +STiH407 based platforms.
> +
> +Required properties:
> + - compatible	: must be "st,stih407-dwc3"
> + - reg		: glue logic base address and USB syscfg ctrl register offset
> + - reg-names	: should be "reg-glue" and "syscfg-reg"
> + - st,syscon	: should be phandle to system configuration node which
> +		  encompasses the glue registers
> + - resets	: list of phandle and reset specifier pairs. There should be two entries, one
> +		  for the powerdown and softreset lines of the usb3 IP
> + - reset-names	: list of reset signal names. Names should be "powerdown" and "softreset"
> +See: Documentation/devicetree/bindings/reset/st,sti-powerdown.txt
> +See: Documentation/devicetree/bindings/reset/reset.txt
> +
> + - #address-cells, #size-cells : should be '1' if the device has sub-nodes
> +                                 with 'reg' property
> +
> + - pinctl-names	: A pinctrl state named "default" must be defined
> +See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
> +
> + - pinctrl-0	: Pin control group
> +See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
> +
> + - ranges	: allows valid 1:1 translation between child's address space and
> +		  parent's address space
> +
> +Sub-nodes:
> +The dwc3 core should be added as subnode to ST DWC3 glue as shown in the
> +example below. The DT binding details of dwc3 can be found in:
> +Documentation/devicetree/bindings/usb/dwc3.txt
> +
> +NB: The dr_mode property described in [1] is NOT optional for this driver, as the default value
> +is "otg", which isn't supported by this SoC. Valid dr_mode values for dwc3-st are either "host"
> +or "device".
> +
> +[1] Documentation/devicetree/bindings/usb/generic.txt
> +
> +Example:
> +
> +st_dwc3: dwc3@8f94000 {
> +	status		= "disabled";
> +	compatible	= "st,stih407-dwc3";
> +	reg		= <0x08f94000 0x1000>, <0x110 0x4>;
> +	reg-names	= "reg-glue", "syscfg-reg";
> +	st,syscfg	= <&syscfg_core>;
> +	resets		= <&powerdown STIH407_USB3_POWERDOWN>;
> +			  <&softreset STIH407_MIPHY2_SOFTRESET>;
> +	reset-names	= "powerdown",
> +			  "softreset";
> +	#address-cells	= <1>;
> +	#size-cells	= <1>;
> +	pinctrl-names	= "default";
> +	pinctrl-0	= <&pinctrl_usb3>;
> +	ranges;
> +
> +	dwc3: dwc3@9900000 {
> +		compatible	= "snps,dwc3";
> +		reg		= <0x09900000 0x100000>;
> +		interrupts	= <GIC_SPI 155 IRQ_TYPE_NONE>;
> +		dr_mode		= "host"
> +		usb-phy		= <&usb3_phy>;
> +		phy-names	= "usb2-phy";
> +		phys		= <&usb2_picophy2>;

why are you using different binding for usb2 and usb3 phys ? Why can't
you just:

	phys-names	= "usb2-phy", "usb3-phy";
	phys		= <&usb2_picophy2>, <&usb3_phy>;

??

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Peter Griffin]

Hi Felipe,

Thanks for reviewing, see my comments below: -

On Wed, 20 Aug 2014, Felipe Balbi wrote:

> > +	dwc3: dwc3@9900000 {
> > +		compatible	= "snps,dwc3";
> > +		reg		= <0x09900000 0x100000>;
> > +		interrupts	= <GIC_SPI 155 IRQ_TYPE_NONE>;
> > +		dr_mode		= "host"
> > +		usb-phy		= <&usb3_phy>;
> > +		phy-names	= "usb2-phy";
> > +		phys		= <&usb2_picophy2>;
> 
> why are you using different binding for usb2 and usb3 phys ? Why can't
> you just:
> 
> 	phys-names	= "usb2-phy", "usb3-phy";
> 	phys		= <&usb2_picophy2>, <&usb3_phy>;
> 
> ??

Currently (in the vendor tree) one of the phys lives in drivers/usb/phy and the other in drivers/phy.
I believe that is because one is only a usb phy and the other is a multi function phy which can drive
PCI-E or USB3.

So to make that work, when dwc3/core.c gets the PHYS in dwc3_core_get_phy() we need to use the different
bindings.

I think we are the only platform using "one of each", but luckily dwc3_core_get_phy()
has been written generically enough that it "just works" :-).

regards,

Peter.

[PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Peter Griffin]

Hi Felipe,

Thanks for reviewing, see my comments below: -

On Wed, 20 Aug 2014, Felipe Balbi wrote:

> > +	dwc3: dwc3 at 9900000 {
> > +		compatible	= "snps,dwc3";
> > +		reg		= <0x09900000 0x100000>;
> > +		interrupts	= <GIC_SPI 155 IRQ_TYPE_NONE>;
> > +		dr_mode		= "host"
> > +		usb-phy		= <&usb3_phy>;
> > +		phy-names	= "usb2-phy";
> > +		phys		= <&usb2_picophy2>;
> 
> why are you using different binding for usb2 and usb3 phys ? Why can't
> you just:
> 
> 	phys-names	= "usb2-phy", "usb3-phy";
> 	phys		= <&usb2_picophy2>, <&usb3_phy>;
> 
> ??

Currently (in the vendor tree) one of the phys lives in drivers/usb/phy and the other in drivers/phy.
I believe that is because one is only a usb phy and the other is a multi function phy which can drive
PCI-E or USB3.

So to make that work, when dwc3/core.c gets the PHYS in dwc3_core_get_phy() we need to use the different
bindings.

I think we are the only platform using "one of each", but luckily dwc3_core_get_phy()
has been written generically enough that it "just works" :-).

regards,

Peter.

Re: [PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1430 bytes --]

On Thu, Aug 21, 2014 at 02:33:40PM +0100, Peter Griffin wrote:
> Hi Felipe,
> 
> Thanks for reviewing, see my comments below: -
> 
> On Wed, 20 Aug 2014, Felipe Balbi wrote:
> 
> > > +	dwc3: dwc3@9900000 {
> > > +		compatible	= "snps,dwc3";
> > > +		reg		= <0x09900000 0x100000>;
> > > +		interrupts	= <GIC_SPI 155 IRQ_TYPE_NONE>;
> > > +		dr_mode		= "host"
> > > +		usb-phy		= <&usb3_phy>;
> > > +		phy-names	= "usb2-phy";
> > > +		phys		= <&usb2_picophy2>;
> > 
> > why are you using different binding for usb2 and usb3 phys ? Why can't
> > you just:
> > 
> > 	phys-names	= "usb2-phy", "usb3-phy";
> > 	phys		= <&usb2_picophy2>, <&usb3_phy>;
> > 
> > ??
> 
> Currently (in the vendor tree) one of the phys lives in
> drivers/usb/phy and the other in drivers/phy.
> I believe that is because one is only a usb phy and the other is a
> multi function phy which can drive PCI-E or USB3.

right, but for mainline, we can have both PHYs in drivers/phy only.

> So to make that work, when dwc3/core.c gets the PHYS in
> dwc3_core_get_phy() we need to use the different bindings.
> 
> I think we are the only platform using "one of each", but luckily
> dwc3_core_get_phy() has been written generically enough that it "just
> works" :-).

true, but I want to drop support for the legacy drivers/usb/phy layer
from dwc3. I'll try to move all PHYs to drivers/phy for v3.18.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Felipe Balbi]

On Thu, Aug 21, 2014 at 02:33:40PM +0100, Peter Griffin wrote:
> Hi Felipe,
> 
> Thanks for reviewing, see my comments below: -
> 
> On Wed, 20 Aug 2014, Felipe Balbi wrote:
> 
> > > +	dwc3: dwc3 at 9900000 {
> > > +		compatible	= "snps,dwc3";
> > > +		reg		= <0x09900000 0x100000>;
> > > +		interrupts	= <GIC_SPI 155 IRQ_TYPE_NONE>;
> > > +		dr_mode		= "host"
> > > +		usb-phy		= <&usb3_phy>;
> > > +		phy-names	= "usb2-phy";
> > > +		phys		= <&usb2_picophy2>;
> > 
> > why are you using different binding for usb2 and usb3 phys ? Why can't
> > you just:
> > 
> > 	phys-names	= "usb2-phy", "usb3-phy";
> > 	phys		= <&usb2_picophy2>, <&usb3_phy>;
> > 
> > ??
> 
> Currently (in the vendor tree) one of the phys lives in
> drivers/usb/phy and the other in drivers/phy.
> I believe that is because one is only a usb phy and the other is a
> multi function phy which can drive PCI-E or USB3.

right, but for mainline, we can have both PHYs in drivers/phy only.

> So to make that work, when dwc3/core.c gets the PHYS in
> dwc3_core_get_phy() we need to use the different bindings.
> 
> I think we are the only platform using "one of each", but luckily
> dwc3_core_get_phy() has been written generically enough that it "just
> works" :-).

true, but I want to drop support for the legacy drivers/usb/phy layer
from dwc3. I'll try to move all PHYs to drivers/phy for v3.18.

cheers

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140821/300d80b2/attachment.sig>

Re: [PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1430 bytes --]

On Thu, Aug 21, 2014 at 02:33:40PM +0100, Peter Griffin wrote:
> Hi Felipe,
> 
> Thanks for reviewing, see my comments below: -
> 
> On Wed, 20 Aug 2014, Felipe Balbi wrote:
> 
> > > +	dwc3: dwc3@9900000 {
> > > +		compatible	= "snps,dwc3";
> > > +		reg		= <0x09900000 0x100000>;
> > > +		interrupts	= <GIC_SPI 155 IRQ_TYPE_NONE>;
> > > +		dr_mode		= "host"
> > > +		usb-phy		= <&usb3_phy>;
> > > +		phy-names	= "usb2-phy";
> > > +		phys		= <&usb2_picophy2>;
> > 
> > why are you using different binding for usb2 and usb3 phys ? Why can't
> > you just:
> > 
> > 	phys-names	= "usb2-phy", "usb3-phy";
> > 	phys		= <&usb2_picophy2>, <&usb3_phy>;
> > 
> > ??
> 
> Currently (in the vendor tree) one of the phys lives in
> drivers/usb/phy and the other in drivers/phy.
> I believe that is because one is only a usb phy and the other is a
> multi function phy which can drive PCI-E or USB3.

right, but for mainline, we can have both PHYs in drivers/phy only.

> So to make that work, when dwc3/core.c gets the PHYS in
> dwc3_core_get_phy() we need to use the different bindings.
> 
> I think we are the only platform using "one of each", but luckily
> dwc3_core_get_phy() has been written generically enough that it "just
> works" :-).

true, but I want to drop support for the legacy drivers/usb/phy layer
from dwc3. I'll try to move all PHYs to drivers/phy for v3.18.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Peter Griffin]

Hi Felipe,

On Thu, 21 Aug 2014, Felipe Balbi wrote:

> > Currently (in the vendor tree) one of the phys lives in
> > drivers/usb/phy and the other in drivers/phy.
> > I believe that is because one is only a usb phy and the other is a
> > multi function phy which can drive PCI-E or USB3.
> 
> right, but for mainline, we can have both PHYs in drivers/phy only.

Ah ok, I didn't know that, so thanks for that info.
> 
> > So to make that work, when dwc3/core.c gets the PHYS in
> > dwc3_core_get_phy() we need to use the different bindings.
> > 
> > I think we are the only platform using "one of each", but luckily
> > dwc3_core_get_phy() has been written generically enough that it "just
> > works" :-).
> 
> true, but I want to drop support for the legacy drivers/usb/phy layer
> from dwc3. I'll try to move all PHYs to drivers/phy for v3.18.

Ok, so for the next re-spin I will change this to just use the generic phy binding
for both usb2 & usb3 phys.

In parallel I will also migrate over the drivers/usb/phy driver to be just a generic phy
before I attempt to upstream it.

regards,

Peter

[PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Peter Griffin]

Hi Felipe,

On Thu, 21 Aug 2014, Felipe Balbi wrote:

> > Currently (in the vendor tree) one of the phys lives in
> > drivers/usb/phy and the other in drivers/phy.
> > I believe that is because one is only a usb phy and the other is a
> > multi function phy which can drive PCI-E or USB3.
> 
> right, but for mainline, we can have both PHYs in drivers/phy only.

Ah ok, I didn't know that, so thanks for that info.
> 
> > So to make that work, when dwc3/core.c gets the PHYS in
> > dwc3_core_get_phy() we need to use the different bindings.
> > 
> > I think we are the only platform using "one of each", but luckily
> > dwc3_core_get_phy() has been written generically enough that it "just
> > works" :-).
> 
> true, but I want to drop support for the legacy drivers/usb/phy layer
> from dwc3. I'll try to move all PHYs to drivers/phy for v3.18.

Ok, so for the next re-spin I will change this to just use the generic phy binding
for both usb2 & usb3 phys.

In parallel I will also migrate over the drivers/usb/phy driver to be just a generic phy
before I attempt to upstream it.

regards,

Peter

Re: [PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Peter Griffin]

Hi Felipe,

On Thu, 21 Aug 2014, Felipe Balbi wrote:

> > Currently (in the vendor tree) one of the phys lives in
> > drivers/usb/phy and the other in drivers/phy.
> > I believe that is because one is only a usb phy and the other is a
> > multi function phy which can drive PCI-E or USB3.
> 
> right, but for mainline, we can have both PHYs in drivers/phy only.

Ah ok, I didn't know that, so thanks for that info.
> 
> > So to make that work, when dwc3/core.c gets the PHYS in
> > dwc3_core_get_phy() we need to use the different bindings.
> > 
> > I think we are the only platform using "one of each", but luckily
> > dwc3_core_get_phy() has been written generically enough that it "just
> > works" :-).
> 
> true, but I want to drop support for the legacy drivers/usb/phy layer
> from dwc3. I'll try to move all PHYs to drivers/phy for v3.18.

Ok, so for the next re-spin I will change this to just use the generic phy binding
for both usb2 & usb3 phys.

In parallel I will also migrate over the drivers/usb/phy driver to be just a generic phy
before I attempt to upstream it.

regards,

Peter



--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1290 bytes --]

Hi Peter,

On Thu, Aug 21, 2014 at 03:03:47PM +0100, Peter Griffin wrote:
> Hi Felipe,
> 
> On Thu, 21 Aug 2014, Felipe Balbi wrote:
> 
> > > Currently (in the vendor tree) one of the phys lives in
> > > drivers/usb/phy and the other in drivers/phy.
> > > I believe that is because one is only a usb phy and the other is a
> > > multi function phy which can drive PCI-E or USB3.
> > 
> > right, but for mainline, we can have both PHYs in drivers/phy only.
> 
> Ah ok, I didn't know that, so thanks for that info.
> > 
> > > So to make that work, when dwc3/core.c gets the PHYS in
> > > dwc3_core_get_phy() we need to use the different bindings.
> > > 
> > > I think we are the only platform using "one of each", but luckily
> > > dwc3_core_get_phy() has been written generically enough that it "just
> > > works" :-).
> > 
> > true, but I want to drop support for the legacy drivers/usb/phy layer
> > from dwc3. I'll try to move all PHYs to drivers/phy for v3.18.
> 
> Ok, so for the next re-spin I will change this to just use the generic
> phy binding for both usb2 & usb3 phys.
> 
> In parallel I will also migrate over the drivers/usb/phy driver to be
> just a generic phy before I attempt to upstream it.

Thank you for understanding :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Felipe Balbi]

Hi Peter,

On Thu, Aug 21, 2014 at 03:03:47PM +0100, Peter Griffin wrote:
> Hi Felipe,
> 
> On Thu, 21 Aug 2014, Felipe Balbi wrote:
> 
> > > Currently (in the vendor tree) one of the phys lives in
> > > drivers/usb/phy and the other in drivers/phy.
> > > I believe that is because one is only a usb phy and the other is a
> > > multi function phy which can drive PCI-E or USB3.
> > 
> > right, but for mainline, we can have both PHYs in drivers/phy only.
> 
> Ah ok, I didn't know that, so thanks for that info.
> > 
> > > So to make that work, when dwc3/core.c gets the PHYS in
> > > dwc3_core_get_phy() we need to use the different bindings.
> > > 
> > > I think we are the only platform using "one of each", but luckily
> > > dwc3_core_get_phy() has been written generically enough that it "just
> > > works" :-).
> > 
> > true, but I want to drop support for the legacy drivers/usb/phy layer
> > from dwc3. I'll try to move all PHYs to drivers/phy for v3.18.
> 
> Ok, so for the next re-spin I will change this to just use the generic
> phy binding for both usb2 & usb3 phys.
> 
> In parallel I will also migrate over the drivers/usb/phy driver to be
> just a generic phy before I attempt to upstream it.

Thank you for understanding :-)

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140821/9e7b3434/attachment.sig>

Re: [PATCH v4 2/3] usb: dwc3: dwc3-st: Add st-dwc3 devicetree bindings documentation

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1290 bytes --]

Hi Peter,

On Thu, Aug 21, 2014 at 03:03:47PM +0100, Peter Griffin wrote:
> Hi Felipe,
> 
> On Thu, 21 Aug 2014, Felipe Balbi wrote:
> 
> > > Currently (in the vendor tree) one of the phys lives in
> > > drivers/usb/phy and the other in drivers/phy.
> > > I believe that is because one is only a usb phy and the other is a
> > > multi function phy which can drive PCI-E or USB3.
> > 
> > right, but for mainline, we can have both PHYs in drivers/phy only.
> 
> Ah ok, I didn't know that, so thanks for that info.
> > 
> > > So to make that work, when dwc3/core.c gets the PHYS in
> > > dwc3_core_get_phy() we need to use the different bindings.
> > > 
> > > I think we are the only platform using "one of each", but luckily
> > > dwc3_core_get_phy() has been written generically enough that it "just
> > > works" :-).
> > 
> > true, but I want to drop support for the legacy drivers/usb/phy layer
> > from dwc3. I'll try to move all PHYs to drivers/phy for v3.18.
> 
> Ok, so for the next re-spin I will change this to just use the generic
> phy binding for both usb2 & usb3 phys.
> 
> In parallel I will also migrate over the drivers/usb/phy driver to be
> just a generic phy before I attempt to upstream it.

Thank you for understanding :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 3/3] MAINTAINERS: Add dwc3-st.c file to ARCH/STI architecture

[Peter Griffin]

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 702ca10..269ad3b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1325,6 +1325,7 @@ F:	drivers/pinctrl/pinctrl-st.c
 F:	drivers/media/rc/st_rc.c
 F:	drivers/i2c/busses/i2c-st.c
 F:	drivers/tty/serial/st-asc.c
+F:	drivers/usb/dwc3/dwc3-st.c
 
 ARM/TECHNOLOGIC SYSTEMS TS7250 MACHINE SUPPORT
 M:	Lennert Buytenhek <kernel@wantstofly.org>
-- 
1.9.1

[PATCH v4 3/3] MAINTAINERS: Add dwc3-st.c file to ARCH/STI architecture

[Peter Griffin]

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 702ca10..269ad3b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1325,6 +1325,7 @@ F:	drivers/pinctrl/pinctrl-st.c
 F:	drivers/media/rc/st_rc.c
 F:	drivers/i2c/busses/i2c-st.c
 F:	drivers/tty/serial/st-asc.c
+F:	drivers/usb/dwc3/dwc3-st.c
 
 ARM/TECHNOLOGIC SYSTEMS TS7250 MACHINE SUPPORT
 M:	Lennert Buytenhek <kernel@wantstofly.org>
-- 
1.9.1

Re: [PATCH v4 3/3] MAINTAINERS: Add dwc3-st.c file to ARCH/STI architecture

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 719 bytes --]

On Wed, Jul 30, 2014 at 04:28:11PM +0100, Peter Griffin wrote:

-ENOLOG

> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 702ca10..269ad3b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1325,6 +1325,7 @@ F:	drivers/pinctrl/pinctrl-st.c
>  F:	drivers/media/rc/st_rc.c
>  F:	drivers/i2c/busses/i2c-st.c
>  F:	drivers/tty/serial/st-asc.c
> +F:	drivers/usb/dwc3/dwc3-st.c

also doesn't apply, please rebase on v3.17-rc1. As for the other two
patches, I'm reviewing them right now and they apply cleanly, at least.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 3/3] MAINTAINERS: Add dwc3-st.c file to ARCH/STI architecture

[Felipe Balbi]

On Wed, Jul 30, 2014 at 04:28:11PM +0100, Peter Griffin wrote:

-ENOLOG

> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 702ca10..269ad3b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1325,6 +1325,7 @@ F:	drivers/pinctrl/pinctrl-st.c
>  F:	drivers/media/rc/st_rc.c
>  F:	drivers/i2c/busses/i2c-st.c
>  F:	drivers/tty/serial/st-asc.c
> +F:	drivers/usb/dwc3/dwc3-st.c

also doesn't apply, please rebase on v3.17-rc1. As for the other two
patches, I'm reviewing them right now and they apply cleanly, at least.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140820/22087b06/attachment.sig>

Re: [PATCH v4 3/3] MAINTAINERS: Add dwc3-st.c file to ARCH/STI architecture

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 777 bytes --]

On Wed, Jul 30, 2014 at 04:28:11PM +0100, Peter Griffin wrote:

-ENOLOG

> Signed-off-by: Peter Griffin <peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 702ca10..269ad3b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1325,6 +1325,7 @@ F:	drivers/pinctrl/pinctrl-st.c
>  F:	drivers/media/rc/st_rc.c
>  F:	drivers/i2c/busses/i2c-st.c
>  F:	drivers/tty/serial/st-asc.c
> +F:	drivers/usb/dwc3/dwc3-st.c

also doesn't apply, please rebase on v3.17-rc1. As for the other two
patches, I'm reviewing them right now and they apply cleanly, at least.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v4 3/3] MAINTAINERS: Add dwc3-st.c file to ARCH/STI architecture

[Peter Griffin]

Hi Felipe,

Will fix the commit log and rebase onto 3.17-rc1 for the next iteration.

regards,

Peter.

[PATCH v4 3/3] MAINTAINERS: Add dwc3-st.c file to ARCH/STI architecture

[Peter Griffin]

Hi Felipe,

Will fix the commit log and rebase onto 3.17-rc1 for the next iteration.

regards,

Peter.

Re: [PATCH v4 3/3] MAINTAINERS: Add dwc3-st.c file to ARCH/STI architecture

[Peter Griffin]

Hi Felipe,

Will fix the commit log and rebase onto 3.17-rc1 for the next iteration.

regards,

Peter.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html