Hi Kishon,

On Tue, 24 Jul 2018 09:31:34 +0530 [off-list ref] wrote:

Hi,

On Tuesday 17 July 2018 04:57 PM, Kunihiko Hayashi wrote:

quoted

Hi Kishon,

On Fri, 13 Jul 2018 12:45:06 +0530 [off-list ref] wrote:

quoted

Hi,

On Wednesday 11 July 2018 05:35 PM, Kunihiko Hayashi wrote:

quoted

On Mon, 9 Jul 2018 20:23:19 +0900 [off-list ref] wrote:

quoted

Hi Kishon,
Thank you for your comments.

On Mon, 9 Jul 2018 10:49:50 +0530 [off-list ref] wrote:

quoted

Hi,

On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote:

quoted

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 <redacted>
Signed-off-by: Masami Hiramatsu <redacted>
---
 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...)

quoted

quoted

--- /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