[PATCH 0/4] phy: socionext: add new UniPhier USB PHY driver support

[PATCH 0/4] phy: socionext: add new UniPhier USB PHY driver support

[Kunihiko Hayashi]

This series adds support for PHY interface built into USB controller
implemented in Socionext UniPhier SoCs.

The USB3 PHY driver supports High-Speed PHY and Super-Speed PHY included in
the USB3 glue layer, and the USB2 PHY driver supports High-Speed PHY
integrated into system controller.

Kunihiko Hayashi (4):
  dt-bindings: phy: add DT bindings for UniPhier USB3 PHY driver
  phy: socionext: add USB3 PHY driver for UniPhier SoC
  dt-bindings: phy: add DT bindings for UniPhier USB2 PHY driver
  phy: socionext: add USB2 PHY driver for UniPhier SoC

 .../devicetree/bindings/phy/uniphier-usb2-phy.txt  |  45 +++
 .../devicetree/bindings/phy/uniphier-usb3-phy.txt  | 118 ++++++
 drivers/phy/Kconfig                                |   1 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/socionext/Kconfig                      |  25 ++
 drivers/phy/socionext/Makefile                     |   7 +
 drivers/phy/socionext/phy-uniphier-usb2.c          | 218 +++++++++++
 drivers/phy/socionext/phy-uniphier-usb3hs.c        | 422 +++++++++++++++++++++
 drivers/phy/socionext/phy-uniphier-usb3ss.c        | 369 ++++++++++++++++++
 9 files changed, 1206 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/uniphier-usb2-phy.txt
 create mode 100644 Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt
 create mode 100644 drivers/phy/socionext/Kconfig
 create mode 100644 drivers/phy/socionext/Makefile
 create mode 100644 drivers/phy/socionext/phy-uniphier-usb2.c
 create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c
 create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c

-- 
2.7.4

[PATCH 1/4] dt-bindings: phy: add DT bindings for UniPhier USB3 PHY driver

[Kunihiko Hayashi]

Add DT bindings for PHY interface built into USB3 controller
implemented in UniPhier SoCs.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 .../devicetree/bindings/phy/uniphier-usb3-phy.txt  | 118 +++++++++++++++++++++
 1 file changed, 118 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt b/Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt
new file mode 100644
index 0000000..3df4a486
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt
@@ -0,0 +1,118 @@
+Socionext UniPhier USB3 PHY
+
+This describes the devicetree bindings for PHY interfaces built into
+USB3 controller implemented on Socionext UniPhier SoCs.
+The controller includes High-Speed PHY and Super-Speed PHY.
+
+USB3 High-Speed (HS) PHY
+------------------------
+
+Required properties:
+- compatible: Should contain one of the following:
+    "socionext,uniphier-pro4-usb3-hsphy" - for Pro4 SoC
+    "socionext,uniphier-pxs2-usb3-hsphy" - for PXs2 SoC
+    "socionext,uniphier-ld20-usb3-hsphy" - for LD20 SoC
+    "socionext,uniphier-pxs3-usb3-hsphy" - for PXs3 SoC
+- reg: Specifies offset and length of the register set for the device.
+- #phy-cells: Should be 0.
+- clocks: A list of phandles to the clock gate for USB3 glue layer.
+	According to the clock-names, appropriate clocks are required.
+- clock-names: Should contain the following:
+    "gio", "link" - for Pro4 SoC
+    "link", "phy", "phy-ext" - for PXs3 SoC, "phy-ext" is optional.
+    "link", "phy" - for others
+- resets: A list of phandles to the reset control for USB3 glue layer.
+	According to the reset-names, appropriate resets are required.
+- reset-names: Should contain the following:
+    "gio", "link" - for Pro4 SoC
+    "link", "phy" - for others
+
+Optional properties:
+- phy-supply: A phandle to the regulator for USB VBUS.
+- nvmem-cells: Phandles to nvmem cell that contains the trimming data.
+	Available only for HS-PHY implemented on LD20 and PXs3, and
+	if unspecified, default value is used.
+- nvmem-cell-names: Should be the following names, which correspond to
+	each nvmem-cells.
+	All of the 3 parameters associated with the following names are
+	required for each port, if any one is omitted, the trimming data
+	of the port will not be set at all.
+    "rterm", "sel_t", "hs_i" - Each cell name for phy parameters
+
+Refer to phy/phy-bindings.txt for the generic PHY binding properties.
+
+Example:
+
+	usb-glue@65b00000 {
+		compatible = "socionext,uniphier-ld20-dwc3-glue",
+			     "simple-mfd";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x65b00000 0x400>;
+
+		usb_hsphy0: hs-phy@200 {
+			compatible = "socionext,uniphier-ld20-usb3-hsphy";
+			reg = <0x200 0x10>;
+			#phy-cells = <0>;
+			clock-names = "link", "phy";
+			clocks = <&sys_clk 14>, <&sys_clk 16>;
+			reset-names = "link", "phy";
+			resets = <&sys_rst 14>, <&sys_rst 16>;
+			phy-supply = <&usb_vbus0>;
+			nvmem-cell-names = "rterm", "sel_t", "hs_i";
+			nvmem-cells = <&usb_rterm0>, <&usb_sel_t0>,
+				      <&usb_hs_i0>;
+		};
+	};
+
+
+USB3 Super-Speed (SS) PHY
+-------------------------
+
+Required properties:
+- compatible: Should contain one of the following:
+    "socionext,uniphier-pro4-usb3-ssphy" - for Pro4 SoC
+    "socionext,uniphier-pxs2-usb3-ssphy" - for PXs2 SoC
+    "socionext,uniphier-ld20-usb3-ssphy" - for LD20 SoC
+    "socionext,uniphier-pxs3-usb3-ssphy" - for PXs3 SoC
+- reg: Specifies offset and length of the register set for the device.
+- #phy-cells: Should be 0.
+- clocks: A list of phandles to the clock gate for USB3 glue layer.
+	According to the clock-names, appropriate clocks are required.
+- clock-names:
+    "gio", "link" - for Pro4 SoC
+    "link", "phy", "phy-ext" - for PXs3 SoC, "phy-ext" is optional.
+    "link", "phy" - for others
+- resets: A list of phandles to the reset control for USB3 glue layer.
+	According to the reset-names, appropriate resets are required.
+- reset-names:
+    "gio", "link" - for Pro4 SoC
+    "link", "phy" - for others
+
+Optional properties:
+- phy-supply: A phandle to the regulator for USB VBUS.
+
+Refer to phy/phy-bindings.txt for the generic PHY binding properties.
+
+Example:
+
+	usb-glue@65b00000 {
+		compatible = "socionext,uniphier-ld20-dwc3-glue",
+			     "simple-mfd";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x65b00000 0x400>;
+
+		usb_ssphy0: ss-phy@300 {
+			compatible = "socionext,uniphier-ld20-usb3-ssphy";
+			reg = <0x300 0x10>;
+			#phy-cells = <0>;
+			clock-names = "link", "phy";
+			clocks = <&sys_clk 14>, <&sys_clk 16>;
+			reset-names = "link", "phy";
+			resets = <&sys_rst 14>, <&sys_rst 16>;
+			phy-supply = <&usb_vbus0>;
+		};
+
+		other nodes ...
+	};
-- 
2.7.4

[PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC

[Kunihiko Hayashi]

Add a driver for PHY interface built into USB3 controller
implemented in UniPhier SoCs.
This driver supports High-Speed PHY and Super-Speed PHY.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Signed-off-by: Motoya Tanigawa <tanigawa.motoya@socionext.com>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/phy/Kconfig                         |   1 +
 drivers/phy/Makefile                        |   1 +
 drivers/phy/socionext/Kconfig               |  12 +
 drivers/phy/socionext/Makefile              |   6 +
 drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++
 drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++
 6 files changed, 811 insertions(+)
 create mode 100644 drivers/phy/socionext/Kconfig
 create mode 100644 drivers/phy/socionext/Makefile
 create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c
 create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 5c8d452..b752589 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -53,6 +53,7 @@ source "drivers/phy/ralink/Kconfig"
 source "drivers/phy/renesas/Kconfig"
 source "drivers/phy/rockchip/Kconfig"
 source "drivers/phy/samsung/Kconfig"
+source "drivers/phy/socionext/Kconfig"
 source "drivers/phy/st/Kconfig"
 source "drivers/phy/tegra/Kconfig"
 source "drivers/phy/ti/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 84e3bd9..5539cde 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -21,5 +21,6 @@ obj-y					+= broadcom/	\
 					   qualcomm/	\
 					   ralink/	\
 					   samsung/	\
+					   socionext/	\
 					   st/		\
 					   ti/
diff --git a/drivers/phy/socionext/Kconfig b/drivers/phy/socionext/Kconfig
new file mode 100644
index 0000000..4a172fc
--- /dev/null
+++ b/drivers/phy/socionext/Kconfig
@@ -0,0 +1,12 @@
+#
+# PHY drivers for Socionext platforms.
+#
+
+config PHY_UNIPHIER_USB3
+	tristate "UniPhier USB3 PHY driver"
+	depends on ARCH_UNIPHIER || COMPILE_TEST
+	depends on OF && HAS_IOMEM
+	select GENERIC_PHY
+	help
+	  Enable this to support USB PHY implemented in USB3 controller
+	  on UniPhier SoCs. This controller supports USB3.0 and lower speed.
diff --git a/drivers/phy/socionext/Makefile b/drivers/phy/socionext/Makefile
new file mode 100644
index 0000000..dfa5cec
--- /dev/null
+++ b/drivers/phy/socionext/Makefile
@@ -0,0 +1,6 @@
+# SPDX-identifier: GPL-2.0
+#
+# Makefile for the phy drivers.
+#
+
+obj-$(CONFIG_PHY_UNIPHIER_USB3)	+= phy-uniphier-usb3hs.o phy-uniphier-usb3ss.o
diff --git a/drivers/phy/socionext/phy-uniphier-usb3hs.c b/drivers/phy/socionext/phy-uniphier-usb3hs.c
new file mode 100644
index 0000000..cea8983
--- /dev/null
+++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c
@@ -0,0 +1,422 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller
+ * Copyright 2015-2018 Socionext Inc.
+ * Author:
+ *	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
+ * Contributors:
+ *      Motoya Tanigawa <tanigawa.motoya@socionext.com>
+ *      Masami Hiramatsu <masami.hiramatsu@linaro.org>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+
+#define HSPHY_CFG0		0x0
+#define HSPHY_CFG0_HS_I_MASK	GENMASK(31, 28)
+#define HSPHY_CFG0_HSDISC_MASK	GENMASK(27, 26)
+#define HSPHY_CFG0_SWING_MASK	GENMASK(17, 16)
+#define HSPHY_CFG0_SEL_T_MASK	GENMASK(15, 12)
+#define HSPHY_CFG0_RTERM_MASK	GENMASK(7, 6)
+#define HSPHY_CFG0_TRIMMASK	(HSPHY_CFG0_HS_I_MASK \
+				 | HSPHY_CFG0_SEL_T_MASK \
+				 | HSPHY_CFG0_RTERM_MASK)
+
+#define HSPHY_CFG1		0x4
+#define HSPHY_CFG1_DAT_EN	BIT(29)
+#define HSPHY_CFG1_ADR_EN	BIT(28)
+#define HSPHY_CFG1_ADR_MASK	GENMASK(27, 16)
+#define HSPHY_CFG1_DAT_MASK	GENMASK(23, 16)
+
+#define MAX_CLKS	3
+#define MAX_RSTS	2
+#define MAX_PHY_PARAMS	1
+
+struct uniphier_u3hsphy_param {
+	u32 addr;
+	u32 mask;
+	u32 val;
+};
+
+struct uniphier_u3hsphy_trim_param {
+	unsigned int rterm;
+	unsigned int sel_t;
+	unsigned int hs_i;
+};
+
+#define trim_param_is_valid(p)	((p)->rterm || (p)->sel_t || (p)->hs_i)
+
+struct uniphier_u3hsphy_priv {
+	struct device *dev;
+	void __iomem *base;
+	int nclks;
+	struct clk *clk[MAX_CLKS], *clk_phy, *clk_phy_ext;
+	int nrsts;
+	struct reset_control *rst[MAX_RSTS], *rst_phy;
+	const struct uniphier_u3hsphy_soc_data *data;
+};
+
+struct uniphier_u3hsphy_soc_data {
+	const char *clock_names[MAX_CLKS];
+	const char *reset_names[MAX_RSTS];
+	int nparams;
+	const struct uniphier_u3hsphy_param param[MAX_PHY_PARAMS];
+	u32 config0;
+	u32 config1;
+	void (*trim_func)(struct uniphier_u3hsphy_priv *priv, u32 *pconfig,
+			  struct uniphier_u3hsphy_trim_param *pt);
+};
+
+static void uniphier_u3hsphy_trim_ld20(struct uniphier_u3hsphy_priv *priv,
+				       u32 *pconfig,
+				       struct uniphier_u3hsphy_trim_param *pt)
+{
+	*pconfig &= ~HSPHY_CFG0_RTERM_MASK;
+	*pconfig |= FIELD_PREP(HSPHY_CFG0_RTERM_MASK, pt->rterm);
+
+	*pconfig &= ~HSPHY_CFG0_SEL_T_MASK;
+	*pconfig |= FIELD_PREP(HSPHY_CFG0_SEL_T_MASK, pt->sel_t);
+
+	*pconfig &= ~HSPHY_CFG0_HS_I_MASK;
+	*pconfig |= FIELD_PREP(HSPHY_CFG0_HS_I_MASK,  pt->hs_i);
+}
+
+static int uniphier_u3hsphy_get_nvparam(struct uniphier_u3hsphy_priv *priv,
+					const char *name, unsigned int *val)
+{
+	struct nvmem_cell *cell;
+	u8 *buf;
+
+	cell = devm_nvmem_cell_get(priv->dev, name);
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	buf = nvmem_cell_read(cell, NULL);
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+
+	*val = *buf;
+
+	kfree(buf);
+
+	return 0;
+}
+
+static int uniphier_u3hsphy_get_nvparams(struct uniphier_u3hsphy_priv *priv,
+					 struct uniphier_u3hsphy_trim_param *pt)
+{
+	int ret;
+
+	ret = uniphier_u3hsphy_get_nvparam(priv, "rterm", &pt->rterm);
+	if (ret)
+		return ret;
+
+	ret = uniphier_u3hsphy_get_nvparam(priv, "sel_t", &pt->sel_t);
+	if (ret)
+		return ret;
+
+	ret = uniphier_u3hsphy_get_nvparam(priv, "hs_i", &pt->hs_i);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int uniphier_u3hsphy_update_config(struct uniphier_u3hsphy_priv *priv,
+					  u32 *pconfig)
+{
+	struct uniphier_u3hsphy_trim_param trim;
+	int ret, trimmed = 0;
+
+	if (priv->data->trim_func) {
+		ret = uniphier_u3hsphy_get_nvparams(priv, &trim);
+		if (ret == -EPROBE_DEFER)
+			return ret;
+
+		/*
+		 * call trim_func only when trimming parameters that aren't
+		 * all-zero can be acquired. All-zero parameters mean nothing
+		 * has been written to nvmem.
+		 */
+		if (!ret && trim_param_is_valid(&trim)) {
+			priv->data->trim_func(priv, pconfig, &trim);
+			trimmed = 1;
+		} else {
+			dev_dbg(priv->dev, "can't get parameter from nvmem\n");
+		}
+	}
+
+	/* use default parameters without trimming values */
+	if (!trimmed) {
+		*pconfig &= ~HSPHY_CFG0_HSDISC_MASK;
+		*pconfig |= FIELD_PREP(HSPHY_CFG0_HSDISC_MASK, 3);
+	}
+
+	return 0;
+}
+
+static void uniphier_u3hsphy_set_param(struct uniphier_u3hsphy_priv *priv,
+				       const struct uniphier_u3hsphy_param *p)
+{
+	u32 val;
+
+	val = readl(priv->base + HSPHY_CFG1);
+	val &= ~HSPHY_CFG1_ADR_MASK;
+	val |= FIELD_PREP(HSPHY_CFG1_ADR_MASK, p->addr) | HSPHY_CFG1_ADR_EN;
+	writel(val, priv->base + HSPHY_CFG1);
+
+	val = readl(priv->base + HSPHY_CFG1);
+	val &= ~HSPHY_CFG1_ADR_EN;
+	writel(val, priv->base + HSPHY_CFG1);
+
+	val = readl(priv->base + HSPHY_CFG1);
+	val &= ~FIELD_PREP(HSPHY_CFG1_DAT_MASK, p->mask);
+	val |=  FIELD_PREP(HSPHY_CFG1_DAT_MASK, p->val) | HSPHY_CFG1_DAT_EN;
+	writel(val, priv->base + HSPHY_CFG1);
+
+	val = readl(priv->base + HSPHY_CFG1);
+	val &= ~HSPHY_CFG1_DAT_EN;
+	writel(val, priv->base + HSPHY_CFG1);
+}
+
+static int uniphier_u3hsphy_init(struct phy *phy)
+{
+	struct uniphier_u3hsphy_priv *priv = phy_get_drvdata(phy);
+	u32 config0, config1;
+	int i, ret;
+
+	ret = clk_prepare_enable(priv->clk_phy_ext);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk_phy);
+	if (ret)
+		goto out_clk_ext_disable;
+
+	ret = reset_control_deassert(priv->rst_phy);
+	if (ret)
+		goto out_clk_disable;
+
+	if (!priv->data->config0 && !priv->data->config1)
+		return 0;
+
+	config0 = priv->data->config0;
+	config1 = priv->data->config1;
+
+	ret = uniphier_u3hsphy_update_config(priv, &config0);
+	if (ret)
+		goto out_rst_assert;
+
+	writel(config0, priv->base + HSPHY_CFG0);
+	writel(config1, priv->base + HSPHY_CFG1);
+
+	for (i = 0; i < priv->data->nparams; i++)
+		uniphier_u3hsphy_set_param(priv, &priv->data->param[i]);
+
+	return 0;
+
+out_rst_assert:
+	reset_control_assert(priv->rst_phy);
+out_clk_disable:
+	clk_disable_unprepare(priv->clk_phy);
+out_clk_ext_disable:
+	clk_disable_unprepare(priv->clk_phy_ext);
+
+	return ret;
+}
+
+static int uniphier_u3hsphy_exit(struct phy *phy)
+{
+	struct uniphier_u3hsphy_priv *priv = phy_get_drvdata(phy);
+
+	reset_control_assert(priv->rst_phy);
+	clk_disable_unprepare(priv->clk_phy);
+	clk_disable_unprepare(priv->clk_phy_ext);
+
+	return 0;
+}
+
+static const struct phy_ops uniphier_u3hsphy_ops = {
+	.init           = uniphier_u3hsphy_init,
+	.exit           = uniphier_u3hsphy_exit,
+	.owner          = THIS_MODULE,
+};
+
+static int uniphier_u3hsphy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct uniphier_u3hsphy_priv *priv;
+	struct phy_provider *phy_provider;
+	struct resource *res;
+	struct phy *phy;
+	struct clk *clk;
+	struct reset_control *rst;
+	const char *name;
+	int i, ret, nc, nr;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->data = of_device_get_match_data(dev);
+	if (WARN_ON(!priv->data ||
+		    priv->data->nparams > MAX_PHY_PARAMS))
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	for (i = 0; i < MAX_CLKS; i++) {
+		name = priv->data->clock_names[i];
+		if (!name)
+			break;
+		clk = devm_clk_get(dev, name);
+		/* "phy-ext" is optional */
+		if (!strcmp(name, "phy-ext")) {
+			if (PTR_ERR(clk) == -ENOENT)
+				clk = NULL;
+			priv->clk_phy_ext = clk;
+		} else if (!strcmp(name, "phy")) {
+			priv->clk_phy = clk;
+		} else {
+			priv->clk[priv->nclks++] = clk;
+		}
+		if (IS_ERR(clk))
+			return PTR_ERR(clk);
+	}
+
+	for (i = 0; i < MAX_RSTS; i++) {
+		name = priv->data->reset_names[i];
+		if (!name)
+			break;
+		rst = devm_reset_control_get_shared(dev, name);
+		if (IS_ERR(rst))
+			return PTR_ERR(rst);
+
+		if (!strcmp(name, "phy"))
+			priv->rst_phy = rst;
+		else
+			priv->rst[priv->nrsts++] = rst;
+	}
+
+	phy = devm_phy_create(dev, dev->of_node, &uniphier_u3hsphy_ops);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	for (nc = 0; nc < priv->nclks; nc++) {
+		ret = clk_prepare_enable(priv->clk[nc]);
+		if (ret)
+			goto out_clk_disable;
+	}
+
+	for (nr = 0; nr < priv->nrsts; nr++) {
+		ret = reset_control_deassert(priv->rst[nr]);
+		if (ret)
+			goto out_rst_assert;
+	}
+
+	platform_set_drvdata(pdev, priv);
+	phy_set_drvdata(phy, priv);
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider)) {
+		ret = PTR_ERR(phy_provider);
+		goto out_rst_assert;
+	}
+
+	return 0;
+
+out_rst_assert:
+	while (nr--)
+		reset_control_assert(priv->rst[nr]);
+out_clk_disable:
+	while (nc--)
+		clk_disable_unprepare(priv->clk[nc]);
+
+	return ret;
+}
+
+static int uniphier_u3hsphy_remove(struct platform_device *pdev)
+{
+	struct uniphier_u3hsphy_priv *priv = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < priv->nrsts; i++)
+		reset_control_assert(priv->rst[i]);
+	for (i = 0; i < priv->nclks; i++)
+		clk_disable_unprepare(priv->clk[i]);
+
+	return 0;
+}
+
+static const struct uniphier_u3hsphy_soc_data uniphier_pxs2_data = {
+	.clock_names = { "link", "phy", },
+	.reset_names = { "link", "phy", },
+	.nparams = 0,
+};
+
+static const struct uniphier_u3hsphy_soc_data uniphier_ld20_data = {
+	.clock_names = { "link", "phy", },
+	.reset_names = { "link", "phy", },
+	.nparams = 1,
+	.param = {
+		{ 10, 0x60, 0x60 },
+	},
+	.trim_func = uniphier_u3hsphy_trim_ld20,
+	.config0 = 0x92316680,
+	.config1 = 0x00000106,
+};
+
+static const struct uniphier_u3hsphy_soc_data uniphier_pxs3_data = {
+	.clock_names = { "link", "phy", "phy-ext", },
+	.reset_names = { "link", "phy", },
+	.nparams = 0,
+	.trim_func = uniphier_u3hsphy_trim_ld20,
+	.config0 = 0x92316680,
+	.config1 = 0x00000106,
+};
+
+static const struct of_device_id uniphier_u3hsphy_match[] = {
+	{
+		.compatible = "socionext,uniphier-pxs2-usb3-hsphy",
+		.data = &uniphier_pxs2_data,
+	},
+	{
+		.compatible = "socionext,uniphier-ld20-usb3-hsphy",
+		.data = &uniphier_ld20_data,
+	},
+	{
+		.compatible = "socionext,uniphier-pxs3-usb3-hsphy",
+		.data = &uniphier_pxs3_data,
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, uniphier_u3hsphy_match);
+
+static struct platform_driver uniphier_u3hsphy_driver = {
+	.probe = uniphier_u3hsphy_probe,
+	.remove = uniphier_u3hsphy_remove,
+	.driver	= {
+		.name = "uniphier-usb3-hsphy",
+		.of_match_table	= uniphier_u3hsphy_match,
+	},
+};
+
+module_platform_driver(uniphier_u3hsphy_driver);
+
+MODULE_AUTHOR("Kunihiko Hayashi <hayashi.kunihiko@socionext.com>");
+MODULE_DESCRIPTION("UniPhier HS-PHY driver for USB3 controller");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/phy/socionext/phy-uniphier-usb3ss.c b/drivers/phy/socionext/phy-uniphier-usb3ss.c
new file mode 100644
index 0000000..6c2fc36
--- /dev/null
+++ b/drivers/phy/socionext/phy-uniphier-usb3ss.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * phy-uniphier-usb3ss.c - SS-PHY driver for Socionext UniPhier USB3 controller
+ * Copyright 2015-2018 Socionext Inc.
+ * Author:
+ *	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
+ * Contributors:
+ *      Motoya Tanigawa <tanigawa.motoya@socionext.com>
+ *      Masami Hiramatsu <masami.hiramatsu@linaro.org>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#define SSPHY_TESTI		0x0
+#define SSPHY_TESTO		0x4
+#define TESTI_DAT_MASK		GENMASK(13, 6)
+#define TESTI_ADR_MASK		GENMASK(5, 1)
+#define TESTI_WR_EN		BIT(0)
+
+#define MAX_CLKS	3
+#define MAX_RSTS	2
+#define MAX_PHY_PARAMS	7
+
+struct uniphier_u3ssphy_param {
+	u32 addr;
+	u32 mask;
+	u32 val;
+};
+
+struct uniphier_u3ssphy_priv {
+	struct device *dev;
+	void __iomem *base;
+	int nclks;
+	struct clk *clk[MAX_CLKS], *clk_phy, *clk_phy_ext;
+	int nrsts;
+	struct reset_control *rst[MAX_RSTS], *rst_phy;
+	const struct uniphier_u3ssphy_soc_data *data;
+};
+
+struct uniphier_u3ssphy_soc_data {
+	const char *clock_names[MAX_CLKS];
+	const char *reset_names[MAX_RSTS];
+	int nparams;
+	const struct uniphier_u3ssphy_param param[MAX_PHY_PARAMS];
+	bool is_legacy;
+};
+
+static void uniphier_u3ssphy_testio_write(struct uniphier_u3ssphy_priv *priv,
+					  u32 data)
+{
+	/* need to read TESTO twice after accessing TESTI */
+	writel(data, priv->base + SSPHY_TESTI);
+	readl(priv->base + SSPHY_TESTI);
+	readl(priv->base + SSPHY_TESTI);
+}
+
+static void uniphier_u3ssphy_set_param(struct uniphier_u3ssphy_priv *priv,
+				       const struct uniphier_u3ssphy_param *p)
+{
+	u32 val, val_prev;
+
+	/* read previous data */
+	val  = FIELD_PREP(TESTI_DAT_MASK, 1);
+	val |= FIELD_PREP(TESTI_ADR_MASK, p->addr);
+	uniphier_u3ssphy_testio_write(priv, val);
+	val_prev = readl(priv->base + SSPHY_TESTO);
+
+	/* update value */
+	val  = FIELD_PREP(TESTI_DAT_MASK,
+			  (val_prev & ~p->mask) | (p->val & p->mask));
+	val |= FIELD_PREP(TESTI_ADR_MASK, p->addr);
+	uniphier_u3ssphy_testio_write(priv, val);
+	uniphier_u3ssphy_testio_write(priv, val | TESTI_WR_EN);
+	uniphier_u3ssphy_testio_write(priv, val);
+
+	/* read current data as dummy */
+	val  = FIELD_PREP(TESTI_DAT_MASK, 1);
+	val |= FIELD_PREP(TESTI_ADR_MASK, p->addr);
+	uniphier_u3ssphy_testio_write(priv, val);
+	readl(priv->base + SSPHY_TESTO);
+}
+
+static void
+uniphier_u3ssphy_legacy_testio_write(struct uniphier_u3ssphy_priv *priv,
+				     u32 data)
+{
+	int i;
+
+	/* need to read TESTO 10 times after accessing TESTI */
+	writel(data, priv->base + SSPHY_TESTI);
+	for (i = 0; i < 10; i++)
+		readl(priv->base + SSPHY_TESTO);
+}
+
+static void
+uniphier_u3ssphy_legacy_set_param(struct uniphier_u3ssphy_priv *priv,
+				  const struct uniphier_u3ssphy_param *p)
+{
+	u32 val;
+
+	val  = FIELD_PREP(TESTI_DAT_MASK, p->val & p->mask);
+	val |= FIELD_PREP(TESTI_ADR_MASK, p->addr);
+	uniphier_u3ssphy_legacy_testio_write(priv, val);
+	uniphier_u3ssphy_legacy_testio_write(priv, val | TESTI_WR_EN);
+	uniphier_u3ssphy_legacy_testio_write(priv, val);
+}
+
+static int uniphier_u3ssphy_init(struct phy *phy)
+{
+	struct uniphier_u3ssphy_priv *priv = phy_get_drvdata(phy);
+	int i, ret;
+
+	ret = clk_prepare_enable(priv->clk_phy_ext);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(priv->clk_phy);
+	if (ret)
+		goto out_clk_ext_disable;
+
+	ret = reset_control_deassert(priv->rst_phy);
+	if (ret)
+		goto out_clk_disable;
+
+	for (i = 0; i < priv->data->nparams; i++)
+		if (priv->data->is_legacy)
+			uniphier_u3ssphy_legacy_set_param(priv,
+							&priv->data->param[i]);
+		else
+			uniphier_u3ssphy_set_param(priv,
+						   &priv->data->param[i]);
+
+	return 0;
+
+out_clk_disable:
+	clk_disable_unprepare(priv->clk_phy);
+out_clk_ext_disable:
+	clk_disable_unprepare(priv->clk_phy_ext);
+
+	return ret;
+}
+
+static int uniphier_u3ssphy_exit(struct phy *phy)
+{
+	struct uniphier_u3ssphy_priv *priv = phy_get_drvdata(phy);
+
+	reset_control_assert(priv->rst_phy);
+	clk_disable_unprepare(priv->clk_phy);
+	clk_disable_unprepare(priv->clk_phy_ext);
+
+	return 0;
+}
+
+static const struct phy_ops uniphier_u3ssphy_ops = {
+	.init           = uniphier_u3ssphy_init,
+	.exit           = uniphier_u3ssphy_exit,
+	.owner          = THIS_MODULE,
+};
+
+static int uniphier_u3ssphy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct uniphier_u3ssphy_priv *priv;
+	struct phy_provider *phy_provider;
+	struct resource *res;
+	struct phy *phy;
+	struct clk *clk;
+	struct reset_control *rst;
+	const char *name;
+	int i, ret, nc, nr;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->data = of_device_get_match_data(dev);
+	if (WARN_ON(!priv->data ||
+		    priv->data->nparams > MAX_PHY_PARAMS))
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	for (i = 0; i < MAX_CLKS; i++) {
+		name = priv->data->clock_names[i];
+		if (!name)
+			break;
+		clk = devm_clk_get(dev, name);
+		/* "phy-ext" is optional */
+		if (!strcmp(name, "phy-ext")) {
+			if (PTR_ERR(clk) == -ENOENT)
+				clk = NULL;
+			priv->clk_phy_ext = clk;
+		} else if (!strcmp(name, "phy")) {
+			priv->clk_phy = clk;
+		} else {
+			priv->clk[priv->nclks++] = clk;
+		}
+		if (IS_ERR(clk))
+			return PTR_ERR(clk);
+	}
+
+	for (i = 0; i < MAX_RSTS; i++) {
+		name = priv->data->reset_names[i];
+		if (!name)
+			break;
+		rst = devm_reset_control_get_shared(dev, name);
+		if (IS_ERR(rst))
+			return PTR_ERR(rst);
+
+		if (!strcmp(name, "phy"))
+			priv->rst_phy = rst;
+		else
+			priv->rst[priv->nrsts++] = rst;
+	}
+
+	phy = devm_phy_create(dev, dev->of_node, &uniphier_u3ssphy_ops);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	for (nc = 0; nc < priv->nclks; nc++) {
+		ret = clk_prepare_enable(priv->clk[nc]);
+		if (ret)
+			goto out_clk_disable;
+	}
+
+	for (nr = 0; nr < priv->nrsts; nr++) {
+		ret = reset_control_deassert(priv->rst[nr]);
+		if (ret)
+			goto out_rst_assert;
+	}
+
+	platform_set_drvdata(pdev, priv);
+	phy_set_drvdata(phy, priv);
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider)) {
+		ret = PTR_ERR(phy_provider);
+		goto out_rst_assert;
+	}
+
+	return 0;
+
+out_rst_assert:
+	while (nr--)
+		reset_control_assert(priv->rst[nr]);
+out_clk_disable:
+	while (nc--)
+		clk_disable_unprepare(priv->clk[nc]);
+
+	return ret;
+}
+
+static int uniphier_u3ssphy_remove(struct platform_device *pdev)
+{
+	struct uniphier_u3ssphy_priv *priv = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < priv->nrsts; i++)
+		reset_control_assert(priv->rst[i]);
+	for (i = 0; i < priv->nclks; i++)
+		clk_disable_unprepare(priv->clk[i]);
+
+	return 0;
+}
+
+static const struct uniphier_u3ssphy_soc_data uniphier_pro4_data = {
+	.clock_names = { "gio", "link", },
+	.reset_names = { "gio", "link", },
+	.nparams = 7,
+	.param = {
+		{  0, 0x0f, 0x04 },
+		{  3, 0x0f, 0x08 },
+		{  5, 0x0f, 0x08 },
+		{  6, 0x0f, 0x07 },
+		{  7, 0x0f, 0x02 },
+		{ 28, 0x0f, 0x0a },
+		{ 30, 0x0f, 0x09 },
+	},
+	.is_legacy = true,
+};
+
+static const struct uniphier_u3ssphy_soc_data uniphier_pxs2_data = {
+	.clock_names = { "link", "phy", },
+	.reset_names = { "link", "phy", },
+	.nparams = 7,
+	.param = {
+		{  7, 0x0f, 0x0a },
+		{  8, 0x0f, 0x03 },
+		{  9, 0x0f, 0x05 },
+		{ 11, 0x0f, 0x09 },
+		{ 13, 0x60, 0x40 },
+		{ 27, 0x07, 0x07 },
+		{ 28, 0x03, 0x01 },
+	},
+	.is_legacy = false,
+};
+
+static const struct uniphier_u3ssphy_soc_data uniphier_ld20_data = {
+	.clock_names = { "link", "phy", },
+	.reset_names = { "link", "phy", },
+	.nparams = 3,
+	.param = {
+		{  7, 0x0f, 0x06 },
+		{ 13, 0xff, 0xcc },
+		{ 26, 0xf0, 0x50 },
+	},
+	.is_legacy = false,
+};
+
+static const struct uniphier_u3ssphy_soc_data uniphier_pxs3_data = {
+	.clock_names = { "link", "phy", "phy-ext", },
+	.reset_names = { "link", "phy", },
+	.nparams = 3,
+	.param = {
+		{  7, 0x0f, 0x06 },
+		{ 13, 0xff, 0xcc },
+		{ 26, 0xf0, 0x50 },
+	},
+	.is_legacy = false,
+};
+
+static const struct of_device_id uniphier_u3ssphy_match[] = {
+	{
+		.compatible = "socionext,uniphier-pro4-usb3-ssphy",
+		.data = &uniphier_pro4_data,
+	},
+	{
+		.compatible = "socionext,uniphier-pxs2-usb3-ssphy",
+		.data = &uniphier_pxs2_data,
+	},
+	{
+		.compatible = "socionext,uniphier-ld20-usb3-ssphy",
+		.data = &uniphier_ld20_data,
+	},
+	{
+		.compatible = "socionext,uniphier-pxs3-usb3-ssphy",
+		.data = &uniphier_pxs3_data,
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, uniphier_u3ssphy_match);
+
+static struct platform_driver uniphier_u3ssphy_driver = {
+	.probe = uniphier_u3ssphy_probe,
+	.remove = uniphier_u3ssphy_remove,
+	.driver	= {
+		.name = "uniphier-usb3-ssphy",
+		.of_match_table	= uniphier_u3ssphy_match,
+	},
+};
+
+module_platform_driver(uniphier_u3ssphy_driver);
+
+MODULE_AUTHOR("Kunihiko Hayashi <hayashi.kunihiko@socionext.com>");
+MODULE_DESCRIPTION("UniPhier SS-PHY driver for USB3 controller");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH 3/4] dt-bindings: phy: add DT bindings for UniPhier USB2 PHY driver

[Kunihiko Hayashi]

Add DT bindings for PHY interface built into USB2 controller
implemented on Socionext UniPhier SoCs.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 .../devicetree/bindings/phy/uniphier-usb2-phy.txt  | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/uniphier-usb2-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/uniphier-usb2-phy.txt b/Documentation/devicetree/bindings/phy/uniphier-usb2-phy.txt
new file mode 100644
index 0000000..95ee393
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/uniphier-usb2-phy.txt
@@ -0,0 +1,45 @@
+Socionext UniPhier USB2 PHY
+
+This describes the devicetree bindings for PHY interface built into
+USB2 controller implemented on Socionext UniPhier SoCs.
+
+Pro4 SoC has both USB2 and USB3 host controllers, however, this USB3
+controller doesn't include its own High-Speed PHY. This needs to specify
+USB2 PHY instead of USB3 HS-PHY.
+
+Required properties:
+- compatible: Should contain one of the following:
+    "socionext,uniphier-pro4-usb2-phy" - for Pro4 SoC
+    "socionext,uniphier-ld11-usb2-phy" - for LD11 SoC
+
+Sub-nodes:
+Each PHY should be represented as a sub-node.
+
+Sub-nodes required properties:
+- #phy-cells: Should be 0.
+- reg: The number of the PHY.
+
+Sub-nodes optional properties:
+- phy-supply: A phandle to the regulator for USB VBUS.
+
+Refer to phy/phy-bindings.txt for the generic PHY binding properties.
+
+Example:
+	soc-glue@5f800000 {
+		...
+		usb-phy {
+			compatible = "socionext,uniphier-ld11-usb2-phy";
+			usb_phy0: phy@0 {
+				reg = <0>;
+				#phy-cells = <0>;
+			};
+			...
+		};
+	};
+
+	usb@5a800100 {
+		compatible = "socionext,uniphier-ehci", "generic-ehci";
+		...
+		phy-names = "usb";
+		phys = <&usb_phy0>;
+	};
-- 
2.7.4

[PATCH 4/4] phy: socionext: add USB2 PHY driver for UniPhier SoC

[Kunihiko Hayashi]

Add a driver for PHY interface built into USB2 controller implemented on
UniPhier SoCs. This driver supports HS-PHY for Pro4 and LD11.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/phy/socionext/Kconfig             |  13 ++
 drivers/phy/socionext/Makefile            |   1 +
 drivers/phy/socionext/phy-uniphier-usb2.c | 218 ++++++++++++++++++++++++++++++
 3 files changed, 232 insertions(+)
 create mode 100644 drivers/phy/socionext/phy-uniphier-usb2.c

diff --git a/drivers/phy/socionext/Kconfig b/drivers/phy/socionext/Kconfig
index 4a172fc..497ca38 100644
--- a/drivers/phy/socionext/Kconfig
+++ b/drivers/phy/socionext/Kconfig
@@ -2,6 +2,19 @@
 # PHY drivers for Socionext platforms.
 #
 
+config PHY_UNIPHIER_USB2
+	tristate "UniPhier USB2 PHY driver"
+	depends on ARCH_UNIPHIER || COMPILE_TEST
+	depends on OF && HAS_IOMEM
+	select GENERIC_PHY
+	select MFD_SYSCON
+	help
+	  Enable this to support USB PHY implemented on USB2 controller
+	  on UniPhier SoCs. This driver provides interface to interact
+	  with USB 2.0 PHY that is part of the UniPhier SoC.
+	  In case of Pro4, it is necessary to specify this USB2 PHY instead
+	  of USB3 HS-PHY.
+
 config PHY_UNIPHIER_USB3
 	tristate "UniPhier USB3 PHY driver"
 	depends on ARCH_UNIPHIER || COMPILE_TEST
diff --git a/drivers/phy/socionext/Makefile b/drivers/phy/socionext/Makefile
index dfa5cec..6b4763e 100644
--- a/drivers/phy/socionext/Makefile
+++ b/drivers/phy/socionext/Makefile
@@ -3,4 +3,5 @@
 # Makefile for the phy drivers.
 #
 
+obj-$(CONFIG_PHY_UNIPHIER_USB2)	+= phy-uniphier-usb2.o
 obj-$(CONFIG_PHY_UNIPHIER_USB3)	+= phy-uniphier-usb3hs.o phy-uniphier-usb3ss.o
diff --git a/drivers/phy/socionext/phy-uniphier-usb2.c b/drivers/phy/socionext/phy-uniphier-usb2.c
new file mode 100644
index 0000000..bab9e38
--- /dev/null
+++ b/drivers/phy/socionext/phy-uniphier-usb2.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * phy-uniphier-usb2.c - PHY driver for UniPhier USB2 controller
+ * Copyright 2015-2018 Socionext Inc.
+ * Author:
+ *	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define PHY_PARAMS	2
+
+struct uniphier_u2phy_soc_data {
+	struct {
+		u32 addr;
+		u32 val;
+	} param[PHY_PARAMS];
+};
+
+struct uniphier_u2phy_priv {
+	struct regmap *regmap;
+	struct phy *phy;
+	const struct uniphier_u2phy_soc_data *data;
+	struct uniphier_u2phy_priv *next;
+};
+
+static int uniphier_u2phy_init(struct phy *phy)
+{
+	struct uniphier_u2phy_priv *priv = phy_get_drvdata(phy);
+	int i;
+
+	if (!priv->data)
+		return 0;
+
+	for (i = 0; i < PHY_PARAMS; i++)
+		regmap_write(priv->regmap,
+			     priv->data->param[i].addr,
+			     priv->data->param[i].val);
+
+	return 0;
+}
+
+static struct phy *uniphier_u2phy_xlate(struct device *dev,
+					struct of_phandle_args *args)
+{
+	struct uniphier_u2phy_priv *priv = dev_get_drvdata(dev);
+
+	while (priv && args->np != priv->phy->dev.of_node)
+		priv = priv->next;
+
+	if (!priv) {
+		dev_err(dev, "Failed to find appropriate phy\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	return priv->phy;
+}
+
+static const struct phy_ops uniphier_u2phy_ops = {
+	.init  = uniphier_u2phy_init,
+	.owner = THIS_MODULE,
+};
+
+static int uniphier_u2phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *parent, *child;
+	struct uniphier_u2phy_priv *priv = NULL, *next = NULL;
+	struct phy_provider *phy_provider;
+	struct regmap *regmap;
+	struct phy *phy;
+	const struct uniphier_u2phy_soc_data *data;
+	int ret, data_idx, ndatas;
+
+	data = of_device_get_match_data(dev);
+	if (WARN_ON(!data))
+		return -EINVAL;
+
+	/* get number of data */
+	for (ndatas = 0; data[ndatas].param[0].addr; ndatas++)
+		;
+
+	parent = of_get_parent(dev->of_node);
+	regmap = syscon_node_to_regmap(parent);
+	of_node_put(parent);
+	if (IS_ERR(regmap)) {
+		dev_err(dev, "Failed to get regmap\n");
+		return PTR_ERR(regmap);
+	}
+
+	for_each_child_of_node(dev->of_node, child) {
+		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+		if (!priv) {
+			ret = -ENOMEM;
+			goto out_put_child;
+		}
+		priv->regmap = regmap;
+
+		phy = devm_phy_create(dev, child, &uniphier_u2phy_ops);
+		if (IS_ERR(phy)) {
+			dev_err(dev, "Failed to create phy\n");
+			ret = PTR_ERR(phy);
+			goto out_put_child;
+		}
+		priv->phy = phy;
+
+		ret = of_property_read_u32(child, "reg", &data_idx);
+		if (ret) {
+			dev_err(dev, "Failed to get reg property\n");
+			goto out_put_child;
+		}
+
+		if (data_idx < ndatas)
+			priv->data = &data[data_idx];
+		else
+			dev_warn(dev, "No phy configuration: %s\n",
+				 child->full_name);
+
+		phy_set_drvdata(phy, priv);
+		priv->next = next;
+		next = priv;
+	}
+
+	dev_set_drvdata(dev, priv);
+	phy_provider = devm_of_phy_provider_register(dev,
+						     uniphier_u2phy_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	return 0;
+
+out_put_child:
+	of_node_put(child);
+
+	return ret;
+}
+
+static const struct uniphier_u2phy_soc_data uniphier_pro4_data[] = {
+	{
+		.param = {
+			{ 0x500, 0x05142400 },
+			{ 0x50c, 0x00010010 },
+		},
+	},
+	{
+		.param = {
+			{ 0x508, 0x05142400 },
+			{ 0x50c, 0x00010010 },
+		},
+	},
+	{
+		.param = {
+			{ 0x510, 0x05142400 },
+			{ 0x51c, 0x00010010 },
+		},
+	},
+	{
+		.param = {
+			{ 0x518, 0x05142400 },
+			{ 0x51c, 0x00010010 },
+		},
+	},
+	{ /* sentinel */ }
+};
+
+static const struct uniphier_u2phy_soc_data uniphier_ld11_data[] = {
+	{
+		.param = {
+			{ 0x500, 0x82280000 },
+			{ 0x504, 0x00000106 },
+		},
+	},
+	{
+		.param = {
+			{ 0x508, 0x82280000 },
+			{ 0x50c, 0x00000106 },
+		},
+	},
+	{
+		.param = {
+			{ 0x510, 0x82280000 },
+			{ 0x514, 0x00000106 },
+		},
+	},
+	{ /* sentinel */ }
+};
+
+static const struct of_device_id uniphier_u2phy_match[] = {
+	{
+		.compatible = "socionext,uniphier-pro4-usb2-phy",
+		.data = &uniphier_pro4_data,
+	},
+	{
+		.compatible = "socionext,uniphier-ld11-usb2-phy",
+		.data = &uniphier_ld11_data,
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, uniphier_u2phy_match);
+
+static struct platform_driver uniphier_u2phy_driver = {
+	.probe = uniphier_u2phy_probe,
+	.driver = {
+		.name = "uniphier-usb2-phy",
+		.of_match_table = uniphier_u2phy_match,
+	},
+};
+module_platform_driver(uniphier_u2phy_driver);
+
+MODULE_AUTHOR("Kunihiko Hayashi <hayashi.kunihiko@socionext.com>");
+MODULE_DESCRIPTION("UniPhier PHY driver for USB2 controller");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC

[Kishon Vijay Abraham I]

Hi,

On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote:
> Add a driver for PHY interface built into USB3 controller
> implemented in UniPhier SoCs.
> This driver supports High-Speed PHY and Super-Speed PHY.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> Signed-off-by: Motoya Tanigawa <tanigawa.motoya@socionext.com>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  drivers/phy/Kconfig                         |   1 +
>  drivers/phy/Makefile                        |   1 +
>  drivers/phy/socionext/Kconfig               |  12 +
>  drivers/phy/socionext/Makefile              |   6 +
>  drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++
>  drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++
>  6 files changed, 811 insertions(+)
>  create mode 100644 drivers/phy/socionext/Kconfig
>  create mode 100644 drivers/phy/socionext/Makefile
>  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c
>  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 5c8d452..b752589 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -53,6 +53,7 @@ source "drivers/phy/ralink/Kconfig"
>  source "drivers/phy/renesas/Kconfig"
>  source "drivers/phy/rockchip/Kconfig"
>  source "drivers/phy/samsung/Kconfig"
> +source "drivers/phy/socionext/Kconfig"
>  source "drivers/phy/st/Kconfig"
>  source "drivers/phy/tegra/Kconfig"
>  source "drivers/phy/ti/Kconfig"
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 84e3bd9..5539cde 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -21,5 +21,6 @@ obj-y					+= broadcom/	\
>  					   qualcomm/	\
>  					   ralink/	\
>  					   samsung/	\
> +					   socionext/	\
>  					   st/		\
>  					   ti/
> diff --git a/drivers/phy/socionext/Kconfig b/drivers/phy/socionext/Kconfig
> new file mode 100644
> index 0000000..4a172fc
> --- /dev/null
> +++ b/drivers/phy/socionext/Kconfig
> @@ -0,0 +1,12 @@
> +#
> +# PHY drivers for Socionext platforms.
> +#
> +
> +config PHY_UNIPHIER_USB3
> +	tristate "UniPhier USB3 PHY driver"
> +	depends on ARCH_UNIPHIER || COMPILE_TEST
> +	depends on OF && HAS_IOMEM
> +	select GENERIC_PHY
> +	help
> +	  Enable this to support USB PHY implemented in USB3 controller
> +	  on UniPhier SoCs. This controller supports USB3.0 and lower speed.
> diff --git a/drivers/phy/socionext/Makefile b/drivers/phy/socionext/Makefile
> new file mode 100644
> index 0000000..dfa5cec
> --- /dev/null
> +++ b/drivers/phy/socionext/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-identifier: GPL-2.0
> +#
> +# Makefile for the phy drivers.
> +#
> +
> +obj-$(CONFIG_PHY_UNIPHIER_USB3)	+= phy-uniphier-usb3hs.o phy-uniphier-usb3ss.o
> diff --git a/drivers/phy/socionext/phy-uniphier-usb3hs.c b/drivers/phy/socionext/phy-uniphier-usb3hs.c
> new file mode 100644
> index 0000000..cea8983
> --- /dev/null
> +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c
> @@ -0,0 +1,422 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller
> + * Copyright 2015-2018 Socionext Inc.
> + * Author:
> + *	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> + * Contributors:
> + *      Motoya Tanigawa <tanigawa.motoya@socionext.com>
> + *      Masami Hiramatsu <masami.hiramatsu@linaro.org>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#define HSPHY_CFG0		0x0
> +#define HSPHY_CFG0_HS_I_MASK	GENMASK(31, 28)
> +#define HSPHY_CFG0_HSDISC_MASK	GENMASK(27, 26)
> +#define HSPHY_CFG0_SWING_MASK	GENMASK(17, 16)
> +#define HSPHY_CFG0_SEL_T_MASK	GENMASK(15, 12)
> +#define HSPHY_CFG0_RTERM_MASK	GENMASK(7, 6)
> +#define HSPHY_CFG0_TRIMMASK	(HSPHY_CFG0_HS_I_MASK \
> +				 | HSPHY_CFG0_SEL_T_MASK \
> +				 | HSPHY_CFG0_RTERM_MASK)
> +
> +#define HSPHY_CFG1		0x4
> +#define HSPHY_CFG1_DAT_EN	BIT(29)
> +#define HSPHY_CFG1_ADR_EN	BIT(28)
> +#define HSPHY_CFG1_ADR_MASK	GENMASK(27, 16)
> +#define HSPHY_CFG1_DAT_MASK	GENMASK(23, 16)
> +
> +#define MAX_CLKS	3
> +#define MAX_RSTS	2
> +#define MAX_PHY_PARAMS	1
> +
> +struct uniphier_u3hsphy_param {
> +	u32 addr;
> +	u32 mask;
> +	u32 val;
> +};

I'd like to avoid configure the PHY this way, since it's impossible to know
which register is being configured.
> +
> +struct uniphier_u3hsphy_trim_param {
> +	unsigned int rterm;
> +	unsigned int sel_t;
> +	unsigned int hs_i;
> +};
> +
> +#define trim_param_is_valid(p)	((p)->rterm || (p)->sel_t || (p)->hs_i)
> +
> +struct uniphier_u3hsphy_priv {
> +	struct device *dev;
> +	void __iomem *base;
> +	int nclks;
> +	struct clk *clk[MAX_CLKS], *clk_phy, *clk_phy_ext;
> +	int nrsts;
> +	struct reset_control *rst[MAX_RSTS], *rst_phy;
> +	const struct uniphier_u3hsphy_soc_data *data;
> +};
> +
> +struct uniphier_u3hsphy_soc_data {
> +	const char *clock_names[MAX_CLKS];
> +	const char *reset_names[MAX_RSTS];
> +	int nparams;
> +	const struct uniphier_u3hsphy_param param[MAX_PHY_PARAMS];
> +	u32 config0;
> +	u32 config1;
> +	void (*trim_func)(struct uniphier_u3hsphy_priv *priv, u32 *pconfig,
> +			  struct uniphier_u3hsphy_trim_param *pt);
> +};
> +
> +static void uniphier_u3hsphy_trim_ld20(struct uniphier_u3hsphy_priv *priv,
> +				       u32 *pconfig,
> +				       struct uniphier_u3hsphy_trim_param *pt)
> +{
> +	*pconfig &= ~HSPHY_CFG0_RTERM_MASK;
> +	*pconfig |= FIELD_PREP(HSPHY_CFG0_RTERM_MASK, pt->rterm);
> +
> +	*pconfig &= ~HSPHY_CFG0_SEL_T_MASK;
> +	*pconfig |= FIELD_PREP(HSPHY_CFG0_SEL_T_MASK, pt->sel_t);
> +
> +	*pconfig &= ~HSPHY_CFG0_HS_I_MASK;
> +	*pconfig |= FIELD_PREP(HSPHY_CFG0_HS_I_MASK,  pt->hs_i);
> +}
> +
> +static int uniphier_u3hsphy_get_nvparam(struct uniphier_u3hsphy_priv *priv,
> +					const char *name, unsigned int *val)
> +{
> +	struct nvmem_cell *cell;
> +	u8 *buf;
> +
> +	cell = devm_nvmem_cell_get(priv->dev, name);
> +	if (IS_ERR(cell))
> +		return PTR_ERR(cell);
> +
> +	buf = nvmem_cell_read(cell, NULL);
> +	if (IS_ERR(buf))
> +		return PTR_ERR(buf);
> +
> +	*val = *buf;
> +
> +	kfree(buf);
> +
> +	return 0;
> +}
> +
> +static int uniphier_u3hsphy_get_nvparams(struct uniphier_u3hsphy_priv *priv,
> +					 struct uniphier_u3hsphy_trim_param *pt)
> +{
> +	int ret;
> +
> +	ret = uniphier_u3hsphy_get_nvparam(priv, "rterm", &pt->rterm);
> +	if (ret)
> +		return ret;
> +
> +	ret = uniphier_u3hsphy_get_nvparam(priv, "sel_t", &pt->sel_t);
> +	if (ret)
> +		return ret;
> +
> +	ret = uniphier_u3hsphy_get_nvparam(priv, "hs_i", &pt->hs_i);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int uniphier_u3hsphy_update_config(struct uniphier_u3hsphy_priv *priv,
> +					  u32 *pconfig)
> +{
> +	struct uniphier_u3hsphy_trim_param trim;
> +	int ret, trimmed = 0;
> +
> +	if (priv->data->trim_func) {
> +		ret = uniphier_u3hsphy_get_nvparams(priv, &trim);
> +		if (ret == -EPROBE_DEFER)
> +			return ret;
> +
> +		/*
> +		 * call trim_func only when trimming parameters that aren't
> +		 * all-zero can be acquired. All-zero parameters mean nothing
> +		 * has been written to nvmem.
> +		 */
> +		if (!ret && trim_param_is_valid(&trim)) {
> +			priv->data->trim_func(priv, pconfig, &trim);
> +			trimmed = 1;
> +		} else {
> +			dev_dbg(priv->dev, "can't get parameter from nvmem\n");
> +		}
> +	}
> +
> +	/* use default parameters without trimming values */
> +	if (!trimmed) {
> +		*pconfig &= ~HSPHY_CFG0_HSDISC_MASK;
> +		*pconfig |= FIELD_PREP(HSPHY_CFG0_HSDISC_MASK, 3);
> +	}
> +
> +	return 0;
> +}
> +
> +static void uniphier_u3hsphy_set_param(struct uniphier_u3hsphy_priv *priv,
> +				       const struct uniphier_u3hsphy_param *p)
> +{
> +	u32 val;
> +
> +	val = readl(priv->base + HSPHY_CFG1);
> +	val &= ~HSPHY_CFG1_ADR_MASK;
> +	val |= FIELD_PREP(HSPHY_CFG1_ADR_MASK, p->addr) | HSPHY_CFG1_ADR_EN;
> +	writel(val, priv->base + HSPHY_CFG1);
> +
> +	val = readl(priv->base + HSPHY_CFG1);
> +	val &= ~HSPHY_CFG1_ADR_EN;
> +	writel(val, priv->base + HSPHY_CFG1);
> +
> +	val = readl(priv->base + HSPHY_CFG1);
> +	val &= ~FIELD_PREP(HSPHY_CFG1_DAT_MASK, p->mask);
> +	val |=  FIELD_PREP(HSPHY_CFG1_DAT_MASK, p->val) | HSPHY_CFG1_DAT_EN;
> +	writel(val, priv->base + HSPHY_CFG1);
> +
> +	val = readl(priv->base + HSPHY_CFG1);
> +	val &= ~HSPHY_CFG1_DAT_EN;
> +	writel(val, priv->base + HSPHY_CFG1);
> +}
> +
> +static int uniphier_u3hsphy_init(struct phy *phy)
> +{
> +	struct uniphier_u3hsphy_priv *priv = phy_get_drvdata(phy);
> +	u32 config0, config1;
> +	int i, ret;
> +
> +	ret = clk_prepare_enable(priv->clk_phy_ext);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(priv->clk_phy);
> +	if (ret)
> +		goto out_clk_ext_disable;
> +
> +	ret = reset_control_deassert(priv->rst_phy);
> +	if (ret)
> +		goto out_clk_disable;
> +
> +	if (!priv->data->config0 && !priv->data->config1)
> +		return 0;
> +
> +	config0 = priv->data->config0;
> +	config1 = priv->data->config1;
> +
> +	ret = uniphier_u3hsphy_update_config(priv, &config0);
> +	if (ret)
> +		goto out_rst_assert;
> +
> +	writel(config0, priv->base + HSPHY_CFG0);
> +	writel(config1, priv->base + HSPHY_CFG1);
> +
> +	for (i = 0; i < priv->data->nparams; i++)
> +		uniphier_u3hsphy_set_param(priv, &priv->data->param[i]);
> +
> +	return 0;
> +
> +out_rst_assert:
> +	reset_control_assert(priv->rst_phy);
> +out_clk_disable:
> +	clk_disable_unprepare(priv->clk_phy);
> +out_clk_ext_disable:
> +	clk_disable_unprepare(priv->clk_phy_ext);
> +
> +	return ret;
> +}
> +
> +static int uniphier_u3hsphy_exit(struct phy *phy)
> +{
> +	struct uniphier_u3hsphy_priv *priv = phy_get_drvdata(phy);
> +
> +	reset_control_assert(priv->rst_phy);
> +	clk_disable_unprepare(priv->clk_phy);
> +	clk_disable_unprepare(priv->clk_phy_ext);
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops uniphier_u3hsphy_ops = {
> +	.init           = uniphier_u3hsphy_init,
> +	.exit           = uniphier_u3hsphy_exit,
> +	.owner          = THIS_MODULE,
> +};
> +
> +static int uniphier_u3hsphy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct uniphier_u3hsphy_priv *priv;
> +	struct phy_provider *phy_provider;
> +	struct resource *res;
> +	struct phy *phy;
> +	struct clk *clk;
> +	struct reset_control *rst;
> +	const char *name;
> +	int i, ret, nc, nr;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dev = dev;
> +	priv->data = of_device_get_match_data(dev);
> +	if (WARN_ON(!priv->data ||
> +		    priv->data->nparams > MAX_PHY_PARAMS))
> +		return -EINVAL;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	for (i = 0; i < MAX_CLKS; i++) {
> +		name = priv->data->clock_names[i];
> +		if (!name)
> +			break;
> +		clk = devm_clk_get(dev, name);
> +		/* "phy-ext" is optional */
> +		if (!strcmp(name, "phy-ext")) {
> +			if (PTR_ERR(clk) == -ENOENT)
> +				clk = NULL;
> +			priv->clk_phy_ext = clk;
> +		} else if (!strcmp(name, "phy")) {
> +			priv->clk_phy = clk;
> +		} else {
> +			priv->clk[priv->nclks++] = clk;

Only "link" clock will be here? Do we need an array for this?

I think the above can be replaced with 3 separate clk_get calls instead of
storing clock names in uniphier_u3hsphy_soc_data.

Thanks
Kishon

Re: [PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC

[Kunihiko Hayashi]

Hi Kishon,
Thank you for your comments.

On Mon, 9 Jul 2018 10:49:50 +0530 <kishon@ti.com> wrote:

> Hi,
> 
> On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote:
> > Add a driver for PHY interface built into USB3 controller
> > implemented in UniPhier SoCs.
> > This driver supports High-Speed PHY and Super-Speed PHY.
> > 
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > Signed-off-by: Motoya Tanigawa <tanigawa.motoya@socionext.com>
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > ---
> >  drivers/phy/Kconfig                         |   1 +
> >  drivers/phy/Makefile                        |   1 +
> >  drivers/phy/socionext/Kconfig               |  12 +
> >  drivers/phy/socionext/Makefile              |   6 +
> >  drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++
> >  drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++
> >  6 files changed, 811 insertions(+)
> >  create mode 100644 drivers/phy/socionext/Kconfig
> >  create mode 100644 drivers/phy/socionext/Makefile
> >  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c
> >  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c

(snip...)

> > --- /dev/null
> > +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c
> > @@ -0,0 +1,422 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller
> > + * Copyright 2015-2018 Socionext Inc.
> > + * Author:
> > + *	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > + * Contributors:
> > + *      Motoya Tanigawa <tanigawa.motoya@socionext.com>
> > + *      Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/bitops.h>
> > +#include <linux/clk.h>
> > +#include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/of.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/reset.h>
> > +#include <linux/slab.h>
> > +
> > +#define HSPHY_CFG0		0x0
> > +#define HSPHY_CFG0_HS_I_MASK	GENMASK(31, 28)
> > +#define HSPHY_CFG0_HSDISC_MASK	GENMASK(27, 26)
> > +#define HSPHY_CFG0_SWING_MASK	GENMASK(17, 16)
> > +#define HSPHY_CFG0_SEL_T_MASK	GENMASK(15, 12)
> > +#define HSPHY_CFG0_RTERM_MASK	GENMASK(7, 6)
> > +#define HSPHY_CFG0_TRIMMASK	(HSPHY_CFG0_HS_I_MASK \
> > +				 | HSPHY_CFG0_SEL_T_MASK \
> > +				 | HSPHY_CFG0_RTERM_MASK)
> > +
> > +#define HSPHY_CFG1		0x4
> > +#define HSPHY_CFG1_DAT_EN	BIT(29)
> > +#define HSPHY_CFG1_ADR_EN	BIT(28)
> > +#define HSPHY_CFG1_ADR_MASK	GENMASK(27, 16)
> > +#define HSPHY_CFG1_DAT_MASK	GENMASK(23, 16)
> > +
> > +#define MAX_CLKS	3
> > +#define MAX_RSTS	2
> > +#define MAX_PHY_PARAMS	1
> > +
> > +struct uniphier_u3hsphy_param {
> > +	u32 addr;
> > +	u32 mask;
> > +	u32 val;
> > +};
> 
> I'd like to avoid configure the PHY this way, since it's impossible to know
> which register is being configured.

I see. 
In order to know which register is set, I'll add definitions for address
and mask.
And I think the driver might have functions for each SoC to configure
the registers instead of uniphier_u3hsphy_param.

Furthermore, I'll omit the register values that are already set
at power on if the configurations are not affected.


> > +
> > +struct uniphier_u3hsphy_trim_param {
> > +	unsigned int rterm;
> > +	unsigned int sel_t;
> > +	unsigned int hs_i;
> > +};
> > +
> > +#define trim_param_is_valid(p)	((p)->rterm || (p)->sel_t || (p)->hs_i)

(snip...)

> > +static int uniphier_u3hsphy_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct uniphier_u3hsphy_priv *priv;
> > +	struct phy_provider *phy_provider;
> > +	struct resource *res;
> > +	struct phy *phy;
> > +	struct clk *clk;
> > +	struct reset_control *rst;
> > +	const char *name;
> > +	int i, ret, nc, nr;
> > +
> > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->dev = dev;
> > +	priv->data = of_device_get_match_data(dev);
> > +	if (WARN_ON(!priv->data ||
> > +		    priv->data->nparams > MAX_PHY_PARAMS))
> > +		return -EINVAL;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	priv->base = devm_ioremap_resource(dev, res);
> > +	if (IS_ERR(priv->base))
> > +		return PTR_ERR(priv->base);
> > +
> > +	for (i = 0; i < MAX_CLKS; i++) {
> > +		name = priv->data->clock_names[i];
> > +		if (!name)
> > +			break;
> > +		clk = devm_clk_get(dev, name);
> > +		/* "phy-ext" is optional */
> > +		if (!strcmp(name, "phy-ext")) {
> > +			if (PTR_ERR(clk) == -ENOENT)
> > +				clk = NULL;
> > +			priv->clk_phy_ext = clk;
> > +		} else if (!strcmp(name, "phy")) {
> > +			priv->clk_phy = clk;
> > +		} else {
> > +			priv->clk[priv->nclks++] = clk;
> 
> Only "link" clock will be here? Do we need an array for this?
> 
> I think the above can be replaced with 3 separate clk_get calls instead of
> storing clock names in uniphier_u3hsphy_soc_data.

Indeed, in case of HS, we don't need such an array.
I'll rewrite it.

Thank you,

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC

[Kunihiko Hayashi]

On Mon, 9 Jul 2018 20:23:19 +0900 <hayashi.kunihiko@socionext.com> wrote:

> Hi Kishon,
> Thank you for your comments.
> 
> On Mon, 9 Jul 2018 10:49:50 +0530 <kishon@ti.com> wrote:
> 
> > Hi,
> > 
> > On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote:
> > > Add a driver for PHY interface built into USB3 controller
> > > implemented in UniPhier SoCs.
> > > This driver supports High-Speed PHY and Super-Speed PHY.
> > > 
> > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > > Signed-off-by: Motoya Tanigawa <tanigawa.motoya@socionext.com>
> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > ---
> > >  drivers/phy/Kconfig                         |   1 +
> > >  drivers/phy/Makefile                        |   1 +
> > >  drivers/phy/socionext/Kconfig               |  12 +
> > >  drivers/phy/socionext/Makefile              |   6 +
> > >  drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++
> > >  drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++
> > >  6 files changed, 811 insertions(+)
> > >  create mode 100644 drivers/phy/socionext/Kconfig
> > >  create mode 100644 drivers/phy/socionext/Makefile
> > >  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c
> > >  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c
> 
> (snip...)
> 
> > > --- /dev/null
> > > +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c
> > > @@ -0,0 +1,422 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller
> > > + * Copyright 2015-2018 Socionext Inc.
> > > + * Author:
> > > + *	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > > + * Contributors:
> > > + *      Motoya Tanigawa <tanigawa.motoya@socionext.com>
> > > + *      Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > + */
> > > +
> > > +#include <linux/bitfield.h>
> > > +#include <linux/bitops.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/io.h>
> > > +#include <linux/mfd/syscon.h>
> > > +#include <linux/module.h>
> > > +#include <linux/nvmem-consumer.h>
> > > +#include <linux/of.h>
> > > +#include <linux/of_platform.h>
> > > +#include <linux/phy/phy.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/reset.h>
> > > +#include <linux/slab.h>
> > > +
> > > +#define HSPHY_CFG0		0x0
> > > +#define HSPHY_CFG0_HS_I_MASK	GENMASK(31, 28)
> > > +#define HSPHY_CFG0_HSDISC_MASK	GENMASK(27, 26)
> > > +#define HSPHY_CFG0_SWING_MASK	GENMASK(17, 16)
> > > +#define HSPHY_CFG0_SEL_T_MASK	GENMASK(15, 12)
> > > +#define HSPHY_CFG0_RTERM_MASK	GENMASK(7, 6)
> > > +#define HSPHY_CFG0_TRIMMASK	(HSPHY_CFG0_HS_I_MASK \
> > > +				 | HSPHY_CFG0_SEL_T_MASK \
> > > +				 | HSPHY_CFG0_RTERM_MASK)
> > > +
> > > +#define HSPHY_CFG1		0x4
> > > +#define HSPHY_CFG1_DAT_EN	BIT(29)
> > > +#define HSPHY_CFG1_ADR_EN	BIT(28)
> > > +#define HSPHY_CFG1_ADR_MASK	GENMASK(27, 16)
> > > +#define HSPHY_CFG1_DAT_MASK	GENMASK(23, 16)
> > > +
> > > +#define MAX_CLKS	3
> > > +#define MAX_RSTS	2
> > > +#define MAX_PHY_PARAMS	1
> > > +
> > > +struct uniphier_u3hsphy_param {
> > > +	u32 addr;
> > > +	u32 mask;
> > > +	u32 val;
> > > +};
> > 
> > I'd like to avoid configure the PHY this way, since it's impossible to know
> > which register is being configured.
> 

This way might be misunderstood.
These HS-PHY and SS-PHY have "internal" registers, which are not memory-mapped.

And to access these internal registers, we need to specify the number
corresponding to the register.

The "addr" in "uniphier_u3hsphy_param" is just the number of the register.
The "mask" shows a bitfield of the register, that means one of PHY parameters.
The "value" shows a parameter value to set to the bitfield.


> I see. 
> In order to know which register is set, I'll add definitions for address
> and mask.
> And I think the driver might have functions for each SoC to configure
> the registers instead of uniphier_u3hsphy_param.

Then, I think it's sufficient to define a macro with a meaningful label for
each bitfield, and PHY parameters are expressed like that.

  { 10, FOO_BAR_MODE, 0x2 }, /* set 2 to FOO_BAR_MODE in register 10 */

In this case, we need to shift "value" according to "mask".

Thank you,

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC

[Kishon Vijay Abraham I]

Hi,

On Wednesday 11 July 2018 05:35 PM, Kunihiko Hayashi wrote:
> On Mon, 9 Jul 2018 20:23:19 +0900 <hayashi.kunihiko@socionext.com> wrote:
> 
>> Hi Kishon,
>> Thank you for your comments.
>>
>> On Mon, 9 Jul 2018 10:49:50 +0530 <kishon@ti.com> wrote:
>>
>>> Hi,
>>>
>>> On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote:
>>>> Add a driver for PHY interface built into USB3 controller
>>>> implemented in UniPhier SoCs.
>>>> This driver supports High-Speed PHY and Super-Speed PHY.
>>>>
>>>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
>>>> Signed-off-by: Motoya Tanigawa <tanigawa.motoya@socionext.com>
>>>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
>>>> ---
>>>>  drivers/phy/Kconfig                         |   1 +
>>>>  drivers/phy/Makefile                        |   1 +
>>>>  drivers/phy/socionext/Kconfig               |  12 +
>>>>  drivers/phy/socionext/Makefile              |   6 +
>>>>  drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++
>>>>  drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++
>>>>  6 files changed, 811 insertions(+)
>>>>  create mode 100644 drivers/phy/socionext/Kconfig
>>>>  create mode 100644 drivers/phy/socionext/Makefile
>>>>  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c
>>>>  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c
>>
>> (snip...)
>>
>>>> --- /dev/null
>>>> +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c
>>>> @@ -0,0 +1,422 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller
>>>> + * Copyright 2015-2018 Socionext Inc.
>>>> + * Author:
>>>> + *	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
>>>> + * Contributors:
>>>> + *      Motoya Tanigawa <tanigawa.motoya@socionext.com>
>>>> + *      Masami Hiramatsu <masami.hiramatsu@linaro.org>
>>>> + */
>>>> +
>>>> +#include <linux/bitfield.h>
>>>> +#include <linux/bitops.h>
>>>> +#include <linux/clk.h>
>>>> +#include <linux/io.h>
>>>> +#include <linux/mfd/syscon.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/nvmem-consumer.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/of_platform.h>
>>>> +#include <linux/phy/phy.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/reset.h>
>>>> +#include <linux/slab.h>
>>>> +
>>>> +#define HSPHY_CFG0		0x0
>>>> +#define HSPHY_CFG0_HS_I_MASK	GENMASK(31, 28)
>>>> +#define HSPHY_CFG0_HSDISC_MASK	GENMASK(27, 26)
>>>> +#define HSPHY_CFG0_SWING_MASK	GENMASK(17, 16)
>>>> +#define HSPHY_CFG0_SEL_T_MASK	GENMASK(15, 12)
>>>> +#define HSPHY_CFG0_RTERM_MASK	GENMASK(7, 6)
>>>> +#define HSPHY_CFG0_TRIMMASK	(HSPHY_CFG0_HS_I_MASK \
>>>> +				 | HSPHY_CFG0_SEL_T_MASK \
>>>> +				 | HSPHY_CFG0_RTERM_MASK)
>>>> +
>>>> +#define HSPHY_CFG1		0x4
>>>> +#define HSPHY_CFG1_DAT_EN	BIT(29)
>>>> +#define HSPHY_CFG1_ADR_EN	BIT(28)
>>>> +#define HSPHY_CFG1_ADR_MASK	GENMASK(27, 16)
>>>> +#define HSPHY_CFG1_DAT_MASK	GENMASK(23, 16)
>>>> +
>>>> +#define MAX_CLKS	3
>>>> +#define MAX_RSTS	2
>>>> +#define MAX_PHY_PARAMS	1
>>>> +
>>>> +struct uniphier_u3hsphy_param {
>>>> +	u32 addr;
>>>> +	u32 mask;
>>>> +	u32 val;
>>>> +};
>>>
>>> I'd like to avoid configure the PHY this way, since it's impossible to know
>>> which register is being configured.
>>
> 
> This way might be misunderstood.
> These HS-PHY and SS-PHY have "internal" registers, which are not memory-mapped.
> 
> And to access these internal registers, we need to specify the number
> corresponding to the register.
> 
> The "addr" in "uniphier_u3hsphy_param" is just the number of the register.
> The "mask" shows a bitfield of the register, that means one of PHY parameters.
> The "value" shows a parameter value to set to the bitfield.

What does each of these bitfields represent? Which PHY parameter does it
configure. Are they really PHY parameters or they also configure clocks/mux
etc? I would like the configurations to be more descriptive so that we get to
know at least what's getting configured here.

Thanks
Kishon

Re: [PATCH 1/4] dt-bindings: phy: add DT bindings for UniPhier USB3 PHY driver

[Rob Herring]

On Fri, Jun 29, 2018 at 05:38:58PM +0900, Kunihiko Hayashi wrote:
> Add DT bindings for PHY interface built into USB3 controller
> implemented in UniPhier SoCs.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>  .../devicetree/bindings/phy/uniphier-usb3-phy.txt  | 118 +++++++++++++++++++++
>  1 file changed, 118 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt b/Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt
> new file mode 100644
> index 0000000..3df4a486
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt
> @@ -0,0 +1,118 @@
> +Socionext UniPhier USB3 PHY
> +
> +This describes the devicetree bindings for PHY interfaces built into
> +USB3 controller implemented on Socionext UniPhier SoCs.
> +The controller includes High-Speed PHY and Super-Speed PHY.
> +
> +USB3 High-Speed (HS) PHY
> +------------------------
> +
> +Required properties:
> +- compatible: Should contain one of the following:
> +    "socionext,uniphier-pro4-usb3-hsphy" - for Pro4 SoC
> +    "socionext,uniphier-pxs2-usb3-hsphy" - for PXs2 SoC
> +    "socionext,uniphier-ld20-usb3-hsphy" - for LD20 SoC
> +    "socionext,uniphier-pxs3-usb3-hsphy" - for PXs3 SoC
> +- reg: Specifies offset and length of the register set for the device.
> +- #phy-cells: Should be 0.
> +- clocks: A list of phandles to the clock gate for USB3 glue layer.
> +	According to the clock-names, appropriate clocks are required.
> +- clock-names: Should contain the following:
> +    "gio", "link" - for Pro4 SoC
> +    "link", "phy", "phy-ext" - for PXs3 SoC, "phy-ext" is optional.
> +    "link", "phy" - for others

Can't you make 'link' always first.

> +- resets: A list of phandles to the reset control for USB3 glue layer.
> +	According to the reset-names, appropriate resets are required.
> +- reset-names: Should contain the following:
> +    "gio", "link" - for Pro4 SoC
> +    "link", "phy" - for others
> +
> +Optional properties:
> +- phy-supply: A phandle to the regulator for USB VBUS.

The phy actually needs Vbus or you just want to control Vbus from the 
phy driver?

> +- nvmem-cells: Phandles to nvmem cell that contains the trimming data.
> +	Available only for HS-PHY implemented on LD20 and PXs3, and
> +	if unspecified, default value is used.
> +- nvmem-cell-names: Should be the following names, which correspond to
> +	each nvmem-cells.
> +	All of the 3 parameters associated with the following names are
> +	required for each port, if any one is omitted, the trimming data
> +	of the port will not be set at all.
> +    "rterm", "sel_t", "hs_i" - Each cell name for phy parameters
> +
> +Refer to phy/phy-bindings.txt for the generic PHY binding properties.
> +
> +Example:
> +
> +	usb-glue@65b00000 {
> +		compatible = "socionext,uniphier-ld20-dwc3-glue",
> +			     "simple-mfd";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0x65b00000 0x400>;
> +
> +		usb_hsphy0: hs-phy@200 {
> +			compatible = "socionext,uniphier-ld20-usb3-hsphy";
> +			reg = <0x200 0x10>;
> +			#phy-cells = <0>;
> +			clock-names = "link", "phy";
> +			clocks = <&sys_clk 14>, <&sys_clk 16>;
> +			reset-names = "link", "phy";
> +			resets = <&sys_rst 14>, <&sys_rst 16>;
> +			phy-supply = <&usb_vbus0>;
> +			nvmem-cell-names = "rterm", "sel_t", "hs_i";
> +			nvmem-cells = <&usb_rterm0>, <&usb_sel_t0>,
> +				      <&usb_hs_i0>;
> +		};
> +	};
> +
> +
> +USB3 Super-Speed (SS) PHY
> +-------------------------

Nothing seems to be shared here. Make this 2 docs.

> +
> +Required properties:
> +- compatible: Should contain one of the following:
> +    "socionext,uniphier-pro4-usb3-ssphy" - for Pro4 SoC
> +    "socionext,uniphier-pxs2-usb3-ssphy" - for PXs2 SoC
> +    "socionext,uniphier-ld20-usb3-ssphy" - for LD20 SoC
> +    "socionext,uniphier-pxs3-usb3-ssphy" - for PXs3 SoC
> +- reg: Specifies offset and length of the register set for the device.
> +- #phy-cells: Should be 0.
> +- clocks: A list of phandles to the clock gate for USB3 glue layer.
> +	According to the clock-names, appropriate clocks are required.
> +- clock-names:
> +    "gio", "link" - for Pro4 SoC
> +    "link", "phy", "phy-ext" - for PXs3 SoC, "phy-ext" is optional.
> +    "link", "phy" - for others

Can't you make 'link' always first.

> +- resets: A list of phandles to the reset control for USB3 glue layer.
> +	According to the reset-names, appropriate resets are required.
> +- reset-names:
> +    "gio", "link" - for Pro4 SoC
> +    "link", "phy" - for others

And here.

> +
> +Optional properties:
> +- phy-supply: A phandle to the regulator for USB VBUS.
> +
> +Refer to phy/phy-bindings.txt for the generic PHY binding properties.
> +
> +Example:
> +
> +	usb-glue@65b00000 {
> +		compatible = "socionext,uniphier-ld20-dwc3-glue",
> +			     "simple-mfd";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0x65b00000 0x400>;
> +
> +		usb_ssphy0: ss-phy@300 {
> +			compatible = "socionext,uniphier-ld20-usb3-ssphy";
> +			reg = <0x300 0x10>;
> +			#phy-cells = <0>;
> +			clock-names = "link", "phy";
> +			clocks = <&sys_clk 14>, <&sys_clk 16>;
> +			reset-names = "link", "phy";
> +			resets = <&sys_rst 14>, <&sys_rst 16>;
> +			phy-supply = <&usb_vbus0>;
> +		};
> +
> +		other nodes ...
> +	};
> -- 
> 2.7.4
> 

Re: [PATCH 1/4] dt-bindings: phy: add DT bindings for UniPhier USB3 PHY driver

[Kunihiko Hayashi]

Hi Rob,
Thank you for your comments.

On Mon, 16 Jul 2018 14:50:49 -0600 <robh@kernel.org> wrote:

> On Fri, Jun 29, 2018 at 05:38:58PM +0900, Kunihiko Hayashi wrote:
> > Add DT bindings for PHY interface built into USB3 controller
> > implemented in UniPhier SoCs.
> > 
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > ---
> >  .../devicetree/bindings/phy/uniphier-usb3-phy.txt  | 118 +++++++++++++++++++++
> >  1 file changed, 118 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt b/Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt
> > new file mode 100644
> > index 0000000..3df4a486
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/uniphier-usb3-phy.txt
> > @@ -0,0 +1,118 @@
> > +Socionext UniPhier USB3 PHY
> > +
> > +This describes the devicetree bindings for PHY interfaces built into
> > +USB3 controller implemented on Socionext UniPhier SoCs.
> > +The controller includes High-Speed PHY and Super-Speed PHY.
> > +
> > +USB3 High-Speed (HS) PHY
> > +------------------------
> > +
> > +Required properties:
> > +- compatible: Should contain one of the following:
> > +    "socionext,uniphier-pro4-usb3-hsphy" - for Pro4 SoC
> > +    "socionext,uniphier-pxs2-usb3-hsphy" - for PXs2 SoC
> > +    "socionext,uniphier-ld20-usb3-hsphy" - for LD20 SoC
> > +    "socionext,uniphier-pxs3-usb3-hsphy" - for PXs3 SoC
> > +- reg: Specifies offset and length of the register set for the device.
> > +- #phy-cells: Should be 0.
> > +- clocks: A list of phandles to the clock gate for USB3 glue layer.
> > +	According to the clock-names, appropriate clocks are required.
> > +- clock-names: Should contain the following:
> > +    "gio", "link" - for Pro4 SoC
> > +    "link", "phy", "phy-ext" - for PXs3 SoC, "phy-ext" is optional.
> > +    "link", "phy" - for others
> 
> Can't you make 'link' always first.

I see. I'll change the order.

> > +- resets: A list of phandles to the reset control for USB3 glue layer.
> > +	According to the reset-names, appropriate resets are required.
> > +- reset-names: Should contain the following:
> > +    "gio", "link" - for Pro4 SoC
> > +    "link", "phy" - for others
> > +
> > +Optional properties:
> > +- phy-supply: A phandle to the regulator for USB VBUS.
> 
> The phy actually needs Vbus or you just want to control Vbus from the 
> phy driver?

To enable each USB port correspond with phy, we must enable Vbus for each port.
However, the host driver doesn't have the way to control regulators directly,
then I think it is reasonable to control Vbus by using phy regulator.


> > +- nvmem-cells: Phandles to nvmem cell that contains the trimming data.
> > +	Available only for HS-PHY implemented on LD20 and PXs3, and
> > +	if unspecified, default value is used.
> > +- nvmem-cell-names: Should be the following names, which correspond to
> > +	each nvmem-cells.
> > +	All of the 3 parameters associated with the following names are
> > +	required for each port, if any one is omitted, the trimming data
> > +	of the port will not be set at all.
> > +    "rterm", "sel_t", "hs_i" - Each cell name for phy parameters
> > +
> > +Refer to phy/phy-bindings.txt for the generic PHY binding properties.
> > +
> > +Example:
> > +
> > +	usb-glue@65b00000 {
> > +		compatible = "socionext,uniphier-ld20-dwc3-glue",
> > +			     "simple-mfd";
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		ranges = <0 0x65b00000 0x400>;
> > +
> > +		usb_hsphy0: hs-phy@200 {
> > +			compatible = "socionext,uniphier-ld20-usb3-hsphy";
> > +			reg = <0x200 0x10>;
> > +			#phy-cells = <0>;
> > +			clock-names = "link", "phy";
> > +			clocks = <&sys_clk 14>, <&sys_clk 16>;
> > +			reset-names = "link", "phy";
> > +			resets = <&sys_rst 14>, <&sys_rst 16>;
> > +			phy-supply = <&usb_vbus0>;
> > +			nvmem-cell-names = "rterm", "sel_t", "hs_i";
> > +			nvmem-cells = <&usb_rterm0>, <&usb_sel_t0>,
> > +				      <&usb_hs_i0>;
> > +		};
> > +	};
> > +
> > +
> > +USB3 Super-Speed (SS) PHY
> > +-------------------------
> 
> Nothing seems to be shared here. Make this 2 docs.

I see. I'll split it.

> 
> > +
> > +Required properties:
> > +- compatible: Should contain one of the following:
> > +    "socionext,uniphier-pro4-usb3-ssphy" - for Pro4 SoC
> > +    "socionext,uniphier-pxs2-usb3-ssphy" - for PXs2 SoC
> > +    "socionext,uniphier-ld20-usb3-ssphy" - for LD20 SoC
> > +    "socionext,uniphier-pxs3-usb3-ssphy" - for PXs3 SoC
> > +- reg: Specifies offset and length of the register set for the device.
> > +- #phy-cells: Should be 0.
> > +- clocks: A list of phandles to the clock gate for USB3 glue layer.
> > +	According to the clock-names, appropriate clocks are required.
> > +- clock-names:
> > +    "gio", "link" - for Pro4 SoC
> > +    "link", "phy", "phy-ext" - for PXs3 SoC, "phy-ext" is optional.
> > +    "link", "phy" - for others
> 
> Can't you make 'link' always first.

ditto.

> > +- resets: A list of phandles to the reset control for USB3 glue layer.
> > +	According to the reset-names, appropriate resets are required.
> > +- reset-names:
> > +    "gio", "link" - for Pro4 SoC
> > +    "link", "phy" - for others
> 
> And here.
> 
> > +
> > +Optional properties:
> > +- phy-supply: A phandle to the regulator for USB VBUS.
> > +
> > +Refer to phy/phy-bindings.txt for the generic PHY binding properties.
> > +
> > +Example:
> > +
> > +	usb-glue@65b00000 {
> > +		compatible = "socionext,uniphier-ld20-dwc3-glue",
> > +			     "simple-mfd";
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		ranges = <0 0x65b00000 0x400>;
> > +
> > +		usb_ssphy0: ss-phy@300 {
> > +			compatible = "socionext,uniphier-ld20-usb3-ssphy";
> > +			reg = <0x300 0x10>;
> > +			#phy-cells = <0>;
> > +			clock-names = "link", "phy";
> > +			clocks = <&sys_clk 14>, <&sys_clk 16>;
> > +			reset-names = "link", "phy";
> > +			resets = <&sys_rst 14>, <&sys_rst 16>;
> > +			phy-supply = <&usb_vbus0>;
> > +		};
> > +
> > +		other nodes ...
> > +	};
> > -- 
> > 2.7.4
> > 

Thank you,

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC

[Kunihiko Hayashi]

Hi Kishon,

On Fri, 13 Jul 2018 12:45:06 +0530 <kishon@ti.com> wrote:

> Hi,
> 
> On Wednesday 11 July 2018 05:35 PM, Kunihiko Hayashi wrote:
> > On Mon, 9 Jul 2018 20:23:19 +0900 <hayashi.kunihiko@socionext.com> wrote:
> > 
> >> Hi Kishon,
> >> Thank you for your comments.
> >>
> >> On Mon, 9 Jul 2018 10:49:50 +0530 <kishon@ti.com> wrote:
> >>
> >>> Hi,
> >>>
> >>> On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote:
> >>>> Add a driver for PHY interface built into USB3 controller
> >>>> implemented in UniPhier SoCs.
> >>>> This driver supports High-Speed PHY and Super-Speed PHY.
> >>>>
> >>>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> >>>> Signed-off-by: Motoya Tanigawa <tanigawa.motoya@socionext.com>
> >>>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> >>>> ---
> >>>>  drivers/phy/Kconfig                         |   1 +
> >>>>  drivers/phy/Makefile                        |   1 +
> >>>>  drivers/phy/socionext/Kconfig               |  12 +
> >>>>  drivers/phy/socionext/Makefile              |   6 +
> >>>>  drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++
> >>>>  drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++
> >>>>  6 files changed, 811 insertions(+)
> >>>>  create mode 100644 drivers/phy/socionext/Kconfig
> >>>>  create mode 100644 drivers/phy/socionext/Makefile
> >>>>  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c
> >>>>  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c
> >>
> >> (snip...)
> >>
> >>>> --- /dev/null
> >>>> +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c
> >>>> @@ -0,0 +1,422 @@
> >>>> +// SPDX-License-Identifier: GPL-2.0
> >>>> +/*
> >>>> + * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller
> >>>> + * Copyright 2015-2018 Socionext Inc.
> >>>> + * Author:
> >>>> + *	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> >>>> + * Contributors:
> >>>> + *      Motoya Tanigawa <tanigawa.motoya@socionext.com>
> >>>> + *      Masami Hiramatsu <masami.hiramatsu@linaro.org>
> >>>> + */
> >>>> +
> >>>> +#include <linux/bitfield.h>
> >>>> +#include <linux/bitops.h>
> >>>> +#include <linux/clk.h>
> >>>> +#include <linux/io.h>
> >>>> +#include <linux/mfd/syscon.h>
> >>>> +#include <linux/module.h>
> >>>> +#include <linux/nvmem-consumer.h>
> >>>> +#include <linux/of.h>
> >>>> +#include <linux/of_platform.h>
> >>>> +#include <linux/phy/phy.h>
> >>>> +#include <linux/platform_device.h>
> >>>> +#include <linux/reset.h>
> >>>> +#include <linux/slab.h>
> >>>> +
> >>>> +#define HSPHY_CFG0		0x0
> >>>> +#define HSPHY_CFG0_HS_I_MASK	GENMASK(31, 28)
> >>>> +#define HSPHY_CFG0_HSDISC_MASK	GENMASK(27, 26)
> >>>> +#define HSPHY_CFG0_SWING_MASK	GENMASK(17, 16)
> >>>> +#define HSPHY_CFG0_SEL_T_MASK	GENMASK(15, 12)
> >>>> +#define HSPHY_CFG0_RTERM_MASK	GENMASK(7, 6)
> >>>> +#define HSPHY_CFG0_TRIMMASK	(HSPHY_CFG0_HS_I_MASK \
> >>>> +				 | HSPHY_CFG0_SEL_T_MASK \
> >>>> +				 | HSPHY_CFG0_RTERM_MASK)
> >>>> +
> >>>> +#define HSPHY_CFG1		0x4
> >>>> +#define HSPHY_CFG1_DAT_EN	BIT(29)
> >>>> +#define HSPHY_CFG1_ADR_EN	BIT(28)
> >>>> +#define HSPHY_CFG1_ADR_MASK	GENMASK(27, 16)
> >>>> +#define HSPHY_CFG1_DAT_MASK	GENMASK(23, 16)
> >>>> +
> >>>> +#define MAX_CLKS	3
> >>>> +#define MAX_RSTS	2
> >>>> +#define MAX_PHY_PARAMS	1
> >>>> +
> >>>> +struct uniphier_u3hsphy_param {
> >>>> +	u32 addr;
> >>>> +	u32 mask;
> >>>> +	u32 val;
> >>>> +};
> >>>
> >>> I'd like to avoid configure the PHY this way, since it's impossible to know
> >>> which register is being configured.
> >>
> > 
> > This way might be misunderstood.
> > These HS-PHY and SS-PHY have "internal" registers, which are not memory-mapped.
> > 
> > And to access these internal registers, we need to specify the number
> > corresponding to the register.
> > 
> > The "addr" in "uniphier_u3hsphy_param" is just the number of the register.
> > The "mask" shows a bitfield of the register, that means one of PHY parameters.
> > The "value" shows a parameter value to set to the bitfield.
> 
> What does each of these bitfields represent? Which PHY parameter does it
> configure. Are they really PHY parameters or they also configure clocks/mux
> etc? I would like the configurations to be more descriptive so that we get to
> know at least what's getting configured here.

These bitfields represent phy parameters only, not include clocks/mux etc.
We can express the register (bitfield) name with macros.

However, only recommended values are noted for the bitfields in the specsheet,
because there are the trimmed values that aren't set as power-on values,
and we should use the values for stable operation.

This driver includes only minimal parameter set, so I think it's difficult
to express them more configurable.

Thank you,

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH 1/4] dt-bindings: phy: add DT bindings for UniPhier USB3 PHY driver

[Rob Herring]

On Tue, Jul 17, 2018 at 4:55 AM Kunihiko Hayashi
<hayashi.kunihiko@socionext.com> wrote:
>
> Hi Rob,
> Thank you for your comments.
>
> On Mon, 16 Jul 2018 14:50:49 -0600 <robh@kernel.org> wrote:
>
> > On Fri, Jun 29, 2018 at 05:38:58PM +0900, Kunihiko Hayashi wrote:
> > > Add DT bindings for PHY interface built into USB3 controller
> > > implemented in UniPhier SoCs.
> > >
> > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > > ---
> > >  .../devicetree/bindings/phy/uniphier-usb3-phy.txt  | 118 +++++++++++++++++++++

[...]

> > > +Optional properties:
> > > +- phy-supply: A phandle to the regulator for USB VBUS.
> >
> > The phy actually needs Vbus or you just want to control Vbus from the
> > phy driver?
>
> To enable each USB port correspond with phy, we must enable Vbus for each port.
> However, the host driver doesn't have the way to control regulators directly,
> then I think it is reasonable to control Vbus by using phy regulator.

DT should reflect the h/w. If Vbus rail is powering the phy then it
should be in the phy node. If you just happen to want to put Vbus
control in the phy driver, that's an OS problem and doesn't belong in
DT. In that case, Vbus should be described as part of a USB connector
node and any driver can walk the tree or graph and get Vbus regulator.

Rob

Re: [PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC

[Kishon Vijay Abraham I]

Hi,

On Tuesday 17 July 2018 04:57 PM, Kunihiko Hayashi wrote:
> Hi Kishon,
> 
> On Fri, 13 Jul 2018 12:45:06 +0530 <kishon@ti.com> wrote:
> 
>> Hi,
>>
>> On Wednesday 11 July 2018 05:35 PM, Kunihiko Hayashi wrote:
>>> On Mon, 9 Jul 2018 20:23:19 +0900 <hayashi.kunihiko@socionext.com> wrote:
>>>
>>>> Hi Kishon,
>>>> Thank you for your comments.
>>>>
>>>> On Mon, 9 Jul 2018 10:49:50 +0530 <kishon@ti.com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote:
>>>>>> Add a driver for PHY interface built into USB3 controller
>>>>>> implemented in UniPhier SoCs.
>>>>>> This driver supports High-Speed PHY and Super-Speed PHY.
>>>>>>
>>>>>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
>>>>>> Signed-off-by: Motoya Tanigawa <tanigawa.motoya@socionext.com>
>>>>>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
>>>>>> ---
>>>>>>  drivers/phy/Kconfig                         |   1 +
>>>>>>  drivers/phy/Makefile                        |   1 +
>>>>>>  drivers/phy/socionext/Kconfig               |  12 +
>>>>>>  drivers/phy/socionext/Makefile              |   6 +
>>>>>>  drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++
>>>>>>  drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++
>>>>>>  6 files changed, 811 insertions(+)
>>>>>>  create mode 100644 drivers/phy/socionext/Kconfig
>>>>>>  create mode 100644 drivers/phy/socionext/Makefile
>>>>>>  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c
>>>>>>  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c
>>>>
>>>> (snip...)
>>>>
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c
>>>>>> @@ -0,0 +1,422 @@
>>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>>> +/*
>>>>>> + * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller
>>>>>> + * Copyright 2015-2018 Socionext Inc.
>>>>>> + * Author:
>>>>>> + *	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
>>>>>> + * Contributors:
>>>>>> + *      Motoya Tanigawa <tanigawa.motoya@socionext.com>
>>>>>> + *      Masami Hiramatsu <masami.hiramatsu@linaro.org>
>>>>>> + */
>>>>>> +
>>>>>> +#include <linux/bitfield.h>
>>>>>> +#include <linux/bitops.h>
>>>>>> +#include <linux/clk.h>
>>>>>> +#include <linux/io.h>
>>>>>> +#include <linux/mfd/syscon.h>
>>>>>> +#include <linux/module.h>
>>>>>> +#include <linux/nvmem-consumer.h>
>>>>>> +#include <linux/of.h>
>>>>>> +#include <linux/of_platform.h>
>>>>>> +#include <linux/phy/phy.h>
>>>>>> +#include <linux/platform_device.h>
>>>>>> +#include <linux/reset.h>
>>>>>> +#include <linux/slab.h>
>>>>>> +
>>>>>> +#define HSPHY_CFG0		0x0
>>>>>> +#define HSPHY_CFG0_HS_I_MASK	GENMASK(31, 28)
>>>>>> +#define HSPHY_CFG0_HSDISC_MASK	GENMASK(27, 26)
>>>>>> +#define HSPHY_CFG0_SWING_MASK	GENMASK(17, 16)
>>>>>> +#define HSPHY_CFG0_SEL_T_MASK	GENMASK(15, 12)
>>>>>> +#define HSPHY_CFG0_RTERM_MASK	GENMASK(7, 6)
>>>>>> +#define HSPHY_CFG0_TRIMMASK	(HSPHY_CFG0_HS_I_MASK \
>>>>>> +				 | HSPHY_CFG0_SEL_T_MASK \
>>>>>> +				 | HSPHY_CFG0_RTERM_MASK)
>>>>>> +
>>>>>> +#define HSPHY_CFG1		0x4
>>>>>> +#define HSPHY_CFG1_DAT_EN	BIT(29)
>>>>>> +#define HSPHY_CFG1_ADR_EN	BIT(28)
>>>>>> +#define HSPHY_CFG1_ADR_MASK	GENMASK(27, 16)
>>>>>> +#define HSPHY_CFG1_DAT_MASK	GENMASK(23, 16)
>>>>>> +
>>>>>> +#define MAX_CLKS	3
>>>>>> +#define MAX_RSTS	2
>>>>>> +#define MAX_PHY_PARAMS	1
>>>>>> +
>>>>>> +struct uniphier_u3hsphy_param {
>>>>>> +	u32 addr;
>>>>>> +	u32 mask;
>>>>>> +	u32 val;
>>>>>> +};
>>>>>
>>>>> I'd like to avoid configure the PHY this way, since it's impossible to know
>>>>> which register is being configured.
>>>>
>>>
>>> This way might be misunderstood.
>>> These HS-PHY and SS-PHY have "internal" registers, which are not memory-mapped.
>>>
>>> And to access these internal registers, we need to specify the number
>>> corresponding to the register.
>>>
>>> The "addr" in "uniphier_u3hsphy_param" is just the number of the register.
>>> The "mask" shows a bitfield of the register, that means one of PHY parameters.
>>> The "value" shows a parameter value to set to the bitfield.
>>
>> What does each of these bitfields represent? Which PHY parameter does it
>> configure. Are they really PHY parameters or they also configure clocks/mux
>> etc? I would like the configurations to be more descriptive so that we get to
>> know at least what's getting configured here.
> 
> These bitfields represent phy parameters only, not include clocks/mux etc.
> We can express the register (bitfield) name with macros.
> 
> However, only recommended values are noted for the bitfields in the specsheet,
> because there are the trimmed values that aren't set as power-on values,
> and we should use the values for stable operation.

Calibration values are fine but at least the registers and bitfields has to be
described using names.

Thanks
Kishon

Re: [PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC

[Kunihiko Hayashi]

Hi Kishon,

On Tue, 24 Jul 2018 09:31:34 +0530 <kishon@ti.com> wrote:

> Hi,
> 
> On Tuesday 17 July 2018 04:57 PM, Kunihiko Hayashi wrote:
> > Hi Kishon,
> > 
> > On Fri, 13 Jul 2018 12:45:06 +0530 <kishon@ti.com> wrote:
> > 
> >> Hi,
> >>
> >> On Wednesday 11 July 2018 05:35 PM, Kunihiko Hayashi wrote:
> >>> On Mon, 9 Jul 2018 20:23:19 +0900 <hayashi.kunihiko@socionext.com> wrote:
> >>>
> >>>> Hi Kishon,
> >>>> Thank you for your comments.
> >>>>
> >>>> On Mon, 9 Jul 2018 10:49:50 +0530 <kishon@ti.com> wrote:
> >>>>
> >>>>> Hi,
> >>>>>
> >>>>> On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote:
> >>>>>> Add a driver for PHY interface built into USB3 controller
> >>>>>> implemented in UniPhier SoCs.
> >>>>>> This driver supports High-Speed PHY and Super-Speed PHY.
> >>>>>>
> >>>>>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> >>>>>> Signed-off-by: Motoya Tanigawa <tanigawa.motoya@socionext.com>
> >>>>>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> >>>>>> ---
> >>>>>>  drivers/phy/Kconfig                         |   1 +
> >>>>>>  drivers/phy/Makefile                        |   1 +
> >>>>>>  drivers/phy/socionext/Kconfig               |  12 +
> >>>>>>  drivers/phy/socionext/Makefile              |   6 +
> >>>>>>  drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++
> >>>>>>  drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++
> >>>>>>  6 files changed, 811 insertions(+)
> >>>>>>  create mode 100644 drivers/phy/socionext/Kconfig
> >>>>>>  create mode 100644 drivers/phy/socionext/Makefile
> >>>>>>  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c
> >>>>>>  create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c
> >>>>
> >>>> (snip...)
> >>>>
> >>>>>> --- /dev/null
> >>>>>> +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c
> >>>>>> @@ -0,0 +1,422 @@
> >>>>>> +// SPDX-License-Identifier: GPL-2.0
> >>>>>> +/*
> >>>>>> + * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller
> >>>>>> + * Copyright 2015-2018 Socionext Inc.
> >>>>>> + * Author:
> >>>>>> + *	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> >>>>>> + * Contributors:
> >>>>>> + *      Motoya Tanigawa <tanigawa.motoya@socionext.com>
> >>>>>> + *      Masami Hiramatsu <masami.hiramatsu@linaro.org>
> >>>>>> + */
> >>>>>> +
> >>>>>> +#include <linux/bitfield.h>
> >>>>>> +#include <linux/bitops.h>
> >>>>>> +#include <linux/clk.h>
> >>>>>> +#include <linux/io.h>
> >>>>>> +#include <linux/mfd/syscon.h>
> >>>>>> +#include <linux/module.h>
> >>>>>> +#include <linux/nvmem-consumer.h>
> >>>>>> +#include <linux/of.h>
> >>>>>> +#include <linux/of_platform.h>
> >>>>>> +#include <linux/phy/phy.h>
> >>>>>> +#include <linux/platform_device.h>
> >>>>>> +#include <linux/reset.h>
> >>>>>> +#include <linux/slab.h>
> >>>>>> +
> >>>>>> +#define HSPHY_CFG0		0x0
> >>>>>> +#define HSPHY_CFG0_HS_I_MASK	GENMASK(31, 28)
> >>>>>> +#define HSPHY_CFG0_HSDISC_MASK	GENMASK(27, 26)
> >>>>>> +#define HSPHY_CFG0_SWING_MASK	GENMASK(17, 16)
> >>>>>> +#define HSPHY_CFG0_SEL_T_MASK	GENMASK(15, 12)
> >>>>>> +#define HSPHY_CFG0_RTERM_MASK	GENMASK(7, 6)
> >>>>>> +#define HSPHY_CFG0_TRIMMASK	(HSPHY_CFG0_HS_I_MASK \
> >>>>>> +				 | HSPHY_CFG0_SEL_T_MASK \
> >>>>>> +				 | HSPHY_CFG0_RTERM_MASK)
> >>>>>> +
> >>>>>> +#define HSPHY_CFG1		0x4
> >>>>>> +#define HSPHY_CFG1_DAT_EN	BIT(29)
> >>>>>> +#define HSPHY_CFG1_ADR_EN	BIT(28)
> >>>>>> +#define HSPHY_CFG1_ADR_MASK	GENMASK(27, 16)
> >>>>>> +#define HSPHY_CFG1_DAT_MASK	GENMASK(23, 16)
> >>>>>> +
> >>>>>> +#define MAX_CLKS	3
> >>>>>> +#define MAX_RSTS	2
> >>>>>> +#define MAX_PHY_PARAMS	1
> >>>>>> +
> >>>>>> +struct uniphier_u3hsphy_param {
> >>>>>> +	u32 addr;
> >>>>>> +	u32 mask;
> >>>>>> +	u32 val;
> >>>>>> +};
> >>>>>
> >>>>> I'd like to avoid configure the PHY this way, since it's impossible to know
> >>>>> which register is being configured.
> >>>>
> >>>
> >>> This way might be misunderstood.
> >>> These HS-PHY and SS-PHY have "internal" registers, which are not memory-mapped.
> >>>
> >>> And to access these internal registers, we need to specify the number
> >>> corresponding to the register.
> >>>
> >>> The "addr" in "uniphier_u3hsphy_param" is just the number of the register.
> >>> The "mask" shows a bitfield of the register, that means one of PHY parameters.
> >>> The "value" shows a parameter value to set to the bitfield.
> >>
> >> What does each of these bitfields represent? Which PHY parameter does it
> >> configure. Are they really PHY parameters or they also configure clocks/mux
> >> etc? I would like the configurations to be more descriptive so that we get to
> >> know at least what's getting configured here.
> > 
> > These bitfields represent phy parameters only, not include clocks/mux etc.
> > We can express the register (bitfield) name with macros.
> > 
> > However, only recommended values are noted for the bitfields in the specsheet,
> > because there are the trimmed values that aren't set as power-on values,
> > and we should use the values for stable operation.
> 
> Calibration values are fine but at least the registers and bitfields has to be
> described using names.

I see. I'll describe bitfields using defined macros.
The register numbers are like a page number, so they have no names even
in the specsheet because there are various phy parameters in one register.

Thank you,

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH 1/4] dt-bindings: phy: add DT bindings for UniPhier USB3 PHY driver

[Kunihiko Hayashi]

Hi Rob,

On Tue, 17 Jul 2018 08:16:23 -0600 <robh@kernel.org> wrote:

> On Tue, Jul 17, 2018 at 4:55 AM Kunihiko Hayashi
> <hayashi.kunihiko@socionext.com> wrote:
> >
> > Hi Rob,
> > Thank you for your comments.
> >
> > On Mon, 16 Jul 2018 14:50:49 -0600 <robh@kernel.org> wrote:
> >
> > > On Fri, Jun 29, 2018 at 05:38:58PM +0900, Kunihiko Hayashi wrote:
> > > > Add DT bindings for PHY interface built into USB3 controller
> > > > implemented in UniPhier SoCs.
> > > >
> > > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > > > ---
> > > >  .../devicetree/bindings/phy/uniphier-usb3-phy.txt  | 118 +++++++++++++++++++++
> 
> [...]
> 
> > > > +Optional properties:
> > > > +- phy-supply: A phandle to the regulator for USB VBUS.
> > >
> > > The phy actually needs Vbus or you just want to control Vbus from the
> > > phy driver?
> >
> > To enable each USB port correspond with phy, we must enable Vbus for each port.
> > However, the host driver doesn't have the way to control regulators directly,
> > then I think it is reasonable to control Vbus by using phy regulator.
> 
> DT should reflect the h/w. If Vbus rail is powering the phy then it
> should be in the phy node. If you just happen to want to put Vbus
> control in the phy driver, that's an OS problem and doesn't belong in
> DT. In that case, Vbus should be described as part of a USB connector
> node and any driver can walk the tree or graph and get Vbus regulator.

I think it's difficult to describe port nodes related to connectors that
has vbus regulator.

This controller has a glue layer, that consists of "reset", "regulator",
and "phy", and there are their respective drivers and their own nodes,
but no glue layer driver [1].

[1] https://www.spinics.net/lists/linux-usb/msg165060.html

For example, when the glue layer node has each port that includes vbus,
we will describe like that:

--------------------------------
usb {
	...
	resets = <&usb_reset>;
	phys = <&usb_phy0>, <&usb_phy1>;
};

usb-glue {
	usb_reset: reset {
		...
	};
	usb_vbus0: regulator {
		...
	};
	usb_vbus1: regulator {
		...
	};
	usb_phy0: phy {
		...
	};
	usb_phy1: phy {
		...
	};

	ports {
		port@0 {
			...
			vbus-supply = <&usb_vbus0>;
		};
		port@1 {
			...
			vbus-supply = <&usb_vbus1>;
		};
	};
};
--------------------------------

However, there is no driver that can walk the tree to get vbus regulator,
and we can't describe that the vbus and phy are related.

In this controller, each port with vbus corresponds to each phy,
and we need to enable the phy and control the vbus.

For the above reasons, and since vbus is not phy's power,
I think it's reasonable to add "vbus-supply" only instead of "phy-supply"
in the phy node like sun4i-usb-phy, rockchip-usb-phy, etc.

--------------------------------
usb {
	...
	resets = <&usb_reset>;
	phys = <&usb_phy0>, <&usb_phy1>;
};

usb-glue {
	usb_reset: reset {
		...
	};
	usb_vbus0: regulator {
		...
	};
	usb_vbus1: regulator {
		...
	};
	usb_phy0: phy {
		...
		vbus-supply = <&usb_vbus0>;
	};
	usb_phy1: phy {
		...
		vbus-supply = <&usb_vbus1>;
	};
};
--------------------------------

Thank you,

---
Best Regards,
Kunihiko Hayashi