linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Stingray USB PHY driver support
@ 2019-02-20 10:33 Srinath Mannam
  2019-02-20 10:34 ` [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document Srinath Mannam
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Srinath Mannam @ 2019-02-20 10:33 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Kishon Vijay Abraham I
  Cc: Tejun Heo, Jayachandran C, devicetree, linux-kernel,
	bcm-kernel-feedback-list, Srinath Mannam

These patches add Stingray USB PHY driver and its corresponding
DT nodes with documentation.

This patch set is based on Linux-5.0-rc2.

Changes from v3:
  - Addressed Rob Herring review comments
     - Changed compatible strings.
     - updated driver based on new compatible strings.

Changes from v2:
  - Addressed Kishon review comments
  - Updated patchset to Linux-5.0-rc2

Changes from v1:
  - Addressed Kishon review comments
  - phy init call return value handle
  
Srinath Mannam (3):
  dt-bindings: phy: Add Stingray USB PHY binding document
  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              | 371 +++++++++++++++++++++
 6 files changed, 541 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document
  2019-02-20 10:33 [PATCH v4 0/3] Stingray USB PHY driver support Srinath Mannam
@ 2019-02-20 10:34 ` Srinath Mannam
  2019-02-22 17:20   ` Rob Herring
  2019-02-20 10:34 ` [PATCH v4 2/3] phy: sr-usb: Add Stingray USB PHY driver Srinath Mannam
  2019-02-20 10:34 ` [PATCH v4 3/3] arm64: dts: Add USB DT nodes for Stingray SoC Srinath Mannam
  2 siblings, 1 reply; 10+ messages in thread
From: Srinath Mannam @ 2019-02-20 10:34 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Kishon Vijay Abraham I
  Cc: Tejun Heo, Jayachandran C, devicetree, linux-kernel,
	bcm-kernel-feedback-list, 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..da19236
--- /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-combo-phy" is a combo PHY has one SS PHY and one HS PHY.
+	- "brcm,sr-usb-hs-phy" has a single HS PHY.
+ - reg: offset and length of the PHY blocks registers
+ - address-cells: should be 1
+ - size-cells: should be 0
+
+Sub-nodes:
+  brcm,sr-usb-combo-phy have two sub-nodes for one SS PHY and one HS PHY.
+
+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-combo-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-combo-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-hs-phy";
+		reg = <0x00020000 0x100>,
+		#address-cells = <1>;
+		#size-cells = <0>;
+		#phy-cells = <0>;
+	};
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/3] phy: sr-usb: Add Stingray USB PHY driver
  2019-02-20 10:33 [PATCH v4 0/3] Stingray USB PHY driver support Srinath Mannam
  2019-02-20 10:34 ` [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document Srinath Mannam
@ 2019-02-20 10:34 ` Srinath Mannam
  2019-02-20 10:34 ` [PATCH v4 3/3] arm64: dts: Add USB DT nodes for Stingray SoC Srinath Mannam
  2 siblings, 0 replies; 10+ messages in thread
From: Srinath Mannam @ 2019-02-20 10:34 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Kishon Vijay Abraham I
  Cc: Tejun Heo, Jayachandran C, devicetree, linux-kernel,
	bcm-kernel-feedback-list, Srinath Mannam

USB PHY driver supports two versions of stingray USB PHYs
 - Version 1 is a combo PHY contains one SS and one HS PHYs.
 - Version 2 is a single HS PHY.

These two PHY versons support both Generic xHCI host controller 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 | 371 ++++++++++++++++++++++++++++++++++
 3 files changed, 383 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 aa917a6..f30f481 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..3b70d28
--- /dev/null
+++ b/drivers/phy/broadcom/phy-bcm-sr-usb.c
@@ -0,0 +1,371 @@
+// 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_SR_USB_COMBO_PHY,
+	BCM_SR_USB_HS_PHY,
+};
+
+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_combo_phy_ss[] = {
+	[PLL_CTRL]		= 0x18,
+	[PHY_CTRL]		= 0x14,
+};
+
+static const u8 bcm_usb_combo_phy_hs[] = {
+	[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_hs_phy[] = {
+	[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 version;
+	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]));
+
+	/* Maximum timeout for PLL reset done */
+	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]));
+
+	/* Maximum timeout for Core Ready done */
+	msleep(30);
+
+	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]);
+
+	/* Maximum timeout for PLL reset done */
+	msleep(30);
+
+	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);
+	int ret = -EINVAL;
+
+	if (phy_cfg->type == USB_SS_PHY)
+		ret = bcm_usb_ss_phy_init(phy_cfg);
+	else if (phy_cfg->type == USB_HS_PHY)
+		ret = bcm_usb_hs_phy_init(phy_cfg);
+
+	return ret;
+}
+
+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;
+
+	phy_cfg = devm_kzalloc(dev, sizeof(struct bcm_usb_phy_cfg), GFP_KERNEL);
+	if (!phy_cfg)
+		return -ENOMEM;
+
+	phy_cfg->regs = regs;
+	phy_cfg->version = version;
+
+	if (phy_cfg->version == BCM_SR_USB_COMBO_PHY) {
+		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_combo_phy_hs;
+			phy_cfg->type = USB_HS_PHY;
+		} else if (id == 1) {
+			phy_cfg->offset = bcm_usb_combo_phy_ss;
+			phy_cfg->type = USB_SS_PHY;
+		} else {
+			return -ENODEV;
+		}
+	} else if (phy_cfg->version == BCM_SR_USB_HS_PHY) {
+		phy_cfg->offset = bcm_usb_hs_phy;
+		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);
+
+	return 0;
+}
+
+static const struct of_device_id bcm_usb_phy_of_match[] = {
+	{
+		.compatible = "brcm,sr-usb-combo-phy",
+		.data = (void *)BCM_SR_USB_COMBO_PHY,
+	},
+	{
+		.compatible = "brcm,sr-usb-hs-phy",
+		.data = (void *)BCM_SR_USB_HS_PHY,
+	},
+	{ /* 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;
+	struct phy_provider *phy_provider;
+
+	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) {
+		ret = bcm_usb_phy_create(dev, dn, regs, version);
+		if (ret)
+			return ret;
+	} else {
+		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;
+			}
+		}
+	}
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/3] arm64: dts: Add USB DT nodes for Stingray SoC
  2019-02-20 10:33 [PATCH v4 0/3] Stingray USB PHY driver support Srinath Mannam
  2019-02-20 10:34 ` [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document Srinath Mannam
  2019-02-20 10:34 ` [PATCH v4 2/3] phy: sr-usb: Add Stingray USB PHY driver Srinath Mannam
@ 2019-02-20 10:34 ` Srinath Mannam
  2 siblings, 0 replies; 10+ messages in thread
From: Srinath Mannam @ 2019-02-20 10:34 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Kishon Vijay Abraham I
  Cc: Tejun Heo, Jayachandran C, devicetree, linux-kernel,
	bcm-kernel-feedback-list, 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 has one SS and
one HS PHYs. [HS-PHY2] is a single HS PHY.

xHCI use SS-PHY to detect SS devices and HS-PHY to detect HS/FS/LS
devices. BDC use SS-PHY in SS mode and HS-PHY in HS mode.

xHCI0 port1 is SS-PHY0, port2 is HS-PHY0.
xHCI1 port1 is SS-PHY1, port2 is HS-PHY2 and port3 is HS-PHY1.

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..c0748ee
--- /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-combo-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-combo-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-hs-phy";
+			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 cfeaa85..21dbb12 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document
  2019-02-20 10:34 ` [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document Srinath Mannam
@ 2019-02-22 17:20   ` Rob Herring
  2019-02-22 17:29     ` Srinath Mannam
  0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2019-02-22 17:20 UTC (permalink / raw)
  To: Srinath Mannam
  Cc: Mark Rutland, Kishon Vijay Abraham I, Tejun Heo, Jayachandran C,
	devicetree, linux-kernel, bcm-kernel-feedback-list

On Wed, Feb 20, 2019 at 04:04:00PM +0530, Srinath Mannam wrote:
> 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..da19236
> --- /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-combo-phy" is a combo PHY has one SS PHY and one HS PHY.
> +	- "brcm,sr-usb-hs-phy" has a single HS PHY.
> + - reg: offset and length of the PHY blocks registers
> + - address-cells: should be 1
> + - size-cells: should be 0
> +
> +Sub-nodes:
> +  brcm,sr-usb-combo-phy have two sub-nodes for one SS PHY and one HS PHY.
> +
> +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-combo-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>;
> +		};

Again, you don't need child nodes here. There are not any per child 
resources. Clients can refer to <&usbphy0 1> just as easily as 
<&usb0_phy1>. This is why we have #phy-cells.

Rob

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document
  2019-02-22 17:20   ` Rob Herring
@ 2019-02-22 17:29     ` Srinath Mannam
  2019-02-22 19:35       ` Rob Herring
  0 siblings, 1 reply; 10+ messages in thread
From: Srinath Mannam @ 2019-02-22 17:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Kishon Vijay Abraham I, Tejun Heo, Jayachandran C,
	devicetree, Linux Kernel Mailing List, BCM Kernel Feedback

Hi Rob,

Thanks for the review, Please find my comments below in line.

On Fri, Feb 22, 2019 at 10:50 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Feb 20, 2019 at 04:04:00PM +0530, Srinath Mannam wrote:
> > 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..da19236
> > --- /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-combo-phy" is a combo PHY has one SS PHY and one HS PHY.
> > +     - "brcm,sr-usb-hs-phy" has a single HS PHY.
> > + - reg: offset and length of the PHY blocks registers
> > + - address-cells: should be 1
> > + - size-cells: should be 0
> > +
> > +Sub-nodes:
> > +  brcm,sr-usb-combo-phy have two sub-nodes for one SS PHY and one HS PHY.
> > +
> > +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-combo-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>;
> > +             };
>
> Again, you don't need child nodes here. There are not any per child
> resources. Clients can refer to <&usbphy0 1> just as easily as
> <&usb0_phy1>. This is why we have #phy-cells.
This phy controller is combo PHY it has one Super Speed USB PHY and
one High Speed USB PHY.
We required to create two PHY devices inside driver to initialize and
service(reset) both SS and HS PHYs separately.
That is the reason we used two child nodes.

Regards,
Srinath.
>
> Rob

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document
  2019-02-22 17:29     ` Srinath Mannam
@ 2019-02-22 19:35       ` Rob Herring
  2019-03-11  3:32         ` Srinath Mannam
  0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2019-02-22 19:35 UTC (permalink / raw)
  To: Srinath Mannam
  Cc: Mark Rutland, Kishon Vijay Abraham I, Tejun Heo, Jayachandran C,
	devicetree, Linux Kernel Mailing List, BCM Kernel Feedback

On Fri, Feb 22, 2019 at 11:29 AM Srinath Mannam
<srinath.mannam@broadcom.com> wrote:
>
> Hi Rob,
>
> Thanks for the review, Please find my comments below in line.
>
> On Fri, Feb 22, 2019 at 10:50 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Feb 20, 2019 at 04:04:00PM +0530, Srinath Mannam wrote:
> > > 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..da19236
> > > --- /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-combo-phy" is a combo PHY has one SS PHY and one HS PHY.
> > > +     - "brcm,sr-usb-hs-phy" has a single HS PHY.
> > > + - reg: offset and length of the PHY blocks registers
> > > + - address-cells: should be 1
> > > + - size-cells: should be 0
> > > +
> > > +Sub-nodes:
> > > +  brcm,sr-usb-combo-phy have two sub-nodes for one SS PHY and one HS PHY.
> > > +
> > > +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-combo-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>;
> > > +             };
> >
> > Again, you don't need child nodes here. There are not any per child
> > resources. Clients can refer to <&usbphy0 1> just as easily as
> > <&usb0_phy1>. This is why we have #phy-cells.
> This phy controller is combo PHY it has one Super Speed USB PHY and
> one High Speed USB PHY.
> We required to create two PHY devices inside driver to initialize and
> service(reset) both SS and HS PHYs separately.
> That is the reason we used two child nodes.

What you do in the driver is your business. That is independent of the
binding. Go look at other phy drivers which use #phy-cells=1.
.of_xlate() function is what converts the phy cells to a struct phy.

Rob

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document
  2019-02-22 19:35       ` Rob Herring
@ 2019-03-11  3:32         ` Srinath Mannam
  2019-03-11 21:30           ` Rob Herring
  0 siblings, 1 reply; 10+ messages in thread
From: Srinath Mannam @ 2019-03-11  3:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Kishon Vijay Abraham I, Tejun Heo, Jayachandran C,
	devicetree, Linux Kernel Mailing List, BCM Kernel Feedback

Hi Rob,

Please find my comments below,

On Sat, Feb 23, 2019 at 1:05 AM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Feb 22, 2019 at 11:29 AM Srinath Mannam
> <srinath.mannam@broadcom.com> wrote:
> >
> > Hi Rob,
> >
> > Thanks for the review, Please find my comments below in line.
> >
> > On Fri, Feb 22, 2019 at 10:50 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Wed, Feb 20, 2019 at 04:04:00PM +0530, Srinath Mannam wrote:
> > > > 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..da19236
> > > > --- /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-combo-phy" is a combo PHY has one SS PHY and one HS PHY.
> > > > +     - "brcm,sr-usb-hs-phy" has a single HS PHY.
> > > > + - reg: offset and length of the PHY blocks registers
> > > > + - address-cells: should be 1
> > > > + - size-cells: should be 0
> > > > +
> > > > +Sub-nodes:
> > > > +  brcm,sr-usb-combo-phy have two sub-nodes for one SS PHY and one HS PHY.
> > > > +
> > > > +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-combo-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>;
> > > > +             };
> > >
> > > Again, you don't need child nodes here. There are not any per child
> > > resources. Clients can refer to <&usbphy0 1> just as easily as
> > > <&usb0_phy1>. This is why we have #phy-cells.
> > This phy controller is combo PHY it has one Super Speed USB PHY and
> > one High Speed USB PHY.
> > We required to create two PHY devices inside driver to initialize and
> > service(reset) both SS and HS PHYs separately.
> > That is the reason we used two child nodes.
>
> What you do in the driver is your business. That is independent of the
> binding. Go look at other phy drivers which use #phy-cells=1.
> .of_xlate() function is what converts the phy cells to a struct phy.
>
I have followed exactly same pattern available in open source.
ex: Documentation/devicetree/bindings/phy/brcm-sata-phy.txt
In this also, two child nodes are used with #phy-cells 0.
> Rob

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document
  2019-03-11  3:32         ` Srinath Mannam
@ 2019-03-11 21:30           ` Rob Herring
  2019-03-12  3:54             ` Srinath Mannam
  0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2019-03-11 21:30 UTC (permalink / raw)
  To: Srinath Mannam
  Cc: Mark Rutland, Kishon Vijay Abraham I, Tejun Heo, Jayachandran C,
	devicetree, Linux Kernel Mailing List, BCM Kernel Feedback

On Sun, Mar 10, 2019 at 10:32 PM Srinath Mannam
<srinath.mannam@broadcom.com> wrote:
>
> Hi Rob,
>
> Please find my comments below,
>
> On Sat, Feb 23, 2019 at 1:05 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, Feb 22, 2019 at 11:29 AM Srinath Mannam
> > <srinath.mannam@broadcom.com> wrote:
> > >
> > > Hi Rob,
> > >
> > > Thanks for the review, Please find my comments below in line.
> > >
> > > On Fri, Feb 22, 2019 at 10:50 PM Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > On Wed, Feb 20, 2019 at 04:04:00PM +0530, Srinath Mannam wrote:
> > > > > 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..da19236
> > > > > --- /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-combo-phy" is a combo PHY has one SS PHY and one HS PHY.
> > > > > +     - "brcm,sr-usb-hs-phy" has a single HS PHY.
> > > > > + - reg: offset and length of the PHY blocks registers
> > > > > + - address-cells: should be 1
> > > > > + - size-cells: should be 0
> > > > > +
> > > > > +Sub-nodes:
> > > > > +  brcm,sr-usb-combo-phy have two sub-nodes for one SS PHY and one HS PHY.
> > > > > +
> > > > > +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-combo-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>;
> > > > > +             };
> > > >
> > > > Again, you don't need child nodes here. There are not any per child
> > > > resources. Clients can refer to <&usbphy0 1> just as easily as
> > > > <&usb0_phy1>. This is why we have #phy-cells.
> > > This phy controller is combo PHY it has one Super Speed USB PHY and
> > > one High Speed USB PHY.
> > > We required to create two PHY devices inside driver to initialize and
> > > service(reset) both SS and HS PHYs separately.
> > > That is the reason we used two child nodes.
> >
> > What you do in the driver is your business. That is independent of the
> > binding. Go look at other phy drivers which use #phy-cells=1.
> > .of_xlate() function is what converts the phy cells to a struct phy.
> >
> I have followed exactly same pattern available in open source.
> ex: Documentation/devicetree/bindings/phy/brcm-sata-phy.txt
> In this also, two child nodes are used with #phy-cells 0.

You'll notice DT maintainers did not review that binding (only changes
to it). There's no shortage of DT examples of how not to do things.

Rob

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document
  2019-03-11 21:30           ` Rob Herring
@ 2019-03-12  3:54             ` Srinath Mannam
  0 siblings, 0 replies; 10+ messages in thread
From: Srinath Mannam @ 2019-03-12  3:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Kishon Vijay Abraham I, Tejun Heo, Jayachandran C,
	devicetree, Linux Kernel Mailing List, BCM Kernel Feedback

Hi Rob,

Thank you, I will send next patch set with the changes as you suggested.

Regards,
Srinath.
On Tue, Mar 12, 2019 at 3:00 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Mar 10, 2019 at 10:32 PM Srinath Mannam
> <srinath.mannam@broadcom.com> wrote:
> >
> > Hi Rob,
> >
> > Please find my comments below,
> >
> > On Sat, Feb 23, 2019 at 1:05 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Fri, Feb 22, 2019 at 11:29 AM Srinath Mannam
> > > <srinath.mannam@broadcom.com> wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > Thanks for the review, Please find my comments below in line.
> > > >
> > > > On Fri, Feb 22, 2019 at 10:50 PM Rob Herring <robh@kernel.org> wrote:
> > > > >
> > > > > On Wed, Feb 20, 2019 at 04:04:00PM +0530, Srinath Mannam wrote:
> > > > > > 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..da19236
> > > > > > --- /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-combo-phy" is a combo PHY has one SS PHY and one HS PHY.
> > > > > > +     - "brcm,sr-usb-hs-phy" has a single HS PHY.
> > > > > > + - reg: offset and length of the PHY blocks registers
> > > > > > + - address-cells: should be 1
> > > > > > + - size-cells: should be 0
> > > > > > +
> > > > > > +Sub-nodes:
> > > > > > +  brcm,sr-usb-combo-phy have two sub-nodes for one SS PHY and one HS PHY.
> > > > > > +
> > > > > > +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-combo-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>;
> > > > > > +             };
> > > > >
> > > > > Again, you don't need child nodes here. There are not any per child
> > > > > resources. Clients can refer to <&usbphy0 1> just as easily as
> > > > > <&usb0_phy1>. This is why we have #phy-cells.
> > > > This phy controller is combo PHY it has one Super Speed USB PHY and
> > > > one High Speed USB PHY.
> > > > We required to create two PHY devices inside driver to initialize and
> > > > service(reset) both SS and HS PHYs separately.
> > > > That is the reason we used two child nodes.
> > >
> > > What you do in the driver is your business. That is independent of the
> > > binding. Go look at other phy drivers which use #phy-cells=1.
> > > .of_xlate() function is what converts the phy cells to a struct phy.
> > >
> > I have followed exactly same pattern available in open source.
> > ex: Documentation/devicetree/bindings/phy/brcm-sata-phy.txt
> > In this also, two child nodes are used with #phy-cells 0.
>
> You'll notice DT maintainers did not review that binding (only changes
> to it). There's no shortage of DT examples of how not to do things.
>
> Rob

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-03-12  3:54 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-20 10:33 [PATCH v4 0/3] Stingray USB PHY driver support Srinath Mannam
2019-02-20 10:34 ` [PATCH v4 1/3] dt-bindings: phy: Add Stingray USB PHY binding document Srinath Mannam
2019-02-22 17:20   ` Rob Herring
2019-02-22 17:29     ` Srinath Mannam
2019-02-22 19:35       ` Rob Herring
2019-03-11  3:32         ` Srinath Mannam
2019-03-11 21:30           ` Rob Herring
2019-03-12  3:54             ` Srinath Mannam
2019-02-20 10:34 ` [PATCH v4 2/3] phy: sr-usb: Add Stingray USB PHY driver Srinath Mannam
2019-02-20 10:34 ` [PATCH v4 3/3] arm64: dts: Add USB DT nodes for Stingray SoC Srinath Mannam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).