[PATCH V2 0/2] reset: support Broadcom's PMB

[PATCH V2 0/2] reset: support Broadcom's PMB

[Rafał Miłecki]

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

PMB is Broadcom's old reset controller. Later it was replaced by PMC
which is a wrapper around PMB.

It's still important to support older PMB as:
1. It's the only reset option on older devices
2. Sometimes PMC may be unable to handle device reset and PMB should
   be used as fallback
3. Some devices may be configured not to use PMC at all

I'll get back to working on PMC later.

Rafał Miłecki (2):
  dt-bindings: reset: document Broadcom's PMB binding
  reset: brcm-pmb: add driver for Broadcom's PMB

 .../devicetree/bindings/reset/brcm,pmb.yaml   |  54 +++
 drivers/reset/Kconfig                         |   7 +
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-brcm-pmb.c                | 307 ++++++++++++++++++
 include/dt-bindings/reset/brcm,pmb.h          |   9 +
 5 files changed, 378 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/brcm,pmb.yaml
 create mode 100644 drivers/reset/reset-brcm-pmb.c
 create mode 100644 include/dt-bindings/reset/brcm,pmb.h

-- 
2.26.2

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

[Rafał Miłecki]

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

Broadcom's PMB is reset controller used for disabling and enabling SoC
devices.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
V2: Fix example compilation
    Describe #reset-cells
---
 .../devicetree/bindings/reset/brcm,pmb.yaml   | 54 +++++++++++++++++++
 include/dt-bindings/reset/brcm,pmb.h          |  9 ++++
 2 files changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/brcm,pmb.yaml
 create mode 100644 include/dt-bindings/reset/brcm,pmb.h

diff --git a/Documentation/devicetree/bindings/reset/brcm,pmb.yaml b/Documentation/devicetree/bindings/reset/brcm,pmb.yaml
new file mode 100644
index 000000000000..2b5dd2faa4b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/brcm,pmb.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/brcm,pmb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom PMB Master reset controller
+
+description: This document describes Broadcom's PMB controller. It supports
+  resetting various types of connected devices (e.g. PCIe, USB, SATA). It
+  requires specifying device address.
+
+maintainers:
+  - Rafał Miłecki <rafal@milecki.pl>
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm4908-pmb # PMB on BCM4908 and compatible SoCs
+
+  reg:
+    maxItems: 1
+
+  big-endian:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Flag to use for block working in big endian mode.
+
+  "#reset-cells":
+    description: |
+      The first cell defines type of device to reset (see brcm,pmb.h)
+      The second cell specifies device address (range: 0 - 255)
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/reset/brcm,pmb.h>
+
+    pmb: reset-controller@802800e0 {
+        compatible = "brcm,bcm4908-pmb";
+        reg = <0x802800e0 0x20>;
+        #reset-cells = <2>;
+    };
+
+    foo {
+        resets = <&pmb BRCM_PMB_USB 17>;
+    };
diff --git a/include/dt-bindings/reset/brcm,pmb.h b/include/dt-bindings/reset/brcm,pmb.h
new file mode 100644
index 000000000000..1b724e451de1
--- /dev/null
+++ b/include/dt-bindings/reset/brcm,pmb.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later OR MIT */
+
+#ifndef _DT_BINDINGS_RESET_BRCM_PMB
+#define _DT_BINDINGS_RESET_BRCM_PMB
+
+#define BRCM_PMB_USB		0x01
+#define BRCM_PMB_PCIE		0x02
+
+#endif
-- 
2.26.2

[PATCH V2 2/2] reset: brcm-pmb: add driver for Broadcom's PMB

[Rafał Miłecki]

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

PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
It's needed to power on and off SoC blocks like PCIe, SATA, USB.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Update subject to "brcm-pmb"
    Fix validation in xlate
    Use device_get_match_data()
---
 drivers/reset/Kconfig          |   7 +
 drivers/reset/Makefile         |   1 +
 drivers/reset/reset-brcm-pmb.c | 307 +++++++++++++++++++++++++++++++++
 3 files changed, 315 insertions(+)
 create mode 100644 drivers/reset/reset-brcm-pmb.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 84baec01aa30..b7e377a32c92 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -41,6 +41,13 @@ config RESET_BERLIN
 	help
 	  This enables the reset controller driver for Marvell Berlin SoCs.
 
+config RESET_BRCM_PMB
+	tristate "Broadcom PMB reset controller"
+	depends on ARM64 || COMPILE_TEST
+	default ARCH_BCM4908
+	help
+	  This enables the Broadcom PMB reset controller driver.
+
 config RESET_BRCMSTB
 	tristate "Broadcom STB reset controller"
 	depends on ARCH_BRCMSTB || COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 16947610cc3b..0dd5d42050dc 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_RESET_A10SR) += reset-a10sr.o
 obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
 obj-$(CONFIG_RESET_AXS10X) += reset-axs10x.o
 obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
+obj-$(CONFIG_RESET_BRCM_PMB) += reset-brcm-pmb.o
 obj-$(CONFIG_RESET_BRCMSTB) += reset-brcmstb.o
 obj-$(CONFIG_RESET_BRCMSTB_RESCAL) += reset-brcmstb-rescal.o
 obj-$(CONFIG_RESET_HSDK) += reset-hsdk.o
diff --git a/drivers/reset/reset-brcm-pmb.c b/drivers/reset/reset-brcm-pmb.c
new file mode 100644
index 000000000000..780df6260d69
--- /dev/null
+++ b/drivers/reset/reset-brcm-pmb.c
@@ -0,0 +1,307 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2013 Broadcom
+ * Copyright (C) 2020 Rafał Miłecki <rafal@milecki.pl>
+ */
+
+#include <dt-bindings/reset/brcm,pmb.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+#include <linux/reset/bcm63xx_pmb.h>
+
+#define BPCM_ID_REG					0x00
+#define BPCM_CAPABILITIES				0x04
+#define  BPCM_CAP_NUM_ZONES				0x000000ff
+#define  BPCM_CAP_SR_REG_BITS				0x0000ff00
+#define  BPCM_CAP_PLLTYPE				0x00030000
+#define  BPCM_CAP_UBUS					0x00080000
+#define BPCM_CONTROL					0x08
+#define BPCM_STATUS					0x0c
+#define BPCM_ROSC_CONTROL				0x10
+#define BPCM_ROSC_THRESH_H				0x14
+#define BPCM_ROSC_THRESHOLD_BCM6838			0x14
+#define BPCM_ROSC_THRESH_S				0x18
+#define BPCM_ROSC_COUNT_BCM6838				0x18
+#define BPCM_ROSC_COUNT					0x1c
+#define BPCM_PWD_CONTROL_BCM6838			0x1c
+#define BPCM_PWD_CONTROL				0x20
+#define BPCM_SR_CONTROL_BCM6838				0x20
+#define BPCM_PWD_ACCUM_CONTROL				0x24
+#define BPCM_SR_CONTROL					0x28
+#define BPCM_GLOBAL_CONTROL				0x2c
+#define BPCM_MISC_CONTROL				0x30
+#define BPCM_MISC_CONTROL2				0x34
+#define BPCM_SGPHY_CNTL					0x38
+#define BPCM_SGPHY_STATUS				0x3c
+#define BPCM_ZONE0					0x40
+#define  BPCM_ZONE_CONTROL				0x00
+#define   BPCM_ZONE_CONTROL_MANUAL_CLK_EN		0x00000001
+#define   BPCM_ZONE_CONTROL_MANUAL_RESET_CTL		0x00000002
+#define   BPCM_ZONE_CONTROL_FREQ_SCALE_USED		0x00000004	/* R/O */
+#define   BPCM_ZONE_CONTROL_DPG_CAPABLE			0x00000008	/* R/O */
+#define   BPCM_ZONE_CONTROL_MANUAL_MEM_PWR		0x00000030
+#define   BPCM_ZONE_CONTROL_MANUAL_ISO_CTL		0x00000040
+#define   BPCM_ZONE_CONTROL_MANUAL_CTL			0x00000080
+#define   BPCM_ZONE_CONTROL_DPG_CTL_EN			0x00000100
+#define   BPCM_ZONE_CONTROL_PWR_DN_REQ			0x00000200
+#define   BPCM_ZONE_CONTROL_PWR_UP_REQ			0x00000400
+#define   BPCM_ZONE_CONTROL_MEM_PWR_CTL_EN		0x00000800
+#define   BPCM_ZONE_CONTROL_BLK_RESET_ASSERT		0x00001000
+#define   BPCM_ZONE_CONTROL_MEM_STBY			0x00002000
+#define   BPCM_ZONE_CONTROL_RESERVED			0x0007c000
+#define   BPCM_ZONE_CONTROL_PWR_CNTL_STATE		0x00f80000
+#define   BPCM_ZONE_CONTROL_FREQ_SCALAR_DYN_SEL		0x01000000	/* R/O */
+#define   BPCM_ZONE_CONTROL_PWR_OFF_STATE		0x02000000	/* R/O */
+#define   BPCM_ZONE_CONTROL_PWR_ON_STATE		0x04000000	/* R/O */
+#define   BPCM_ZONE_CONTROL_PWR_GOOD			0x08000000	/* R/O */
+#define   BPCM_ZONE_CONTROL_DPG_PWR_STATE		0x10000000	/* R/O */
+#define   BPCM_ZONE_CONTROL_MEM_PWR_STATE		0x20000000	/* R/O */
+#define   BPCM_ZONE_CONTROL_ISO_STATE			0x40000000	/* R/O */
+#define   BPCM_ZONE_CONTROL_RESET_STATE			0x80000000	/* R/O */
+#define  BPCM_ZONE_CONFIG1				0x04
+#define  BPCM_ZONE_CONFIG2				0x08
+#define  BPCM_ZONE_FREQ_SCALAR_CONTROL			0x0c
+#define  BPCM_ZONE_SIZE					0x10
+
+enum brcm_pmb_chipset {
+	BCM4908,
+};
+
+struct brcm_pmb_data {
+	enum brcm_pmb_chipset chipset;
+};
+
+static const struct brcm_pmb_data brcm_pmb_4908_data = {
+	.chipset				= BCM4908,
+};
+
+struct brcm_pmb {
+	const struct brcm_pmb_data *data;
+	void __iomem *base;
+	struct device *dev;
+	struct reset_controller_dev rcdev;
+	spinlock_t lock;
+	bool little_endian;
+};
+
+static int brcm_pmb_bpcm_read(struct brcm_pmb *pmb, u16 device, int offset, u32 *val)
+{
+	unsigned long flags;
+	int err;
+
+	spin_lock_irqsave(&pmb->lock, flags);
+	err = bpcm_rd(pmb->base, device, offset, val);
+	spin_unlock_irqrestore(&pmb->lock, flags);
+
+	if (!err)
+		*val = pmb->little_endian ? le32_to_cpu(*val) : be32_to_cpu(*val);
+
+	return err;
+}
+
+static int brcm_pmb_bpcm_write(struct brcm_pmb *pmb, u16 device, int offset, u32 val)
+{
+	unsigned long flags;
+	int err;
+
+	val = pmb->little_endian ? cpu_to_le32(val) : cpu_to_be32(val);
+
+	spin_lock_irqsave(&pmb->lock, flags);
+	err = bpcm_wr(pmb->base, device, offset, val);
+	spin_unlock_irqrestore(&pmb->lock, flags);
+
+	return err;
+}
+
+static int brcm_pmb_power_off_zone(struct brcm_pmb *pmb, u16 device, int zone)
+{
+	int offset;
+	u32 val;
+	int err;
+
+	offset = BPCM_ZONE0 + zone * BPCM_ZONE_SIZE + BPCM_ZONE_CONTROL;
+
+	err = brcm_pmb_bpcm_read(pmb, device, offset, &val);
+	if (err)
+		return err;
+
+	val |= BPCM_ZONE_CONTROL_PWR_DN_REQ;
+	val &= ~BPCM_ZONE_CONTROL_PWR_UP_REQ;
+
+	err = brcm_pmb_bpcm_write(pmb, device, offset, val);
+
+	return err;
+}
+
+static int brcm_pmb_power_on_zone(struct brcm_pmb *pmb, u16 device, int zone)
+{
+	int offset;
+	u32 val;
+	int err;
+
+	offset = BPCM_ZONE0 + zone * BPCM_ZONE_SIZE + BPCM_ZONE_CONTROL;
+
+	err = brcm_pmb_bpcm_read(pmb, device, offset, &val);
+	if (err)
+		return err;
+
+	if (!(val & BPCM_ZONE_CONTROL_PWR_ON_STATE)) {
+		val &= ~BPCM_ZONE_CONTROL_PWR_DN_REQ;
+		val |= BPCM_ZONE_CONTROL_DPG_CTL_EN;
+		val |= BPCM_ZONE_CONTROL_PWR_UP_REQ;
+		val |= BPCM_ZONE_CONTROL_MEM_PWR_CTL_EN;
+		val |= BPCM_ZONE_CONTROL_BLK_RESET_ASSERT;
+
+		err = brcm_pmb_bpcm_write(pmb, device, offset, val);
+	}
+
+	return err;
+}
+
+static int brcm_pmb_power_off_device(struct brcm_pmb *pmb, u16 device)
+{
+	int offset;
+	u32 val;
+	int err;
+
+	/* Entire device can be powered off by powering off the 0th zone */
+	offset = BPCM_ZONE0 + BPCM_ZONE_CONTROL;
+
+	err = brcm_pmb_bpcm_read(pmb, device, offset, &val);
+	if (err)
+		return err;
+
+	if (!(val & BPCM_ZONE_CONTROL_PWR_OFF_STATE)) {
+		val = BPCM_ZONE_CONTROL_PWR_DN_REQ;
+
+		err = brcm_pmb_bpcm_write(pmb, device, offset, val);
+	}
+
+	return err;
+}
+
+static int brcm_pmb_power_on_device(struct brcm_pmb *pmb, u16 device)
+{
+	u32 val;
+	int err;
+	int i;
+
+	err = brcm_pmb_bpcm_read(pmb, device, BPCM_CAPABILITIES, &val);
+	if (err)
+		return err;
+
+	for (i = 0; i < (val & BPCM_CAP_NUM_ZONES); i++) {
+		err = brcm_pmb_power_on_zone(pmb, device, i);
+		if (err)
+			return err;
+	}
+
+	return err;
+}
+
+static int brcm_pmb_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct brcm_pmb *pmb = container_of(rcdev, struct brcm_pmb, rcdev);
+	int device = id & 0xff;
+
+	switch (id >> 8) {
+	case BRCM_PMB_USB:
+		return brcm_pmb_power_off_device(pmb, device);
+	case BRCM_PMB_PCIE:
+		return brcm_pmb_power_off_zone(pmb, device, 0);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int brcm_pmb_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct brcm_pmb *pmb = container_of(rcdev, struct brcm_pmb, rcdev);
+	int device = id & 0xff;
+
+	switch (id >> 8) {
+	case BRCM_PMB_USB:
+		return brcm_pmb_power_on_device(pmb, device);
+	case BRCM_PMB_PCIE:
+		return brcm_pmb_power_on_zone(pmb, device, 0);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int brcm_pmb_reset_xlate(struct reset_controller_dev *rcdev,
+				const struct of_phandle_args *reset_spec)
+{
+	u32 type = reset_spec->args[0];
+	u32 device = reset_spec->args[1];
+
+	if (type > 0xff || device > 0xff)
+		return -EINVAL;
+
+	return (type << 8) | device;
+}
+
+static const struct reset_control_ops brcm_pmb_reset_control_ops = {
+	.assert = brcm_pmb_assert,
+	.deassert = brcm_pmb_deassert,
+};
+
+static const struct of_device_id brcm_pmb_reset_of_match[] = {
+	{ .compatible = "brcm,bcm4908-pmb", .data = &brcm_pmb_4908_data, },
+	{ },
+};
+
+static int brcm_pmb_reset_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct brcm_pmb *pmb;
+	struct resource *res;
+
+	pmb = devm_kzalloc(dev, sizeof(*pmb), GFP_KERNEL);
+	if (!pmb)
+		return -ENOMEM;
+
+	pmb->data = device_get_match_data(dev);
+	if (!pmb->data) {
+		dev_err(dev, "Failed to find chipset data\n");
+		return -EINVAL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pmb->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(pmb->base))
+		return PTR_ERR(pmb->base);
+
+	pmb->dev = dev;
+
+	pmb->rcdev.ops = &brcm_pmb_reset_control_ops;
+	pmb->rcdev.owner = THIS_MODULE;
+	pmb->rcdev.of_node = dev->of_node;
+	pmb->rcdev.of_reset_n_cells = 2;
+	pmb->rcdev.of_xlate = brcm_pmb_reset_xlate;
+
+	spin_lock_init(&pmb->lock);
+
+	pmb->little_endian = !of_device_is_big_endian(dev->of_node);
+
+	return devm_reset_controller_register(dev, &pmb->rcdev);
+}
+
+static struct platform_driver brcm_pmb_reset_driver = {
+	.probe = brcm_pmb_reset_probe,
+	.driver = {
+		.name	= "brcm-pmb",
+		.of_match_table	= brcm_pmb_reset_of_match,
+	}
+};
+module_platform_driver(brcm_pmb_reset_driver);
+
+MODULE_AUTHOR("Rafał Miłecki");
+MODULE_LICENSE("GPL v2");
+MODULE_DEVICE_TABLE(of, brcm_pmb_reset_of_match);
-- 
2.26.2

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

[Rob Herring]

On Thu, 19 Nov 2020 13:55:59 +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Broadcom's PMB is reset controller used for disabling and enabling SoC
> devices.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> V2: Fix example compilation
>     Describe #reset-cells
> ---
>  .../devicetree/bindings/reset/brcm,pmb.yaml   | 54 +++++++++++++++++++
>  include/dt-bindings/reset/brcm,pmb.h          |  9 ++++
>  2 files changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/brcm,pmb.yaml
>  create mode 100644 include/dt-bindings/reset/brcm,pmb.h
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH V2 2/2] reset: brcm-pmb: add driver for Broadcom's PMB

[Philipp Zabel]

Hi Rafał,

On Thu, 2020-11-19 at 13:56 +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
> It's needed to power on and off SoC blocks like PCIe, SATA, USB.

This sentence, the register names, and
the brcm_pmb_power_on/off_device/zone functions below make me worry that
this should be a power domain controller (pm_genpd in drivers/soc)
instead.
Does PM in PMB stand for power management by chance?

If this actually cuts power to the USB and PCIe cores instead of just
controlling reset lines, it would be better to implement this
differently.

> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Update subject to "brcm-pmb"
>     Fix validation in xlate
>     Use device_get_match_data()
> ---
>  drivers/reset/Kconfig          |   7 +
>  drivers/reset/Makefile         |   1 +
>  drivers/reset/reset-brcm-pmb.c | 307 +++++++++++++++++++++++++++++++++
>  3 files changed, 315 insertions(+)
>  create mode 100644 drivers/reset/reset-brcm-pmb.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 84baec01aa30..b7e377a32c92 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -41,6 +41,13 @@ config RESET_BERLIN
>  	help
>  	  This enables the reset controller driver for Marvell Berlin SoCs.
>  
> +config RESET_BRCM_PMB
> +	tristate "Broadcom PMB reset controller"
> +	depends on ARM64 || COMPILE_TEST
> +	default ARCH_BCM4908
> +	help
> +	  This enables the Broadcom PMB reset controller driver.
> +
>  config RESET_BRCMSTB
>  	tristate "Broadcom STB reset controller"
>  	depends on ARCH_BRCMSTB || COMPILE_TEST
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 16947610cc3b..0dd5d42050dc 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_RESET_A10SR) += reset-a10sr.o
>  obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
>  obj-$(CONFIG_RESET_AXS10X) += reset-axs10x.o
>  obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
> +obj-$(CONFIG_RESET_BRCM_PMB) += reset-brcm-pmb.o
>  obj-$(CONFIG_RESET_BRCMSTB) += reset-brcmstb.o
>  obj-$(CONFIG_RESET_BRCMSTB_RESCAL) += reset-brcmstb-rescal.o
>  obj-$(CONFIG_RESET_HSDK) += reset-hsdk.o
> diff --git a/drivers/reset/reset-brcm-pmb.c b/drivers/reset/reset-brcm-pmb.c
> new file mode 100644
> index 000000000000..780df6260d69
> --- /dev/null
> +++ b/drivers/reset/reset-brcm-pmb.c
> @@ -0,0 +1,307 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2013 Broadcom
> + * Copyright (C) 2020 Rafał Miłecki <rafal@milecki.pl>
> + */
> +
> +#include <dt-bindings/reset/brcm,pmb.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>

Unused header.

> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>

Unused header.

> +#include <linux/reset-controller.h>
> +#include <linux/reset/bcm63xx_pmb.h>
> +
> +#define BPCM_ID_REG					0x00
> +#define BPCM_CAPABILITIES				0x04
> +#define  BPCM_CAP_NUM_ZONES				0x000000ff
> +#define  BPCM_CAP_SR_REG_BITS				0x0000ff00
> +#define  BPCM_CAP_PLLTYPE				0x00030000
> +#define  BPCM_CAP_UBUS					0x00080000
> +#define BPCM_CONTROL					0x08
> +#define BPCM_STATUS					0x0c
> +#define BPCM_ROSC_CONTROL				0x10
> +#define BPCM_ROSC_THRESH_H				0x14
> +#define BPCM_ROSC_THRESHOLD_BCM6838			0x14
> +#define BPCM_ROSC_THRESH_S				0x18
> +#define BPCM_ROSC_COUNT_BCM6838				0x18
> +#define BPCM_ROSC_COUNT					0x1c
> +#define BPCM_PWD_CONTROL_BCM6838			0x1c
> +#define BPCM_PWD_CONTROL				0x20
> +#define BPCM_SR_CONTROL_BCM6838				0x20
> +#define BPCM_PWD_ACCUM_CONTROL				0x24
> +#define BPCM_SR_CONTROL					0x28
> +#define BPCM_GLOBAL_CONTROL				0x2c
> +#define BPCM_MISC_CONTROL				0x30
> +#define BPCM_MISC_CONTROL2				0x34
> +#define BPCM_SGPHY_CNTL					0x38
> +#define BPCM_SGPHY_STATUS				0x3c
> +#define BPCM_ZONE0					0x40
> +#define  BPCM_ZONE_CONTROL				0x00
> +#define   BPCM_ZONE_CONTROL_MANUAL_CLK_EN		0x00000001
> +#define   BPCM_ZONE_CONTROL_MANUAL_RESET_CTL		0x00000002
> +#define   BPCM_ZONE_CONTROL_FREQ_SCALE_USED		0x00000004	/* R/O */
> +#define   BPCM_ZONE_CONTROL_DPG_CAPABLE			0x00000008	/* R/O */
> +#define   BPCM_ZONE_CONTROL_MANUAL_MEM_PWR		0x00000030
> +#define   BPCM_ZONE_CONTROL_MANUAL_ISO_CTL		0x00000040
> +#define   BPCM_ZONE_CONTROL_MANUAL_CTL			0x00000080
> +#define   BPCM_ZONE_CONTROL_DPG_CTL_EN			0x00000100
> +#define   BPCM_ZONE_CONTROL_PWR_DN_REQ			0x00000200
> +#define   BPCM_ZONE_CONTROL_PWR_UP_REQ			0x00000400
> +#define   BPCM_ZONE_CONTROL_MEM_PWR_CTL_EN		0x00000800
> +#define   BPCM_ZONE_CONTROL_BLK_RESET_ASSERT		0x00001000
> +#define   BPCM_ZONE_CONTROL_MEM_STBY			0x00002000
> +#define   BPCM_ZONE_CONTROL_RESERVED			0x0007c000
> +#define   BPCM_ZONE_CONTROL_PWR_CNTL_STATE		0x00f80000
> +#define   BPCM_ZONE_CONTROL_FREQ_SCALAR_DYN_SEL		0x01000000	/* R/O */
> +#define   BPCM_ZONE_CONTROL_PWR_OFF_STATE		0x02000000	/* R/O */
> +#define   BPCM_ZONE_CONTROL_PWR_ON_STATE		0x04000000	/* R/O */
> +#define   BPCM_ZONE_CONTROL_PWR_GOOD			0x08000000	/* R/O */
> +#define   BPCM_ZONE_CONTROL_DPG_PWR_STATE		0x10000000	/* R/O */
> +#define   BPCM_ZONE_CONTROL_MEM_PWR_STATE		0x20000000	/* R/O */
> +#define   BPCM_ZONE_CONTROL_ISO_STATE			0x40000000	/* R/O */
> +#define   BPCM_ZONE_CONTROL_RESET_STATE			0x80000000	/* R/O */
> +#define  BPCM_ZONE_CONFIG1				0x04
> +#define  BPCM_ZONE_CONFIG2				0x08
> +#define  BPCM_ZONE_FREQ_SCALAR_CONTROL			0x0c
> +#define  BPCM_ZONE_SIZE					0x10
> +
> +enum brcm_pmb_chipset {
> +	BCM4908,
> +};
> +
> +struct brcm_pmb_data {
> +	enum brcm_pmb_chipset chipset;
> +};
> +
> +static const struct brcm_pmb_data brcm_pmb_4908_data = {
> +	.chipset				= BCM4908,
> +};

These three are not used at all.

> +struct brcm_pmb {
> +	const struct brcm_pmb_data *data;
> +	void __iomem *base;
> +	struct device *dev;
> +	struct reset_controller_dev rcdev;
> +	spinlock_t lock;
> +	bool little_endian;
> +};
> +
> +static int brcm_pmb_bpcm_read(struct brcm_pmb *pmb, u16 device, int offset, u32 *val)
> +{
> +	unsigned long flags;
> +	int err;
> +
> +	spin_lock_irqsave(&pmb->lock, flags);
> +	err = bpcm_rd(pmb->base, device, offset, val);
> +	spin_unlock_irqrestore(&pmb->lock, flags);
> +
> +	if (!err)
> +		*val = pmb->little_endian ? le32_to_cpu(*val) : be32_to_cpu(*val);
> +
> +	return err;
> +}
> +
> +static int brcm_pmb_bpcm_write(struct brcm_pmb *pmb, u16 device, int offset, u32 val)
> +{
> +	unsigned long flags;
> +	int err;
> +
> +	val = pmb->little_endian ? cpu_to_le32(val) : cpu_to_be32(val);
> +
> +	spin_lock_irqsave(&pmb->lock, flags);
> +	err = bpcm_wr(pmb->base, device, offset, val);
> +	spin_unlock_irqrestore(&pmb->lock, flags);
> +
> +	return err;
> +}
> +
> +static int brcm_pmb_power_off_zone(struct brcm_pmb *pmb, u16 device, int zone)
> +{
> +	int offset;
> +	u32 val;
> +	int err;
> +
> +	offset = BPCM_ZONE0 + zone * BPCM_ZONE_SIZE + BPCM_ZONE_CONTROL;
> +
> +	err = brcm_pmb_bpcm_read(pmb, device, offset, &val);
> +	if (err)
> +		return err;
> +
> +	val |= BPCM_ZONE_CONTROL_PWR_DN_REQ;
> +	val &= ~BPCM_ZONE_CONTROL_PWR_UP_REQ;
> +
> +	err = brcm_pmb_bpcm_write(pmb, device, offset, val);
> +
> +	return err;
> +}
> +
> +static int brcm_pmb_power_on_zone(struct brcm_pmb *pmb, u16 device, int zone)
> +{
> +	int offset;
> +	u32 val;
> +	int err;
> +
> +	offset = BPCM_ZONE0 + zone * BPCM_ZONE_SIZE + BPCM_ZONE_CONTROL;
> +
> +	err = brcm_pmb_bpcm_read(pmb, device, offset, &val);
> +	if (err)
> +		return err;
> +
> +	if (!(val & BPCM_ZONE_CONTROL_PWR_ON_STATE)) {
> +		val &= ~BPCM_ZONE_CONTROL_PWR_DN_REQ;
> +		val |= BPCM_ZONE_CONTROL_DPG_CTL_EN;
> +		val |= BPCM_ZONE_CONTROL_PWR_UP_REQ;
> +		val |= BPCM_ZONE_CONTROL_MEM_PWR_CTL_EN;
> +		val |= BPCM_ZONE_CONTROL_BLK_RESET_ASSERT;
> +
> +		err = brcm_pmb_bpcm_write(pmb, device, offset, val);
> +	}
> +
> +	return err;
> +}
> +
> +static int brcm_pmb_power_off_device(struct brcm_pmb *pmb, u16 device)
> +{
> +	int offset;
> +	u32 val;
> +	int err;
> +
> +	/* Entire device can be powered off by powering off the 0th zone */
> +	offset = BPCM_ZONE0 + BPCM_ZONE_CONTROL;
> +
> +	err = brcm_pmb_bpcm_read(pmb, device, offset, &val);
> +	if (err)
> +		return err;
> +
> +	if (!(val & BPCM_ZONE_CONTROL_PWR_OFF_STATE)) {
> +		val = BPCM_ZONE_CONTROL_PWR_DN_REQ;
> +
> +		err = brcm_pmb_bpcm_write(pmb, device, offset, val);
> +	}
> +
> +	return err;
> +}
> +
> +static int brcm_pmb_power_on_device(struct brcm_pmb *pmb, u16 device)
> +{
> +	u32 val;
> +	int err;
> +	int i;
> +
> +	err = brcm_pmb_bpcm_read(pmb, device, BPCM_CAPABILITIES, &val);
> +	if (err)
> +		return err;
> +
> +	for (i = 0; i < (val & BPCM_CAP_NUM_ZONES); i++) {
> +		err = brcm_pmb_power_on_zone(pmb, device, i);
> +		if (err)
> +			return err;
> +	}
> +
> +	return err;
> +}
> +
> +static int brcm_pmb_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	struct brcm_pmb *pmb = container_of(rcdev, struct brcm_pmb, rcdev);
> +	int device = id & 0xff;
> +
> +	switch (id >> 8) {
> +	case BRCM_PMB_USB:
> +		return brcm_pmb_power_off_device(pmb, device);
> +	case BRCM_PMB_PCIE:
> +		return brcm_pmb_power_off_zone(pmb, device, 0);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int brcm_pmb_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	struct brcm_pmb *pmb = container_of(rcdev, struct brcm_pmb, rcdev);
> +	int device = id & 0xff;
> +
> +	switch (id >> 8) {
> +	case BRCM_PMB_USB:
> +		return brcm_pmb_power_on_device(pmb, device);
> +	case BRCM_PMB_PCIE:
> +		return brcm_pmb_power_on_zone(pmb, device, 0);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int brcm_pmb_reset_xlate(struct reset_controller_dev *rcdev,
> +				const struct of_phandle_args *reset_spec)
> +{
> +	u32 type = reset_spec->args[0];
> +	u32 device = reset_spec->args[1];
> +
> +	if (type > 0xff || device > 0xff)
> +		return -EINVAL;
> +
> +	return (type << 8) | device;
> +}
> +
> +static const struct reset_control_ops brcm_pmb_reset_control_ops = {
> +	.assert = brcm_pmb_assert,
> +	.deassert = brcm_pmb_deassert,
> +};
> +
> +static const struct of_device_id brcm_pmb_reset_of_match[] = {
> +	{ .compatible = "brcm,bcm4908-pmb", .data = &brcm_pmb_4908_data, },
> +	{ },
> +};
> +
> +static int brcm_pmb_reset_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct brcm_pmb *pmb;
> +	struct resource *res;
> +
> +	pmb = devm_kzalloc(dev, sizeof(*pmb), GFP_KERNEL);
> +	if (!pmb)
> +		return -ENOMEM;
> +
> +	pmb->data = device_get_match_data(dev);
> +	if (!pmb->data) {
> +		dev_err(dev, "Failed to find chipset data\n");
> +		return -EINVAL;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	pmb->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(pmb->base))
> +		return PTR_ERR(pmb->base);
> +
> +	pmb->dev = dev;
> +
> +	pmb->rcdev.ops = &brcm_pmb_reset_control_ops;
> +	pmb->rcdev.owner = THIS_MODULE;
> +	pmb->rcdev.of_node = dev->of_node;
> +	pmb->rcdev.of_reset_n_cells = 2;
> +	pmb->rcdev.of_xlate = brcm_pmb_reset_xlate;
> +
> +	spin_lock_init(&pmb->lock);
> +
> +	pmb->little_endian = !of_device_is_big_endian(dev->of_node);
> +
> +	return devm_reset_controller_register(dev, &pmb->rcdev);
> +}
> +
> +static struct platform_driver brcm_pmb_reset_driver = {
> +	.probe = brcm_pmb_reset_probe,
> +	.driver = {
> +		.name	= "brcm-pmb",
> +		.of_match_table	= brcm_pmb_reset_of_match,
> +	}
> +};
> +module_platform_driver(brcm_pmb_reset_driver);
> +
> +MODULE_AUTHOR("Rafał Miłecki");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DEVICE_TABLE(of, brcm_pmb_reset_of_match);

regards
Philipp

Re: [PATCH V2 2/2] reset: brcm-pmb: add driver for Broadcom's PMB

[Florian Fainelli]

On 12/8/20 3:02 AM, Philipp Zabel wrote:
> Hi Rafał,
> 
> On Thu, 2020-11-19 at 13:56 +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
>> It's needed to power on and off SoC blocks like PCIe, SATA, USB.
> 
> This sentence, the register names, and
> the brcm_pmb_power_on/off_device/zone functions below make me worry that
> this should be a power domain controller (pm_genpd in drivers/soc)
> instead.
> Does PM in PMB stand for power management by chance?

It does, PMB stands for Power Management Bus.

> 
> If this actually cuts power to the USB and PCIe cores instead of just
> controlling reset lines, it would be better to implement this
> differently.

What is tricky is this is a combined reset/clock/power zones controller.
You rarely turn off/assert one of those without also turning
off/asserting the others.
-- 
Florian

Re: [PATCH V2 2/2] reset: brcm-pmb: add driver for Broadcom's PMB

[Philipp Zabel]

Hi Florian,

On Tue, 2020-12-08 at 12:20 -0800, Florian Fainelli wrote:
> On 12/8/20 3:02 AM, Philipp Zabel wrote:
> > Hi Rafał,
> > 
> > On Thu, 2020-11-19 at 13:56 +0100, Rafał Miłecki wrote:
> > > From: Rafał Miłecki <rafal@milecki.pl>
> > > 
> > > PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
> > > It's needed to power on and off SoC blocks like PCIe, SATA, USB.
> > 
> > This sentence, the register names, and
> > the brcm_pmb_power_on/off_device/zone functions below make me worry that
> > this should be a power domain controller (pm_genpd in drivers/soc)
> > instead.
> > Does PM in PMB stand for power management by chance?
> 
> It does, PMB stands for Power Management Bus.
> 
> > If this actually cuts power to the USB and PCIe cores instead of just
> > controlling reset lines, it would be better to implement this
> > differently.
> 
> What is tricky is this is a combined reset/clock/power zones controller.
> You rarely turn off/assert one of those without also turning
> off/asserting the others.

It is common to have power management controllers also control clocks
and resets.
If the PMB controlled clocks and resets are only required for the power
on/off sequences, you should be able to move the current .(de)assert
implementations into a generic_pm_domain's .power_on and .power_off
methods.

regards
Philipp

Re: [PATCH V2 2/2] reset: brcm-pmb: add driver for Broadcom's PMB

[Rafał Miłecki]

On 09.12.2020 12:24, Philipp Zabel wrote:
> On Tue, 2020-12-08 at 12:20 -0800, Florian Fainelli wrote:
>> On 12/8/20 3:02 AM, Philipp Zabel wrote:
>>> Hi Rafał,
>>>
>>> On Thu, 2020-11-19 at 13:56 +0100, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> PMB can be found on BCM4908 and many other chipsets (e.g. BCM63138).
>>>> It's needed to power on and off SoC blocks like PCIe, SATA, USB.
>>>
>>> This sentence, the register names, and
>>> the brcm_pmb_power_on/off_device/zone functions below make me worry that
>>> this should be a power domain controller (pm_genpd in drivers/soc)
>>> instead.
>>> Does PM in PMB stand for power management by chance?
>>
>> It does, PMB stands for Power Management Bus.
>>
>>> If this actually cuts power to the USB and PCIe cores instead of just
>>> controlling reset lines, it would be better to implement this
>>> differently.
>>
>> What is tricky is this is a combined reset/clock/power zones controller.
>> You rarely turn off/assert one of those without also turning
>> off/asserting the others.
> 
> It is common to have power management controllers also control clocks
> and resets.
> If the PMB controlled clocks and resets are only required for the power
> on/off sequences, you should be able to move the current .(de)assert
> implementations into a generic_pm_domain's .power_on and .power_off
> methods.

Thanks for your comments Philipp! I just took a look at generic PM domains.
While I accept you know better that this hw support should be implemented
using generic PM domains, I'm not sure how to correctly proceed with that.

***

One great thing about reset controllers is that they have:
1. xlate function retuning int
2. (de)assert functions taking unsigned long id argument

That fits PMB hardware design very nicely as I can specify in DT:
1. Type of device to reset
2. ID of device to reset

Basically: I can use:
#reset-cells = <2>;
and then sth like:
resets = <&pmb BRCM_PMB_USB 17>;

***

In the struct generic_pm_domain we have "power_off" and "power_on" that take
no extra arguments however. Also its xlate has to return a single "struct
generic_pm_domain".

Please note that PMB can support for up 256 devices.

If I try to use generic PM domains to handle PMB hw in the same way I'll hit
two problems:

1. I'll have to register 256 "struct generic_pm_domain"s.
2. I won't have a clean way of passing device type (xlate limitation).

I can try to workaround the later just like in case of "ti,sci-pm-domain"
which uses #power-domain-cells = <2>. It seems hacky though as its "xlate"
handler stores args[1] in internal struct and hopes no another xlate call
will get executed before "power_on" or "power_off" call.

***

So I guess my real option is to store info about SoC devices in the PM
driver and create one "struct generic_pm_domain" for each known device only.

Then have something like:
#define BRCM_PMB_HOST_USB		0x01
#define BRCM_PMB_HOST_USB2		0x02
#define BRCM_PMB_HOST_USB3		0x03
#define BRCM_PMB_PCIE0			0x04
#define BRCM_PMB_PCIE1			0x05
#define BRCM_PMB_PCIE2			0x06

And finally:
power-domains = <&pmb0 BRCM_PMB_HOST_USB>;
power-domains = <&pmb0 BRCM_PMB_PCIE0>;
power-domains = <&pmb0 BRCM_PMB_PCIE1>;

What do you think about it? Is it the way to go?