Skip to contentSkip to conversation

dt-bindings: reset: document Broadcom's BCM4908 USB reset binding

PATCH In review Rafał Miłecki 4 participants 10 messages Dec 4, 2020 – Dec 10, 2020

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 dt-bindings: reset: document Broadcom's BCM4908 USB reset binding

From: Rafał Miłecki <[email protected]>

Document binding of block responsible for initializing USB controllers (OHCI, EHCI, XHCI).

---
 .../reset/brcm,bcm4908-usb-reset.yaml         | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml new file +60 -0
new file mode 100644
index 000000000000..31beb1c8f3cd
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/brcm,bcm4908-usb-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM4908 USB host controller reset
+
+description: >
+  BCM4908 has a separated block controlling all USB controllers. It handles the
+  whole setup process and takes care of initializing PHYs at the right time
+  (state).
+
+maintainers:
+  - Rafał Miłecki <[email protected]>
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm4908-usb-reset
+
+  reg:
+    maxItems: 1
+
+  resets:
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  phys:
+    minItems: 2
+    maxItems: 2
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
+  phy-names:
+    items:
+      - const: usb2
+      - const: usb3
+
+  "#reset-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - phys
+  - phy-names
+  - "#reset-cells"
+
+additionalProperties: true
+
+examples:
+  - |
+    reset-controller@8000c200 {
+        compatible = "brcm,bcm4908-usb-reset";
+        reg = <0x8000c200 0x100>;
+
+        phys = <&usb2_phy>, <&usb3_phy>;
+        phy-names = "usb2", "usb3";
+
+        #reset-cells = <0>;
+    };
Signature · 1 line
2.26.2
#1 lore raw

PATCH 2/2 reset: bcm4908-usb: add driver for BCM4908 USB reset controller

From: Rafał Miłecki <[email protected]>

This controller is responsible for OHCI, EHCI, XHCI and PHYs setup that has to be handled in the proper order.

One unusual thing about this controller is that is provides access to the MDIO bus. There are two registers (in the middle of block space) responsible for that. For that reason this driver initializes regmap so a proper MDIO driver can use them.

---
 drivers/reset/Kconfig             |   8 +
 drivers/reset/Makefile            |   1 +
 drivers/reset/reset-bcm4908-usb.c | 250 ++++++++++++++++++++++++++++++
 3 files changed, 259 insertions(+)
 create mode 100644 drivers/reset/reset-bcm4908-usb.c
drivers/reset/Kconfig +8 -0
index f393f7e17e33..bb4d2bea9e95 100644
@@ -35,6 +35,14 @@ config RESET_AXS10X
 	help
 	  This enables the reset controller driver for AXS10x.
 
+config RESET_BCM4908_USB
+	tristate "Broadcom BCM4908 USB controller"
+	depends on ARM64 || COMPILE_TEST
+	default ARCH_BCM4908
+	help
+	  This enables driver for the Broadcom BCM4908 USB controller
+	  responsible for initializing OHCI, EHCI, XHCI and PHYs.
+
 config RESET_BERLIN
 	bool "Berlin Reset Driver" if COMPILE_TEST
 	default ARCH_BERLIN
drivers/reset/Makefile +1 -0
index 0dd5d42050dc..f2627bbc7ad4 100644
@@ -6,6 +6,7 @@ obj-$(CONFIG_ARCH_TEGRA) += tegra/
 obj-$(CONFIG_RESET_A10SR) += reset-a10sr.o
 obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
 obj-$(CONFIG_RESET_AXS10X) += reset-axs10x.o
+obj-$(CONFIG_RESET_BCM4908_USB) += reset-bcm4908-usb.o
 obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
 obj-$(CONFIG_RESET_BRCM_PMB) += reset-brcm-pmb.o
 obj-$(CONFIG_RESET_BRCMSTB) += reset-brcmstb.o
drivers/reset/reset-bcm4908-usb.c new file +250 -0

250 lines not on this page · raw message

Signature · 1 line
2.26.2
maintainer
#2 lore raw

Re: [PATCH 2/2] reset: bcm4908-usb: add driver for BCM4908 USB reset controller

Hi Rafał,

Quoted 11 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

This controller is responsible for OHCI, EHCI, XHCI and PHYs setup that has to be handled in the proper order.

One unusual thing about this controller is that is provides access to the MDIO bus. There are two registers (in the middle of block space) responsible for that. For that reason this driver initializes regmap so a proper MDIO driver can use them.

This doesn't look like a reset controller to me, but rather like something that belongs in drivers/usb.

regards
Philipp

Quoted 294 lines
---
 drivers/reset/Kconfig             |   8 +
 drivers/reset/Makefile            |   1 +
 drivers/reset/reset-bcm4908-usb.c | 250 ++++++++++++++++++++++++++++++
 3 files changed, 259 insertions(+)
 create mode 100644 drivers/reset/reset-bcm4908-usb.c
drivers/reset/Kconfig +8 -0
index f393f7e17e33..bb4d2bea9e95 100644
@@ -35,6 +35,14 @@ config RESET_AXS10X
 	help
 	  This enables the reset controller driver for AXS10x.
 
+config RESET_BCM4908_USB
+	tristate "Broadcom BCM4908 USB controller"
+	depends on ARM64 || COMPILE_TEST
+	default ARCH_BCM4908
+	help
+	  This enables driver for the Broadcom BCM4908 USB controller
+	  responsible for initializing OHCI, EHCI, XHCI and PHYs.
+
 config RESET_BERLIN
 	bool "Berlin Reset Driver" if COMPILE_TEST
 	default ARCH_BERLIN
drivers/reset/Makefile +1 -0
index 0dd5d42050dc..f2627bbc7ad4 100644
@@ -6,6 +6,7 @@ obj-$(CONFIG_ARCH_TEGRA) += tegra/
 obj-$(CONFIG_RESET_A10SR) += reset-a10sr.o
 obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
 obj-$(CONFIG_RESET_AXS10X) += reset-axs10x.o
+obj-$(CONFIG_RESET_BCM4908_USB) += reset-bcm4908-usb.o
 obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
 obj-$(CONFIG_RESET_BRCM_PMB) += reset-brcm-pmb.o
 obj-$(CONFIG_RESET_BRCMSTB) += reset-brcmstb.o
drivers/reset/reset-bcm4908-usb.c new file +250 -0
new file mode 100644
index 000000000000..e9b7d369c894
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2013 Broadcom
+ * Copyright (C) 2020 Rafał Miłecki <[email protected]>
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+#include <linux/reset.h>
+
+#define BCM4908_USB_RESET_SETUP						0x0000
+#define  BCM4908_USBH_IPP						(1<<5)
+#define  BCM4908_USBH_IOC						(1<<4)
+#define  BCM4908_USB2_OC_DISABLE_PORT0					(1<<28)
+#define  BCM4908_USB2_OC_DISABLE_PORT1					(1<<29)
+#define  BCM4908_USB3_OC_DISABLE_PORT0					(1<<30)
+#define  BCM4908_USB3_OC_DISABLE_PORT1					(1<<31)
+#define BCM4908_USB_RESET_PLL_CTL					0x0004
+#define BCM4908_USB_RESET_FLADJ_VALUE					0x0008
+#define BCM4908_USB_RESET_BRIDGE_CTL					0x000c
+#define BCM4908_USB_RESET_SPARE1					0x0010
+#define BCM4908_USB_RESET_MDIO						0x0014
+#define BCM4908_USB_RESET_MDIO2						0x0018
+#define BCM4908_USB_RESET_TEST_PORT_CONTROL				0x001c
+#define BCM4908_USB_RESET_USB_SIMCTL					0x0020
+#define  BCM4908_USBH_OHCI_MEM_REQ_DIS					(1<<1)
+#define BCM4908_USB_RESET_USB_TESTCTL					0x0024
+#define BCM4908_USB_RESET_USB_TESTMON					0x0028
+#define BCM4908_USB_RESET_UTMI_CTL_1					0x002c
+#define BCM4908_USB_RESET_SPARE2					0x0030
+#define BCM4908_USB_RESET_USB_PM					0x0034
+#define  BCM4908_XHC_SOFT_RESETB					(1<<22)
+#define  BCM4908_USB_PWRDWN						(1<<31)
+#define BCM4908_USB_RESET_USB_PM_STATUS					0x0038
+#define BCM4908_USB_RESET_SPARE3					0x003c
+#define BCM4908_USB_RESET_PLL_LDO_CTL					0x0040
+#define BCM4908_USB_RESET_PLL_LDO_PLLBIAS				0x0044
+#define BCM4908_USB_RESET_PLL_AFE_BG_CNTL				0x0048
+#define BCM4908_USB_RESET_AFE_USBIO_TST					0x004c
+#define BCM4908_USB_RESET_PLL_NDIV_FRAC					0x0050
+#define BCM4908_USB_RESET_TP_DIAG					0x0054
+#define BCM4908_USB_RESET_AHB_CAPTURE_FIFO				0x0058
+#define BCM4908_USB_RESET_SPARE4					0x005c
+#define BCM4908_USB_RESET_USB30_CTL1					0x0060
+#define  BCM4908_PHY3_PLL_SEQ_START					(1<<4)
+#define BCM4908_USB_RESET_USB30_CTL2					0x0064
+#define BCM4908_USB_RESET_USB30_CTL3					0x0068
+#define BCM4908_USB_RESET_USB30_CTL4					0x006c
+#define BCM4908_USB_RESET_USB30_PCTL					0x0070
+#define BCM4908_USB_RESET_USB30_CTL5					0x0074
+#define BCM4908_USB_RESET_SPARE5					0x0078
+#define BCM4908_USB_RESET_SPARE6					0x007c
+#define BCM4908_USB_RESET_SPARE7					0x0080
+#define BCM4908_USB_RESET_USB_DEVICE_CTL1				0x0090
+#define BCM4908_USB_RESET_USB_DEVICE_CTL2				0x0094
+#define BCM4908_USB_RESET_USB20_ID					0x0150
+#define BCM4908_USB_RESET_USB30_ID					0x0154
+#define BCM4908_USB_RESET_BDC_COREID					0x0158
+#define BCM4908_USB_RESET_USB_REVID					0x015c
+
+struct bcm4908usb {
+	struct device *dev;
+	void __iomem *base;
+	struct regmap *regmap;
+	struct reset_control *reset;
+	struct phy *usb2_phy;
+	struct phy *usb3_phy;
+	struct reset_controller_dev rcdev;
+};
+
+static const struct regmap_config bcm4908_usb_reset_regmap_config = {
+	.reg_bits =	32,
+	.reg_stride =	4,
+	.val_bits =	32,
+	.fast_io =	true,
+};
+
+static u32 bcm4908_usb_reset_read(struct bcm4908usb *usb, u32 reg)
+{
+	return readl(usb->base + reg);
+}
+
+static void bcm4908_usb_reset_write(struct bcm4908usb *usb, u32 reg, u32 value)
+{
+	writel(value, usb->base + reg);
+}
+
+static void bcm4908_usb_reset_update_bits(struct bcm4908usb *usb, u32 reg, u32 mask, u32 val)
+{
+	u32 tmp;
+
+	WARN_ON(val & ~mask);
+
+	tmp = readl(usb->base + reg);
+	tmp &= ~mask;
+	tmp |= val & mask;
+	writel(tmp, usb->base + reg);
+}
+
+static int bcm4908_usb_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct bcm4908usb *usb = container_of(rcdev, struct bcm4908usb, rcdev);
+	struct device *dev = usb->dev;
+	u32 val;
+	int err;
+
+	err = reset_control_deassert(usb->reset);
+	if (err) {
+		dev_err(dev, "failed to deassert");
+		return err;
+	}
+
+	mdelay(1);
+
+	/* adjust over current & port power polarity */
+	bcm4908_usb_reset_update_bits(usb, BCM4908_USB_RESET_SETUP,
+				      BCM4908_USBH_IOC, BCM4908_USBH_IOC);
+	bcm4908_usb_reset_update_bits(usb, BCM4908_USB_RESET_SETUP,
+				      BCM4908_USBH_IPP, BCM4908_USBH_IPP);
+
+	/* enable USB PHYs */
+	bcm4908_usb_reset_update_bits(usb, BCM4908_USB_RESET_USB_PM,
+				      BCM4908_USB_PWRDWN, 0);
+	mdelay(1);
+
+	err = phy_init(usb->usb3_phy);
+	if (err) {
+		dev_err(usb->dev, "failed to init USB 3.0 PHY: %d\n", err);
+		return err;
+	}
+	mdelay(300);
+
+	bcm4908_usb_reset_update_bits(usb, BCM4908_USB_RESET_USB30_CTL1,
+				      BCM4908_PHY3_PLL_SEQ_START, BCM4908_PHY3_PLL_SEQ_START);
+	bcm4908_usb_reset_update_bits(usb, BCM4908_USB_RESET_USB_PM,
+				      BCM4908_XHC_SOFT_RESETB, BCM4908_XHC_SOFT_RESETB);
+
+	err = phy_init(usb->usb2_phy);
+	if (err) {
+		dev_err(usb->dev, "failed to init USB 2.0 PHY: %d\n", err);
+		return err;
+	}
+
+	/* no swap for data & descriptors */
+	bcm4908_usb_reset_update_bits(usb, BCM4908_USB_RESET_BRIDGE_CTL, 0xf, 0);
+
+	/* reset host controllers for possible fake overcurrent indications */
+	val = bcm4908_usb_reset_read(usb, BCM4908_USB_RESET_USB_PM);
+	bcm4908_usb_reset_write(usb, BCM4908_USB_RESET_USB_PM, 0);
+	bcm4908_usb_reset_write(usb, BCM4908_USB_RESET_USB_PM, val);
+	mdelay(1);
+
+	/* TODO: erdy nump bypass */
+
+	/* Reduce accesses to DDR during USB idle time when Self-Refresh feature is compiled in */
+	bcm4908_usb_reset_update_bits(usb, BCM4908_USB_RESET_USB_SIMCTL,
+				      BCM4908_USBH_OHCI_MEM_REQ_DIS, BCM4908_USBH_OHCI_MEM_REQ_DIS);
+
+	return 0;
+}
+
+static const struct reset_control_ops bcm4908_usb_reset_control_ops = {
+	.deassert = bcm4908_usb_deassert,
+};
+
+static int bcm4908_usb_reset_xlate(struct reset_controller_dev *rcdev,
+				   const struct of_phandle_args *reset_spec)
+{
+	if (WARN_ON(reset_spec->args_count))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int bcm4908_usb_reset_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct bcm4908usb *usb;
+	int err;
+
+	usb = devm_kzalloc(dev, sizeof(*usb), GFP_KERNEL);
+	if (!usb)
+		return -ENOMEM;
+
+	usb->dev = dev;
+
+	usb->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(usb->base))
+		return PTR_ERR(usb->base);
+
+	usb->regmap = devm_regmap_init_mmio(dev, usb->base, &bcm4908_usb_reset_regmap_config);
+	if (IS_ERR(usb->regmap)) {
+		err = PTR_ERR(usb->regmap);
+		dev_err(dev, "failed to init regmap: %d\n", err);
+		return err;
+	}
+
+	usb->reset = of_reset_control_array_get_optional_exclusive(usb->dev->of_node);
+	if (IS_ERR(usb->reset)) {
+		err = PTR_ERR(usb->reset);
+		return err;
+	}
+
+	usb->usb2_phy = devm_phy_get(dev, "usb2");
+	if (IS_ERR(usb->usb2_phy)) {
+		err = PTR_ERR(usb->usb2_phy);
+		dev_err(dev, "Failed to get USB 2.0 PHY: %d\n", err);
+		return err;
+	}
+
+	usb->usb3_phy = devm_phy_get(dev, "usb3");
+	if (IS_ERR(usb->usb3_phy)) {
+		err = PTR_ERR(usb->usb3_phy);
+		dev_err(dev, "Failed to get USB 2.0 PHY: %d\n", err);
+		return err;
+	}
+
+	usb->rcdev.ops = &bcm4908_usb_reset_control_ops;
+	usb->rcdev.owner = THIS_MODULE;
+	usb->rcdev.of_node = dev->of_node;
+	usb->rcdev.of_reset_n_cells = 0;
+	usb->rcdev.of_xlate = bcm4908_usb_reset_xlate;
+
+	return devm_reset_controller_register(dev, &usb->rcdev);
+}
+
+static const struct of_device_id bcm4908_usb_reset_of_match[] = {
+	{ .compatible = "brcm,bcm4908-usb-reset" },
+	{ },
+};
+
+static struct platform_driver bcm4908_usb_reset_driver = {
+	.probe = bcm4908_usb_reset_probe,
+	.driver = {
+		.name	= "bcm4908-usb-reset",
+		.of_match_table	= bcm4908_usb_reset_of_match,
+	}
+};
+module_platform_driver(bcm4908_usb_reset_driver);
+
+MODULE_AUTHOR("Rafał Miłecki");
+MODULE_LICENSE("GPL v2");
+MODULE_DEVICE_TABLE(of, bcm4908_usb_reset_of_match);
#3 lore raw

Re: [PATCH 2/2] reset: bcm4908-usb: add driver for BCM4908 USB reset controller

Quoted 11 lines · Philipp Zabel
Quoted 10 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

This controller is responsible for OHCI, EHCI, XHCI and PHYs setup that has to be handled in the proper order.

One unusual thing about this controller is that is provides access to the MDIO bus. There are two registers (in the middle of block space) responsible for that. For that reason this driver initializes regmap so a proper MDIO driver can use them.

Quoted 4 lines
Quoted 1 line

This doesn't look like a reset controller to me, but rather like something that belongs in drivers/usb.

I think I found the reset API to match the best setup requirements and assumed it should be treated as a reset controller.

Any advice, idea, how should I integrate this driver with the USB subsystem? Rested made it easy as all I needed was:

usb@c300 {
    compatible = "generic-ehci";
    reg = <0xc300 0x100>;
    interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
    resets = <&usb_reset>;
};
Signature · 1 line
Rafał
#4 lore raw

Re: [PATCH 2/2] reset: bcm4908-usb: add driver for BCM4908 USB reset controller

In reply to Rafał Miłecki
Quoted 12 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

This controller is responsible for OHCI, EHCI, XHCI and PHYs setup that has to be handled in the proper order.

One unusual thing about this controller is that is provides access to the MDIO bus. There are two registers (in the middle of block space) responsible for that. For that reason this driver initializes regmap so a proper MDIO driver can use them.

---

[snip]

Quoted 40 lines
drivers/reset/reset-bcm4908-usb.c new file +40 -0
+
+#define BCM4908_USB_RESET_SETUP						0x0000
+#define  BCM4908_USBH_IPP						(1<<5)
+#define  BCM4908_USBH_IOC						(1<<4)
+#define  BCM4908_USB2_OC_DISABLE_PORT0					(1<<28)
+#define  BCM4908_USB2_OC_DISABLE_PORT1					(1<<29)
+#define  BCM4908_USB3_OC_DISABLE_PORT0					(1<<30)
+#define  BCM4908_USB3_OC_DISABLE_PORT1					(1<<31)
+#define BCM4908_USB_RESET_PLL_CTL					0x0004
+#define BCM4908_USB_RESET_FLADJ_VALUE					0x0008
+#define BCM4908_USB_RESET_BRIDGE_CTL					0x000c
+#define BCM4908_USB_RESET_SPARE1					0x0010
+#define BCM4908_USB_RESET_MDIO						0x0014
+#define BCM4908_USB_RESET_MDIO2						0x0018
+#define BCM4908_USB_RESET_TEST_PORT_CONTROL				0x001c
+#define BCM4908_USB_RESET_USB_SIMCTL					0x0020
+#define  BCM4908_USBH_OHCI_MEM_REQ_DIS					(1<<1)
+#define BCM4908_USB_RESET_USB_TESTCTL					0x0024
+#define BCM4908_USB_RESET_USB_TESTMON					0x0028
+#define BCM4908_USB_RESET_UTMI_CTL_1					0x002c
+#define BCM4908_USB_RESET_SPARE2					0x0030
+#define BCM4908_USB_RESET_USB_PM					0x0034
+#define  BCM4908_XHC_SOFT_RESETB					(1<<22)
+#define  BCM4908_USB_PWRDWN						(1<<31)
+#define BCM4908_USB_RESET_USB_PM_STATUS					0x0038
+#define BCM4908_USB_RESET_SPARE3					0x003c
+#define BCM4908_USB_RESET_PLL_LDO_CTL					0x0040
+#define BCM4908_USB_RESET_PLL_LDO_PLLBIAS				0x0044
+#define BCM4908_USB_RESET_PLL_AFE_BG_CNTL				0x0048
+#define BCM4908_USB_RESET_AFE_USBIO_TST					0x004c
+#define BCM4908_USB_RESET_PLL_NDIV_FRAC					0x0050
+#define BCM4908_USB_RESET_TP_DIAG					0x0054
+#define BCM4908_USB_RESET_AHB_CAPTURE_FIFO				0x0058
+#define BCM4908_USB_RESET_SPARE4					0x005c
+#define BCM4908_USB_RESET_USB30_CTL1					0x0060
+#define  BCM4908_PHY3_PLL_SEQ_START					(1<<4)
+#define BCM4908_USB_RESET_USB30_CTL2					0x0064
+#define BCM4908_USB_RESET_USB30_CTL3					0x0068
+#define BCM4908_USB_RESET_USB30_CTL4					0x006c
+#define BCM4908_USB_RESET_USB30_PCTL					0x0070

drivers/reset/reset-bcm4908-usb.c line 66

Quoted 10 lines
drivers/reset/reset-bcm4908-usb.c new file +10 -0
+#define BCM4908_USB_RESET_USB30_CTL5					0x0074
+#define BCM4908_USB_RESET_SPARE5					0x0078
+#define BCM4908_USB_RESET_SPARE6					0x007c
+#define BCM4908_USB_RESET_SPARE7					0x0080
+#define BCM4908_USB_RESET_USB_DEVICE_CTL1				0x0090
+#define BCM4908_USB_RESET_USB_DEVICE_CTL2				0x0094
+#define BCM4908_USB_RESET_USB20_ID					0x0150
+#define BCM4908_USB_RESET_USB30_ID					0x0154
+#define BCM4908_USB_RESET_BDC_COREID					0x0158
+#define BCM4908_USB_RESET_USB_REVID					0x015c

This register layout is nearly identical to the one described under drivers/phy/broadcom/phy-brcm-usb-init.c and this is because within Broadcom the same design group has been supplying the USB PHY and host controllers to the DSL and STB product lines.

I would model this the same way we have done it for the Broadcom STB HCI drivers and add a separate compatible string along with an optional reset line.

As far as MDIO goes as you can see the USB PHY driver uses a mix of memory mapped and MDIO accesses (eye fix, etc.) so it was deemed cleaner to not use the MDIO subsystem for the very few accesses that are required.

This is different from the Northstar platform you have been working on where the USB PHYs are not memory maapped at all and only accessible over MDIO.

Let me know if you think the existing driver would not be extensible to support 4908.

Signature · 1 line
Florian
#5 lore raw

Re: [PATCH 2/2] reset: bcm4908-usb: add driver for BCM4908 USB reset controller

Quoted 80 lines · Florian Fainelli
Quoted 12 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

This controller is responsible for OHCI, EHCI, XHCI and PHYs setup that has to be handled in the proper order.

One unusual thing about this controller is that is provides access to the MDIO bus. There are two registers (in the middle of block space) responsible for that. For that reason this driver initializes regmap so a proper MDIO driver can use them.

---

[snip]

Quoted 40 lines
drivers/reset/reset-bcm4908-usb.c new file +40 -0
+
+#define BCM4908_USB_RESET_SETUP						0x0000
+#define  BCM4908_USBH_IPP						(1<<5)
+#define  BCM4908_USBH_IOC						(1<<4)
+#define  BCM4908_USB2_OC_DISABLE_PORT0					(1<<28)
+#define  BCM4908_USB2_OC_DISABLE_PORT1					(1<<29)
+#define  BCM4908_USB3_OC_DISABLE_PORT0					(1<<30)
+#define  BCM4908_USB3_OC_DISABLE_PORT1					(1<<31)
+#define BCM4908_USB_RESET_PLL_CTL					0x0004
+#define BCM4908_USB_RESET_FLADJ_VALUE					0x0008
+#define BCM4908_USB_RESET_BRIDGE_CTL					0x000c
+#define BCM4908_USB_RESET_SPARE1					0x0010
+#define BCM4908_USB_RESET_MDIO						0x0014
+#define BCM4908_USB_RESET_MDIO2						0x0018
+#define BCM4908_USB_RESET_TEST_PORT_CONTROL				0x001c
+#define BCM4908_USB_RESET_USB_SIMCTL					0x0020
+#define  BCM4908_USBH_OHCI_MEM_REQ_DIS					(1<<1)
+#define BCM4908_USB_RESET_USB_TESTCTL					0x0024
+#define BCM4908_USB_RESET_USB_TESTMON					0x0028
+#define BCM4908_USB_RESET_UTMI_CTL_1					0x002c
+#define BCM4908_USB_RESET_SPARE2					0x0030
+#define BCM4908_USB_RESET_USB_PM					0x0034
+#define  BCM4908_XHC_SOFT_RESETB					(1<<22)
+#define  BCM4908_USB_PWRDWN						(1<<31)
+#define BCM4908_USB_RESET_USB_PM_STATUS					0x0038
+#define BCM4908_USB_RESET_SPARE3					0x003c
+#define BCM4908_USB_RESET_PLL_LDO_CTL					0x0040
+#define BCM4908_USB_RESET_PLL_LDO_PLLBIAS				0x0044
+#define BCM4908_USB_RESET_PLL_AFE_BG_CNTL				0x0048
+#define BCM4908_USB_RESET_AFE_USBIO_TST					0x004c
+#define BCM4908_USB_RESET_PLL_NDIV_FRAC					0x0050
+#define BCM4908_USB_RESET_TP_DIAG					0x0054
+#define BCM4908_USB_RESET_AHB_CAPTURE_FIFO				0x0058
+#define BCM4908_USB_RESET_SPARE4					0x005c
+#define BCM4908_USB_RESET_USB30_CTL1					0x0060
+#define  BCM4908_PHY3_PLL_SEQ_START					(1<<4)
+#define BCM4908_USB_RESET_USB30_CTL2					0x0064
+#define BCM4908_USB_RESET_USB30_CTL3					0x0068
+#define BCM4908_USB_RESET_USB30_CTL4					0x006c
+#define BCM4908_USB_RESET_USB30_PCTL					0x0070

drivers/reset/reset-bcm4908-usb.c line 66

Quoted 10 lines
drivers/reset/reset-bcm4908-usb.c new file +10 -0
+#define BCM4908_USB_RESET_USB30_CTL5					0x0074
+#define BCM4908_USB_RESET_SPARE5					0x0078
+#define BCM4908_USB_RESET_SPARE6					0x007c
+#define BCM4908_USB_RESET_SPARE7					0x0080
+#define BCM4908_USB_RESET_USB_DEVICE_CTL1				0x0090
+#define BCM4908_USB_RESET_USB_DEVICE_CTL2				0x0094
+#define BCM4908_USB_RESET_USB20_ID					0x0150
+#define BCM4908_USB_RESET_USB30_ID					0x0154
+#define BCM4908_USB_RESET_BDC_COREID					0x0158
+#define BCM4908_USB_RESET_USB_REVID					0x015c

This register layout is nearly identical to the one described under drivers/phy/broadcom/phy-brcm-usb-init.c and this is because within Broadcom the same design group has been supplying the USB PHY and host controllers to the DSL and STB product lines.

I would model this the same way we have done it for the Broadcom STB HCI drivers and add a separate compatible string along with an optional reset line.

As far as MDIO goes as you can see the USB PHY driver uses a mix of memory mapped and MDIO accesses (eye fix, etc.) so it was deemed cleaner to not use the MDIO subsystem for the very few accesses that are required.

Quoted 6 lines

This is different from the Northstar platform you have been working on where the USB PHYs are not memory maapped at all and only accessible over MDIO.

Let me know if you think the existing driver would not be extensible to support 4908.

I see some / many similarities in that PHY driver, I'll try to reuse it, thanks!

#6 lore raw
In reply to Rafał Miłecki
Quoted 64 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

Document binding of block responsible for initializing USB controllers (OHCI, EHCI, XHCI).

---
 .../reset/brcm,bcm4908-usb-reset.yaml         | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml new file +47 -0
new file mode 100644
index 000000000000..31beb1c8f3cd
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/brcm,bcm4908-usb-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM4908 USB host controller reset
+
+description: >
+  BCM4908 has a separated block controlling all USB controllers. It handles the
+  whole setup process and takes care of initializing PHYs at the right time
+  (state).
+
+maintainers:
+  - Rafał Miłecki <[email protected]>
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm4908-usb-reset
+
+  reg:
+    maxItems: 1
+
+  resets:
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  phys:
+    minItems: 2
+    maxItems: 2
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
+  phy-names:
+    items:
+      - const: usb2
+      - const: usb3
+
+  "#reset-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - phys
+  - phy-names
+  - "#reset-cells"
+

Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml line 57

Quoted 10 lines
Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml new file +10 -0
+additionalProperties: true
+
+examples:
+  - |
+    reset-controller@8000c200 {
+        compatible = "brcm,bcm4908-usb-reset";
+        reg = <0x8000c200 0x100>;
+
+        phys = <&usb2_phy>, <&usb3_phy>;
+        phy-names = "usb2", "usb3";

This looks quite unusual, usually the *HCI controllers would be consumers of the PHY and the PHY may be a consumer of the reset controller.

(still going through my emails have not fully read your separate email on the topic, so pardon me if this is being discussed twice).

Signature · 1 line
Florian
#7 lore raw
Quoted 79 lines · Florian Fainelli
Quoted 64 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

Document binding of block responsible for initializing USB controllers (OHCI, EHCI, XHCI).

---
  .../reset/brcm,bcm4908-usb-reset.yaml         | 60 +++++++++++++++++++
  1 file changed, 60 insertions(+)

create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml

Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml new file +47 -0
new file mode 100644
index 000000000000..31beb1c8f3cd
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/brcm,bcm4908-usb-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM4908 USB host controller reset
+
+description: >
+  BCM4908 has a separated block controlling all USB controllers. It handles the
+  whole setup process and takes care of initializing PHYs at the right time
+  (state).
+
+maintainers:
+  - Rafał Miłecki <[email protected]>
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm4908-usb-reset
+
+  reg:
+    maxItems: 1
+
+  resets:
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  phys:
+    minItems: 2
+    maxItems: 2
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
+  phy-names:
+    items:
+      - const: usb2
+      - const: usb3
+
+  "#reset-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - phys
+  - phy-names
+  - "#reset-cells"
+

Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml line 57

Quoted 10 lines
Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml new file +10 -0
+additionalProperties: true
+
+examples:
+  - |
+    reset-controller@8000c200 {
+        compatible = "brcm,bcm4908-usb-reset";
+        reg = <0x8000c200 0x100>;
+
+        phys = <&usb2_phy>, <&usb3_phy>;
+        phy-names = "usb2", "usb3";

This looks quite unusual, usually the *HCI controllers would be consumers of the PHY and the PHY may be a consumer of the reset controller.

Quoted 2 lines

(still going through my emails have not fully read your separate email on the topic, so pardon me if this is being discussed twice).

I agree, it's the the best solution I found for this specific design.

This specific hw block perform various operations before, in the middle and after PHY initialization. That made me make reset controlller initialize PHYs.

I'm happy to implement a more proper design if someone can just suggest how. I don't have any better idea :(

maintainer
#8 lore raw
Quoted 83 lines · Rafał Miłecki
Quoted 79 lines · Florian Fainelli
Quoted 64 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

Document binding of block responsible for initializing USB controllers (OHCI, EHCI, XHCI).

---
  .../reset/brcm,bcm4908-usb-reset.yaml         | 60 +++++++++++++++++++
  1 file changed, 60 insertions(+)

create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml

Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml new file +47 -0
new file mode 100644
index 000000000000..31beb1c8f3cd
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/brcm,bcm4908-usb-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM4908 USB host controller reset
+
+description: >
+  BCM4908 has a separated block controlling all USB controllers. It handles the
+  whole setup process and takes care of initializing PHYs at the right time
+  (state).
+
+maintainers:
+  - Rafał Miłecki <[email protected]>
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm4908-usb-reset
+
+  reg:
+    maxItems: 1
+
+  resets:
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  phys:
+    minItems: 2
+    maxItems: 2
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
+  phy-names:
+    items:
+      - const: usb2
+      - const: usb3
+
+  "#reset-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - phys
+  - phy-names
+  - "#reset-cells"
+

Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml line 57

Quoted 10 lines
Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml new file +10 -0
+additionalProperties: true
+
+examples:
+  - |
+    reset-controller@8000c200 {
+        compatible = "brcm,bcm4908-usb-reset";
+        reg = <0x8000c200 0x100>;
+
+        phys = <&usb2_phy>, <&usb3_phy>;
+        phy-names = "usb2", "usb3";

This looks quite unusual, usually the *HCI controllers would be consumers of the PHY and the PHY may be a consumer of the reset controller.

Quoted 2 lines

(still going through my emails have not fully read your separate email on the topic, so pardon me if this is being discussed twice).

Quoted 7 lines

I agree, it's the the best solution I found for this specific design.

This specific hw block perform various operations before, in the middle and after PHY initialization. That made me make reset controlller initialize PHYs.

I'm happy to implement a more proper design if someone can just suggest how. I don't have any better idea :(

So the reset controller block has more than just resets? I'd hide all this in the phy driver rather than hide the phy in the reset driver. So for DT provide the phy a phandle to the reset block to poke the registers directly.

Rob

#9 lore raw
Quoted 89 lines · Rob Herring
Quoted 88 lines · Rafał Miłecki
Quoted 79 lines · Florian Fainelli
Quoted 64 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

Document binding of block responsible for initializing USB controllers (OHCI, EHCI, XHCI).

---
  .../reset/brcm,bcm4908-usb-reset.yaml         | 60 +++++++++++++++++++
  1 file changed, 60 insertions(+)

create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml

Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml new file +47 -0
new file mode 100644
index 000000000000..31beb1c8f3cd
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/brcm,bcm4908-usb-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM4908 USB host controller reset
+
+description: >
+  BCM4908 has a separated block controlling all USB controllers. It handles the
+  whole setup process and takes care of initializing PHYs at the right time
+  (state).
+
+maintainers:
+  - Rafał Miłecki <[email protected]>
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm4908-usb-reset
+
+  reg:
+    maxItems: 1
+
+  resets:
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  phys:
+    minItems: 2
+    maxItems: 2
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
+  phy-names:
+    items:
+      - const: usb2
+      - const: usb3
+
+  "#reset-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - phys
+  - phy-names
+  - "#reset-cells"
+

Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml line 57

Quoted 10 lines
Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml new file +10 -0
+additionalProperties: true
+
+examples:
+  - |
+    reset-controller@8000c200 {
+        compatible = "brcm,bcm4908-usb-reset";
+        reg = <0x8000c200 0x100>;
+
+        phys = <&usb2_phy>, <&usb3_phy>;
+        phy-names = "usb2", "usb3";

This looks quite unusual, usually the *HCI controllers would be consumers of the PHY and the PHY may be a consumer of the reset controller.

Quoted 2 lines

(still going through my emails have not fully read your separate email on the topic, so pardon me if this is being discussed twice).

I agree, it's the the best solution I found for this specific design.

This specific hw block perform various operations before, in the middle and after PHY initialization. That made me make reset controlller initialize PHYs.

Quoted 7 lines
Quoted 2 lines

I'm happy to implement a more proper design if someone can just suggest how. I don't have any better idea :(

So the reset controller block has more than just resets? I'd hide all this in the phy driver rather than hide the phy in the reset driver. So for DT provide the phy a phandle to the reset block to poke the registers directly.

Rafal was pointed to drivers/phy/broadcom/phy-brcm-usb-init.c which should be a good starting point for driving the 4908 USB PHY. From there the USB PHY may optionally be a consumer of a reset controller if it needs to, not clear if that is necessary.

Signature · 1 line
Florian