[PATCH v3 00/12] ARM: berlin: USB support

[PATCH v3 00/12] ARM: berlin: USB support

[Antoine Ténart]

This series adds the support for ChipIdea USB2 (ci13xxx) controllers,
the USB PHYs of the Marvell Berlin SoCs and also adds a reset
controller for these SoCs.

The reset controller is used by the PHY driver and shares the
existing chip controller node with the clocks and one pin controller.

The Marvell Berlin USB controllers are host only on the BG2Q and are
compatible with USB ChipIdea. We here add a glue to use the available
common functions for this kind of controllers, and add a generic USB2
ChipIdea driver. A PHY driver is also added to control the USB PHY.

This series applies on top of the generic PHY support in the USB
framework[1].

Changes since v2:
	- moved the PHY driver to the generic PHY framework
	- changed the compatible to 'chipidea,usb2'
	- added a property to set the DMA mask in the USB2 CI driver
	- separated dt specific calls in the CI probing function
	- rebased on top of the generic PHY support for CI[1]

Changes since v1:
        - made the Berlin CI USB driver a generic one
        - added support to custom offset for the reset register
        - added fixed regulators to support supply the VBUS
        - modified the PHY driver to support the one one the BG2CD as
          well
        - documented the reset properties
        - added bindings for the BG2CD
        - cosmetic fixes

[1] https://lkml.org/lkml/2014/7/15/330

Antoine Ténart (10):
  reset: add the Berlin reset controller driver
  Documentation: bindings: add reset bindings docs for Marvell Berlin
    SoCs
  ARM: Berlin: select the reset controller
  ARM: dts: berlin: add a required reset property in the chip controller
    node
  phy: add the Berlin USB PHY driver
  Documentation: bindings: add doc for the Berlin USB PHY
  usb: chipidea: add a usb2 driver for ci13xxx
  Documentation: bindings: add doc for the USB2 ChipIdea USB driver
  ARM: dts: berlin: add BG2Q nodes for USB support
  ARM: dts: Berlin: enable USB on the BG2Q DMP

Sebastian Hesselbarth (2):
  ARM: dts: berlin: add BG2CD nodes for USB support
  ARM: dts: berlin: enable USB on the Google Chromecast

 .../devicetree/bindings/arm/marvell,berlin.txt     |  10 +
 .../devicetree/bindings/phy/berlin-usb-phy.txt     |  16 ++
 .../devicetree/bindings/usb/ci-hdrc-usb2.txt       |  23 +++
 arch/arm/boot/dts/berlin2.dtsi                     |   1 +
 arch/arm/boot/dts/berlin2cd-google-chromecast.dts  |   4 +
 arch/arm/boot/dts/berlin2cd.dtsi                   |  35 ++++
 arch/arm/boot/dts/berlin2q-marvell-dmp.dts         |  53 +++++
 arch/arm/boot/dts/berlin2q.dtsi                    |  53 +++++
 arch/arm/mach-berlin/Kconfig                       |   2 +
 drivers/phy/Kconfig                                |   7 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-berlin-usb.c                       | 223 +++++++++++++++++++++
 drivers/reset/Makefile                             |   1 +
 drivers/reset/reset-berlin.c                       | 131 ++++++++++++
 drivers/usb/chipidea/Makefile                      |   1 +
 drivers/usb/chipidea/ci_hdrc_usb2.c                | 147 ++++++++++++++
 16 files changed, 708 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/berlin-usb-phy.txt
 create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
 create mode 100644 drivers/phy/phy-berlin-usb.c
 create mode 100644 drivers/reset/reset-berlin.c
 create mode 100644 drivers/usb/chipidea/ci_hdrc_usb2.c

-- 
1.9.1

[PATCH v3 01/12] reset: add the Berlin reset controller driver

[Antoine Ténart]

Add a reset controller for Marvell Berlin SoCs which is used by the
USB PHYs drivers (for now).

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/reset/Makefile       |   1 +
 drivers/reset/reset-berlin.c | 131 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 132 insertions(+)
 create mode 100644 drivers/reset/reset-berlin.c

diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 60fed3d7820b..157d421f755b 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_RESET_CONTROLLER) += core.o
 obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
+obj-$(CONFIG_ARCH_BERLIN) += reset-berlin.o
 obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_ARCH_STI) += sti/
diff --git a/drivers/reset/reset-berlin.c b/drivers/reset/reset-berlin.c
new file mode 100644
index 000000000000..7b894047a81d
--- /dev/null
+++ b/drivers/reset/reset-berlin.c
@@ -0,0 +1,131 @@
+/*
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <antoine.tenart@free-electrons.com>
+ * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#define BERLIN_MAX_RESETS	32
+
+#define to_berlin_reset_priv(p)		\
+	container_of((p), struct berlin_reset_priv, rcdev)
+
+struct berlin_reset_priv {
+	void __iomem			*base;
+	unsigned int			size;
+	struct reset_controller_dev	rcdev;
+};
+
+static int berlin_reset_reset(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev);
+	int offset = id >> 8;
+	int mask = BIT(id & 0xff);
+
+	writel(mask, priv->base + offset);
+
+	/* let the reset be effective */
+	udelay(10);
+
+	return 0;
+}
+
+static struct reset_control_ops berlin_reset_ops = {
+	.reset	= berlin_reset_reset,
+};
+
+static int berlin_reset_xlate(struct reset_controller_dev *rcdev,
+			      const struct of_phandle_args *reset_spec)
+{
+	struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev);
+	unsigned offset, bit;
+
+	if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
+		return -EINVAL;
+
+	offset = reset_spec->args[0];
+	bit = reset_spec->args[1];
+
+	if (offset >= priv->size)
+		return -EINVAL;
+
+	if (bit >= BERLIN_MAX_RESETS)
+		return -EINVAL;
+
+	return (offset << 8) | bit;
+}
+
+static int __berlin_reset_init(struct device_node *np)
+{
+	struct berlin_reset_priv *priv;
+	struct resource res;
+	resource_size_t size;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = of_address_to_resource(np, 0, &res);
+	if (ret)
+		goto err;
+
+	size = resource_size(&res);
+	priv->base = ioremap(res.start, size);
+	if (!priv->base) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	priv->size = size;
+
+	priv->rcdev.owner = THIS_MODULE;
+	priv->rcdev.ops = &berlin_reset_ops;
+	priv->rcdev.of_node = np;
+	priv->rcdev.of_reset_n_cells = 2;
+	priv->rcdev.of_xlate = berlin_reset_xlate;
+
+	reset_controller_register(&priv->rcdev);
+
+	return 0;
+
+err:
+	kfree(priv);
+	return ret;
+}
+
+static const struct of_device_id berlin_reset_of_match[] __initconst = {
+	{ .compatible = "marvell,berlin2-chip-ctrl" },
+	{ .compatible = "marvell,berlin2cd-chip-ctrl" },
+	{ .compatible = "marvell,berlin2q-chip-ctrl" },
+	{ },
+};
+
+static int __init berlin_reset_init(void)
+{
+	struct device_node *np;
+	int ret;
+
+	for_each_matching_node(np, berlin_reset_of_match) {
+		ret = __berlin_reset_init(np);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+arch_initcall(berlin_reset_init);
-- 
1.9.1

[PATCH v3 02/12] Documentation: bindings: add reset bindings docs for Marvell Berlin SoCs

[Antoine Ténart]

Add the reset binding documentation to the SoC binding documentation as
the reset driver in Marvell Berlin SoC is part of the chip/system
control registers. This patch adds the required properties to configure
the reset controller.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 Documentation/devicetree/bindings/arm/marvell,berlin.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell,berlin.txt b/Documentation/devicetree/bindings/arm/marvell,berlin.txt
index 94013a9a8769..32a1a8a652ad 100644
--- a/Documentation/devicetree/bindings/arm/marvell,berlin.txt
+++ b/Documentation/devicetree/bindings/arm/marvell,berlin.txt
@@ -90,11 +90,21 @@ Required subnode-properties:
 - groups: a list of strings describing the group names.
 - function: a string describing the function used to mux the groups.
 
+* Reset controller binding
+
+A reset controller is part of the chip control registers set. The chip control
+node also provides the reset. The register set is not at the same offset between
+Berlin SoCs.
+
+Required property:
+- #reset-cells: must be set to 2
+
 Example:
 
 chip: chip-control@ea0000 {
 	compatible = "marvell,berlin2-chip-ctrl";
 	#clock-cells = <1>;
+	#reset-cells = <2>;
 	reg = <0xea0000 0x400>;
 	clocks = <&refclk>, <&externaldev 0>;
 	clock-names = "refclk", "video_ext0";
-- 
1.9.1

[PATCH v3 03/12] ARM: Berlin: select the reset controller

[Antoine Ténart]

The Marvell Berlin SoCs now has a reset controller. Add the needed
configuration.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 arch/arm/mach-berlin/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig
index 2631cfc5ab0d..3a4fd1e41801 100644
--- a/arch/arm/mach-berlin/Kconfig
+++ b/arch/arm/mach-berlin/Kconfig
@@ -1,11 +1,13 @@
 menuconfig ARCH_BERLIN
 	bool "Marvell Berlin SoCs" if ARCH_MULTI_V7
+	select ARCH_HAS_RESET_CONTROLLER
 	select ARCH_REQUIRE_GPIOLIB
 	select ARM_GIC
 	select GENERIC_IRQ_CHIP
 	select DW_APB_ICTL
 	select DW_APB_TIMER_OF
 	select PINCTRL
+	select RESET_CONTROLLER
 
 if ARCH_BERLIN
 
-- 
1.9.1

[PATCH v3 04/12] ARM: dts: berlin: add a required reset property in the chip controller node

[Antoine Ténart]

The chip controller node now also describes the Marvell Berlin reset
controller. Add the required 'reset-cells' property.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 arch/arm/boot/dts/berlin2.dtsi   | 1 +
 arch/arm/boot/dts/berlin2cd.dtsi | 1 +
 arch/arm/boot/dts/berlin2q.dtsi  | 1 +
 3 files changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi
index 2477dac4d643..97b12492675e 100644
--- a/arch/arm/boot/dts/berlin2.dtsi
+++ b/arch/arm/boot/dts/berlin2.dtsi
@@ -243,6 +243,7 @@
 		chip: chip-control@ea0000 {
 			compatible = "marvell,berlin2-chip-ctrl";
 			#clock-cells = <1>;
+			#reset-cells = <2>;
 			reg = <0xea0000 0x400>;
 			clocks = <&refclk>;
 			clock-names = "refclk";
diff --git a/arch/arm/boot/dts/berlin2cd.dtsi b/arch/arm/boot/dts/berlin2cd.dtsi
index cc1df65da504..68f7032b4686 100644
--- a/arch/arm/boot/dts/berlin2cd.dtsi
+++ b/arch/arm/boot/dts/berlin2cd.dtsi
@@ -231,6 +231,7 @@
 		chip: chip-control@ea0000 {
 			compatible = "marvell,berlin2cd-chip-ctrl";
 			#clock-cells = <1>;
+			#reset-cells = <2>;
 			reg = <0xea0000 0x400>;
 			clocks = <&refclk>;
 			clock-names = "refclk";
diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index 635a16a64cb4..a6f7ec325200 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -298,6 +298,7 @@
 		chip: chip-control@ea0000 {
 			compatible = "marvell,berlin2q-chip-ctrl";
 			#clock-cells = <1>;
+			#reset-cells = <2>;
 			reg = <0xea0000 0x400>, <0xdd0170 0x10>;
 			clocks = <&refclk>;
 			clock-names = "refclk";
-- 
1.9.1

[PATCH v3 05/12] phy: add the Berlin USB PHY driver

[Antoine Ténart]

Add the driver driving the Marvell Berlin USB PHY. This allows to
initialize the PHY and to use it from the USB driver later.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 drivers/phy/Kconfig          |   7 ++
 drivers/phy/Makefile         |   1 +
 drivers/phy/phy-berlin-usb.c | 223 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 231 insertions(+)
 create mode 100644 drivers/phy/phy-berlin-usb.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 64b98d242ea6..a24dd7e9f5e5 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -15,6 +15,13 @@ config GENERIC_PHY
 	  phy users can obtain reference to the PHY. All the users of this
 	  framework should select this config.
 
+config PHY_BERLIN_USB
+	tristate "Marvell Berlin USB PHY Driver"
+	depends on ARCH_BERLIN && HAS_IOMEM && OF
+	select GENERIC_PHY
+	help
+	  Enable this to support the USB PHY on Marvell Berlin SoCs.
+
 config PHY_EXYNOS_MIPI_VIDEO
 	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
 	depends on HAS_IOMEM
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index b4f1d5770601..20afda923175 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,6 +3,7 @@
 #
 
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
+obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
 obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
 obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
 obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
diff --git a/drivers/phy/phy-berlin-usb.c b/drivers/phy/phy-berlin-usb.c
new file mode 100644
index 000000000000..db8837e3ba24
--- /dev/null
+++ b/drivers/phy/phy-berlin-usb.c
@@ -0,0 +1,223 @@
+/*
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <antoine.tenart@free-electrons.com>
+ * Jisheng Zhang <jszhang@marvell.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/gpio.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#define USB_PHY_PLL		0x04
+#define USB_PHY_PLL_CONTROL	0x08
+#define USB_PHY_TX_CTRL0	0x10
+#define USB_PHY_TX_CTRL1	0x14
+#define USB_PHY_TX_CTRL2	0x18
+#define USB_PHY_RX_CTRL		0x20
+#define USB_PHY_ANALOG		0x34
+
+/* USB_PHY_PLL */
+#define CLK_REF_DIV(x)		((x) << 4)
+#define FEEDBACK_CLK_DIV(x)	((x) << 8)
+
+/* USB_PHY_PLL_CONTROL */
+#define CLK_STABLE		BIT(0)
+#define PLL_CTRL_PIN		BIT(1)
+#define PLL_CTRL_REG		BIT(2)
+#define PLL_ON			BIT(3)
+#define PHASE_OFF_TOL_125	(0x0 << 5)
+#define PHASE_OFF_TOL_250	BIT(5)
+#define KVC0_CALIB		(0x0 << 9)
+#define KVC0_REG_CTRL		BIT(9)
+#define KVC0_HIGH		(0x0 << 10)
+#define KVC0_LOW		(0x3 << 10)
+#define CLK_BLK_EN		BIT(13)
+
+/* USB_PHY_TX_CTRL0 */
+#define EXT_HS_RCAL_EN		BIT(3)
+#define EXT_FS_RCAL_EN		BIT(4)
+#define IMPCAL_VTH_DIV(x)	((x) << 5)
+#define EXT_RS_RCAL_DIV(x)	((x) << 8)
+#define EXT_FS_RCAL_DIV(x)	((x) << 12)
+
+/* USB_PHY_TX_CTRL1 */
+#define TX_VDD15_14		(0x0 << 4)
+#define TX_VDD15_15		BIT(4)
+#define TX_VDD15_16		(0x2 << 4)
+#define TX_VDD15_17		(0x3 << 4)
+#define TX_VDD12_VDD		(0x0 << 6)
+#define TX_VDD12_11		BIT(6)
+#define TX_VDD12_12		(0x2 << 6)
+#define TX_VDD12_13		(0x3 << 6)
+#define LOW_VDD_EN		BIT(8)
+#define TX_OUT_AMP(x)		((x) << 9)
+
+/* USB_PHY_TX_CTRL2 */
+#define TX_CHAN_CTRL_REG(x)	((x) << 0)
+#define DRV_SLEWRATE(x)		((x) << 4)
+#define IMP_CAL_FS_HS_DLY_0	(0x0 << 6)
+#define IMP_CAL_FS_HS_DLY_1	BIT(6)
+#define IMP_CAL_FS_HS_DLY_2	(0x2 << 6)
+#define IMP_CAL_FS_HS_DLY_3	(0x3 << 6)
+#define FS_DRV_EN_MASK(x)	((x) << 8)
+#define HS_DRV_EN_MASK(x)	((x) << 12)
+
+/* USB_PHY_RX_CTRL */
+#define PHASE_FREEZE_DLY_2_CL	(0x0 << 0)
+#define PHASE_FREEZE_DLY_4_CL	BIT(0)
+#define ACK_LENGTH_8_CL		(0x0 << 2)
+#define ACK_LENGTH_12_CL	BIT(2)
+#define ACK_LENGTH_16_CL	(0x2 << 2)
+#define ACK_LENGTH_20_CL	(0x3 << 2)
+#define SQ_LENGTH_3		(0x0 << 4)
+#define SQ_LENGTH_6		BIT(4)
+#define SQ_LENGTH_9		(0x2 << 4)
+#define SQ_LENGTH_12		(0x3 << 4)
+#define DISCON_THRESHOLD_260	(0x0 << 6)
+#define DISCON_THRESHOLD_270	BIT(6)
+#define DISCON_THRESHOLD_280	(0x2 << 6)
+#define DISCON_THRESHOLD_290	(0x3 << 6)
+#define SQ_THRESHOLD(x)		((x) << 8)
+#define LPF_COEF(x)		((x) << 12)
+#define INTPL_CUR_10		(0x0 << 14)
+#define INTPL_CUR_20		BIT(14)
+#define INTPL_CUR_30		(0x2 << 14)
+#define INTPL_CUR_40		(0x3 << 14)
+
+/* USB_PHY_ANALOG */
+#define ANA_PWR_UP		BIT(1)
+#define ANA_PWR_DOWN		BIT(2)
+#define V2I_VCO_RATIO(x)	((x) << 7)
+#define R_ROTATE_90		(0x0 << 10)
+#define R_ROTATE_0		BIT(10)
+#define MODE_TEST_EN		BIT(11)
+#define ANA_TEST_DC_CTRL(x)	((x) << 12)
+
+#define to_phy_berlin_usb_priv(p)	\
+	container_of((p), struct phy_berlin_usb_priv, phy)
+
+static const u32 phy_berlin_pll_dividers[] = {
+	/* Berlin 2 */
+	CLK_REF_DIV(0xc) | FEEDBACK_CLK_DIV(0x54),
+	/* Berlin 2CD */
+	CLK_REF_DIV(0x6) | FEEDBACK_CLK_DIV(0x55),
+};
+
+struct phy_berlin_usb_priv {
+	void __iomem		*base;
+	struct phy		*phy;
+	struct reset_control	*rst_ctrl;
+	u32			pll_divider;
+};
+
+static int phy_berlin_usb_power_on(struct phy *phy)
+{
+	struct phy_berlin_usb_priv *priv = dev_get_drvdata(phy->dev.parent);
+
+	reset_control_reset(priv->rst_ctrl);
+
+	writel(priv->pll_divider,
+	       priv->base + USB_PHY_PLL);
+	writel(CLK_STABLE | PLL_CTRL_REG | PHASE_OFF_TOL_250 | KVC0_REG_CTRL |
+	       CLK_BLK_EN, priv->base + USB_PHY_PLL_CONTROL);
+	writel(V2I_VCO_RATIO(0x5) | R_ROTATE_0 | ANA_TEST_DC_CTRL(0x5),
+	       priv->base + USB_PHY_ANALOG);
+	writel(PHASE_FREEZE_DLY_4_CL | ACK_LENGTH_16_CL | SQ_LENGTH_12 |
+	       DISCON_THRESHOLD_260 | SQ_THRESHOLD(0xa) | LPF_COEF(0x2) |
+	       INTPL_CUR_30, priv->base + USB_PHY_RX_CTRL);
+
+	writel(TX_VDD12_13 | TX_OUT_AMP(0x3), priv->base + USB_PHY_TX_CTRL1);
+	writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) | EXT_RS_RCAL_DIV(0x4),
+	       priv->base + USB_PHY_TX_CTRL0);
+
+	writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) | EXT_RS_RCAL_DIV(0x4) |
+	       EXT_FS_RCAL_DIV(0x2), priv->base + USB_PHY_TX_CTRL0);
+
+	writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) | EXT_RS_RCAL_DIV(0x4),
+	       priv->base + USB_PHY_TX_CTRL0);
+	writel(TX_CHAN_CTRL_REG(0xf) | DRV_SLEWRATE(0x3) | IMP_CAL_FS_HS_DLY_3 |
+	       FS_DRV_EN_MASK(0xd), priv->base + USB_PHY_TX_CTRL2);
+
+	return 0;
+}
+
+static struct phy_ops phy_berlin_usb_ops = {
+	.power_on	= phy_berlin_usb_power_on,
+	.owner		= THIS_MODULE,
+};
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+	{
+		.compatible = "marvell,berlin2-usb-phy",
+		.data = &phy_berlin_pll_dividers[0],
+	},
+	{
+		.compatible = "marvell,berlin2cd-usb-phy",
+		.data = &phy_berlin_pll_dividers[1],
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, phy_berlin_sata_of_match);
+
+static int phy_berlin_usb_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match =
+		of_match_device(phy_berlin_sata_of_match, &pdev->dev);
+	struct phy_berlin_usb_priv *priv;
+	struct resource *res;
+	struct phy_provider *phy_provider;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->rst_ctrl = devm_reset_control_get(&pdev->dev, NULL);
+	if (IS_ERR(priv->rst_ctrl))
+		return PTR_ERR(priv->rst_ctrl);
+
+	priv->pll_divider = *((u32 *)match->data);
+
+	priv->phy = devm_phy_create(&pdev->dev, &phy_berlin_usb_ops, NULL);
+	if (IS_ERR(priv->phy)) {
+		dev_err(&pdev->dev, "failed to create PHY\n");
+		return PTR_ERR(priv->phy);
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	phy_provider =
+		devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	return 0;
+}
+
+static struct platform_driver phy_berlin_usb_driver = {
+	.probe	= phy_berlin_usb_probe,
+	.driver	= {
+		.name		= "phy-berlin-usb",
+		.owner		= THIS_MODULE,
+		.of_match_table	= phy_berlin_sata_of_match,
+	 },
+};
+module_platform_driver(phy_berlin_usb_driver);
+
+MODULE_AUTHOR("Antoine Ténart <antoine.tenart@free-electrons.com>");
+MODULE_DESCRIPTION("Marvell Berlin PHY driver for USB");
+MODULE_LICENSE("GPL");
-- 
1.9.1

[PATCH v3 06/12] Documentation: bindings: add doc for the Berlin USB PHY

[Antoine Ténart]

Document the bindings of the Marvell Berlin USB PHY driver.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 Documentation/devicetree/bindings/phy/berlin-usb-phy.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/berlin-usb-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/berlin-usb-phy.txt b/Documentation/devicetree/bindings/phy/berlin-usb-phy.txt
new file mode 100644
index 000000000000..be33780f668e
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/berlin-usb-phy.txt
@@ -0,0 +1,16 @@
+* Marvell Berlin USB PHY
+
+Required properties:
+- compatible: "marvell,berlin2-usb-phy" or "marvell,berlin2cd-usb-phy"
+- reg: base address and length of the registers
+- #phys-cells: should be 0
+- resets: reference to the reset controller
+
+Example:
+
+	usb-phy@f774000 {
+		compatible = "marvell,berlin2-usb-phy";
+		reg = <0xf774000 0x128>;
+		#phy-cells = <0>;
+		resets = <&chip 0x104 14>;
+	};
-- 
1.9.1

[PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Antoine Ténart]

Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock
and DMA mask, to support USB2 ChipIdea controllers that don't need
specific functions.

Needed for the Marvell Berlin SoCs USB controllers.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 drivers/usb/chipidea/Makefile       |   1 +
 drivers/usb/chipidea/ci_hdrc_usb2.c | 147 ++++++++++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+)
 create mode 100644 drivers/usb/chipidea/ci_hdrc_usb2.c

diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index 2f099c7df7b5..1fc86a2ca22d 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -10,6 +10,7 @@ ci_hdrc-$(CONFIG_USB_OTG_FSM)		+= otg_fsm.o
 
 # Glue/Bridge layers go here
 
+obj-$(CONFIG_USB_CHIPIDEA)	+= ci_hdrc_usb2.o
 obj-$(CONFIG_USB_CHIPIDEA)	+= ci_hdrc_msm.o
 obj-$(CONFIG_USB_CHIPIDEA)	+= ci_hdrc_zevio.o
 
diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
new file mode 100644
index 000000000000..4ec2240d00a1
--- /dev/null
+++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
@@ -0,0 +1,147 @@
+/*
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <antoine.tenart@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/usb/chipidea.h>
+#include <linux/usb/hcd.h>
+#include <linux/usb/ulpi.h>
+
+#include "ci.h"
+
+struct ci_hdrc_usb2_priv {
+	struct platform_device	*ci_pdev;
+	struct clk		*clk;
+	u32			dma_mask;
+};
+
+static int ci_hdrc_usb2_dt_probe(struct device *dev,
+				    struct ci_hdrc_platform_data *ci_pdata,
+				    struct ci_hdrc_usb2_priv *priv)
+{
+	u32 mask;
+	int ret;
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (!IS_ERR(priv->clk)) {
+		ret = clk_prepare_enable(priv->clk);
+		if (ret) {
+			dev_err(dev, "failed to enable the clock: %d\n", ret);
+			return ret;
+		}
+	}
+
+	ci_pdata->phy = of_phy_get(dev->of_node, 0);
+	if (IS_ERR(ci_pdata->phy)) {
+		if (PTR_ERR(ci_pdata->phy) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		/* PHY is optional */
+		ci_pdata->phy = NULL;
+	}
+
+	if (of_property_read_u32(dev->of_node, "dma-mask", &mask))
+		priv->dma_mask = mask;
+
+	return 0;
+}
+
+static struct ci_hdrc_platform_data ci_default_pdata = {
+	.capoffset	= DEF_CAPOFFSET,
+	.flags		= CI_HDRC_REQUIRE_TRANSCEIVER |
+			  CI_HDRC_DISABLE_STREAMING,
+};
+
+static int ci_hdrc_usb2_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ci_hdrc_usb2_priv *priv;
+	struct ci_hdrc_platform_data *ci_pdata = dev_get_platdata(&pdev->dev);
+	int ret;
+
+	if (!ci_pdata)
+		ci_pdata = &ci_default_pdata;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	if (dev->of_node) {
+		ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata, priv);
+		if (ret)
+			return ret;
+	}
+
+	ci_pdata->name = dev_name(&pdev->dev);
+
+	if (priv->dma_mask) {
+		ret = dma_coerce_mask_and_coherent(&pdev->dev, priv->dma_mask);
+		if (ret)
+			return ret;
+	}
+
+	priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource,
+				     pdev->num_resources, ci_pdata);
+	if (IS_ERR(priv->ci_pdev)) {
+		ret = PTR_ERR(priv->ci_pdev);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev,
+				"failed to register ci_hdrc platform device: %d\n",
+				ret);
+		goto clk_err;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	pm_runtime_no_callbacks(dev);
+	pm_runtime_enable(dev);
+
+	return 0;
+
+clk_err:
+	if (!IS_ERR(priv->clk))
+		clk_disable_unprepare(priv->clk);
+	return ret;
+}
+
+static int ci_hdrc_usb2_remove(struct platform_device *pdev)
+{
+	struct ci_hdrc_usb2_priv *priv = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(&pdev->dev);
+	ci_hdrc_remove_device(priv->ci_pdev);
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static const struct of_device_id ci_hdrc_usb2_of_match[] = {
+	{ .compatible = "chipidea,usb2" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match);
+
+static struct platform_driver ci_hdrc_usb2_driver = {
+	.probe	= ci_hdrc_usb2_probe,
+	.remove	= ci_hdrc_usb2_remove,
+	.driver	= {
+		.name		= "chipidea-usb2",
+		.owner		= THIS_MODULE,
+		.of_match_table	= of_match_ptr(ci_hdrc_usb2_of_match),
+	},
+};
+module_platform_driver(ci_hdrc_usb2_driver);
+
+MODULE_DESCRIPTION("ChipIdea HDRC USB2 binding for ci13xxx");
+MODULE_AUTHOR("Antoine Ténart <antoine.tenart@free-electrons.com>");
+MODULE_LICENSE("GPL");
-- 
1.9.1

[PATCH v3 08/12] Documentation: bindings: add doc for the USB2 ChipIdea USB driver

[Antoine Ténart]

Document the USB2 ChipIdea driver (ci13xxx) bindings.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 .../devicetree/bindings/usb/ci-hdrc-usb2.txt       | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
new file mode 100644
index 000000000000..fb3157183e6a
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -0,0 +1,23 @@
+* USB2 ChipIdea USB controller for ci13xxx
+
+Required properties:
+- compatible: should be "chipidea,usb2"
+- reg: base address and length of the registers
+- interrupts: interrupt for the USB controller
+
+Optional properties:
+- clocks: reference to the USB clock
+- usb-phy: reference to the USB PHY
+- vbus-supply: reference to the VBUS regulator
+- dma-mask: DMA bit mask
+
+Example:
+
+	usb@f7ed0000 {
+		compatible = "chipidea,usb2";
+		reg = <0xf7ed0000 0x10000>;
+		interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&chip CLKID_USB0>;
+		usb-phy = <&usb_phy0>;
+		vbus-supply = <&reg_usb0_vbus>;
+	};
-- 
1.9.1

[PATCH v3 09/12] ARM: dts: berlin: add BG2Q nodes for USB support

[Antoine Ténart]

Adds nodes describing the Marvell Berlin BG2Q USB PHY and USB. The BG2Q
SoC has 3 USB host controller, compatible with ChipIdea.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2q.dtsi | 52 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index a6f7ec325200..a56ee2ca196e 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -111,6 +111,39 @@
 			#interrupt-cells = <3>;
 		};
 
+		usb_phy2: phy@a2f400 {
+			compatible = "marvell,berlin2-usb-phy";
+			reg = <0xa2f400 0x128>;
+			#phy-cells = <0>;
+			resets = <&chip 0x104 14>;
+			status = "disabled";
+		};
+
+		usb2: usb@a30000 {
+			compatible = "chipidea,usb2";
+			reg = <0xa30000 0x10000>;
+			interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&chip CLKID_USB2>;
+			phys = <&usb_phy2>;
+			status = "disabled";
+		};
+
+		usb_phy0: phy@b74000 {
+			compatible = "marvell,berlin2-usb-phy";
+			reg = <0xb74000 0x128>;
+			#phy-cells = <0>;
+			resets = <&chip 0x104 12>;
+			status = "disabled";
+		};
+
+		usb_phy1: phy@b78000 {
+			compatible = "marvell,berlin2-usb-phy";
+			reg = <0xb78000 0x128>;
+			#phy-cells = <0>;
+			resets = <&chip 0x104 13>;
+			status = "disabled";
+		};
+
 		apb@e80000 {
 			compatible = "simple-bus";
 			#address-cells = <1>;
@@ -304,6 +337,24 @@
 			clock-names = "refclk";
 		};
 
+		usb0: usb@ed0000 {
+			compatible = "chipidea,usb2";
+			reg = <0xed0000 0x10000>;
+			interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&chip CLKID_USB0>;
+			phys = <&usb_phy0>;
+			status = "disabled";
+		};
+
+		usb1: usb@ee0000 {
+			compatible = "chipidea,usb2";
+			reg = <0xee0000 0x10000>;
+			interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&chip CLKID_USB1>;
+			phys = <&usb_phy1>;
+			status = "disabled";
+		};
+
 		apb@fc0000 {
 			compatible = "simple-bus";
 			#address-cells = <1>;
@@ -360,5 +411,6 @@
 				interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
 			};
 		};
+
 	};
 };
-- 
1.9.1

[PATCH v3 10/12] ARM: dts: Berlin: enable USB on the BG2Q DMP

[Antoine Ténart]

Enable the 2 available USB PHY and USB nodes on the Marvell Berlin BG2Q
DMP.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 53 ++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
index 995150f93795..0d515046f5f5 100644
--- a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
+++ b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
@@ -7,6 +7,8 @@
  */
 
 /dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
 #include "berlin2q.dtsi"
 
 / {
@@ -21,6 +23,39 @@
 	choosen {
 		bootargs = "console=ttyS0,115200 earlyprintk";
 	};
+
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		reg_usb0_vbus: regulator@0 {
+			compatible = "regulator-fixed";
+			regulator-name = "usb0_vbus";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			gpio = <&portb 8 GPIO_ACTIVE_HIGH>;
+			enable-active-high;
+		};
+
+		reg_usb1_vbus: regulator@1 {
+			compatible = "regulator-fixed";
+			regulator-name = "usb1_vbus";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			gpio = <&portb 10 GPIO_ACTIVE_HIGH>;
+			enable-active-high;
+		};
+
+		reg_usb2_vbus: regulator@2 {
+			compatible = "regulator-fixed";
+			regulator-name = "usb2_vbus";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			gpio = <&portb 12 GPIO_ACTIVE_HIGH>;
+			enable-active-high;
+		};
+	};
 };
 
 &sdhci1 {
@@ -37,3 +72,21 @@
 &uart0 {
 	status = "okay";
 };
+
+&usb_phy0 {
+	status = "okay";
+};
+
+&usb_phy2 {
+	status = "okay";
+};
+
+&usb0 {
+	vbus-supply = <&reg_usb0_vbus>;
+	status = "okay";
+};
+
+&usb2 {
+	vbus-supply = <&reg_usb2_vbus>;
+	status = "okay";
+};
-- 
1.9.1

[PATCH v3 11/12] ARM: dts: berlin: add BG2CD nodes for USB support

[Antoine Ténart]

From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

Adds nodes describing the Marvell Berlin BG2CD USB PHY and USB. The BG2CD
SoC has 2 USB ChipIdea controllers, with usb0 host-only and usb1 dual-role
capable.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2cd.dtsi | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2cd.dtsi b/arch/arm/boot/dts/berlin2cd.dtsi
index 68f7032b4686..5c6950531327 100644
--- a/arch/arm/boot/dts/berlin2cd.dtsi
+++ b/arch/arm/boot/dts/berlin2cd.dtsi
@@ -66,6 +66,22 @@
 			clocks = <&chip CLKID_TWD>;
 		};
 
+		usb_phy0: usb-phy@b74000 {
+			compatible = "marvell,berlin2cd-usb-phy";
+			reg = <0xb74000 0x128>;
+			#phy-cells = <0>;
+			resets = <&chip 0x178 23>;
+			status = "disabled";
+		};
+
+		usb_phy1: usb-phy@b78000 {
+			compatible = "marvell,berlin2cd-usb-phy";
+			reg = <0xb78000 0x128>;
+			#phy-cells = <0>;
+			resets = <&chip 0x178 24>;
+			status = "disabled";
+		};
+
 		apb@e80000 {
 			compatible = "simple-bus";
 			#address-cells = <1>;
@@ -242,6 +258,24 @@
 			};
 		};
 
+		usb0: usb@ed0000 {
+			compatible = "chipidea,usb2";
+			reg = <0xed0000 0x200>;
+			interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&chip CLKID_USB0>;
+			usb-phy = <&usb_phy0>;
+			status = "disabled";
+		};
+
+		usb1: usb@ee0000 {
+			compatible = "chipidea,usb2";
+			reg = <0xee0000 0x200>;
+			interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&chip CLKID_USB1>;
+			usb-phy = <&usb_phy1>;
+			status = "disabled";
+		};
+
 		apb@fc0000 {
 			compatible = "simple-bus";
 			#address-cells = <1>;
-- 
1.9.1

[PATCH v3 12/12] ARM: dts: berlin: enable USB on the Google Chromecast

[Antoine Ténart]

From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

Enable usb1 on Google Chromecast which is connected to micro-USB
plug used for external power supply, too.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
 arch/arm/boot/dts/berlin2cd-google-chromecast.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2cd-google-chromecast.dts b/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
index bcd81ffc495d..5c42c3bfb613 100644
--- a/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
+++ b/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
@@ -27,3 +27,7 @@
 };
 
 &uart0 { status = "okay"; };
+
+&usb_phy1 { status = "okay"; };
+
+&usb1 { status = "okay"; };
-- 
1.9.1

Re: [PATCH v3 08/12] Documentation: bindings: add doc for the USB2 ChipIdea USB driver

[Arnd Bergmann]

On Wednesday 16 July 2014 10:26:02 Antoine Ténart wrote:
> +
> +Required properties:
> +- compatible: should be "chipidea,usb2"
> +- reg: base address and length of the registers
> +- interrupts: interrupt for the USB controller
> +
> +Optional properties:
> +- clocks: reference to the USB clock
> +- usb-phy: reference to the USB PHY

I think the phy reference should use the standard phy binding, using
"phys" property to refer to a node that has a "#phy-cells" property.

It seems the code gets this right, but just the documentation is wrong.

> +- vbus-supply: reference to the VBUS regulator
> +- dma-mask: DMA bit mask

No. Do not use "dma-mask" as a property, that is a pure Linux specific
concept. The DT way to express this is to use the 'dma-ranges' property
in the device's parent to describe any bus-related restrictions, and
for the driver to ask for a mask that matches what the hardware is
capable of (which may be smaller or larger than what the bus supports).

	Arnd

Re: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Arnd Bergmann]

On Wednesday 16 July 2014 10:26:01 Antoine Ténart wrote:
> +
> +       if (priv->dma_mask) {
> +               ret = dma_coerce_mask_and_coherent(&pdev->dev, priv->dma_mask);
> +               if (ret)
> +                       return ret;
> +       }
> +
> 

As mentioned in my comment for the binding, this is the wrong way to do it.
Russell has in the past converted all drivers that did this manually to 
do dma_coerce_mask_and_coherent() so we can spot them more easily, but we
should really be doing this better for new drivers.

Can you describe what the restriction is that you want to put on the dma mask?

	Arnd

Re: [PATCH v3 08/12] Documentation: bindings: add doc for the USB2 ChipIdea USB driver

[Antoine Ténart]

Hi Arnd,

On Wed, Jul 16, 2014 at 10:39:36AM +0200, Arnd Bergmann wrote:
> On Wednesday 16 July 2014 10:26:02 Antoine Ténart wrote:
> > +
> > +Required properties:
> > +- compatible: should be "chipidea,usb2"
> > +- reg: base address and length of the registers
> > +- interrupts: interrupt for the USB controller
> > +
> > +Optional properties:
> > +- clocks: reference to the USB clock
> > +- usb-phy: reference to the USB PHY
> 
> I think the phy reference should use the standard phy binding, using
> "phys" property to refer to a node that has a "#phy-cells" property.
> 
> It seems the code gets this right, but just the documentation is wrong.

I moved this PHY driver from usb/phy/ to phy/ and forgot to update this
property. I'll fix it.

> 
> > +- vbus-supply: reference to the VBUS regulator
> > +- dma-mask: DMA bit mask
> 
> No. Do not use "dma-mask" as a property, that is a pure Linux specific
> concept. The DT way to express this is to use the 'dma-ranges' property
> in the device's parent to describe any bus-related restrictions, and
> for the driver to ask for a mask that matches what the hardware is
> capable of (which may be smaller or larger than what the bus supports).

Ok. I'll answer in the CI driver's thread.

Thanks!

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Antoine Ténart]

Hi Arnd,

On Wed, Jul 16, 2014 at 10:41:10AM +0200, Arnd Bergmann wrote:
> On Wednesday 16 July 2014 10:26:01 Antoine Ténart wrote:
> > +
> > +       if (priv->dma_mask) {
> > +               ret = dma_coerce_mask_and_coherent(&pdev->dev, priv->dma_mask);
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> > 
> 
> As mentioned in my comment for the binding, this is the wrong way to do it.
> Russell has in the past converted all drivers that did this manually to 
> do dma_coerce_mask_and_coherent() so we can spot them more easily, but we
> should really be doing this better for new drivers.
> 
> Can you describe what the restriction is that you want to put on the dma mask?

Some people wanted the possibility to set the DMA mask as this USB2 CI
driver does not do specific Berlin operation and can be reused later.
I don't particularly need to call dma_coerce_mask_and_coherent() in my
case, as far as I know.

They can maybe give the restrictions they might want to put on the DMA
mask.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v3 05/12] phy: add the Berlin USB PHY driver

[Varka Bhadram]

On 07/16/2014 01:55 PM, Antoine Ténart wrote:
> +static const struct of_device_id phy_berlin_sata_of_match[] = {
> +	{
> +		.compatible = "marvell,berlin2-usb-phy",
> +		.data = &phy_berlin_pll_dividers[0],
> +	},
> +	{
> +		.compatible = "marvell,berlin2cd-usb-phy",
> +		.data = &phy_berlin_pll_dividers[1],
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, phy_berlin_sata_of_match);
> +

It looks good if we move this after probe().. So that we can see of_match_table directly...

> +static int phy_berlin_usb_probe(struct platform_device *pdev)
> +{
> +	const struct of_device_id *match =
> +		of_match_device(phy_berlin_sata_of_match, &pdev->dev);
> +	struct phy_berlin_usb_priv *priv;
> +	struct resource *res;
> +	struct phy_provider *phy_provider;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	priv->rst_ctrl = devm_reset_control_get(&pdev->dev, NULL);
> +	if (IS_ERR(priv->rst_ctrl))
> +		return PTR_ERR(priv->rst_ctrl);
> +
> +	priv->pll_divider = *((u32 *)match->data);
> +
> +	priv->phy = devm_phy_create(&pdev->dev, &phy_berlin_usb_ops, NULL);
> +	if (IS_ERR(priv->phy)) {
> +		dev_err(&pdev->dev, "failed to create PHY\n");
> +		return PTR_ERR(priv->phy);
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	phy_provider =
> +		devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver phy_berlin_usb_driver = {
> +	.probe	= phy_berlin_usb_probe,
> +	.driver	= {
> +		.name		= "phy-berlin-usb",
> +		.owner		= THIS_MODULE,

No need to update owner field.

-- 
Regards,
Varka Bhadram.

Re: [PATCH v3 05/12] phy: add the Berlin USB PHY driver

[Antoine Ténart]

Hi Varka,

On Wed, Jul 16, 2014 at 02:49:05PM +0530, Varka Bhadram wrote:
> On 07/16/2014 01:55 PM, Antoine Ténart wrote:
> >+static const struct of_device_id phy_berlin_sata_of_match[] = {
> >+	{
> >+		.compatible = "marvell,berlin2-usb-phy",
> >+		.data = &phy_berlin_pll_dividers[0],
> >+	},
> >+	{
> >+		.compatible = "marvell,berlin2cd-usb-phy",
> >+		.data = &phy_berlin_pll_dividers[1],
> >+	},
> >+	{ },
> >+};
> >+MODULE_DEVICE_TABLE(of, phy_berlin_sata_of_match);
> >+
> 
> It looks good if we move this after probe().. So that we can see of_match_table directly...

We use the of match table in the probe, when calling of_match_device().

> >+static int phy_berlin_usb_probe(struct platform_device *pdev)
> >+{
> >+	const struct of_device_id *match =
> >+		of_match_device(phy_berlin_sata_of_match, &pdev->dev);

Here.

Antoine

> >+	struct phy_berlin_usb_priv *priv;
> >+	struct resource *res;
> >+	struct phy_provider *phy_provider;
> >+
> >+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> >+	if (!priv)
> >+		return -ENOMEM;
> >+
> >+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >+	priv->base = devm_ioremap_resource(&pdev->dev, res);
> >+	if (IS_ERR(priv->base))
> >+		return PTR_ERR(priv->base);
> >+
> >+	priv->rst_ctrl = devm_reset_control_get(&pdev->dev, NULL);
> >+	if (IS_ERR(priv->rst_ctrl))
> >+		return PTR_ERR(priv->rst_ctrl);
> >+
> >+	priv->pll_divider = *((u32 *)match->data);
> >+
> >+	priv->phy = devm_phy_create(&pdev->dev, &phy_berlin_usb_ops, NULL);
> >+	if (IS_ERR(priv->phy)) {
> >+		dev_err(&pdev->dev, "failed to create PHY\n");
> >+		return PTR_ERR(priv->phy);
> >+	}
> >+
> >+	platform_set_drvdata(pdev, priv);
> >+
> >+	phy_provider =
> >+		devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
> >+	if (IS_ERR(phy_provider))
> >+		return PTR_ERR(phy_provider);
> >+
> >+	return 0;
> >+}

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v3 05/12] phy: add the Berlin USB PHY driver

[Varka Bhadram]

On 07/16/2014 02:55 PM, Antoine Ténart wrote:
> Hi Varka,
>
> On Wed, Jul 16, 2014 at 02:49:05PM +0530, Varka Bhadram wrote:
>> On 07/16/2014 01:55 PM, Antoine Ténart wrote:
>>> +static const struct of_device_id phy_berlin_sata_of_match[] = {
>>> +	{
>>> +		.compatible = "marvell,berlin2-usb-phy",
>>> +		.data = &phy_berlin_pll_dividers[0],
>>> +	},
>>> +	{
>>> +		.compatible = "marvell,berlin2cd-usb-phy",
>>> +		.data = &phy_berlin_pll_dividers[1],
>>> +	},
>>> +	{ },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, phy_berlin_sata_of_match);
>>> +
>> It looks good if we move this after probe().. So that we can see of_match_table directly...
> We use the of match table in the probe, when calling of_match_device().
>
>>> +static int phy_berlin_usb_probe(struct platform_device *pdev)
>>> +{
>>> +	const struct of_device_id *match =
>>> +		of_match_device(phy_berlin_sata_of_match, &pdev->dev);

We are updating of_match_table in platform_driver struct.

Every driver follows the concept of giving the of_device_ids
above the platform_driver declaration...

> Here.
>
> Antoine
>

Re: [PATCH v3 05/12] phy: add the Berlin USB PHY driver

[Antoine Ténart]

On Wed, Jul 16, 2014 at 03:03:14PM +0530, Varka Bhadram wrote:
> On 07/16/2014 02:55 PM, Antoine Ténart wrote:
> >Hi Varka,
> >
> >On Wed, Jul 16, 2014 at 02:49:05PM +0530, Varka Bhadram wrote:
> >>On 07/16/2014 01:55 PM, Antoine Ténart wrote:
> >>>+static const struct of_device_id phy_berlin_sata_of_match[] = {
> >>>+	{
> >>>+		.compatible = "marvell,berlin2-usb-phy",
> >>>+		.data = &phy_berlin_pll_dividers[0],
> >>>+	},
> >>>+	{
> >>>+		.compatible = "marvell,berlin2cd-usb-phy",
> >>>+		.data = &phy_berlin_pll_dividers[1],
> >>>+	},
> >>>+	{ },
> >>>+};
> >>>+MODULE_DEVICE_TABLE(of, phy_berlin_sata_of_match);
> >>>+
> >>It looks good if we move this after probe().. So that we can see of_match_table directly...
> >We use the of match table in the probe, when calling of_match_device().
> >
> >>>+static int phy_berlin_usb_probe(struct platform_device *pdev)
> >>>+{
> >>>+	const struct of_device_id *match =
> >>>+		of_match_device(phy_berlin_sata_of_match, &pdev->dev);
> 
> We are updating of_match_table in platform_driver struct.

Yes. This is not the same thing. of_match_device() allows to get a
pointer to the of_device_id that matched, to retrieve its data.

> 
> Every driver follows the concept of giving the of_device_ids
> above the platform_driver declaration...

Lots of drivers using of_match_device() declare their of_device_id
structure before the probe.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v3 05/12] phy: add the Berlin USB PHY driver

[Varka Bhadram]

On 07/16/2014 03:17 PM, Antoine Ténart wrote:
> On Wed, Jul 16, 2014 at 03:03:14PM +0530, Varka Bhadram wrote:
>> On 07/16/2014 02:55 PM, Antoine Ténart wrote:
>>> Hi Varka,
>>>
>>> On Wed, Jul 16, 2014 at 02:49:05PM +0530, Varka Bhadram wrote:
>>>> On 07/16/2014 01:55 PM, Antoine Ténart wrote:
>>>>> +static const struct of_device_id phy_berlin_sata_of_match[] = {
>>>>> +	{
>>>>> +		.compatible = "marvell,berlin2-usb-phy",
>>>>> +		.data = &phy_berlin_pll_dividers[0],
>>>>> +	},
>>>>> +	{
>>>>> +		.compatible = "marvell,berlin2cd-usb-phy",
>>>>> +		.data = &phy_berlin_pll_dividers[1],
>>>>> +	},
>>>>> +	{ },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, phy_berlin_sata_of_match);
>>>>> +
>>>> It looks good if we move this after probe().. So that we can see of_match_table directly...
>>> We use the of match table in the probe, when calling of_match_device().
>>>
>>>>> +static int phy_berlin_usb_probe(struct platform_device *pdev)
>>>>> +{
>>>>> +	const struct of_device_id *match =
>>>>> +		of_match_device(phy_berlin_sata_of_match, &pdev->dev);
>> We are updating of_match_table in platform_driver struct.
> Yes. This is not the same thing. of_match_device() allows to get a
> pointer to the of_device_id that matched, to retrieve its data.
>
>> Every driver follows the concept of giving the of_device_ids
>> above the platform_driver declaration...
> Lots of drivers using of_match_device() declare their of_device_id
> structure before the probe.
>
Yes . I also checked that. Thanks  :-)

-- 
Regards,
Varka Bhadram.

Re: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Arnd Bergmann]

On Wednesday 16 July 2014 11:15:33 Antoine Ténart wrote:
> 
> On Wed, Jul 16, 2014 at 10:41:10AM +0200, Arnd Bergmann wrote:
> > On Wednesday 16 July 2014 10:26:01 Antoine Ténart wrote:
> > > +
> > > +       if (priv->dma_mask) {
> > > +               ret = dma_coerce_mask_and_coherent(&pdev->dev, priv->dma_mask);
> > > +               if (ret)
> > > +                       return ret;
> > > +       }
> > > +
> > > 
> > 
> > As mentioned in my comment for the binding, this is the wrong way to do it.
> > Russell has in the past converted all drivers that did this manually to 
> > do dma_coerce_mask_and_coherent() so we can spot them more easily, but we
> > should really be doing this better for new drivers.
> > 
> > Can you describe what the restriction is that you want to put on the dma mask?
> 
> Some people wanted the possibility to set the DMA mask as this USB2 CI
> driver does not do specific Berlin operation and can be reused later.
> I don't particularly need to call dma_coerce_mask_and_coherent() in my
> case, as far as I know.

Ok, just remove the call then and rely on the default mask.
 
> They can maybe give the restrictions they might want to put on the DMA
> mask.

If the restriction is from the bus, it should get handled automatically
by the device probe as long as the correct dma-ranges property is there
(though we have a small bug there at the moment). If there is a variation
of ci13xxx that can't do 32-bit DMA, that should use a different compatible
string and pass a fixed mask into dma_set_mask_and_coherent() based on
the device.

	Arnd

RE: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Peter Chen]

 
> >
> > Some people wanted the possibility to set the DMA mask as this USB2 CI
> > driver does not do specific Berlin operation and can be reused later.
> > I don't particularly need to call dma_coerce_mask_and_coherent() in my
> > case, as far as I know.
> 
> Ok, just remove the call then and rely on the default mask.
> 
> > They can maybe give the restrictions they might want to put on the DMA
> > mask.
> 
> If the restriction is from the bus, it should get handled automatically
> by the device probe as long as the correct dma-ranges property is there
> (though we have a small bug there at the moment). If there is a variation
> of ci13xxx that can't do 32-bit DMA, that should use a different
> compatible string and pass a fixed mask into dma_set_mask_and_coherent()
> based on the device.
> 

Correct me if my below understanding is wrong please.

For three chipidea users:
user_a: don't need dma mask
user_b: dma mask value is dma_mask_b
user_c: dma mask value is dma_mask_c

We don't need to call dma_coerce_mask_and_coherent() at generic chipidea
glue driver, and set dma-ranges as dma_mask_b at user_b's dts, and set
dma-ranges as dma_mask_c at user_c's dts.

Peter 

Re: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Arnd Bergmann]

On Wednesday 16 July 2014 11:58:14 Peter Chen wrote:
> > >
> > > Some people wanted the possibility to set the DMA mask as this USB2 CI
> > > driver does not do specific Berlin operation and can be reused later.
> > > I don't particularly need to call dma_coerce_mask_and_coherent() in my
> > > case, as far as I know.
> > 
> > Ok, just remove the call then and rely on the default mask.
> > 
> > > They can maybe give the restrictions they might want to put on the DMA
> > > mask.
> > 
> > If the restriction is from the bus, it should get handled automatically
> > by the device probe as long as the correct dma-ranges property is there
> > (though we have a small bug there at the moment). If there is a variation
> > of ci13xxx that can't do 32-bit DMA, that should use a different
> > compatible string and pass a fixed mask into dma_set_mask_and_coherent()
> > based on the device.
> > 
> 
> Correct me if my below understanding is wrong please.
> 
> For three chipidea users:
> user_a: don't need dma mask
> user_b: dma mask value is dma_mask_b
> user_c: dma mask value is dma_mask_c
> 
> We don't need to call dma_coerce_mask_and_coherent() at generic chipidea
> glue driver, and set dma-ranges as dma_mask_b at user_b's dts, and set
> dma-ranges as dma_mask_c at user_c's dts.

The dma-ranges property doesn't just encode a dma-mask for a device, but
rather how the children of a bus see the memory address space of the parent.

For traditional reasons we default to assuming that a 32-bit dma_mask
is correct, but there may be multiple reasons why this is not true:

- you have a device connected to a bus that is smaller than 32-bit wide
  (and that would have a dma-ranges property describing that)
- you have a device that has fewer than 32 address lines but is connected
  to a 32-bit upstream bus (hence the dma-ranges describe all 32 bit,
  but the driver must set a smaller mask)
- you have a 64-bit capable device connected to a 32-bit bus: the driver
  will ask for a 64-bit mask, but the dma_set_mask() call refuses this
  because of the bus capabilities.
- you have a 64-bit capable device on a 64-bit capable bus, and the
  dma_set_mask call should succeed.

The mask is definitely never "user" specific, but is a combination of
the device you have and the bus it is connected to.

Things get more complicated when you add in IOMMUs to this, but I'm
skipping this for the sake of limiting the confusion.

	Arnd

Re: [PATCH v3 01/12] reset: add the Berlin reset controller driver

[Andrew Lunn]

On Wed, Jul 16, 2014 at 10:25:55AM +0200, Antoine Ténart wrote:
> Add a reset controller for Marvell Berlin SoCs which is used by the
> USB PHYs drivers (for now).
> 
> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/reset/Makefile       |   1 +
>  drivers/reset/reset-berlin.c | 131 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 132 insertions(+)
>  create mode 100644 drivers/reset/reset-berlin.c
> 
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 60fed3d7820b..157d421f755b 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_RESET_CONTROLLER) += core.o
>  obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
> +obj-$(CONFIG_ARCH_BERLIN) += reset-berlin.o
>  obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
>  obj-$(CONFIG_ARCH_STI) += sti/
> diff --git a/drivers/reset/reset-berlin.c b/drivers/reset/reset-berlin.c
> new file mode 100644
> index 000000000000..7b894047a81d
> --- /dev/null
> +++ b/drivers/reset/reset-berlin.c
> @@ -0,0 +1,131 @@
> +/*
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <antoine.tenart@free-electrons.com>
> + * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#define BERLIN_MAX_RESETS	32
> +
> +#define to_berlin_reset_priv(p)		\
> +	container_of((p), struct berlin_reset_priv, rcdev)
> +
> +struct berlin_reset_priv {
> +	void __iomem			*base;
> +	unsigned int			size;
> +	struct reset_controller_dev	rcdev;
> +};
> +
> +static int berlin_reset_reset(struct reset_controller_dev *rcdev,
> +			      unsigned long id)
> +{
> +	struct berlin_reset_priv *priv = to_berlin_reset_priv(rcdev);
> +	int offset = id >> 8;
> +	int mask = BIT(id & 0xff);

nit: 

The parameter to BIT() should be 0-31. So you want to & with 5. Given
your xlate function, it should never happen that it is greater than
31. Hence it is only a nit. If you need to respin, you can change
this, but don't bother to respin just because of this.

    Andrew

RE: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Peter Chen]

> > > >
> > > > Some people wanted the possibility to set the DMA mask as this
> > > > USB2 CI driver does not do specific Berlin operation and can be
> reused later.
> > > > I don't particularly need to call dma_coerce_mask_and_coherent()
> > > > in my case, as far as I know.
> > >
> > > Ok, just remove the call then and rely on the default mask.
> > >
> > > > They can maybe give the restrictions they might want to put on the
> > > > DMA mask.
> > >
> > > If the restriction is from the bus, it should get handled
> > > automatically by the device probe as long as the correct dma-ranges
> > > property is there (though we have a small bug there at the moment).
> > > If there is a variation of ci13xxx that can't do 32-bit DMA, that
> > > should use a different compatible string and pass a fixed mask into
> > > dma_set_mask_and_coherent() based on the device.
> > >
> >
> > Correct me if my below understanding is wrong please.
> >
> > For three chipidea users:
> > user_a: don't need dma mask
> > user_b: dma mask value is dma_mask_b
> > user_c: dma mask value is dma_mask_c
> >
> > We don't need to call dma_coerce_mask_and_coherent() at generic
> > chipidea glue driver, and set dma-ranges as dma_mask_b at user_b's
> > dts, and set dma-ranges as dma_mask_c at user_c's dts.
> 
> The dma-ranges property doesn't just encode a dma-mask for a device, but
> rather how the children of a bus see the memory address space of the
> parent.
> 
> For traditional reasons we default to assuming that a 32-bit dma_mask is
> correct, but there may be multiple reasons why this is not true:
> 
> - you have a device connected to a bus that is smaller than 32-bit wide
>   (and that would have a dma-ranges property describing that)
> - you have a device that has fewer than 32 address lines but is connected
>   to a 32-bit upstream bus (hence the dma-ranges describe all 32 bit,
>   but the driver must set a smaller mask)
> - you have a 64-bit capable device connected to a 32-bit bus: the driver
>   will ask for a 64-bit mask, but the dma_set_mask() call refuses this
>   because of the bus capabilities.
> - you have a 64-bit capable device on a 64-bit capable bus, and the
>   dma_set_mask call should succeed.
> 
> The mask is definitely never "user" specific, but is a combination of the
> device you have and the bus it is connected to.
> 

Thanks, arnd.

For chipidea generic glue layer case, if there are three devices who use this
driver, and all devices have 32-bit bus, some devices have less 32 address lines.
For example:

- the device_a doesn't need to use dma_mask
- the device_b needs dma_mask as 0xfffffffff
- the device_c needs dma_mask as 0xfffffff0, assume it has only 28 address lines

My questions are:
- Can we not set dma_mask at driver, and only set dma-ranges at dts for device_b
and device_c as a solution to cover this different dma mask use case?

- If we can't use this solution, would you suggest one?

- If we can use this solution, for device_b and device_c, how can we write dma-ranges?
I can't find any arm platforms use it, only some powerpc platform use it.
According to the definition from Power_ePAPR_APPROVED_v1.1.pdf, it is
dma-ranges = <child-bus-address, parent-bus-address, length>
but I find the powerpc has different way for using dma-ranges. 

Peter

Re: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Arnd Bergmann]

On Thursday 17 July 2014 01:20:54 Peter Chen wrote:
> Thanks, arnd.
> 
> For chipidea generic glue layer case, if there are three devices who use this
> driver, and all devices have 32-bit bus, some devices have less 32 address lines.
> For example:
> 
> - the device_a doesn't need to use dma_mask
> - the device_b needs dma_mask as 0xfffffffff
> - the device_c needs dma_mask as 0xfffffff0, assume it has only 28 address lines

This makes no sense. You always need a dma mask, so the first case doesn't exist,
and the second one is the default.

In the third case, I assume you mean 0x0fffffff, which is a 28-bit mask.

> My questions are:
> - Can we not set dma_mask at driver, and only set dma-ranges at dts for device_b
> and device_c as a solution to cover this different dma mask use case?

try to understand my earlier reply. What is the problem with device_b?
Is that a limitation of the bus it is connected to, or the version of the
chipidea hardware?

> - If we can't use this solution, would you suggest one?

It depends on what the requirement of the hardware is, as I explained
now for three times.

> - If we can use this solution, for device_b and device_c, how can we write dma-ranges?
> I can't find any arm platforms use it, only some powerpc platform use it.
> According to the definition from Power_ePAPR_APPROVED_v1.1.pdf, it is
> dma-ranges = <child-bus-address, parent-bus-address, length>
> but I find the powerpc has different way for using dma-ranges. 

It's now handled by of_dma_configure() in drivers/of/platform.c for all
architectures.

	Arnd

Re: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Peter Chen]

On Thu, Jul 17, 2014 at 12:21:56PM +0200, Arnd Bergmann wrote:
> On Thursday 17 July 2014 01:20:54 Peter Chen wrote:
> > Thanks, arnd.
> > 
> > For chipidea generic glue layer case, if there are three devices who use this
> > driver, and all devices have 32-bit bus, some devices have less 32 address lines.
> > For example:
> > 
> > - the device_a doesn't need to use dma_mask
> > - the device_b needs dma_mask as 0xfffffffff
> > - the device_c needs dma_mask as 0xfffffff0, assume it has only 28 address lines
> 
> This makes no sense. You always need a dma mask, so the first case doesn't exist,
> and the second one is the default.
> 
> In the third case, I assume you mean 0x0fffffff, which is a 28-bit mask.
> 
> > My questions are:
> > - Can we not set dma_mask at driver, and only set dma-ranges at dts for device_b
> > and device_c as a solution to cover this different dma mask use case?
> 
> try to understand my earlier reply. What is the problem with device_b?
> Is that a limitation of the bus it is connected to, or the version of the
> chipidea hardware?
> 
> > - If we can't use this solution, would you suggest one?
> 
> It depends on what the requirement of the hardware is, as I explained
> now for three times.
> 

Thanks, Arnd.

Currently, we are designing a generic driver, we don't know what's the
hardware architecture, we are trying to find a solution how to set
dma mask for all possible devices which will use this driver, Antoine's
this patch is trying to cover this feature.

For example, 

Marvell Berlin doesn't need to set dma mask specific.
http://marc.info/?l=linux-arm-kernel&m=140205228507045&w=2
http://www.spinics.net/lists/linux-usb/msg110598.html

Xilinx zynq needs to set dma mask as 0xFFFFFFF0
http://marc.info/?l=linux-usb&m=140384129921706&w=2

FSL i.mx needs to set dma mask as DMA_BIT_MASK(32)
https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/tree/drivers/usb/chipidea/ci_hdrc_imx.c?h=usb-next

-- 

Best Regards,
Peter Chen

Re: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Arnd Bergmann]

On Thursday 17 July 2014 19:19:15 Peter Chen wrote:
> 
> Currently, we are designing a generic driver, we don't know what's the
> hardware architecture, we are trying to find a solution how to set
> dma mask for all possible devices which will use this driver, Antoine's
> this patch is trying to cover this feature.
> 
> For example, 
> 
> Marvell Berlin doesn't need to set dma mask specific.
> http://marc.info/?l=linux-arm-kernel&m=140205228507045&w=2
> http://www.spinics.net/lists/linux-usb/msg110598.html
> 
> Xilinx zynq needs to set dma mask as 0xFFFFFFF0
> http://marc.info/?l=linux-usb&m=140384129921706&w=2
> 
> FSL i.mx needs to set dma mask as DMA_BIT_MASK(32)
> https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/tree/drivers/usb/chipidea/ci_hdrc_imx.c?h=usb-next

Ok, I see now. I think we can safely drop it completely then:

- Berlin was only recently added and doesn't need to set the
  mask because the common code handles it just fine now.

- i.mx was added earlier, at a time where it was still necessary

- zynq just made a mistake, 0xFFFFFFF0 was never a valid mask
  and just happened to work by accident.

In summary, the 32-bit default mask will work on all of them,
and we need no special code or DT properties for it.

	Arnd

Re: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Russell King - ARM Linux]

On Thu, Jul 17, 2014 at 07:19:15PM +0800, Peter Chen wrote:
> Currently, we are designing a generic driver, we don't know what's the
> hardware architecture, we are trying to find a solution how to set
> dma mask for all possible devices which will use this driver, Antoine's
> this patch is trying to cover this feature.
> 
> For example, 
> 
> Marvell Berlin doesn't need to set dma mask specific.
> http://marc.info/?l=linux-arm-kernel&m=140205228507045&w=2
> http://www.spinics.net/lists/linux-usb/msg110598.html

I can't make sense of those messages.

What are you saying - that ci_hdrc on Berlin does not use DMA at all?
Or that it doesn't care what the DMA mask is set to?

> Xilinx zynq needs to set dma mask as 0xFFFFFFF0
> http://marc.info/?l=linux-usb&m=140384129921706&w=2

Why?  The DMA mask does /not/ convey the DMA alignment requirements of
the transfers.  If you need it to be aligned to 16-bytes, then that's
something which is internal to the driver.  This is no different from
devices which require 32-bit alignment or 64-bit alignment.

You can't really expect the DMA subsystem to take care of that for you.
The DMA mask is about indicating what memory ranges are acceptable for
DMA, and not the alignment.

So, in this case, your DMA mask should be DMA_BIT_MASK(32), the same as
iMX.

However, if your driver does receive memory which is not appropriately
aligned, it is the driver's responsibility, not the DMA API, to deal
with that appropriately.

So, it sounds to me like:

- The DT code should be setting the DMA mask appropriately already.

- If the driver is using streaming DMA, should call:

	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
	if (err)
		handle error;

- If the driver is using coherent DMA only:

	err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
	if (err)
		handle error;

- If the driver uses both coherent DMA and streaming DMA:

	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
	if (err)
		handle error;

Where the _bus_ has restrictions on the available memory (iow, the
higher address bits), these should be dealt with inside the DMA API.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Peter Chen]

On Wed, Jul 16, 2014 at 10:26:01AM +0200, Antoine Ténart wrote:
> Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock
> and DMA mask, to support USB2 ChipIdea controllers that don't need
> specific functions.

You may need to indicate it is a generic usb2 glue layer driver at both
subject and context.
> 
> Needed for the Marvell Berlin SoCs USB controllers.

You can say it is tested at Marvell Berlin SoCs USB controllers.

> 
> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
> ---
>  drivers/usb/chipidea/Makefile       |   1 +
>  drivers/usb/chipidea/ci_hdrc_usb2.c | 147 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 148 insertions(+)
>  create mode 100644 drivers/usb/chipidea/ci_hdrc_usb2.c
> 
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index 2f099c7df7b5..1fc86a2ca22d 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -10,6 +10,7 @@ ci_hdrc-$(CONFIG_USB_OTG_FSM)		+= otg_fsm.o
>  
>  # Glue/Bridge layers go here
>  
> +obj-$(CONFIG_USB_CHIPIDEA)	+= ci_hdrc_usb2.o
>  obj-$(CONFIG_USB_CHIPIDEA)	+= ci_hdrc_msm.o
>  obj-$(CONFIG_USB_CHIPIDEA)	+= ci_hdrc_zevio.o
>  
> diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
> new file mode 100644
> index 000000000000..4ec2240d00a1
> --- /dev/null
> +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
> @@ -0,0 +1,147 @@
> +/*
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <antoine.tenart@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/usb/chipidea.h>
> +#include <linux/usb/hcd.h>
> +#include <linux/usb/ulpi.h>
> +
> +#include "ci.h"
> +
> +struct ci_hdrc_usb2_priv {
> +	struct platform_device	*ci_pdev;
> +	struct clk		*clk;
> +	u32			dma_mask;
> +};
> +
> +static int ci_hdrc_usb2_dt_probe(struct device *dev,
> +				    struct ci_hdrc_platform_data *ci_pdata,
> +				    struct ci_hdrc_usb2_priv *priv)
> +{
> +	u32 mask;
> +	int ret;
> +
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (!IS_ERR(priv->clk)) {
> +		ret = clk_prepare_enable(priv->clk);
> +		if (ret) {
> +			dev_err(dev, "failed to enable the clock: %d\n", ret);
> +			return ret;
> +		}
> +	}

The clk API may be needed for both DT and non-DT cases.

> +
> +	ci_pdata->phy = of_phy_get(dev->of_node, 0);
> +	if (IS_ERR(ci_pdata->phy)) {
> +		if (PTR_ERR(ci_pdata->phy) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +
> +		/* PHY is optional */
> +		ci_pdata->phy = NULL;
> +	}
> +
> +	if (of_property_read_u32(dev->of_node, "dma-mask", &mask))
> +		priv->dma_mask = mask;
> +
According to discussion, you may not need dma_mask in driver any more,
and use below API for both DT and non-DT case.

        err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
        if (err)
        	                handle error;

Peter

> +	return 0;
> +}
> +
> +static struct ci_hdrc_platform_data ci_default_pdata = {
> +	.capoffset	= DEF_CAPOFFSET,
> +	.flags		= CI_HDRC_REQUIRE_TRANSCEIVER |
> +			  CI_HDRC_DISABLE_STREAMING,
> +};
> +
> +static int ci_hdrc_usb2_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ci_hdrc_usb2_priv *priv;
> +	struct ci_hdrc_platform_data *ci_pdata = dev_get_platdata(&pdev->dev);
> +	int ret;
> +
> +	if (!ci_pdata)
> +		ci_pdata = &ci_default_pdata;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	if (dev->of_node) {
> +		ret = ci_hdrc_usb2_dt_probe(dev, ci_pdata, priv);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ci_pdata->name = dev_name(&pdev->dev);
> +
> +	if (priv->dma_mask) {
> +		ret = dma_coerce_mask_and_coherent(&pdev->dev, priv->dma_mask);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource,
> +				     pdev->num_resources, ci_pdata);
> +	if (IS_ERR(priv->ci_pdev)) {
> +		ret = PTR_ERR(priv->ci_pdev);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev,
> +				"failed to register ci_hdrc platform device: %d\n",
> +				ret);
> +		goto clk_err;
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	pm_runtime_no_callbacks(dev);
> +	pm_runtime_enable(dev);
> +
> +	return 0;
> +
> +clk_err:
> +	if (!IS_ERR(priv->clk))
> +		clk_disable_unprepare(priv->clk);
> +	return ret;
> +}
> +
> +static int ci_hdrc_usb2_remove(struct platform_device *pdev)
> +{
> +	struct ci_hdrc_usb2_priv *priv = platform_get_drvdata(pdev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +	ci_hdrc_remove_device(priv->ci_pdev);
> +	clk_disable_unprepare(priv->clk);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ci_hdrc_usb2_of_match[] = {
> +	{ .compatible = "chipidea,usb2" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ci_hdrc_usb2_of_match);
> +
> +static struct platform_driver ci_hdrc_usb2_driver = {
> +	.probe	= ci_hdrc_usb2_probe,
> +	.remove	= ci_hdrc_usb2_remove,
> +	.driver	= {
> +		.name		= "chipidea-usb2",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= of_match_ptr(ci_hdrc_usb2_of_match),
> +	},
> +};
> +module_platform_driver(ci_hdrc_usb2_driver);
> +
> +MODULE_DESCRIPTION("ChipIdea HDRC USB2 binding for ci13xxx");
> +MODULE_AUTHOR("Antoine Ténart <antoine.tenart@free-electrons.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 1.9.1
> 

-- 

Best Regards,
Peter Chen

Re: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Antoine Ténart]

Hi Peter,

On Fri, Jul 25, 2014 at 10:18:54AM +0800, Peter Chen wrote:
> On Wed, Jul 16, 2014 at 10:26:01AM +0200, Antoine Ténart wrote:
> > Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock
> > and DMA mask, to support USB2 ChipIdea controllers that don't need
> > specific functions.
> 
> You may need to indicate it is a generic usb2 glue layer driver at both
> subject and context.

"USB2 ChipIdea driver for ci13xxx" seemed quite generic to me.

> > 
> > Needed for the Marvell Berlin SoCs USB controllers.
> 
> You can say it is tested at Marvell Berlin SoCs USB controllers.

Ok, I'll explicitly say it.

> > +
> > +static int ci_hdrc_usb2_dt_probe(struct device *dev,
> > +				    struct ci_hdrc_platform_data *ci_pdata,
> > +				    struct ci_hdrc_usb2_priv *priv)
> > +{
> > +	u32 mask;
> > +	int ret;
> > +
> > +	priv->clk = devm_clk_get(dev, NULL);
> > +	if (!IS_ERR(priv->clk)) {
> > +		ret = clk_prepare_enable(priv->clk);
> > +		if (ret) {
> > +			dev_err(dev, "failed to enable the clock: %d\n", ret);
> > +			return ret;
> > +		}
> > +	}
> 
> The clk API may be needed for both DT and non-DT cases.

Yes sure, I'll move this to the ci_hdrc_usb2_probe().

> 
> > +
> > +	ci_pdata->phy = of_phy_get(dev->of_node, 0);
> > +	if (IS_ERR(ci_pdata->phy)) {
> > +		if (PTR_ERR(ci_pdata->phy) == -EPROBE_DEFER)
> > +			return -EPROBE_DEFER;
> > +
> > +		/* PHY is optional */
> > +		ci_pdata->phy = NULL;
> > +	}
> > +
> > +	if (of_property_read_u32(dev->of_node, "dma-mask", &mask))
> > +		priv->dma_mask = mask;
> > +
> According to discussion, you may not need dma_mask in driver any more,
> and use below API for both DT and non-DT case.
> 
>         err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>         if (err)
>         	                handle error;

This would only be needed in non-dt cases, as the DMA mask is configured
in drivers/of/platform.c otherwise.

Thanks!

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

RE: [PATCH v3 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Peter Chen]

 
> 
> On Fri, Jul 25, 2014 at 10:18:54AM +0800, Peter Chen wrote:
> > On Wed, Jul 16, 2014 at 10:26:01AM +0200, Antoine Ténart wrote:
> > > Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock and
> > > DMA mask, to support USB2 ChipIdea controllers that don't need
> > > specific functions.
> >
> > You may need to indicate it is a generic usb2 glue layer driver at
> > both subject and context.
> 
> "USB2 ChipIdea driver for ci13xxx" seemed quite generic to me.
> 

ok

> > >
> > > Needed for the Marvell Berlin SoCs USB controllers.
> >
> > You can say it is tested at Marvell Berlin SoCs USB controllers.
> 
> Ok, I'll explicitly say it.
> 
> > > +
> > > +static int ci_hdrc_usb2_dt_probe(struct device *dev,
> > > +				    struct ci_hdrc_platform_data *ci_pdata,
> > > +				    struct ci_hdrc_usb2_priv *priv) {
> > > +	u32 mask;
> > > +	int ret;
> > > +
> > > +	priv->clk = devm_clk_get(dev, NULL);
> > > +	if (!IS_ERR(priv->clk)) {
> > > +		ret = clk_prepare_enable(priv->clk);
> > > +		if (ret) {
> > > +			dev_err(dev, "failed to enable the clock: %d\n", ret);
> > > +			return ret;
> > > +		}
> > > +	}
> >
> > The clk API may be needed for both DT and non-DT cases.
> 
> Yes sure, I'll move this to the ci_hdrc_usb2_probe().
> 
 
> > > +
> > > +	if (of_property_read_u32(dev->of_node, "dma-mask", &mask))
> > > +		priv->dma_mask = mask;
> > > +
> > According to discussion, you may not need dma_mask in driver any more,
> > and use below API for both DT and non-DT case.
> >
> >         err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> >         if (err)
> >         	                handle error;
> 
> This would only be needed in non-dt cases, as the DMA mask is configured
> in drivers/of/platform.c otherwise.
> 
 
Get it, thanks.

Peter