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

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

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

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

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../reset/brcm,bcm4908-usb-reset.yaml         | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml

diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
new file mode 100644
index 000000000000..31beb1c8f3cd
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
@@ -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 <rafal@milecki.pl>
+
+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>;
+    };
-- 
2.26.2

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

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

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.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 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

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index f393f7e17e33..bb4d2bea9e95 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -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
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 0dd5d42050dc..f2627bbc7ad4 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -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
diff --git a/drivers/reset/reset-bcm4908-usb.c b/drivers/reset/reset-bcm4908-usb.c
new file mode 100644
index 000000000000..e9b7d369c894
--- /dev/null
+++ b/drivers/reset/reset-bcm4908-usb.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2013 Broadcom
+ * Copyright (C) 2020 Rafał Miłecki <rafal@milecki.pl>
+ */
+
+#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);
-- 
2.26.2

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

[Philipp Zabel]

Hi Rafał,

On Fri, 2020-12-04 at 10:37 +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> 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.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

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

regards
Philipp

> ---
>  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
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index f393f7e17e33..bb4d2bea9e95 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -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
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 0dd5d42050dc..f2627bbc7ad4 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -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
> diff --git a/drivers/reset/reset-bcm4908-usb.c b/drivers/reset/reset-bcm4908-usb.c
> new file mode 100644
> index 000000000000..e9b7d369c894
> --- /dev/null
> +++ b/drivers/reset/reset-bcm4908-usb.c
> @@ -0,0 +1,250 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2013 Broadcom
> + * Copyright (C) 2020 Rafał Miłecki <rafal@milecki.pl>
> + */
> +
> +#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);

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

[Rafał Miłecki]

On Fri, 4 Dec 2020 at 17:13, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> On Fri, 2020-12-04 at 10:37 +0100, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@milecki.pl>
> >
> > 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.
> >
> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>
> 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>;
};

-- 
Rafał

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

[Florian Fainelli]

On 12/4/2020 1:37 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Document binding of block responsible for initializing USB controllers
> (OHCI, EHCI, XHCI).
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../reset/brcm,bcm4908-usb-reset.yaml         | 60 +++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
> new file mode 100644
> index 000000000000..31beb1c8f3cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
> @@ -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 <rafal@milecki.pl>
> +
> +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";

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).
-- 
Florian

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

[Florian Fainelli]

On 12/4/2020 1:37 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> 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.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---

[snip]

> +
> +#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

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.
-- 
Florian

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

[Rafał Miłecki]

On 04.12.2020 17:32, Florian Fainelli wrote:
> On 12/4/2020 1:37 AM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Document binding of block responsible for initializing USB controllers
>> (OHCI, EHCI, XHCI).
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>   .../reset/brcm,bcm4908-usb-reset.yaml         | 60 +++++++++++++++++++
>>   1 file changed, 60 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
>> new file mode 100644
>> index 000000000000..31beb1c8f3cd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
>> @@ -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 <rafal@milecki.pl>
>> +
>> +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";
> 
> 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).

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 :(

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

[Rafał Miłecki]

On 04.12.2020 17:38, Florian Fainelli wrote:
> On 12/4/2020 1:37 AM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> 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.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
> 
> [snip]
> 
>> +
>> +#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
> 
> 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.

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

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

[Rob Herring]

On Fri, Dec 04, 2020 at 05:39:14PM +0100, Rafał Miłecki wrote:
> On 04.12.2020 17:32, Florian Fainelli wrote:
> > On 12/4/2020 1:37 AM, Rafał Miłecki wrote:
> > > From: Rafał Miłecki <rafal@milecki.pl>
> > > 
> > > Document binding of block responsible for initializing USB controllers
> > > (OHCI, EHCI, XHCI).
> > > 
> > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > > ---
> > >   .../reset/brcm,bcm4908-usb-reset.yaml         | 60 +++++++++++++++++++
> > >   1 file changed, 60 insertions(+)
> > >   create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
> > > new file mode 100644
> > > index 000000000000..31beb1c8f3cd
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
> > > @@ -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 <rafal@milecki.pl>
> > > +
> > > +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";
> > 
> > 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).
> 
> 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

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

[Florian Fainelli]

On 12/9/20 3:35 PM, Rob Herring wrote:
> On Fri, Dec 04, 2020 at 05:39:14PM +0100, Rafał Miłecki wrote:
>> On 04.12.2020 17:32, Florian Fainelli wrote:
>>> On 12/4/2020 1:37 AM, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> Document binding of block responsible for initializing USB controllers
>>>> (OHCI, EHCI, XHCI).
>>>>
>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>> ---
>>>>   .../reset/brcm,bcm4908-usb-reset.yaml         | 60 +++++++++++++++++++
>>>>   1 file changed, 60 insertions(+)
>>>>   create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
>>>> new file mode 100644
>>>> index 000000000000..31beb1c8f3cd
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
>>>> @@ -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 <rafal@milecki.pl>
>>>> +
>>>> +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";
>>>
>>> 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).
>>
>> 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.

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.
-- 
Florian