[PATCH 0/3] Stingray usb phy driver support

[PATCH 0/3] Stingray usb phy driver support

[Srinath Mannam]

These patches add stingray usb phy driver and its
corresponding DT nodes with documentation.

All patches are based on Linux-4.19.

Srinath Mannam (3):
  dt-bindings: phy: Add binding document for stingray usb phy
  phy: sr-usb: Add stingray usb phy driver
  arm64: dts: Add USB DT nodes for Stingray SoC

 .../bindings/phy/brcm,stingray-usb-phy.txt         |  62 ++++
 .../boot/dts/broadcom/stingray/stingray-usb.dtsi   |  95 ++++++
 .../arm64/boot/dts/broadcom/stingray/stingray.dtsi |   1 +
 drivers/phy/broadcom/Kconfig                       |  11 +
 drivers/phy/broadcom/Makefile                      |   1 +
 drivers/phy/broadcom/phy-bcm-sr-usb.c              | 367 +++++++++++++++++++++
 6 files changed, 537 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/brcm,stingray-usb-phy.txt
 create mode 100644 arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi
 create mode 100644 drivers/phy/broadcom/phy-bcm-sr-usb.c

-- 
2.7.4

[PATCH 1/3] dt-bindings: phy: Add binding document for stingray usb phy

[Srinath Mannam]

Add DT binding document for stingray usb phy.

Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 .../bindings/phy/brcm,stingray-usb-phy.txt         | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/brcm,stingray-usb-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/brcm,stingray-usb-phy.txt b/Documentation/devicetree/bindings/phy/brcm,stingray-usb-phy.txt
new file mode 100644
index 0000000..bfe1367
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/brcm,stingray-usb-phy.txt
@@ -0,0 +1,62 @@
+Broadcom Stingray USB PHY
+
+Required properties:
+ - compatible : should be one of the listed compatibles
+	- "brcm,sr-usb-phy"
+	- "brcm,sr-usb-phy-v2"
+ - reg: offset and length of the PHY blocks registers
+ - address-cells: should be 1
+ - size-cells: should be 0
+
+Sub-nodes:
+  Each port's PHY should be represented as a sub-node.
+
+Sub-nodes required properties:
+ - reg: required for brcm,sr-usb-phy model phy.
+	reg value 0 is HS phy and 1 is SS phy.
+ - phy-cells: generic PHY binding; must be 0
+
+Refer to phy/phy-bindings.txt for the generic PHY binding properties
+
+Example:
+	usbphy0: usb-phy@0 {
+		compatible = "brcm,sr-usb-phy";
+		reg = <0x00000000 0x100>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		usb0_phy0: phy@0 {
+			reg = <0>;
+			#phy-cells = <0>;
+		};
+
+		usb0_phy1: phy@1 {
+			reg = <1>;
+			#phy-cells = <0>;
+		};
+	};
+
+	usbphy1: usb-phy@10000 {
+		compatible = "brcm,sr-usb-phy";
+		reg = <0x00010000 0x100>,
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		usb1_phy0: phy@0 {
+			reg = <0>;
+			#phy-cells = <0>;
+		};
+
+		usb1_phy1: phy@1 {
+			reg = <1>;
+			#phy-cells = <0>;
+		};
+	};
+
+	usbphy2: usb-phy@20000 {
+		compatible = "brcm,sr-usb-phy-v2";
+		reg = <0x00020000 0x100>,
+		#address-cells = <1>;
+		#size-cells = <0>;
+		#phy-cells = <0>;
+	};
-- 
2.7.4

[PATCH 2/3] phy: sr-usb: Add stingray usb phy driver

[Srinath Mannam]

This driver supports all versions of stingray SS and HS
USB phys.
In version 1 is combo phy contain both SS and HS phys
in a common IO space.
In version 2 a single HS phy.
These phys support both xHCI host driver and
BDC Broadcom device controller driver.

Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 drivers/phy/broadcom/Kconfig          |  11 +
 drivers/phy/broadcom/Makefile         |   1 +
 drivers/phy/broadcom/phy-bcm-sr-usb.c | 367 ++++++++++++++++++++++++++++++++++
 3 files changed, 379 insertions(+)
 create mode 100644 drivers/phy/broadcom/phy-bcm-sr-usb.c

diff --git a/drivers/phy/broadcom/Kconfig b/drivers/phy/broadcom/Kconfig
index 8786a96..c1e4dd5 100644
--- a/drivers/phy/broadcom/Kconfig
+++ b/drivers/phy/broadcom/Kconfig
@@ -10,6 +10,17 @@ config PHY_CYGNUS_PCIE
 	  Enable this to support the Broadcom Cygnus PCIe PHY.
 	  If unsure, say N.
 
+config PHY_BCM_SR_USB
+	tristate "Broadcom Stingray USB PHY driver"
+	depends on OF && (ARCH_BCM_IPROC || COMPILE_TEST)
+	select GENERIC_PHY
+	default ARCH_BCM_IPROC
+	help
+	  Enable this to support the Broadcom Stingray USB PHY
+	  driver. It supports all versions of Superspeed and
+	  Highspeed PHYs.
+	  If unsure, say N.
+
 config BCM_KONA_USB2_PHY
 	tristate "Broadcom Kona USB2 PHY Driver"
 	depends on HAS_IOMEM
diff --git a/drivers/phy/broadcom/Makefile b/drivers/phy/broadcom/Makefile
index 0f60184..f453c7d 100644
--- a/drivers/phy/broadcom/Makefile
+++ b/drivers/phy/broadcom/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_PHY_BRCM_USB)		+= phy-brcm-usb-dvr.o
 phy-brcm-usb-dvr-objs := phy-brcm-usb.o phy-brcm-usb-init.o
 
 obj-$(CONFIG_PHY_BCM_SR_PCIE)		+= phy-bcm-sr-pcie.o
+obj-$(CONFIG_PHY_BCM_SR_USB)		+= phy-bcm-sr-usb.o
diff --git a/drivers/phy/broadcom/phy-bcm-sr-usb.c b/drivers/phy/broadcom/phy-bcm-sr-usb.c
new file mode 100644
index 0000000..99de49f
--- /dev/null
+++ b/drivers/phy/broadcom/phy-bcm-sr-usb.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016-2018 Broadcom
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+enum bcm_usb_phy_version {
+	BCM_USB_PHY_V1,
+	BCM_USB_PHY_V2,
+};
+
+enum bcm_usb_phy_reg {
+	PLL_NDIV_FRAC,
+	PLL_NDIV_INT,
+	PLL_CTRL,
+	PHY_CTRL,
+	PHY_PLL_CTRL,
+};
+
+/* USB PHY registers */
+
+static const u8 bcm_usb_u3phy_v1[] = {
+	[PLL_CTRL]		= 0x18,
+	[PHY_CTRL]		= 0x14,
+};
+
+static const u8 bcm_usb_u2phy_v1[] = {
+	[PLL_NDIV_FRAC]	= 0x04,
+	[PLL_NDIV_INT]	= 0x08,
+	[PLL_CTRL]	= 0x0c,
+	[PHY_CTRL]	= 0x10,
+};
+
+#define HSPLL_NDIV_INT_VAL	0x13
+#define HSPLL_NDIV_FRAC_VAL	0x1005
+
+static const u8 bcm_usb_u2phy_v2[] = {
+	[PLL_NDIV_FRAC]	= 0x0,
+	[PLL_NDIV_INT]	= 0x4,
+	[PLL_CTRL]	= 0x8,
+	[PHY_CTRL]	= 0xc,
+};
+
+enum pll_ctrl_bits {
+	PLL_RESETB,
+	SSPLL_SUSPEND_EN,
+	PLL_SEQ_START,
+	PLL_LOCK,
+	PLL_PDIV,
+};
+
+static const u8 u3pll_ctrl[] = {
+	[PLL_RESETB]		= 0,
+	[SSPLL_SUSPEND_EN]	= 1,
+	[PLL_SEQ_START]		= 2,
+	[PLL_LOCK]		= 3,
+};
+
+#define HSPLL_PDIV_MASK		0xF
+#define HSPLL_PDIV_VAL		0x1
+
+static const u8 u2pll_ctrl[] = {
+	[PLL_PDIV]	= 1,
+	[PLL_RESETB]	= 5,
+	[PLL_LOCK]	= 6,
+};
+
+enum bcm_usb_phy_ctrl_bits {
+	CORERDY,
+	AFE_LDO_PWRDWNB,
+	AFE_PLL_PWRDWNB,
+	AFE_BG_PWRDWNB,
+	PHY_ISO,
+	PHY_RESETB,
+	PHY_PCTL,
+};
+
+#define PHY_PCTL_MASK	0xffff
+/*
+ * 0x0806 of PCTL_VAL has below bits set
+ * BIT-8 : refclk divider 1
+ * BIT-3:2: device mode; mode is not effect
+ * BIT-1: soft reset active low
+ */
+#define HSPHY_PCTL_VAL	0x0806
+#define SSPHY_PCTL_VAL	0x0006
+
+static const u8 u3phy_ctrl[] = {
+	[PHY_RESETB]	= 1,
+	[PHY_PCTL]	= 2,
+};
+
+static const u8 u2phy_ctrl[] = {
+	[CORERDY]		= 0,
+	[AFE_LDO_PWRDWNB]	= 1,
+	[AFE_PLL_PWRDWNB]	= 2,
+	[AFE_BG_PWRDWNB]	= 3,
+	[PHY_ISO]		= 4,
+	[PHY_RESETB]		= 5,
+	[PHY_PCTL]		= 6,
+};
+
+struct bcm_usb_phy_cfg {
+	uint32_t type;
+	uint32_t ver;
+	void __iomem *regs;
+	struct phy *phy;
+	const u8 *offset;
+};
+
+#define PLL_LOCK_RETRY_COUNT	1000
+
+enum bcm_usb_phy_type {
+	USB_HS_PHY,
+	USB_SS_PHY,
+};
+
+static inline void bcm_usb_reg32_clrbits(void __iomem *addr, uint32_t clear)
+{
+	writel(readl(addr) & ~clear, addr);
+}
+
+static inline void bcm_usb_reg32_setbits(void __iomem *addr, uint32_t set)
+{
+	writel(readl(addr) | set, addr);
+}
+
+static int bcm_usb_pll_lock_check(void __iomem *addr, u32 bit)
+{
+	int retry;
+	u32 rd_data;
+
+	retry = PLL_LOCK_RETRY_COUNT;
+	do {
+		rd_data = readl(addr);
+		if (rd_data & bit)
+			return 0;
+		udelay(1);
+	} while (--retry > 0);
+
+	pr_err("%s: FAIL\n", __func__);
+	return -ETIMEDOUT;
+}
+
+static int bcm_usb_ss_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
+{
+	int ret = 0;
+	void __iomem *regs = phy_cfg->regs;
+	const u8 *offset;
+	u32 rd_data;
+
+	offset = phy_cfg->offset;
+
+	/* Set pctl with mode and soft reset */
+	rd_data = readl(regs + offset[PHY_CTRL]);
+	rd_data &= ~(PHY_PCTL_MASK << u3phy_ctrl[PHY_PCTL]);
+	rd_data |= (SSPHY_PCTL_VAL << u3phy_ctrl[PHY_PCTL]);
+	writel(rd_data, regs + offset[PHY_CTRL]);
+
+	bcm_usb_reg32_clrbits(regs + offset[PLL_CTRL],
+			      BIT(u3pll_ctrl[SSPLL_SUSPEND_EN]));
+	bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
+			      BIT(u3pll_ctrl[PLL_SEQ_START]));
+	bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
+			      BIT(u3pll_ctrl[PLL_RESETB]));
+
+	msleep(30);
+
+	ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
+				     BIT(u3pll_ctrl[PLL_LOCK]));
+
+	return ret;
+}
+
+static int bcm_usb_hs_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
+{
+	int ret = 0;
+	void __iomem *regs = phy_cfg->regs;
+	const u8 *offset;
+	u32 rd_data;
+
+	offset = phy_cfg->offset;
+
+	writel(HSPLL_NDIV_INT_VAL, regs + offset[PLL_NDIV_INT]);
+	writel(HSPLL_NDIV_FRAC_VAL, regs + offset[PLL_NDIV_FRAC]);
+
+	rd_data = readl(regs + offset[PLL_CTRL]);
+	rd_data &= ~(HSPLL_PDIV_MASK << u2pll_ctrl[PLL_PDIV]);
+	rd_data |= (HSPLL_PDIV_VAL << u2pll_ctrl[PLL_PDIV]);
+	writel(rd_data, regs + offset[PLL_CTRL]);
+
+	/* Set Core Ready high */
+	bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
+			      BIT(u2phy_ctrl[CORERDY]));
+
+	msleep(100);
+
+	bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
+			      BIT(u2pll_ctrl[PLL_RESETB]));
+	bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
+			      BIT(u2phy_ctrl[PHY_RESETB]));
+
+
+	rd_data = readl(regs + offset[PHY_CTRL]);
+	rd_data &= ~(PHY_PCTL_MASK << u2phy_ctrl[PHY_PCTL]);
+	rd_data |= (HSPHY_PCTL_VAL << u2phy_ctrl[PHY_PCTL]);
+	writel(rd_data, regs + offset[PHY_CTRL]);
+
+	ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
+				     BIT(u2pll_ctrl[PLL_LOCK]));
+
+	return ret;
+}
+
+static int bcm_usb_phy_reset(struct phy *phy)
+{
+	struct bcm_usb_phy_cfg *phy_cfg = phy_get_drvdata(phy);
+	void __iomem *regs = phy_cfg->regs;
+	const u8 *offset;
+
+	offset = phy_cfg->offset;
+
+	if (phy_cfg->type == USB_HS_PHY) {
+		bcm_usb_reg32_clrbits(regs + offset[PHY_CTRL],
+				      BIT(u2phy_ctrl[CORERDY]));
+		bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
+				      BIT(u2phy_ctrl[CORERDY]));
+	}
+
+	return 0;
+}
+
+static int bcm_usb_phy_init(struct phy *phy)
+{
+	struct bcm_usb_phy_cfg *phy_cfg = phy_get_drvdata(phy);
+
+	if (phy_cfg->type == USB_SS_PHY)
+		bcm_usb_ss_phy_init(phy_cfg);
+	if (phy_cfg->type == USB_HS_PHY)
+		bcm_usb_hs_phy_init(phy_cfg);
+
+	return 0;
+}
+
+static struct phy_ops sr_phy_ops = {
+	.init		= bcm_usb_phy_init,
+	.reset		= bcm_usb_phy_reset,
+	.owner		= THIS_MODULE,
+};
+
+static int bcm_usb_phy_create(struct device *dev, struct device_node *node,
+			     void __iomem *regs, uint32_t version)
+{
+	struct bcm_usb_phy_cfg *phy_cfg;
+	struct phy_provider *phy_provider;
+
+	phy_cfg = devm_kzalloc(dev, sizeof(struct bcm_usb_phy_cfg), GFP_KERNEL);
+	if (!phy_cfg)
+		return -ENOMEM;
+
+	phy_cfg->regs = regs;
+	phy_cfg->ver = version;
+
+	if (phy_cfg->ver == BCM_USB_PHY_V1) {
+		unsigned int id;
+
+		if (of_property_read_u32(node, "reg", &id)) {
+			dev_err(dev, "missing reg property in node %s\n",
+				node->name);
+			return -EINVAL;
+		}
+
+		if (id == 0) {
+			phy_cfg->offset = bcm_usb_u2phy_v1;
+			phy_cfg->type = USB_HS_PHY;
+		} else if (id == 1) {
+			phy_cfg->offset = bcm_usb_u3phy_v1;
+			phy_cfg->type = USB_SS_PHY;
+		} else {
+			return -ENODEV;
+		}
+	} else if (phy_cfg->ver == BCM_USB_PHY_V2) {
+		phy_cfg->offset = bcm_usb_u2phy_v2;
+		phy_cfg->type = USB_HS_PHY;
+	}
+
+	phy_cfg->phy = devm_phy_create(dev, node, &sr_phy_ops);
+	if (IS_ERR(phy_cfg->phy))
+		return PTR_ERR(phy_cfg->phy);
+
+	phy_set_drvdata(phy_cfg->phy, phy_cfg);
+	phy_provider = devm_of_phy_provider_register(&phy_cfg->phy->dev,
+						     of_phy_simple_xlate);
+	if (IS_ERR(phy_provider)) {
+		dev_err(dev, "Failed to register phy provider\n");
+		return PTR_ERR(phy_provider);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id bcm_usb_phy_of_match[] = {
+	{
+		.compatible = "brcm,sr-usb-phy",
+		.data = (void *)BCM_USB_PHY_V1,
+	},
+	{
+		.compatible = "brcm,sr-usb-phy-v2",
+		.data = (void *)BCM_USB_PHY_V2,
+	},
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, bcm_usb_phy_of_match);
+
+static int bcm_usb_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *dn = dev->of_node, *child;
+	const struct of_device_id *of_id;
+	struct resource *res;
+	void __iomem *regs;
+	int ret;
+	enum bcm_usb_phy_version version;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	of_id = of_match_node(bcm_usb_phy_of_match, dn);
+	if (of_id)
+		version = (enum bcm_usb_phy_version)of_id->data;
+	else
+		return -ENODEV;
+
+	if (of_get_child_count(dn) == 0)
+		return bcm_usb_phy_create(dev, dn, regs, version);
+
+	for_each_available_child_of_node(dn, child) {
+		ret = bcm_usb_phy_create(dev, child, regs, version);
+		if (ret) {
+			of_node_put(child);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static struct platform_driver bcm_usb_phy_driver = {
+	.driver = {
+		.name = "phy-bcm-sr-usb",
+		.of_match_table = bcm_usb_phy_of_match,
+	},
+	.probe = bcm_usb_phy_probe,
+};
+module_platform_driver(bcm_usb_phy_driver);
+
+MODULE_AUTHOR("Broadcom");
+MODULE_DESCRIPTION("Broadcom stingray USB Phy driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH 3/3] arm64: dts: Add USB DT nodes for Stingray SoC

[Srinath Mannam]

Add DT nodes for
  - Two xHCI host controllers
  - Two BDC Broadcom USB device controller
  - Five USB PHY controllers

[xHCI0]      [BDC0]            [xHCI1]            [BDC1]
   |           |                  |                 |
  ---------------               -----------------------
   |           |                 |         |         |
[SS-PHY0]   [HS-PHY0]        [SS-PHY1] [HS-PHY2] [HS-PHY1]

[SS-PHY0/HS-PHY0] and [SS-PHY1/HS-PHY1] are combo PHYs of
version1 category has both SS and HS PHYs..
[HS-PHY2] is HS PHY of version2 category.

Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 .../boot/dts/broadcom/stingray/stingray-usb.dtsi   | 95 ++++++++++++++++++++++
 .../arm64/boot/dts/broadcom/stingray/stingray.dtsi |  1 +
 2 files changed, 96 insertions(+)
 create mode 100644 arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi

diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi
new file mode 100644
index 0000000..164a1de
--- /dev/null
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: (GPL-2.0 or BSD-3-Clause)
+/*
+ *Copyright(c) 2018 Broadcom
+ */
+	usb {
+		compatible = "simple-bus";
+		dma-ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0x0 0x0 0x68500000 0x00400000>;
+
+		usbphy0: usb-phy@0 {
+			compatible = "brcm,sr-usb-phy";
+			reg = <0x00000000 0x100>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			usb0_phy0: phy@0 {
+				reg = <0>;
+				#phy-cells = <0>;
+			};
+			usb0_phy1: phy@1 {
+				reg = <1>;
+				#phy-cells = <0>;
+			};
+
+		};
+
+		xhci0: usb@1000 {
+			compatible = "generic-xhci";
+			reg = <0x00001000 0x1000>;
+			interrupts = <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>;
+			phys = <&usb0_phy1>, <&usb0_phy0>;
+			phy-names = "phy0", "phy1";
+			dma-coherent;
+			status = "disabled";
+		};
+
+		bdc0: usb@2000 {
+			compatible = "brcm,bdc-v0.16";
+			reg = <0x00002000 0x1000>;
+			interrupts = <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH>;
+			phys = <&usb0_phy1>, <&usb0_phy0>;
+			phy-names = "phy0", "phy1";
+			dma-coherent;
+			status = "disabled";
+		};
+
+		usbphy1: usb-phy@10000 {
+			compatible = "brcm,sr-usb-phy";
+			reg = <0x00010000 0x100>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			usb1_phy0: phy@0 {
+				reg = <0>;
+				#phy-cells = <0>;
+			};
+			usb1_phy1: phy@1 {
+				reg = <1>;
+				#phy-cells = <0>;
+			};
+		};
+
+		usbphy2: usb-phy@20000 {
+			compatible = "brcm,sr-usb-phy-v2";
+			reg = <0x00020000 0x100>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			#phy-cells = <0>;
+			status = "disabled";
+		};
+
+		xhci1: usb@11000 {
+			compatible = "generic-xhci";
+			reg = <0x00011000 0x1000>;
+			interrupts = <GIC_SPI 263 IRQ_TYPE_LEVEL_HIGH>;
+			phys = <&usb1_phy1>, <&usbphy2>, <&usb1_phy0>;
+			phy-names = "phy0", "phy1", "phy2";
+			dma-coherent;
+			status = "disabled";
+		};
+
+		bdc1: usb@21000 {
+			compatible = "brcm,bdc-v0.16";
+			reg = <0x00021000 0x1000>;
+			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
+			phys = <&usbphy2>;
+			phy-names = "phy0";
+			dma-coherent;
+			status = "disabled";
+		};
+	};
diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
index e283480..6927236 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
@@ -287,6 +287,7 @@
 	#include "stingray-fs4.dtsi"
 	#include "stingray-sata.dtsi"
 	#include "stingray-pcie.dtsi"
+	#include "stingray-usb.dtsi"
 
 	hsls {
 		compatible = "simple-bus";
-- 
2.7.4

Re: [PATCH 0/3] Stingray usb phy driver support

[Srinath Mannam]

Hi Vijay,

Could you please help to review and provide your comments to this
patch series when you have time?

Thank you,
Regards,
Srinath.

On Tue, Nov 13, 2018 at 9:43 AM Srinath Mannam
<srinath.mannam@broadcom.com> wrote:
>
> These patches add stingray usb phy driver and its
> corresponding DT nodes with documentation.
>
> All patches are based on Linux-4.19.
>
> Srinath Mannam (3):
>   dt-bindings: phy: Add binding document for stingray usb phy
>   phy: sr-usb: Add stingray usb phy driver
>   arm64: dts: Add USB DT nodes for Stingray SoC
>
>  .../bindings/phy/brcm,stingray-usb-phy.txt         |  62 ++++
>  .../boot/dts/broadcom/stingray/stingray-usb.dtsi   |  95 ++++++
>  .../arm64/boot/dts/broadcom/stingray/stingray.dtsi |   1 +
>  drivers/phy/broadcom/Kconfig                       |  11 +
>  drivers/phy/broadcom/Makefile                      |   1 +
>  drivers/phy/broadcom/phy-bcm-sr-usb.c              | 367 +++++++++++++++++++++
>  6 files changed, 537 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/brcm,stingray-usb-phy.txt
>  create mode 100644 arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi
>  create mode 100644 drivers/phy/broadcom/phy-bcm-sr-usb.c
>
> --
> 2.7.4
>

Re: [PATCH 2/3] phy: sr-usb: Add stingray usb phy driver

[Kishon Vijay Abraham I]

Hi,

On 13/11/18 9:43 AM, Srinath Mannam wrote:
> This driver supports all versions of stingray SS and HS
> USB phys.
> In version 1 is combo phy contain both SS and HS phys
> in a common IO space.
> In version 2 a single HS phy.
> These phys support both xHCI host driver and
> BDC Broadcom device controller driver.
> 
> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> ---
>  drivers/phy/broadcom/Kconfig          |  11 +
>  drivers/phy/broadcom/Makefile         |   1 +
>  drivers/phy/broadcom/phy-bcm-sr-usb.c | 367 ++++++++++++++++++++++++++++++++++
>  3 files changed, 379 insertions(+)
>  create mode 100644 drivers/phy/broadcom/phy-bcm-sr-usb.c
> 
> diff --git a/drivers/phy/broadcom/Kconfig b/drivers/phy/broadcom/Kconfig
> index 8786a96..c1e4dd5 100644
> --- a/drivers/phy/broadcom/Kconfig
> +++ b/drivers/phy/broadcom/Kconfig
> @@ -10,6 +10,17 @@ config PHY_CYGNUS_PCIE
>  	  Enable this to support the Broadcom Cygnus PCIe PHY.
>  	  If unsure, say N.
>  
> +config PHY_BCM_SR_USB
> +	tristate "Broadcom Stingray USB PHY driver"
> +	depends on OF && (ARCH_BCM_IPROC || COMPILE_TEST)
> +	select GENERIC_PHY
> +	default ARCH_BCM_IPROC
> +	help
> +	  Enable this to support the Broadcom Stingray USB PHY
> +	  driver. It supports all versions of Superspeed and
> +	  Highspeed PHYs.
> +	  If unsure, say N.
> +
>  config BCM_KONA_USB2_PHY
>  	tristate "Broadcom Kona USB2 PHY Driver"
>  	depends on HAS_IOMEM
> diff --git a/drivers/phy/broadcom/Makefile b/drivers/phy/broadcom/Makefile
> index 0f60184..f453c7d 100644
> --- a/drivers/phy/broadcom/Makefile
> +++ b/drivers/phy/broadcom/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_PHY_BRCM_USB)		+= phy-brcm-usb-dvr.o
>  phy-brcm-usb-dvr-objs := phy-brcm-usb.o phy-brcm-usb-init.o
>  
>  obj-$(CONFIG_PHY_BCM_SR_PCIE)		+= phy-bcm-sr-pcie.o
> +obj-$(CONFIG_PHY_BCM_SR_USB)		+= phy-bcm-sr-usb.o
> diff --git a/drivers/phy/broadcom/phy-bcm-sr-usb.c b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> new file mode 100644
> index 0000000..99de49f
> --- /dev/null
> +++ b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> @@ -0,0 +1,367 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2016-2018 Broadcom
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +
> +enum bcm_usb_phy_version {
> +	BCM_USB_PHY_V1,
> +	BCM_USB_PHY_V2,
> +};
> +
> +enum bcm_usb_phy_reg {
> +	PLL_NDIV_FRAC,
> +	PLL_NDIV_INT,
> +	PLL_CTRL,
> +	PHY_CTRL,
> +	PHY_PLL_CTRL,
> +};
> +
> +/* USB PHY registers */
> +
> +static const u8 bcm_usb_u3phy_v1[] = {
> +	[PLL_CTRL]		= 0x18,
> +	[PHY_CTRL]		= 0x14,
> +};
> +
> +static const u8 bcm_usb_u2phy_v1[] = {
> +	[PLL_NDIV_FRAC]	= 0x04,
> +	[PLL_NDIV_INT]	= 0x08,
> +	[PLL_CTRL]	= 0x0c,
> +	[PHY_CTRL]	= 0x10,
> +};
> +
> +#define HSPLL_NDIV_INT_VAL	0x13
> +#define HSPLL_NDIV_FRAC_VAL	0x1005
> +
> +static const u8 bcm_usb_u2phy_v2[] = {
> +	[PLL_NDIV_FRAC]	= 0x0,
> +	[PLL_NDIV_INT]	= 0x4,
> +	[PLL_CTRL]	= 0x8,
> +	[PHY_CTRL]	= 0xc,
> +};
> +
> +enum pll_ctrl_bits {
> +	PLL_RESETB,
> +	SSPLL_SUSPEND_EN,
> +	PLL_SEQ_START,
> +	PLL_LOCK,
> +	PLL_PDIV,
> +};
> +
> +static const u8 u3pll_ctrl[] = {
> +	[PLL_RESETB]		= 0,
> +	[SSPLL_SUSPEND_EN]	= 1,
> +	[PLL_SEQ_START]		= 2,
> +	[PLL_LOCK]		= 3,
> +};
> +
> +#define HSPLL_PDIV_MASK		0xF
> +#define HSPLL_PDIV_VAL		0x1
> +
> +static const u8 u2pll_ctrl[] = {
> +	[PLL_PDIV]	= 1,
> +	[PLL_RESETB]	= 5,
> +	[PLL_LOCK]	= 6,
> +};
> +
> +enum bcm_usb_phy_ctrl_bits {
> +	CORERDY,
> +	AFE_LDO_PWRDWNB,
> +	AFE_PLL_PWRDWNB,
> +	AFE_BG_PWRDWNB,
> +	PHY_ISO,
> +	PHY_RESETB,
> +	PHY_PCTL,
> +};
> +
> +#define PHY_PCTL_MASK	0xffff
> +/*
> + * 0x0806 of PCTL_VAL has below bits set
> + * BIT-8 : refclk divider 1
> + * BIT-3:2: device mode; mode is not effect
> + * BIT-1: soft reset active low
> + */
> +#define HSPHY_PCTL_VAL	0x0806
> +#define SSPHY_PCTL_VAL	0x0006
> +
> +static const u8 u3phy_ctrl[] = {
> +	[PHY_RESETB]	= 1,
> +	[PHY_PCTL]	= 2,
> +};
> +
> +static const u8 u2phy_ctrl[] = {
> +	[CORERDY]		= 0,
> +	[AFE_LDO_PWRDWNB]	= 1,
> +	[AFE_PLL_PWRDWNB]	= 2,
> +	[AFE_BG_PWRDWNB]	= 3,
> +	[PHY_ISO]		= 4,
> +	[PHY_RESETB]		= 5,
> +	[PHY_PCTL]		= 6,
> +};
> +
> +struct bcm_usb_phy_cfg {
> +	uint32_t type;
> +	uint32_t ver;
> +	void __iomem *regs;
> +	struct phy *phy;
> +	const u8 *offset;
> +};
> +
> +#define PLL_LOCK_RETRY_COUNT	1000
> +
> +enum bcm_usb_phy_type {
> +	USB_HS_PHY,
> +	USB_SS_PHY,
> +};
> +
> +static inline void bcm_usb_reg32_clrbits(void __iomem *addr, uint32_t clear)
> +{
> +	writel(readl(addr) & ~clear, addr);
> +}
> +
> +static inline void bcm_usb_reg32_setbits(void __iomem *addr, uint32_t set)
> +{
> +	writel(readl(addr) | set, addr);
> +}
> +
> +static int bcm_usb_pll_lock_check(void __iomem *addr, u32 bit)
> +{
> +	int retry;
> +	u32 rd_data;
> +
> +	retry = PLL_LOCK_RETRY_COUNT;
> +	do {
> +		rd_data = readl(addr);
> +		if (rd_data & bit)
> +			return 0;
> +		udelay(1);
> +	} while (--retry > 0);
> +
> +	pr_err("%s: FAIL\n", __func__);
> +	return -ETIMEDOUT;
> +}
> +
> +static int bcm_usb_ss_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
> +{
> +	int ret = 0;
> +	void __iomem *regs = phy_cfg->regs;
> +	const u8 *offset;
> +	u32 rd_data;
> +
> +	offset = phy_cfg->offset;
> +
> +	/* Set pctl with mode and soft reset */
> +	rd_data = readl(regs + offset[PHY_CTRL]);
> +	rd_data &= ~(PHY_PCTL_MASK << u3phy_ctrl[PHY_PCTL]);
> +	rd_data |= (SSPHY_PCTL_VAL << u3phy_ctrl[PHY_PCTL]);
> +	writel(rd_data, regs + offset[PHY_CTRL]);
> +
> +	bcm_usb_reg32_clrbits(regs + offset[PLL_CTRL],
> +			      BIT(u3pll_ctrl[SSPLL_SUSPEND_EN]));
> +	bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
> +			      BIT(u3pll_ctrl[PLL_SEQ_START]));
> +	bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
> +			      BIT(u3pll_ctrl[PLL_RESETB]));
> +
> +	msleep(30);
> +
> +	ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
> +				     BIT(u3pll_ctrl[PLL_LOCK]));
> +
> +	return ret;
> +}
> +
> +static int bcm_usb_hs_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
> +{
> +	int ret = 0;
> +	void __iomem *regs = phy_cfg->regs;
> +	const u8 *offset;
> +	u32 rd_data;
> +
> +	offset = phy_cfg->offset;
> +
> +	writel(HSPLL_NDIV_INT_VAL, regs + offset[PLL_NDIV_INT]);
> +	writel(HSPLL_NDIV_FRAC_VAL, regs + offset[PLL_NDIV_FRAC]);
> +
> +	rd_data = readl(regs + offset[PLL_CTRL]);
> +	rd_data &= ~(HSPLL_PDIV_MASK << u2pll_ctrl[PLL_PDIV]);
> +	rd_data |= (HSPLL_PDIV_VAL << u2pll_ctrl[PLL_PDIV]);
> +	writel(rd_data, regs + offset[PLL_CTRL]);
> +
> +	/* Set Core Ready high */
> +	bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> +			      BIT(u2phy_ctrl[CORERDY]));
> +
> +	msleep(100);
> +
> +	bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
> +			      BIT(u2pll_ctrl[PLL_RESETB]));
> +	bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> +			      BIT(u2phy_ctrl[PHY_RESETB]));
> +
> +
> +	rd_data = readl(regs + offset[PHY_CTRL]);
> +	rd_data &= ~(PHY_PCTL_MASK << u2phy_ctrl[PHY_PCTL]);
> +	rd_data |= (HSPHY_PCTL_VAL << u2phy_ctrl[PHY_PCTL]);
> +	writel(rd_data, regs + offset[PHY_CTRL]);
> +
> +	ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
> +				     BIT(u2pll_ctrl[PLL_LOCK]));
> +
> +	return ret;
> +}
> +
> +static int bcm_usb_phy_reset(struct phy *phy)
> +{
> +	struct bcm_usb_phy_cfg *phy_cfg = phy_get_drvdata(phy);
> +	void __iomem *regs = phy_cfg->regs;
> +	const u8 *offset;
> +
> +	offset = phy_cfg->offset;
> +
> +	if (phy_cfg->type == USB_HS_PHY) {
> +		bcm_usb_reg32_clrbits(regs + offset[PHY_CTRL],
> +				      BIT(u2phy_ctrl[CORERDY]));
> +		bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> +				      BIT(u2phy_ctrl[CORERDY]));
> +	}
> +
> +	return 0;
> +}
> +
> +static int bcm_usb_phy_init(struct phy *phy)
> +{
> +	struct bcm_usb_phy_cfg *phy_cfg = phy_get_drvdata(phy);
> +
> +	if (phy_cfg->type == USB_SS_PHY)
> +		bcm_usb_ss_phy_init(phy_cfg);
> +	if (phy_cfg->type == USB_HS_PHY)
> +		bcm_usb_hs_phy_init(phy_cfg);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops sr_phy_ops = {
> +	.init		= bcm_usb_phy_init,
> +	.reset		= bcm_usb_phy_reset,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int bcm_usb_phy_create(struct device *dev, struct device_node *node,
> +			     void __iomem *regs, uint32_t version)
> +{
> +	struct bcm_usb_phy_cfg *phy_cfg;
> +	struct phy_provider *phy_provider;
> +
> +	phy_cfg = devm_kzalloc(dev, sizeof(struct bcm_usb_phy_cfg), GFP_KERNEL);
> +	if (!phy_cfg)
> +		return -ENOMEM;
> +
> +	phy_cfg->regs = regs;

Are there any registers shared by all the PHYs?
> +	phy_cfg->ver = version;
> +
> +	if (phy_cfg->ver == BCM_USB_PHY_V1) {
> +		unsigned int id;
> +
> +		if (of_property_read_u32(node, "reg", &id)) {
> +			dev_err(dev, "missing reg property in node %s\n",
> +				node->name);
> +			return -EINVAL;
> +		}
> +
> +		if (id == 0) {
> +			phy_cfg->offset = bcm_usb_u2phy_v1;

The reg space can come from the dt itself.
> +			phy_cfg->type = USB_HS_PHY;
> +		} else if (id == 1) {
> +			phy_cfg->offset = bcm_usb_u3phy_v1;
> +			phy_cfg->type = USB_SS_PHY;
> +		} else {
> +			return -ENODEV;
> +		}
> +	} else if (phy_cfg->ver == BCM_USB_PHY_V2) {
> +		phy_cfg->offset = bcm_usb_u2phy_v2;
> +		phy_cfg->type = USB_HS_PHY;
> +	}
> +
> +	phy_cfg->phy = devm_phy_create(dev, node, &sr_phy_ops);
> +	if (IS_ERR(phy_cfg->phy))
> +		return PTR_ERR(phy_cfg->phy);
> +
> +	phy_set_drvdata(phy_cfg->phy, phy_cfg);
> +	phy_provider = devm_of_phy_provider_register(&phy_cfg->phy->dev,
> +						     of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider)) {
> +		dev_err(dev, "Failed to register phy provider\n");
> +		return PTR_ERR(phy_provider);
> +	}

No need for a separate PHY provider.

Thanks
Kishon

Re: [PATCH 2/3] phy: sr-usb: Add stingray usb phy driver

[Srinath Mannam]

Hi Kishon,

Thank you for review.. Please find my answers below in line.


On Thu, Nov 29, 2018 at 3:55 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Hi,
>
> On 13/11/18 9:43 AM, Srinath Mannam wrote:
> > This driver supports all versions of stingray SS and HS
> > USB phys.
> > In version 1 is combo phy contain both SS and HS phys
> > in a common IO space.
> > In version 2 a single HS phy.
> > These phys support both xHCI host driver and
> > BDC Broadcom device controller driver.
> >
> > Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> > ---
> >  drivers/phy/broadcom/Kconfig          |  11 +
> >  drivers/phy/broadcom/Makefile         |   1 +
> >  drivers/phy/broadcom/phy-bcm-sr-usb.c | 367 ++++++++++++++++++++++++++++++++++
> >  3 files changed, 379 insertions(+)
> >  create mode 100644 drivers/phy/broadcom/phy-bcm-sr-usb.c
> >
> > diff --git a/drivers/phy/broadcom/Kconfig b/drivers/phy/broadcom/Kconfig
> > index 8786a96..c1e4dd5 100644
> > --- a/drivers/phy/broadcom/Kconfig
> > +++ b/drivers/phy/broadcom/Kconfig
> > @@ -10,6 +10,17 @@ config PHY_CYGNUS_PCIE
> >         Enable this to support the Broadcom Cygnus PCIe PHY.
> >         If unsure, say N.
> >
> > +config PHY_BCM_SR_USB
> > +     tristate "Broadcom Stingray USB PHY driver"
> > +     depends on OF && (ARCH_BCM_IPROC || COMPILE_TEST)
> > +     select GENERIC_PHY
> > +     default ARCH_BCM_IPROC
> > +     help
> > +       Enable this to support the Broadcom Stingray USB PHY
> > +       driver. It supports all versions of Superspeed and
> > +       Highspeed PHYs.
> > +       If unsure, say N.
> > +
> >  config BCM_KONA_USB2_PHY
> >       tristate "Broadcom Kona USB2 PHY Driver"
> >       depends on HAS_IOMEM
> > diff --git a/drivers/phy/broadcom/Makefile b/drivers/phy/broadcom/Makefile
> > index 0f60184..f453c7d 100644
> > --- a/drivers/phy/broadcom/Makefile
> > +++ b/drivers/phy/broadcom/Makefile
> > @@ -11,3 +11,4 @@ obj-$(CONFIG_PHY_BRCM_USB)          += phy-brcm-usb-dvr.o
> >  phy-brcm-usb-dvr-objs := phy-brcm-usb.o phy-brcm-usb-init.o
> >
> >  obj-$(CONFIG_PHY_BCM_SR_PCIE)                += phy-bcm-sr-pcie.o
> > +obj-$(CONFIG_PHY_BCM_SR_USB)         += phy-bcm-sr-usb.o
> > diff --git a/drivers/phy/broadcom/phy-bcm-sr-usb.c b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> > new file mode 100644
> > index 0000000..99de49f
> > --- /dev/null
> > +++ b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> > @@ -0,0 +1,367 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2016-2018 Broadcom
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/platform_device.h>
> > +
> > +enum bcm_usb_phy_version {
> > +     BCM_USB_PHY_V1,
> > +     BCM_USB_PHY_V2,
> > +};
> > +
> > +enum bcm_usb_phy_reg {
> > +     PLL_NDIV_FRAC,
> > +     PLL_NDIV_INT,
> > +     PLL_CTRL,
> > +     PHY_CTRL,
> > +     PHY_PLL_CTRL,
> > +};
> > +
> > +/* USB PHY registers */
> > +
> > +static const u8 bcm_usb_u3phy_v1[] = {
> > +     [PLL_CTRL]              = 0x18,
> > +     [PHY_CTRL]              = 0x14,
> > +};
> > +
> > +static const u8 bcm_usb_u2phy_v1[] = {
> > +     [PLL_NDIV_FRAC] = 0x04,
> > +     [PLL_NDIV_INT]  = 0x08,
> > +     [PLL_CTRL]      = 0x0c,
> > +     [PHY_CTRL]      = 0x10,
> > +};
> > +
> > +#define HSPLL_NDIV_INT_VAL   0x13
> > +#define HSPLL_NDIV_FRAC_VAL  0x1005
> > +
> > +static const u8 bcm_usb_u2phy_v2[] = {
> > +     [PLL_NDIV_FRAC] = 0x0,
> > +     [PLL_NDIV_INT]  = 0x4,
> > +     [PLL_CTRL]      = 0x8,
> > +     [PHY_CTRL]      = 0xc,
> > +};
> > +
> > +enum pll_ctrl_bits {
> > +     PLL_RESETB,
> > +     SSPLL_SUSPEND_EN,
> > +     PLL_SEQ_START,
> > +     PLL_LOCK,
> > +     PLL_PDIV,
> > +};
> > +
> > +static const u8 u3pll_ctrl[] = {
> > +     [PLL_RESETB]            = 0,
> > +     [SSPLL_SUSPEND_EN]      = 1,
> > +     [PLL_SEQ_START]         = 2,
> > +     [PLL_LOCK]              = 3,
> > +};
> > +
> > +#define HSPLL_PDIV_MASK              0xF
> > +#define HSPLL_PDIV_VAL               0x1
> > +
> > +static const u8 u2pll_ctrl[] = {
> > +     [PLL_PDIV]      = 1,
> > +     [PLL_RESETB]    = 5,
> > +     [PLL_LOCK]      = 6,
> > +};
> > +
> > +enum bcm_usb_phy_ctrl_bits {
> > +     CORERDY,
> > +     AFE_LDO_PWRDWNB,
> > +     AFE_PLL_PWRDWNB,
> > +     AFE_BG_PWRDWNB,
> > +     PHY_ISO,
> > +     PHY_RESETB,
> > +     PHY_PCTL,
> > +};
> > +
> > +#define PHY_PCTL_MASK        0xffff
> > +/*
> > + * 0x0806 of PCTL_VAL has below bits set
> > + * BIT-8 : refclk divider 1
> > + * BIT-3:2: device mode; mode is not effect
> > + * BIT-1: soft reset active low
> > + */
> > +#define HSPHY_PCTL_VAL       0x0806
> > +#define SSPHY_PCTL_VAL       0x0006
> > +
> > +static const u8 u3phy_ctrl[] = {
> > +     [PHY_RESETB]    = 1,
> > +     [PHY_PCTL]      = 2,
> > +};
> > +
> > +static const u8 u2phy_ctrl[] = {
> > +     [CORERDY]               = 0,
> > +     [AFE_LDO_PWRDWNB]       = 1,
> > +     [AFE_PLL_PWRDWNB]       = 2,
> > +     [AFE_BG_PWRDWNB]        = 3,
> > +     [PHY_ISO]               = 4,
> > +     [PHY_RESETB]            = 5,
> > +     [PHY_PCTL]              = 6,
> > +};
> > +
> > +struct bcm_usb_phy_cfg {
> > +     uint32_t type;
> > +     uint32_t ver;
> > +     void __iomem *regs;
> > +     struct phy *phy;
> > +     const u8 *offset;
> > +};
> > +
> > +#define PLL_LOCK_RETRY_COUNT 1000
> > +
> > +enum bcm_usb_phy_type {
> > +     USB_HS_PHY,
> > +     USB_SS_PHY,
> > +};
> > +
> > +static inline void bcm_usb_reg32_clrbits(void __iomem *addr, uint32_t clear)
> > +{
> > +     writel(readl(addr) & ~clear, addr);
> > +}
> > +
> > +static inline void bcm_usb_reg32_setbits(void __iomem *addr, uint32_t set)
> > +{
> > +     writel(readl(addr) | set, addr);
> > +}
> > +
> > +static int bcm_usb_pll_lock_check(void __iomem *addr, u32 bit)
> > +{
> > +     int retry;
> > +     u32 rd_data;
> > +
> > +     retry = PLL_LOCK_RETRY_COUNT;
> > +     do {
> > +             rd_data = readl(addr);
> > +             if (rd_data & bit)
> > +                     return 0;
> > +             udelay(1);
> > +     } while (--retry > 0);
> > +
> > +     pr_err("%s: FAIL\n", __func__);
> > +     return -ETIMEDOUT;
> > +}
> > +
> > +static int bcm_usb_ss_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
> > +{
> > +     int ret = 0;
> > +     void __iomem *regs = phy_cfg->regs;
> > +     const u8 *offset;
> > +     u32 rd_data;
> > +
> > +     offset = phy_cfg->offset;
> > +
> > +     /* Set pctl with mode and soft reset */
> > +     rd_data = readl(regs + offset[PHY_CTRL]);
> > +     rd_data &= ~(PHY_PCTL_MASK << u3phy_ctrl[PHY_PCTL]);
> > +     rd_data |= (SSPHY_PCTL_VAL << u3phy_ctrl[PHY_PCTL]);
> > +     writel(rd_data, regs + offset[PHY_CTRL]);
> > +
> > +     bcm_usb_reg32_clrbits(regs + offset[PLL_CTRL],
> > +                           BIT(u3pll_ctrl[SSPLL_SUSPEND_EN]));
> > +     bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
> > +                           BIT(u3pll_ctrl[PLL_SEQ_START]));
> > +     bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
> > +                           BIT(u3pll_ctrl[PLL_RESETB]));
> > +
> > +     msleep(30);
> > +
> > +     ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
> > +                                  BIT(u3pll_ctrl[PLL_LOCK]));
> > +
> > +     return ret;
> > +}
> > +
> > +static int bcm_usb_hs_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
> > +{
> > +     int ret = 0;
> > +     void __iomem *regs = phy_cfg->regs;
> > +     const u8 *offset;
> > +     u32 rd_data;
> > +
> > +     offset = phy_cfg->offset;
> > +
> > +     writel(HSPLL_NDIV_INT_VAL, regs + offset[PLL_NDIV_INT]);
> > +     writel(HSPLL_NDIV_FRAC_VAL, regs + offset[PLL_NDIV_FRAC]);
> > +
> > +     rd_data = readl(regs + offset[PLL_CTRL]);
> > +     rd_data &= ~(HSPLL_PDIV_MASK << u2pll_ctrl[PLL_PDIV]);
> > +     rd_data |= (HSPLL_PDIV_VAL << u2pll_ctrl[PLL_PDIV]);
> > +     writel(rd_data, regs + offset[PLL_CTRL]);
> > +
> > +     /* Set Core Ready high */
> > +     bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> > +                           BIT(u2phy_ctrl[CORERDY]));
> > +
> > +     msleep(100);
> > +
> > +     bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
> > +                           BIT(u2pll_ctrl[PLL_RESETB]));
> > +     bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> > +                           BIT(u2phy_ctrl[PHY_RESETB]));
> > +
> > +
> > +     rd_data = readl(regs + offset[PHY_CTRL]);
> > +     rd_data &= ~(PHY_PCTL_MASK << u2phy_ctrl[PHY_PCTL]);
> > +     rd_data |= (HSPHY_PCTL_VAL << u2phy_ctrl[PHY_PCTL]);
> > +     writel(rd_data, regs + offset[PHY_CTRL]);
> > +
> > +     ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
> > +                                  BIT(u2pll_ctrl[PLL_LOCK]));
> > +
> > +     return ret;
> > +}
> > +
> > +static int bcm_usb_phy_reset(struct phy *phy)
> > +{
> > +     struct bcm_usb_phy_cfg *phy_cfg = phy_get_drvdata(phy);
> > +     void __iomem *regs = phy_cfg->regs;
> > +     const u8 *offset;
> > +
> > +     offset = phy_cfg->offset;
> > +
> > +     if (phy_cfg->type == USB_HS_PHY) {
> > +             bcm_usb_reg32_clrbits(regs + offset[PHY_CTRL],
> > +                                   BIT(u2phy_ctrl[CORERDY]));
> > +             bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> > +                                   BIT(u2phy_ctrl[CORERDY]));
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int bcm_usb_phy_init(struct phy *phy)
> > +{
> > +     struct bcm_usb_phy_cfg *phy_cfg = phy_get_drvdata(phy);
> > +
> > +     if (phy_cfg->type == USB_SS_PHY)
> > +             bcm_usb_ss_phy_init(phy_cfg);
> > +     if (phy_cfg->type == USB_HS_PHY)
> > +             bcm_usb_hs_phy_init(phy_cfg);
> > +
> > +     return 0;
> > +}
> > +
> > +static struct phy_ops sr_phy_ops = {
> > +     .init           = bcm_usb_phy_init,
> > +     .reset          = bcm_usb_phy_reset,
> > +     .owner          = THIS_MODULE,
> > +};
> > +
> > +static int bcm_usb_phy_create(struct device *dev, struct device_node *node,
> > +                          void __iomem *regs, uint32_t version)
> > +{
> > +     struct bcm_usb_phy_cfg *phy_cfg;
> > +     struct phy_provider *phy_provider;
> > +
> > +     phy_cfg = devm_kzalloc(dev, sizeof(struct bcm_usb_phy_cfg), GFP_KERNEL);
> > +     if (!phy_cfg)
> > +             return -ENOMEM;
> > +
> > +     phy_cfg->regs = regs;
>
> Are there any registers shared by all the PHYs?
No registers shared between phys. All phys have same type of registers
but at different address offset.
In the same way few bits in registers are same but at different
position in registers.
> > +     phy_cfg->ver = version;
> > +
> > +     if (phy_cfg->ver == BCM_USB_PHY_V1) {
> > +             unsigned int id;
> > +
> > +             if (of_property_read_u32(node, "reg", &id)) {
> > +                     dev_err(dev, "missing reg property in node %s\n",
> > +                             node->name);
> > +                     return -EINVAL;
> > +             }
> > +
> > +             if (id == 0) {
> > +                     phy_cfg->offset = bcm_usb_u2phy_v1;
>
> The reg space can come from the dt itself.
This driver supports two different phys. one is combo phy which has
both USB3 and USB2 phy.
other is single phy it has only USB2 phy.
So that, from DT we passed address of either combo phy address or
single phy address.
In combo phy, USB3 phy and USB2 phy registers are at different offsets
from the base address passed through DT.
> > +                     phy_cfg->type = USB_HS_PHY;
> > +             } else if (id == 1) {
> > +                     phy_cfg->offset = bcm_usb_u3phy_v1;
> > +                     phy_cfg->type = USB_SS_PHY;
> > +             } else {
> > +                     return -ENODEV;
> > +             }
> > +     } else if (phy_cfg->ver == BCM_USB_PHY_V2) {
> > +             phy_cfg->offset = bcm_usb_u2phy_v2;
> > +             phy_cfg->type = USB_HS_PHY;
> > +     }
> > +
> > +     phy_cfg->phy = devm_phy_create(dev, node, &sr_phy_ops);
> > +     if (IS_ERR(phy_cfg->phy))
> > +             return PTR_ERR(phy_cfg->phy);
> > +
> > +     phy_set_drvdata(phy_cfg->phy, phy_cfg);
> > +     phy_provider = devm_of_phy_provider_register(&phy_cfg->phy->dev,
> > +                                                  of_phy_simple_xlate);
> > +     if (IS_ERR(phy_provider)) {
> > +             dev_err(dev, "Failed to register phy provider\n");
> > +             return PTR_ERR(phy_provider);
> > +     }
>
> No need for a separate PHY provider.
Could you please provide more details?
I thought to get phy device in the upper layer we need to register
provider. can we skip this?
In this driver, combo phy provides two usb ports to host controller,
so that I need to add separate phandlers
in host controller DT node and need to create two separate phy
devices. also phy_init and reset handlers need
to call for both phy devices separately.
>
> Thanks
> Kishon

Thank you.
Regards,
Srinath.

Re: [PATCH 2/3] phy: sr-usb: Add stingray usb phy driver

[Kishon Vijay Abraham I]

Hi,

On 29/11/18 6:31 PM, Srinath Mannam wrote:
> Hi Kishon,
> 
> Thank you for review.. Please find my answers below in line.
> 
> 
> On Thu, Nov 29, 2018 at 3:55 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>
>> Hi,
>>
>> On 13/11/18 9:43 AM, Srinath Mannam wrote:
>>> This driver supports all versions of stingray SS and HS
>>> USB phys.
>>> In version 1 is combo phy contain both SS and HS phys
>>> in a common IO space.
>>> In version 2 a single HS phy.
>>> These phys support both xHCI host driver and
>>> BDC Broadcom device controller driver.
>>>
>>> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
>>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>> ---
>>>  drivers/phy/broadcom/Kconfig          |  11 +
>>>  drivers/phy/broadcom/Makefile         |   1 +
>>>  drivers/phy/broadcom/phy-bcm-sr-usb.c | 367 ++++++++++++++++++++++++++++++++++
>>>  3 files changed, 379 insertions(+)
>>>  create mode 100644 drivers/phy/broadcom/phy-bcm-sr-usb.c
>>>
>>> diff --git a/drivers/phy/broadcom/Kconfig b/drivers/phy/broadcom/Kconfig
>>> index 8786a96..c1e4dd5 100644
>>> --- a/drivers/phy/broadcom/Kconfig
>>> +++ b/drivers/phy/broadcom/Kconfig
>>> @@ -10,6 +10,17 @@ config PHY_CYGNUS_PCIE
>>>         Enable this to support the Broadcom Cygnus PCIe PHY.
>>>         If unsure, say N.
>>>
>>> +config PHY_BCM_SR_USB
>>> +     tristate "Broadcom Stingray USB PHY driver"
>>> +     depends on OF && (ARCH_BCM_IPROC || COMPILE_TEST)
>>> +     select GENERIC_PHY
>>> +     default ARCH_BCM_IPROC
>>> +     help
>>> +       Enable this to support the Broadcom Stingray USB PHY
>>> +       driver. It supports all versions of Superspeed and
>>> +       Highspeed PHYs.
>>> +       If unsure, say N.
>>> +
>>>  config BCM_KONA_USB2_PHY
>>>       tristate "Broadcom Kona USB2 PHY Driver"
>>>       depends on HAS_IOMEM
>>> diff --git a/drivers/phy/broadcom/Makefile b/drivers/phy/broadcom/Makefile
>>> index 0f60184..f453c7d 100644
>>> --- a/drivers/phy/broadcom/Makefile
>>> +++ b/drivers/phy/broadcom/Makefile
>>> @@ -11,3 +11,4 @@ obj-$(CONFIG_PHY_BRCM_USB)          += phy-brcm-usb-dvr.o
>>>  phy-brcm-usb-dvr-objs := phy-brcm-usb.o phy-brcm-usb-init.o
>>>
>>>  obj-$(CONFIG_PHY_BCM_SR_PCIE)                += phy-bcm-sr-pcie.o
>>> +obj-$(CONFIG_PHY_BCM_SR_USB)         += phy-bcm-sr-usb.o
>>> diff --git a/drivers/phy/broadcom/phy-bcm-sr-usb.c b/drivers/phy/broadcom/phy-bcm-sr-usb.c
>>> new file mode 100644
>>> index 0000000..99de49f
>>> --- /dev/null
>>> +++ b/drivers/phy/broadcom/phy-bcm-sr-usb.c
>>> @@ -0,0 +1,367 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2016-2018 Broadcom
>>> + */
>>> +
>>> +#include <linux/delay.h>
>>> +#include <linux/io.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/phy/phy.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +enum bcm_usb_phy_version {
>>> +     BCM_USB_PHY_V1,
>>> +     BCM_USB_PHY_V2,
>>> +};
>>> +
>>> +enum bcm_usb_phy_reg {
>>> +     PLL_NDIV_FRAC,
>>> +     PLL_NDIV_INT,
>>> +     PLL_CTRL,
>>> +     PHY_CTRL,
>>> +     PHY_PLL_CTRL,
>>> +};
>>> +
>>> +/* USB PHY registers */
>>> +
>>> +static const u8 bcm_usb_u3phy_v1[] = {
>>> +     [PLL_CTRL]              = 0x18,
>>> +     [PHY_CTRL]              = 0x14,
>>> +};
>>> +
>>> +static const u8 bcm_usb_u2phy_v1[] = {
>>> +     [PLL_NDIV_FRAC] = 0x04,
>>> +     [PLL_NDIV_INT]  = 0x08,
>>> +     [PLL_CTRL]      = 0x0c,
>>> +     [PHY_CTRL]      = 0x10,
>>> +};
>>> +
>>> +#define HSPLL_NDIV_INT_VAL   0x13
>>> +#define HSPLL_NDIV_FRAC_VAL  0x1005
>>> +
>>> +static const u8 bcm_usb_u2phy_v2[] = {
>>> +     [PLL_NDIV_FRAC] = 0x0,
>>> +     [PLL_NDIV_INT]  = 0x4,
>>> +     [PLL_CTRL]      = 0x8,
>>> +     [PHY_CTRL]      = 0xc,
>>> +};
>>> +
>>> +enum pll_ctrl_bits {
>>> +     PLL_RESETB,
>>> +     SSPLL_SUSPEND_EN,
>>> +     PLL_SEQ_START,
>>> +     PLL_LOCK,
>>> +     PLL_PDIV,
>>> +};
>>> +
>>> +static const u8 u3pll_ctrl[] = {
>>> +     [PLL_RESETB]            = 0,
>>> +     [SSPLL_SUSPEND_EN]      = 1,
>>> +     [PLL_SEQ_START]         = 2,
>>> +     [PLL_LOCK]              = 3,
>>> +};
>>> +
>>> +#define HSPLL_PDIV_MASK              0xF
>>> +#define HSPLL_PDIV_VAL               0x1
>>> +
>>> +static const u8 u2pll_ctrl[] = {
>>> +     [PLL_PDIV]      = 1,
>>> +     [PLL_RESETB]    = 5,
>>> +     [PLL_LOCK]      = 6,
>>> +};
>>> +
>>> +enum bcm_usb_phy_ctrl_bits {
>>> +     CORERDY,
>>> +     AFE_LDO_PWRDWNB,
>>> +     AFE_PLL_PWRDWNB,
>>> +     AFE_BG_PWRDWNB,
>>> +     PHY_ISO,
>>> +     PHY_RESETB,
>>> +     PHY_PCTL,
>>> +};
>>> +
>>> +#define PHY_PCTL_MASK        0xffff
>>> +/*
>>> + * 0x0806 of PCTL_VAL has below bits set
>>> + * BIT-8 : refclk divider 1
>>> + * BIT-3:2: device mode; mode is not effect
>>> + * BIT-1: soft reset active low
>>> + */
>>> +#define HSPHY_PCTL_VAL       0x0806
>>> +#define SSPHY_PCTL_VAL       0x0006
>>> +
>>> +static const u8 u3phy_ctrl[] = {
>>> +     [PHY_RESETB]    = 1,
>>> +     [PHY_PCTL]      = 2,
>>> +};
>>> +
>>> +static const u8 u2phy_ctrl[] = {
>>> +     [CORERDY]               = 0,
>>> +     [AFE_LDO_PWRDWNB]       = 1,
>>> +     [AFE_PLL_PWRDWNB]       = 2,
>>> +     [AFE_BG_PWRDWNB]        = 3,
>>> +     [PHY_ISO]               = 4,
>>> +     [PHY_RESETB]            = 5,
>>> +     [PHY_PCTL]              = 6,
>>> +};
>>> +
>>> +struct bcm_usb_phy_cfg {
>>> +     uint32_t type;
>>> +     uint32_t ver;
>>> +     void __iomem *regs;
>>> +     struct phy *phy;
>>> +     const u8 *offset;
>>> +};
>>> +
>>> +#define PLL_LOCK_RETRY_COUNT 1000
>>> +
>>> +enum bcm_usb_phy_type {
>>> +     USB_HS_PHY,
>>> +     USB_SS_PHY,
>>> +};
>>> +
>>> +static inline void bcm_usb_reg32_clrbits(void __iomem *addr, uint32_t clear)
>>> +{
>>> +     writel(readl(addr) & ~clear, addr);
>>> +}
>>> +
>>> +static inline void bcm_usb_reg32_setbits(void __iomem *addr, uint32_t set)
>>> +{
>>> +     writel(readl(addr) | set, addr);
>>> +}
>>> +
>>> +static int bcm_usb_pll_lock_check(void __iomem *addr, u32 bit)
>>> +{
>>> +     int retry;
>>> +     u32 rd_data;
>>> +
>>> +     retry = PLL_LOCK_RETRY_COUNT;
>>> +     do {
>>> +             rd_data = readl(addr);
>>> +             if (rd_data & bit)
>>> +                     return 0;
>>> +             udelay(1);
>>> +     } while (--retry > 0);
>>> +
>>> +     pr_err("%s: FAIL\n", __func__);
>>> +     return -ETIMEDOUT;
>>> +}
>>> +
>>> +static int bcm_usb_ss_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
>>> +{
>>> +     int ret = 0;
>>> +     void __iomem *regs = phy_cfg->regs;
>>> +     const u8 *offset;
>>> +     u32 rd_data;
>>> +
>>> +     offset = phy_cfg->offset;
>>> +
>>> +     /* Set pctl with mode and soft reset */
>>> +     rd_data = readl(regs + offset[PHY_CTRL]);
>>> +     rd_data &= ~(PHY_PCTL_MASK << u3phy_ctrl[PHY_PCTL]);
>>> +     rd_data |= (SSPHY_PCTL_VAL << u3phy_ctrl[PHY_PCTL]);
>>> +     writel(rd_data, regs + offset[PHY_CTRL]);
>>> +
>>> +     bcm_usb_reg32_clrbits(regs + offset[PLL_CTRL],
>>> +                           BIT(u3pll_ctrl[SSPLL_SUSPEND_EN]));
>>> +     bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
>>> +                           BIT(u3pll_ctrl[PLL_SEQ_START]));
>>> +     bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
>>> +                           BIT(u3pll_ctrl[PLL_RESETB]));
>>> +
>>> +     msleep(30);

Please add a comment for each of the delay added to mention if it's documented
in the data manual or was a result of experiment etc..
>>> +
>>> +     ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
>>> +                                  BIT(u3pll_ctrl[PLL_LOCK]));
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static int bcm_usb_hs_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
>>> +{
>>> +     int ret = 0;
>>> +     void __iomem *regs = phy_cfg->regs;
>>> +     const u8 *offset;
>>> +     u32 rd_data;
>>> +
>>> +     offset = phy_cfg->offset;
>>> +
>>> +     writel(HSPLL_NDIV_INT_VAL, regs + offset[PLL_NDIV_INT]);
>>> +     writel(HSPLL_NDIV_FRAC_VAL, regs + offset[PLL_NDIV_FRAC]);
>>> +
>>> +     rd_data = readl(regs + offset[PLL_CTRL]);
>>> +     rd_data &= ~(HSPLL_PDIV_MASK << u2pll_ctrl[PLL_PDIV]);
>>> +     rd_data |= (HSPLL_PDIV_VAL << u2pll_ctrl[PLL_PDIV]);
>>> +     writel(rd_data, regs + offset[PLL_CTRL]);
>>> +
>>> +     /* Set Core Ready high */
>>> +     bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
>>> +                           BIT(u2phy_ctrl[CORERDY]));
>>> +
>>> +     msleep(100);
>>> +
>>> +     bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
>>> +                           BIT(u2pll_ctrl[PLL_RESETB]));
>>> +     bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
>>> +                           BIT(u2phy_ctrl[PHY_RESETB]));
>>> +
>>> +
>>> +     rd_data = readl(regs + offset[PHY_CTRL]);
>>> +     rd_data &= ~(PHY_PCTL_MASK << u2phy_ctrl[PHY_PCTL]);
>>> +     rd_data |= (HSPHY_PCTL_VAL << u2phy_ctrl[PHY_PCTL]);
>>> +     writel(rd_data, regs + offset[PHY_CTRL]);
>>> +
>>> +     ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
>>> +                                  BIT(u2pll_ctrl[PLL_LOCK]));
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static int bcm_usb_phy_reset(struct phy *phy)
>>> +{
>>> +     struct bcm_usb_phy_cfg *phy_cfg = phy_get_drvdata(phy);
>>> +     void __iomem *regs = phy_cfg->regs;
>>> +     const u8 *offset;
>>> +
>>> +     offset = phy_cfg->offset;
>>> +
>>> +     if (phy_cfg->type == USB_HS_PHY) {
>>> +             bcm_usb_reg32_clrbits(regs + offset[PHY_CTRL],
>>> +                                   BIT(u2phy_ctrl[CORERDY]));
>>> +             bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
>>> +                                   BIT(u2phy_ctrl[CORERDY]));
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int bcm_usb_phy_init(struct phy *phy)
>>> +{
>>> +     struct bcm_usb_phy_cfg *phy_cfg = phy_get_drvdata(phy);
>>> +
>>> +     if (phy_cfg->type == USB_SS_PHY)
>>> +             bcm_usb_ss_phy_init(phy_cfg);
>>> +     if (phy_cfg->type == USB_HS_PHY)
>>> +             bcm_usb_hs_phy_init(phy_cfg);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static struct phy_ops sr_phy_ops = {
>>> +     .init           = bcm_usb_phy_init,
>>> +     .reset          = bcm_usb_phy_reset,
>>> +     .owner          = THIS_MODULE,
>>> +};
>>> +
>>> +static int bcm_usb_phy_create(struct device *dev, struct device_node *node,
>>> +                          void __iomem *regs, uint32_t version)
>>> +{
>>> +     struct bcm_usb_phy_cfg *phy_cfg;
>>> +     struct phy_provider *phy_provider;
>>> +
>>> +     phy_cfg = devm_kzalloc(dev, sizeof(struct bcm_usb_phy_cfg), GFP_KERNEL);
>>> +     if (!phy_cfg)
>>> +             return -ENOMEM;
>>> +
>>> +     phy_cfg->regs = regs;
>>
>> Are there any registers shared by all the PHYs?
> No registers shared between phys. All phys have same type of registers
> but at different address offset.
> In the same way few bits in registers are same but at different
> position in registers.
>>> +     phy_cfg->ver = version;
>>> +
>>> +     if (phy_cfg->ver == BCM_USB_PHY_V1) {
>>> +             unsigned int id;
>>> +
>>> +             if (of_property_read_u32(node, "reg", &id)) {
>>> +                     dev_err(dev, "missing reg property in node %s\n",
>>> +                             node->name);
>>> +                     return -EINVAL;
>>> +             }
>>> +
>>> +             if (id == 0) {
>>> +                     phy_cfg->offset = bcm_usb_u2phy_v1;
>>
>> The reg space can come from the dt itself.
> This driver supports two different phys. one is combo phy which has
> both USB3 and USB2 phy.
> other is single phy it has only USB2 phy.
> So that, from DT we passed address of either combo phy address or
> single phy address.
> In combo phy, USB3 phy and USB2 phy registers are at different offsets
> from the base address passed through DT.
>>> +                     phy_cfg->type = USB_HS_PHY;
>>> +             } else if (id == 1) {
>>> +                     phy_cfg->offset = bcm_usb_u3phy_v1;
>>> +                     phy_cfg->type = USB_SS_PHY;
>>> +             } else {
>>> +                     return -ENODEV;
>>> +             }
>>> +     } else if (phy_cfg->ver == BCM_USB_PHY_V2) {
>>> +             phy_cfg->offset = bcm_usb_u2phy_v2;
>>> +             phy_cfg->type = USB_HS_PHY;
>>> +     }
>>> +
>>> +     phy_cfg->phy = devm_phy_create(dev, node, &sr_phy_ops);
>>> +     if (IS_ERR(phy_cfg->phy))
>>> +             return PTR_ERR(phy_cfg->phy);
>>> +
>>> +     phy_set_drvdata(phy_cfg->phy, phy_cfg);
>>> +     phy_provider = devm_of_phy_provider_register(&phy_cfg->phy->dev,
>>> +                                                  of_phy_simple_xlate);
>>> +     if (IS_ERR(phy_provider)) {
>>> +             dev_err(dev, "Failed to register phy provider\n");
>>> +             return PTR_ERR(phy_provider);
>>> +     }
>>
>> No need for a separate PHY provider.
> Could you please provide more details?
> I thought to get phy device in the upper layer we need to register
> provider. can we skip this?
> In this driver, combo phy provides two usb ports to host controller,
> so that I need to add separate phandlers
> in host controller DT node and need to create two separate phy
> devices. also phy_init and reset handlers need
> to call for both phy devices separately.

A single phy_provider can create multiple PHY's. Here anyways you have subnodes
for each of the PHY's. So host controller DT node can have a phandle to the
subnodes directly.

Thanks
Kishon

Re: [PATCH 2/3] phy: sr-usb: Add stingray usb phy driver

[Srinath Mannam]

Hi Kishon,

Thank you for more information. Please find my answers below in line.

On Fri, Nov 30, 2018 at 9:56 AM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Hi,
>
> On 29/11/18 6:31 PM, Srinath Mannam wrote:
> > Hi Kishon,
> >
> > Thank you for review.. Please find my answers below in line.
> >
> >
> > On Thu, Nov 29, 2018 at 3:55 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
> >>
> >> Hi,
> >>
> >> On 13/11/18 9:43 AM, Srinath Mannam wrote:
> >>> This driver supports all versions of stingray SS and HS
> >>> USB phys.
> >>> In version 1 is combo phy contain both SS and HS phys
> >>> in a common IO space.
> >>> In version 2 a single HS phy.
> >>> These phys support both xHCI host driver and
> >>> BDC Broadcom device controller driver.
> >>>
> >>> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> >>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> >>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> >>> ---
> >>>  drivers/phy/broadcom/Kconfig          |  11 +
> >>>  drivers/phy/broadcom/Makefile         |   1 +
> >>>  drivers/phy/broadcom/phy-bcm-sr-usb.c | 367 ++++++++++++++++++++++++++++++++++
> >>>  3 files changed, 379 insertions(+)
> >>>  create mode 100644 drivers/phy/broadcom/phy-bcm-sr-usb.c
> >>>
> >>> diff --git a/drivers/phy/broadcom/Kconfig b/drivers/phy/broadcom/Kconfig
> >>> index 8786a96..c1e4dd5 100644
> >>> --- a/drivers/phy/broadcom/Kconfig
> >>> +++ b/drivers/phy/broadcom/Kconfig
> >>> @@ -10,6 +10,17 @@ config PHY_CYGNUS_PCIE
> >>>         Enable this to support the Broadcom Cygnus PCIe PHY.
> >>>         If unsure, say N.
> >>>
> >>> +config PHY_BCM_SR_USB
> >>> +     tristate "Broadcom Stingray USB PHY driver"
> >>> +     depends on OF && (ARCH_BCM_IPROC || COMPILE_TEST)
> >>> +     select GENERIC_PHY
> >>> +     default ARCH_BCM_IPROC
> >>> +     help
> >>> +       Enable this to support the Broadcom Stingray USB PHY
> >>> +       driver. It supports all versions of Superspeed and
> >>> +       Highspeed PHYs.
> >>> +       If unsure, say N.
> >>> +
> >>>  config BCM_KONA_USB2_PHY
> >>>       tristate "Broadcom Kona USB2 PHY Driver"
> >>>       depends on HAS_IOMEM
> >>> diff --git a/drivers/phy/broadcom/Makefile b/drivers/phy/broadcom/Makefile
> >>> index 0f60184..f453c7d 100644
> >>> --- a/drivers/phy/broadcom/Makefile
> >>> +++ b/drivers/phy/broadcom/Makefile
> >>> @@ -11,3 +11,4 @@ obj-$(CONFIG_PHY_BRCM_USB)          += phy-brcm-usb-dvr.o
> >>>  phy-brcm-usb-dvr-objs := phy-brcm-usb.o phy-brcm-usb-init.o
> >>>
> >>>  obj-$(CONFIG_PHY_BCM_SR_PCIE)                += phy-bcm-sr-pcie.o
> >>> +obj-$(CONFIG_PHY_BCM_SR_USB)         += phy-bcm-sr-usb.o
> >>> diff --git a/drivers/phy/broadcom/phy-bcm-sr-usb.c b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> >>> new file mode 100644
> >>> index 0000000..99de49f
> >>> --- /dev/null
> >>> +++ b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> >>> @@ -0,0 +1,367 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Copyright (C) 2016-2018 Broadcom
> >>> + */
> >>> +
> >>> +#include <linux/delay.h>
> >>> +#include <linux/io.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/of.h>
> >>> +#include <linux/phy/phy.h>
> >>> +#include <linux/platform_device.h>
> >>> +
> >>> +enum bcm_usb_phy_version {
> >>> +     BCM_USB_PHY_V1,
> >>> +     BCM_USB_PHY_V2,
> >>> +};
> >>> +
> >>> +enum bcm_usb_phy_reg {
> >>> +     PLL_NDIV_FRAC,
> >>> +     PLL_NDIV_INT,
> >>> +     PLL_CTRL,
> >>> +     PHY_CTRL,
> >>> +     PHY_PLL_CTRL,
> >>> +};
> >>> +
> >>> +/* USB PHY registers */
> >>> +
> >>> +static const u8 bcm_usb_u3phy_v1[] = {
> >>> +     [PLL_CTRL]              = 0x18,
> >>> +     [PHY_CTRL]              = 0x14,
> >>> +};
> >>> +
> >>> +static const u8 bcm_usb_u2phy_v1[] = {
> >>> +     [PLL_NDIV_FRAC] = 0x04,
> >>> +     [PLL_NDIV_INT]  = 0x08,
> >>> +     [PLL_CTRL]      = 0x0c,
> >>> +     [PHY_CTRL]      = 0x10,
> >>> +};
> >>> +
> >>> +#define HSPLL_NDIV_INT_VAL   0x13
> >>> +#define HSPLL_NDIV_FRAC_VAL  0x1005
> >>> +
> >>> +static const u8 bcm_usb_u2phy_v2[] = {
> >>> +     [PLL_NDIV_FRAC] = 0x0,
> >>> +     [PLL_NDIV_INT]  = 0x4,
> >>> +     [PLL_CTRL]      = 0x8,
> >>> +     [PHY_CTRL]      = 0xc,
> >>> +};
> >>> +
> >>> +enum pll_ctrl_bits {
> >>> +     PLL_RESETB,
> >>> +     SSPLL_SUSPEND_EN,
> >>> +     PLL_SEQ_START,
> >>> +     PLL_LOCK,
> >>> +     PLL_PDIV,
> >>> +};
> >>> +
> >>> +static const u8 u3pll_ctrl[] = {
> >>> +     [PLL_RESETB]            = 0,
> >>> +     [SSPLL_SUSPEND_EN]      = 1,
> >>> +     [PLL_SEQ_START]         = 2,
> >>> +     [PLL_LOCK]              = 3,
> >>> +};
> >>> +
> >>> +#define HSPLL_PDIV_MASK              0xF
> >>> +#define HSPLL_PDIV_VAL               0x1
> >>> +
> >>> +static const u8 u2pll_ctrl[] = {
> >>> +     [PLL_PDIV]      = 1,
> >>> +     [PLL_RESETB]    = 5,
> >>> +     [PLL_LOCK]      = 6,
> >>> +};
> >>> +
> >>> +enum bcm_usb_phy_ctrl_bits {
> >>> +     CORERDY,
> >>> +     AFE_LDO_PWRDWNB,
> >>> +     AFE_PLL_PWRDWNB,
> >>> +     AFE_BG_PWRDWNB,
> >>> +     PHY_ISO,
> >>> +     PHY_RESETB,
> >>> +     PHY_PCTL,
> >>> +};
> >>> +
> >>> +#define PHY_PCTL_MASK        0xffff
> >>> +/*
> >>> + * 0x0806 of PCTL_VAL has below bits set
> >>> + * BIT-8 : refclk divider 1
> >>> + * BIT-3:2: device mode; mode is not effect
> >>> + * BIT-1: soft reset active low
> >>> + */
> >>> +#define HSPHY_PCTL_VAL       0x0806
> >>> +#define SSPHY_PCTL_VAL       0x0006
> >>> +
> >>> +static const u8 u3phy_ctrl[] = {
> >>> +     [PHY_RESETB]    = 1,
> >>> +     [PHY_PCTL]      = 2,
> >>> +};
> >>> +
> >>> +static const u8 u2phy_ctrl[] = {
> >>> +     [CORERDY]               = 0,
> >>> +     [AFE_LDO_PWRDWNB]       = 1,
> >>> +     [AFE_PLL_PWRDWNB]       = 2,
> >>> +     [AFE_BG_PWRDWNB]        = 3,
> >>> +     [PHY_ISO]               = 4,
> >>> +     [PHY_RESETB]            = 5,
> >>> +     [PHY_PCTL]              = 6,
> >>> +};
> >>> +
> >>> +struct bcm_usb_phy_cfg {
> >>> +     uint32_t type;
> >>> +     uint32_t ver;
> >>> +     void __iomem *regs;
> >>> +     struct phy *phy;
> >>> +     const u8 *offset;
> >>> +};
> >>> +
> >>> +#define PLL_LOCK_RETRY_COUNT 1000
> >>> +
> >>> +enum bcm_usb_phy_type {
> >>> +     USB_HS_PHY,
> >>> +     USB_SS_PHY,
> >>> +};
> >>> +
> >>> +static inline void bcm_usb_reg32_clrbits(void __iomem *addr, uint32_t clear)
> >>> +{
> >>> +     writel(readl(addr) & ~clear, addr);
> >>> +}
> >>> +
> >>> +static inline void bcm_usb_reg32_setbits(void __iomem *addr, uint32_t set)
> >>> +{
> >>> +     writel(readl(addr) | set, addr);
> >>> +}
> >>> +
> >>> +static int bcm_usb_pll_lock_check(void __iomem *addr, u32 bit)
> >>> +{
> >>> +     int retry;
> >>> +     u32 rd_data;
> >>> +
> >>> +     retry = PLL_LOCK_RETRY_COUNT;
> >>> +     do {
> >>> +             rd_data = readl(addr);
> >>> +             if (rd_data & bit)
> >>> +                     return 0;
> >>> +             udelay(1);
> >>> +     } while (--retry > 0);
> >>> +
> >>> +     pr_err("%s: FAIL\n", __func__);
> >>> +     return -ETIMEDOUT;
> >>> +}
> >>> +
> >>> +static int bcm_usb_ss_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
> >>> +{
> >>> +     int ret = 0;
> >>> +     void __iomem *regs = phy_cfg->regs;
> >>> +     const u8 *offset;
> >>> +     u32 rd_data;
> >>> +
> >>> +     offset = phy_cfg->offset;
> >>> +
> >>> +     /* Set pctl with mode and soft reset */
> >>> +     rd_data = readl(regs + offset[PHY_CTRL]);
> >>> +     rd_data &= ~(PHY_PCTL_MASK << u3phy_ctrl[PHY_PCTL]);
> >>> +     rd_data |= (SSPHY_PCTL_VAL << u3phy_ctrl[PHY_PCTL]);
> >>> +     writel(rd_data, regs + offset[PHY_CTRL]);
> >>> +
> >>> +     bcm_usb_reg32_clrbits(regs + offset[PLL_CTRL],
> >>> +                           BIT(u3pll_ctrl[SSPLL_SUSPEND_EN]));
> >>> +     bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
> >>> +                           BIT(u3pll_ctrl[PLL_SEQ_START]));
> >>> +     bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
> >>> +                           BIT(u3pll_ctrl[PLL_RESETB]));
> >>> +
> >>> +     msleep(30);
>
> Please add a comment for each of the delay added to mention if it's documented
> in the data manual or was a result of experiment etc..
I will add comments in next patch version.
> >>> +
> >>> +     ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
> >>> +                                  BIT(u3pll_ctrl[PLL_LOCK]));
> >>> +
> >>> +     return ret;
> >>> +}
> >>> +
> >>> +static int bcm_usb_hs_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
> >>> +{
> >>> +     int ret = 0;
> >>> +     void __iomem *regs = phy_cfg->regs;
> >>> +     const u8 *offset;
> >>> +     u32 rd_data;
> >>> +
> >>> +     offset = phy_cfg->offset;
> >>> +
> >>> +     writel(HSPLL_NDIV_INT_VAL, regs + offset[PLL_NDIV_INT]);
> >>> +     writel(HSPLL_NDIV_FRAC_VAL, regs + offset[PLL_NDIV_FRAC]);
> >>> +
> >>> +     rd_data = readl(regs + offset[PLL_CTRL]);
> >>> +     rd_data &= ~(HSPLL_PDIV_MASK << u2pll_ctrl[PLL_PDIV]);
> >>> +     rd_data |= (HSPLL_PDIV_VAL << u2pll_ctrl[PLL_PDIV]);
> >>> +     writel(rd_data, regs + offset[PLL_CTRL]);
> >>> +
> >>> +     /* Set Core Ready high */
> >>> +     bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> >>> +                           BIT(u2phy_ctrl[CORERDY]));
> >>> +
> >>> +     msleep(100);
> >>> +
> >>> +     bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
> >>> +                           BIT(u2pll_ctrl[PLL_RESETB]));
> >>> +     bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> >>> +                           BIT(u2phy_ctrl[PHY_RESETB]));
> >>> +
> >>> +
> >>> +     rd_data = readl(regs + offset[PHY_CTRL]);
> >>> +     rd_data &= ~(PHY_PCTL_MASK << u2phy_ctrl[PHY_PCTL]);
> >>> +     rd_data |= (HSPHY_PCTL_VAL << u2phy_ctrl[PHY_PCTL]);
> >>> +     writel(rd_data, regs + offset[PHY_CTRL]);
> >>> +
> >>> +     ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
> >>> +                                  BIT(u2pll_ctrl[PLL_LOCK]));
> >>> +
> >>> +     return ret;
> >>> +}
> >>> +
> >>> +static int bcm_usb_phy_reset(struct phy *phy)
> >>> +{
> >>> +     struct bcm_usb_phy_cfg *phy_cfg = phy_get_drvdata(phy);
> >>> +     void __iomem *regs = phy_cfg->regs;
> >>> +     const u8 *offset;
> >>> +
> >>> +     offset = phy_cfg->offset;
> >>> +
> >>> +     if (phy_cfg->type == USB_HS_PHY) {
> >>> +             bcm_usb_reg32_clrbits(regs + offset[PHY_CTRL],
> >>> +                                   BIT(u2phy_ctrl[CORERDY]));
> >>> +             bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> >>> +                                   BIT(u2phy_ctrl[CORERDY]));
> >>> +     }
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static int bcm_usb_phy_init(struct phy *phy)
> >>> +{
> >>> +     struct bcm_usb_phy_cfg *phy_cfg = phy_get_drvdata(phy);
> >>> +
> >>> +     if (phy_cfg->type == USB_SS_PHY)
> >>> +             bcm_usb_ss_phy_init(phy_cfg);
> >>> +     if (phy_cfg->type == USB_HS_PHY)
> >>> +             bcm_usb_hs_phy_init(phy_cfg);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static struct phy_ops sr_phy_ops = {
> >>> +     .init           = bcm_usb_phy_init,
> >>> +     .reset          = bcm_usb_phy_reset,
> >>> +     .owner          = THIS_MODULE,
> >>> +};
> >>> +
> >>> +static int bcm_usb_phy_create(struct device *dev, struct device_node *node,
> >>> +                          void __iomem *regs, uint32_t version)
> >>> +{
> >>> +     struct bcm_usb_phy_cfg *phy_cfg;
> >>> +     struct phy_provider *phy_provider;
> >>> +
> >>> +     phy_cfg = devm_kzalloc(dev, sizeof(struct bcm_usb_phy_cfg), GFP_KERNEL);
> >>> +     if (!phy_cfg)
> >>> +             return -ENOMEM;
> >>> +
> >>> +     phy_cfg->regs = regs;
> >>
> >> Are there any registers shared by all the PHYs?
> > No registers shared between phys. All phys have same type of registers
> > but at different address offset.
> > In the same way few bits in registers are same but at different
> > position in registers.
> >>> +     phy_cfg->ver = version;
> >>> +
> >>> +     if (phy_cfg->ver == BCM_USB_PHY_V1) {
> >>> +             unsigned int id;
> >>> +
> >>> +             if (of_property_read_u32(node, "reg", &id)) {
> >>> +                     dev_err(dev, "missing reg property in node %s\n",
> >>> +                             node->name);
> >>> +                     return -EINVAL;
> >>> +             }
> >>> +
> >>> +             if (id == 0) {
> >>> +                     phy_cfg->offset = bcm_usb_u2phy_v1;
> >>
> >> The reg space can come from the dt itself.
> > This driver supports two different phys. one is combo phy which has
> > both USB3 and USB2 phy.
> > other is single phy it has only USB2 phy.
> > So that, from DT we passed address of either combo phy address or
> > single phy address.
> > In combo phy, USB3 phy and USB2 phy registers are at different offsets
> > from the base address passed through DT.
> >>> +                     phy_cfg->type = USB_HS_PHY;
> >>> +             } else if (id == 1) {
> >>> +                     phy_cfg->offset = bcm_usb_u3phy_v1;
> >>> +                     phy_cfg->type = USB_SS_PHY;
> >>> +             } else {
> >>> +                     return -ENODEV;
> >>> +             }
> >>> +     } else if (phy_cfg->ver == BCM_USB_PHY_V2) {
> >>> +             phy_cfg->offset = bcm_usb_u2phy_v2;
> >>> +             phy_cfg->type = USB_HS_PHY;
> >>> +     }
> >>> +
> >>> +     phy_cfg->phy = devm_phy_create(dev, node, &sr_phy_ops);
> >>> +     if (IS_ERR(phy_cfg->phy))
> >>> +             return PTR_ERR(phy_cfg->phy);
> >>> +
> >>> +     phy_set_drvdata(phy_cfg->phy, phy_cfg);
> >>> +     phy_provider = devm_of_phy_provider_register(&phy_cfg->phy->dev,
> >>> +                                                  of_phy_simple_xlate);
> >>> +     if (IS_ERR(phy_provider)) {
> >>> +             dev_err(dev, "Failed to register phy provider\n");
> >>> +             return PTR_ERR(phy_provider);
> >>> +     }
> >>
> >> No need for a separate PHY provider.
> > Could you please provide more details?
> > I thought to get phy device in the upper layer we need to register
> > provider. can we skip this?
> > In this driver, combo phy provides two usb ports to host controller,
> > so that I need to add separate phandlers
> > in host controller DT node and need to create two separate phy
> > devices. also phy_init and reset handlers need
> > to call for both phy devices separately.
>
> A single phy_provider can create multiple PHY's. Here anyways you have subnodes
> for each of the PHY's. So host controller DT node can have a phandle to the
> subnodes directly.
Yes, phandles of subnodes only given in host controller DT node as
shown in below line.
phys = <&usb0_phy1>, <&usb0_phy0>;
I thought phy device is fetched using of_node passed in host
controller DT by fetching
provider attached to it. dev pointer of provide is dev of phy device.
so that I thought it is better to have separate
provider for each phy device. Please correct me if my understanding is
not right.

Thank you very much.

Regards,
Srinath.
>
> Thanks
> Kishon