[PATCH RFC 0/5] BCMBCA PMB reset controller

[PATCH RFC 0/5] BCMBCA PMB reset controller

[Linus Walleij]

This is another of these big upfront design binding and driver
hoards that is the consequence of adding new SoC support. My stake
is to be able to boot and test a full featured Cortex-A5 system and
the BCM6846 XG6846B is one such system on my desk.

It's been cooking for a while in various prototype forms.

IMPLEMENTATION:

Add a shared Processor Monitor Bus (PMB) driver and a child reset
provider for Broadcom BCMBCA blocks. Support BCM6846 XRDP and its Runner
cores, PCIe0/1 and PVTMON. The PMB bus driver uses the third generation
Platform Management Controller (PMC3) keyhole interface. It owns the
register bank and serializes complete sequences for child services.

The fixed SoC transport is built-in and has no sysfs bind/unbind
controls, allowing delayed recovery to retain hardware ownership
until a busy transaction completes. The reset child can be modular.

The series contains three bindings, the PMB bus driver, and the PMB reset
driver. Describe PMC 3.1 as a simple-mfd with two system-controller
children and a bus child, each owning its register window.

The bus child exposes the PMB transport and contains the reset provider.
The parent uses Broadcom's "brcm,bca-pmc-3-1" compatible. The PMB and
reset drivers support only the BCM6846-specific compatibles for now.
Other BCMBCA SoCs have different reset groups or PMB interfaces.

Use one reset specifier cell and define BCM6846_RESET_XRDP as ID 0 for
XRDP and its three Runner cores. The vendor sources describe individual
PMB device addresses and reset masks, but no ID for this combined group.
SoC tables map IDs to blocks, register values, timing and sequence
callbacks. PCIe pulses preserve lane-selection straps. PVTMON uses its
analog-acquisition reset. Consumers must quiesce every affected user;
these resets do not replace clock, power-domain, PHY or UBUS setup.
Power-cycle operations for USB2, WAN, PERIPH, MEMC, BIU and BIU PLL are
left to power-domain and platform power-management support.

This series is based on v7.3-rc1 and is a prerequisite for the
forthcoming Genexis XG6846B Ethernet support series, which works.

The BCM6846 XRDP reset sequence enabled full-MTU traffic on all four LAN
ports and copper WAN and has been tested on that series.

MISSING:

I did not add MAINTAINERS entries because I don't know how the Broadcom
maintainers feel about it. Tell me.

MERGE STRATEGY:

I'm not sure. Possibly through SoC with ACKs. Through an MFD immutable
branch if Lee wants to handle it.

Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
Linus Walleij (5):
      dt-bindings: mfd: Describe the BCM6846 PMC complex
      dt-bindings: soc: Describe the BCM6846 Processor Monitor Bus
      dt-bindings: reset: Describe the BCMBCA PMB reset controller
      bus: Add a shared BCMBCA PMC3 PMB transport
      reset: Add BCMBCA block resets through the PMB

 .../devicetree/bindings/mfd/brcm,bcm6846-pmc.yaml  |  98 +++++++
 .../bindings/reset/brcm,bcm6846-pmb-reset.yaml     |  46 ++++
 .../bindings/soc/bcm/brcm,bcm6846-pmb.yaml         |  51 ++++
 drivers/bus/Kconfig                                |  13 +
 drivers/bus/Makefile                               |   1 +
 drivers/bus/bcmbca_pmc3_pmb.c                      | 232 +++++++++++++++++
 drivers/reset/Kconfig                              |  15 ++
 drivers/reset/Makefile                             |   1 +
 drivers/reset/reset-bcmbca-pmb.c                   | 281 +++++++++++++++++++++
 include/dt-bindings/reset/brcm,bcm6846-pmb.h       |  15 ++
 include/linux/soc/bcm/bcmbca-pmb.h                 |  25 ++
 11 files changed, 778 insertions(+)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20261013-xg6846b-bcmbca-xrdp-reset-de71b80c25a4

Best regards,
--  
Linus Walleij <linusw@kernel.org>

[PATCH RFC 1/5] dt-bindings: mfd: Describe the BCM6846 PMC complex

[Linus Walleij]

The third generation Platform Management Controller (PMC3) has
different subdevices.

Identify the parent with Broadcom's "brcm,bca-pmc-3-1" compatible
for the version 3.1 complex.

Describe the complex as a simple-mfd with two system-controller children
and a bus child, each owning its register resource.

Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 .../devicetree/bindings/mfd/brcm,bcm6846-pmc.yaml  | 98 ++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm6846-pmc.yaml b/Documentation/devicetree/bindings/mfd/brcm,bcm6846-pmc.yaml
new file mode 100644
index 000000000000..7ddf84b561aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/brcm,bcm6846-pmc.yaml
@@ -0,0 +1,98 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/brcm,bcm6846-pmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM6846 Platform Management Controller
+
+maintainers:
+  - Linus Walleij <linusw@kernel.org>
+
+description: |
+  The third generation Platform Management Controller (PMC3) contains
+  separate register banks for PMC control and message queues, the
+  Processor Monitor Bus (PMB) interface within Procmon, and the Maestro
+  processor control registers. Each child owns its register bank. The
+  parent provides address translation and does not own a register bank.
+
+properties:
+  compatible:
+    items:
+      - const: brcm,bca-pmc-3-1
+      - const: simple-mfd
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+  ranges:
+    maxItems: 1
+
+patternProperties:
+  '^system-controller@[0-9a-f]+$':
+    type: object
+    additionalProperties: false
+    description:
+      PMC control and message queue registers, or Maestro processor
+      control, status and interrupt registers
+
+    properties:
+      compatible:
+        enum:
+          - brcm,bcm6846-pmc-control
+          - brcm,bcm6846-pmc-maestro
+
+      reg:
+        maxItems: 1
+
+    required:
+      - compatible
+      - reg
+
+  '^bus@[0-9a-f]+$':
+    $ref: /schemas/soc/bcm/brcm,bcm6846-pmb.yaml#
+    description: PMB configuration, keyhole and address mapping registers
+
+required:
+  - compatible
+  - '#address-cells'
+  - '#size-cells'
+  - ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    system-controller@ffb00000 {
+        /* Broadcom BCA Platform Management Complex version 3.1 */
+        compatible = "brcm,bca-pmc-3-1", "simple-mfd";
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges = <0x0 0xffb00000 0x21000>;
+
+        /* Called PMC Platform Management Controller in vendor tree */
+        system-controller@1018 {
+            compatible = "brcm,bcm6846-pmc-control";
+            reg = <0x1018 0x5080>;
+        };
+
+        /* Called PMB Processor Monitor Bus in vendor tree */
+        bus@20100 {
+            compatible = "brcm,bcm6846-pmb";
+            reg = <0x20100 0x200>;
+
+            reset-controller {
+                compatible = "brcm,bcm6846-pmb-reset";
+                #reset-cells = <1>;
+            };
+        };
+
+        /* Called Maestro in vendor tree */
+        system-controller@20400 {
+            compatible = "brcm,bcm6846-pmc-maestro";
+            reg = <0x20400 0x5d0>;
+        };
+    };

-- 
2.56.0

[PATCH RFC 2/5] dt-bindings: soc: Describe the BCM6846 Processor Monitor Bus

[Linus Walleij]

The third generation Platform Management Controller (PMC3) accesses
SoC block control registers through the Processor Monitor Bus (PMB).
Describe the BCM6846 PMB controller and its shared register bank.

Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 .../bindings/soc/bcm/brcm,bcm6846-pmb.yaml         | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm6846-pmb.yaml b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm6846-pmb.yaml
new file mode 100644
index 000000000000..85850dc16418
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm6846-pmb.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/bcm/brcm,bcm6846-pmb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM6846 Processor Monitor Bus
+
+maintainers:
+  - Linus Walleij <linusw@kernel.org>
+
+description: |
+  The Processor Monitor Bus (PMB) connects the third generation Platform
+  Management Controller (PMC3) to the control registers of SoC blocks.
+  The controller provides configuration, arbitration and four keyhole
+  interfaces for indirect register access. Its children share this
+  transport, including serialization of complete register sequences.
+
+  The PMB interface is part of Procmon within the PMC complex and is
+  described by its bus child node. The register bank starts at the
+  PMB configuration register, not at the beginning of Procmon.
+
+properties:
+  compatible:
+    const: brcm,bcm6846-pmb
+
+  reg:
+    maxItems: 1
+    description: PMB configuration, keyhole and address mapping registers
+
+  reset-controller:
+    $ref: /schemas/reset/brcm,bcm6846-pmb-reset.yaml#
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    /* Called PMB Processor Monitor Bus in vendor tree */
+    bus@ffb20100 {
+        compatible = "brcm,bcm6846-pmb";
+        reg = <0xffb20100 0x200>;
+
+        reset-controller {
+            compatible = "brcm,bcm6846-pmb-reset";
+            #reset-cells = <1>;
+        };
+    };

-- 
2.56.0

[PATCH RFC 3/5] dt-bindings: reset: Describe the BCMBCA PMB reset controller

[Linus Walleij]

Describe a reset provider below the Processor Monitor Bus (PMB)
controller, which owns the register bank and serializes accesses.
Use one specifier cell for a SoC-specific logical reset group ID.

Define BCM6846_RESET_XRDP for the coordinated reset of the XRDP packet
engine and its three Runner cores. The vendor PMB device addresses
identify individual blocks, not this group, so assign it reset ID 0.

Define separate IDs for PCIe0 and PCIe1 soft resets and PVTMON analog
acquisition. Describe the scope and prerequisites of each operation.

Use only the SoC-specific compatible because the reset groups and PMB
interfaces differ between BCMBCA SoCs. There is no common reset
contract for a family-wide fallback compatible.

Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 .../bindings/reset/brcm,bcm6846-pmb-reset.yaml     | 46 ++++++++++++++++++++++
 include/dt-bindings/reset/brcm,bcm6846-pmb.h       | 15 +++++++
 2 files changed, 61 insertions(+)

diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm6846-pmb-reset.yaml b/Documentation/devicetree/bindings/reset/brcm,bcm6846-pmb-reset.yaml
new file mode 100644
index 000000000000..c8a1e5132fa5
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/brcm,bcm6846-pmb-reset.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/brcm,bcm6846-pmb-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCMBCA Processor Monitor Bus reset controller
+
+maintainers:
+  - Linus Walleij <linusw@kernel.org>
+
+description: |
+  The Processor Monitor Bus (PMB) provides access to reset registers in
+  BCMBCA SoC blocks. Each reset specifier identifies a logical reset group,
+  which may span several blocks that must be reset together. Blocks must
+  be powered and all affected users quiesced before requesting a reset.
+
+  BCM6846_RESET_XRDP covers the XRDP packet engine and three separate Runner
+  cores. All participating blocks are asserted before any is released.
+  The bus interfaces remain accessible throughout the reset.
+
+  BCM6846_RESET_PCIE0 and BCM6846_RESET_PCIE1 pulse the PCIe PMB soft
+  resets, including IDDQ, while preserving the lane-selection straps.
+  These are separate from PCIe PERST and the controller-local resets.
+
+  BCM6846_RESET_PVTMON resets analog acquisition in the process, voltage
+  and temperature monitor. Its clock and power configuration are retained.
+  Users must coordinate with any firmware performing voltage monitoring.
+
+  This node is a child of the PMB controller in the third generation
+  Platform Management Controller (PMC3), which owns the shared register
+  bank and serializes access to its keyhole interface.
+
+properties:
+  compatible:
+    const: brcm,bcm6846-pmb-reset
+
+  '#reset-cells':
+    const: 1
+    description: Reset group ID from dt-bindings/reset/brcm,bcm6846-pmb.h
+
+required:
+  - compatible
+  - '#reset-cells'
+
+additionalProperties: false
diff --git a/include/dt-bindings/reset/brcm,bcm6846-pmb.h b/include/dt-bindings/reset/brcm,bcm6846-pmb.h
new file mode 100644
index 000000000000..3cf31ddcb66c
--- /dev/null
+++ b/include/dt-bindings/reset/brcm,bcm6846-pmb.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+#ifndef _DT_BINDINGS_RESET_BRCM_BCM6846_PMB_H
+#define _DT_BINDINGS_RESET_BRCM_BCM6846_PMB_H
+
+/* XRDP packet engine and its three Runner cores. */
+#define BCM6846_RESET_XRDP	0
+
+/* PCIe PMB soft resets, preserving the lane configuration. */
+#define BCM6846_RESET_PCIE0	1
+#define BCM6846_RESET_PCIE1	2
+
+/* Process, voltage and temperature monitor analog acquisition reset. */
+#define BCM6846_RESET_PVTMON	3
+
+#endif

-- 
2.56.0

Re: [PATCH RFC 3/5] dt-bindings: reset: Describe the BCMBCA PMB reset controller

[Philipp Zabel]

On Mi, 2026-10-14 at 11:48 +0200, Linus Walleij wrote:
> Describe a reset provider below the Processor Monitor Bus (PMB)
> controller, which owns the register bank and serializes accesses.
> Use one specifier cell for a SoC-specific logical reset group ID.
> 
> Define BCM6846_RESET_XRDP for the coordinated reset of the XRDP packet
> engine and its three Runner cores. The vendor PMB device addresses
> identify individual blocks, not this group, so assign it reset ID 0.
> 
> Define separate IDs for PCIe0 and PCIe1 soft resets and PVTMON analog
> acquisition. Describe the scope and prerequisites of each operation.
> 
> Use only the SoC-specific compatible because the reset groups and PMB
> interfaces differ between BCMBCA SoCs. There is no common reset
> contract for a family-wide fallback compatible.
> 
> Assisted-by: LLM
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  .../bindings/reset/brcm,bcm6846-pmb-reset.yaml     | 46 ++++++++++++++++++++++
>  include/dt-bindings/reset/brcm,bcm6846-pmb.h       | 15 +++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm6846-pmb-reset.yaml b/Documentation/devicetree/bindings/reset/brcm,bcm6846-pmb-reset.yaml
> new file mode 100644
> index 000000000000..c8a1e5132fa5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/brcm,bcm6846-pmb-reset.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/brcm,bcm6846-pmb-reset.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCMBCA Processor Monitor Bus reset controller
> +
> +maintainers:
> +  - Linus Walleij <linusw@kernel.org>
> +
> +description: |
> +  The Processor Monitor Bus (PMB) provides access to reset registers in
> +  BCMBCA SoC blocks. Each reset specifier identifies a logical reset group,
> +  which may span several blocks that must be reset together. Blocks must
> +  be powered and all affected users quiesced before requesting a reset.
> +
> +  BCM6846_RESET_XRDP covers the XRDP packet engine and three separate Runner
> +  cores. All participating blocks are asserted before any is released.
> +  The bus interfaces remain accessible throughout the reset.
> +
> +  BCM6846_RESET_PCIE0 and BCM6846_RESET_PCIE1 pulse the PCIe PMB soft
> +  resets, including IDDQ, while preserving the lane-selection straps.
> +  These are separate from PCIe PERST and the controller-local resets.
> +
> +  BCM6846_RESET_PVTMON resets analog acquisition in the process, voltage
> +  and temperature monitor. Its clock and power configuration are retained.
> +  Users must coordinate with any firmware performing voltage monitoring.
> +
> +  This node is a child of the PMB controller in the third generation
> +  Platform Management Controller (PMC3), which owns the shared register
> +  bank and serializes access to its keyhole interface.

Why is this a separate child node instead of just #reset-cells property
on the PMB node?

regards
Philipp

[PATCH RFC 4/5] bus: Add a shared BCMBCA PMC3 PMB transport

[Linus Walleij]

Add a bus driver for shared access to the Processor Monitor Bus (PMB)
of the third generation Platform Management Controller (PMC3), initially
for BCM6846.

Serialize complete register sequences with a mutex and the
keyhole hardware lock.

A software timeout does not abort a transaction. If the keyhole is
still busy, retain its hardware lock and reject new callers until
delayed work observes completion and releases it.

Build this fixed SoC transport into the kernel and suppress sysfs
bind/unbind controls. The reset child can remain modular. Runtime
removal of the PMB platform device is unsupported.

The keyhole register layout differs from older Broadcom PMB
controllers. Match only BCM6846 for now.

Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/bus/Kconfig                |  13 +++
 drivers/bus/Makefile               |   1 +
 drivers/bus/bcmbca_pmc3_pmb.c      | 232 +++++++++++++++++++++++++++++++++++++
 include/linux/soc/bcm/bcmbca-pmb.h |  25 ++++
 4 files changed, 271 insertions(+)

diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index e4b1db809187..a3cf367c4327 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -29,6 +29,19 @@ config ARM_INTEGRATOR_LM
 	  Say y here to enable support for the ARM Logic Module bus
 	  found on the ARM Integrator AP (Application Platform)
 
+config BCMBCA_PMB
+	bool "Broadcom BCMBCA Processor Monitor Bus"
+	depends on ARCH_BCMBCA || COMPILE_TEST
+	depends on OF && HAS_IOMEM
+	help
+	  Provide shared Processor Monitor Bus (PMB) access through the third
+	  generation Platform Management Controller (PMC3), initially on
+	  BCM6846. Child drivers use this transport for operations such as
+	  coordinated resets. Firmware must power the target blocks first.
+
+	  This fixed SoC transport is built-in and cannot be unbound through
+	  sysfs, so recovery can retain ownership of an active transaction.
+
 config BRCMSTB_GISB_ARB
 	tristate "Broadcom STB GISB bus arbiter"
 	depends on ARCH_BRCMSTB || BMIPS_GENERIC
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index a01f97fef3e8..209af710bbd0 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -7,6 +7,7 @@
 obj-$(CONFIG_ARM_CCI)		+= arm-cci.o
 obj-$(CONFIG_ARM_INTEGRATOR_LM)	+= arm-integrator-lm.o
 obj-$(CONFIG_HISILICON_LPC)	+= hisi_lpc.o
+obj-$(CONFIG_BCMBCA_PMB)		+= bcmbca_pmc3_pmb.o
 obj-$(CONFIG_BRCMSTB_GISB_ARB)	+= brcmstb_gisb.o
 obj-$(CONFIG_MOXTET)		+= moxtet.o
 
diff --git a/drivers/bus/bcmbca_pmc3_pmb.c b/drivers/bus/bcmbca_pmc3_pmb.c
new file mode 100644
index 000000000000..998a254c57d3
--- /dev/null
+++ b/drivers/bus/bcmbca_pmc3_pmb.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Broadcom BCMBCA Processor Monitor Bus (PMB) transport for the third
+ * generation Platform Management Controller (PMC3).
+ *
+ * The bus child of the PMC complex supplies the PMB register bank.
+ * Children share keyhole 0 and hold its software and hardware locks across
+ * complete sequences. A software timeout does not abort a PMB transaction:
+ * keep the hardware lock until completion, even after returning an error.
+ * Module removal and sysfs unbind are not supported for this fixed SoC
+ * transport, allowing recovery to wait for delayed transaction completion.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/ioport.h>
+#include <linux/log2.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/soc/bcm/bcmbca-pmb.h>
+#include <linux/workqueue.h>
+
+#define PMB_CONFIG		0x00
+#define PMB_REGS_PER_DEVICE	GENMASK(29, 20)
+#define PMB_CONTROL		0x10
+#define PMB_WRITE_DATA		0x14
+#define PMB_MUTEX		0x18
+#define PMB_READ_DATA		0x1c
+#define PMB_START		BIT(31)
+#define PMB_TIMEOUT		BIT(30)
+#define PMB_SLAVE_ERROR		BIT(29)
+#define PMB_BUSY			BIT(28)
+#define PMB_WRITE		BIT(24)
+#define PMB_BUS			GENMASK(21, 20)
+#define PMB_LOCKED		BIT(0)
+
+struct bcmbca_pmb {
+	struct device *dev;
+	void __iomem *base;
+	/* Serializes register sequences and recovery state. */
+	struct mutex lock;
+	struct delayed_work recovery;
+	u32 regs_per_device;
+	bool recovering;
+	bool stopping;
+};
+
+static struct platform_driver bcmbca_pmb_driver;
+
+/* Only children populated by this provider may borrow its transport. */
+struct bcmbca_pmb *bcmbca_pmb_get(struct device *dev)
+{
+	if (!dev->parent || dev->parent->driver != &bcmbca_pmb_driver.driver)
+		return ERR_PTR(-ENODEV);
+
+	return dev_get_drvdata(dev->parent);
+}
+EXPORT_SYMBOL_GPL(bcmbca_pmb_get);
+
+static void bcmbca_pmb_recover(struct work_struct *work)
+{
+	struct bcmbca_pmb *pmb = container_of(to_delayed_work(work),
+					   struct bcmbca_pmb, recovery);
+
+	mutex_lock(&pmb->lock);
+	if (!pmb->stopping) {
+		if (readl(pmb->base + PMB_CONTROL) & PMB_BUSY) {
+			queue_delayed_work(system_dfl_wq, &pmb->recovery,
+					   msecs_to_jiffies(100));
+		} else {
+			writel(PMB_LOCKED, pmb->base + PMB_MUTEX);
+			pmb->recovering = false;
+		}
+	}
+	mutex_unlock(&pmb->lock);
+}
+
+int bcmbca_pmb_lock(struct bcmbca_pmb *pmb)
+{
+	u32 value;
+	int ret;
+
+	mutex_lock(&pmb->lock);
+	if (pmb->stopping) {
+		ret = -ENODEV;
+		goto unlock;
+	}
+	if (pmb->recovering) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	/* Reading bit 0 acquires the hardware lock; writing 1 releases it. */
+	ret = readl_poll_timeout(pmb->base + PMB_MUTEX, value,
+				 value & PMB_LOCKED, 1, 1000);
+	if (!ret)
+		return 0;
+unlock:
+	mutex_unlock(&pmb->lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(bcmbca_pmb_lock);
+
+void bcmbca_pmb_unlock(struct bcmbca_pmb *pmb)
+{
+	lockdep_assert_held(&pmb->lock);
+	if (readl(pmb->base + PMB_CONTROL) & PMB_BUSY) {
+		pmb->recovering = true;
+		dev_err(pmb->dev, "PMB keyhole still busy; deferring unlock\n");
+		queue_delayed_work(system_dfl_wq, &pmb->recovery,
+				   msecs_to_jiffies(100));
+	} else {
+		writel(PMB_LOCKED, pmb->base + PMB_MUTEX);
+	}
+	mutex_unlock(&pmb->lock);
+}
+EXPORT_SYMBOL_GPL(bcmbca_pmb_unlock);
+
+static int bcmbca_pmb_op(struct bcmbca_pmb *pmb, u8 bus, u8 device,
+			 u16 offset, u32 *value, bool write)
+{
+	u32 status, command;
+	int ret;
+
+	lockdep_assert_held(&pmb->lock);
+	if (bus > FIELD_MAX(PMB_BUS) || offset >= pmb->regs_per_device)
+		return -EINVAL;
+
+	ret = readl_poll_timeout(pmb->base + PMB_CONTROL, status,
+				 !(status & PMB_BUSY), 1, 1000);
+	if (ret)
+		return ret;
+	command = PMB_START | FIELD_PREP(PMB_BUS, bus) |
+		  (device * pmb->regs_per_device) | offset;
+	if (write) {
+		writel(*value, pmb->base + PMB_WRITE_DATA);
+		command |= PMB_WRITE;
+	}
+	writel(command, pmb->base + PMB_CONTROL);
+	ret = readl_poll_timeout(pmb->base + PMB_CONTROL, status,
+				 !(status & PMB_BUSY), 1, 1000);
+	if (ret || status & PMB_TIMEOUT)
+		return -ETIMEDOUT;
+	if (status & PMB_SLAVE_ERROR)
+		return -EIO;
+	if (!write)
+		*value = readl(pmb->base + PMB_READ_DATA);
+	return 0;
+}
+
+int bcmbca_pmb_read(struct bcmbca_pmb *pmb, u8 bus, u8 device,
+		    u16 offset, u32 *value)
+{
+	return bcmbca_pmb_op(pmb, bus, device, offset, value, false);
+}
+EXPORT_SYMBOL_GPL(bcmbca_pmb_read);
+
+int bcmbca_pmb_write(struct bcmbca_pmb *pmb, u8 bus, u8 device,
+		     u16 offset, u32 value)
+{
+	return bcmbca_pmb_op(pmb, bus, device, offset, &value, true);
+}
+EXPORT_SYMBOL_GPL(bcmbca_pmb_write);
+
+/* Children are removed before this action, while MMIO is still mapped. */
+static void bcmbca_pmb_stop(void *data)
+{
+	struct bcmbca_pmb *pmb = data;
+
+	mutex_lock(&pmb->lock);
+	pmb->stopping = true;
+	mutex_unlock(&pmb->lock);
+	cancel_delayed_work_sync(&pmb->recovery);
+
+	if (pmb->recovering) {
+		if (readl(pmb->base + PMB_CONTROL) & PMB_BUSY)
+			dev_err(pmb->dev, "PMB keyhole remains busy; hardware reset required\n");
+		else
+			writel(PMB_LOCKED, pmb->base + PMB_MUTEX);
+	}
+}
+
+static int bcmbca_pmb_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct bcmbca_pmb *pmb;
+	struct resource *res;
+	int ret;
+
+	pmb = devm_kzalloc(dev, sizeof(*pmb), GFP_KERNEL);
+	if (!pmb)
+		return -ENOMEM;
+	pmb->dev = dev;
+	pmb->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(pmb->base))
+		return PTR_ERR(pmb->base);
+	if (resource_size(res) < PMB_READ_DATA + sizeof(u32))
+		return dev_err_probe(dev, -EINVAL, "PMB register range is too small\n");
+	pmb->regs_per_device = FIELD_GET(PMB_REGS_PER_DEVICE,
+					 readl(pmb->base + PMB_CONFIG));
+	if (!is_power_of_2(pmb->regs_per_device))
+		return dev_err_probe(dev, -EINVAL, "invalid PMB register stride\n");
+	mutex_init(&pmb->lock);
+	INIT_DELAYED_WORK(&pmb->recovery, bcmbca_pmb_recover);
+	ret = devm_add_action_or_reset(dev, bcmbca_pmb_stop, pmb);
+	if (ret)
+		return ret;
+	platform_set_drvdata(pdev, pmb);
+	return devm_of_platform_populate(dev);
+}
+
+static const struct of_device_id bcmbca_pmb_match[] = {
+	{ .compatible = "brcm,bcm6846-pmb" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, bcmbca_pmb_match);
+
+static struct platform_driver bcmbca_pmb_driver = {
+	.probe = bcmbca_pmb_probe,
+	.driver = {
+		.name = "bcmbca-pmb",
+		.of_match_table = bcmbca_pmb_match,
+		.suppress_bind_attrs = true,
+	},
+};
+builtin_platform_driver(bcmbca_pmb_driver);
+
+MODULE_DESCRIPTION("Broadcom BCMBCA Processor Monitor Bus transport for PMC3");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/soc/bcm/bcmbca-pmb.h b/include/linux/soc/bcm/bcmbca-pmb.h
new file mode 100644
index 000000000000..f95523d52e01
--- /dev/null
+++ b/include/linux/soc/bcm/bcmbca-pmb.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __LINUX_SOC_BCM_BCMBCA_PMB_H
+#define __LINUX_SOC_BCM_BCMBCA_PMB_H
+
+#include <linux/types.h>
+
+struct device;
+struct bcmbca_pmb;
+
+/* The parent bus device owns the Processor Monitor Bus (PMB) transport. */
+struct bcmbca_pmb *bcmbca_pmb_get(struct device *dev);
+
+/*
+ * Processor Monitor Bus (PMB) operations sleep. Hold the transport lock
+ * across a complete sequence, including read/modify/write operations.
+ * A successful lock must be paired with unlock, including on errors.
+ */
+int bcmbca_pmb_lock(struct bcmbca_pmb *pmb);
+void bcmbca_pmb_unlock(struct bcmbca_pmb *pmb);
+int bcmbca_pmb_read(struct bcmbca_pmb *pmb, u8 bus, u8 device,
+		    u16 offset, u32 *value);
+int bcmbca_pmb_write(struct bcmbca_pmb *pmb, u8 bus, u8 device,
+		     u16 offset, u32 value);
+
+#endif

-- 
2.56.0

Re: [PATCH RFC 4/5] bus: Add a shared BCMBCA PMC3 PMB transport

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bus: Missing sparse lock annotations for transport lock functions
--

commit f8e671cc4d072dbd3353e224bb3d5cee80468de7
Author: Linus Walleij <linusw@kernel.org>

bus: Add a shared BCMBCA PMC3 PMB transport

This commit adds a bus driver for shared access to the Processor Monitor Bus
(PMB) of the third generation Platform Management Controller (PMC3), initially
for BCM6846. It serializes complete register sequences with a mutex and a
hardware lock.

> diff --git a/include/linux/soc/bcm/bcmbca-pmb.h b/include/linux/soc/bcm/bcmbca-pmb.h
> new file mode 100644
> index 0000000000000..f95523d52e01b
> --- /dev/null
> +++ b/include/linux/soc/bcm/bcmbca-pmb.h
[ ... ]
> +/*
> + * Processor Monitor Bus (PMB) operations sleep. Hold the transport lock
> + * across a complete sequence, including read/modify/write operations.
> + * A successful lock must be paired with unlock, including on errors.
> + */
> +int bcmbca_pmb_lock(struct bcmbca_pmb *pmb);
> +void bcmbca_pmb_unlock(struct bcmbca_pmb *pmb);

[Severity: Low]
Does this code need sparse lock annotations for the transport lock functions?

Without __acquires(&pmb->lock) and __releases(&pmb->lock) on these
prototypes, sparse will report context imbalance warnings when child drivers
call bcmbca_pmb_lock() and bcmbca_pmb_unlock() (when built with C=1). While
sparse has limited strictness with mutexes compared to spinlocks, adding
the annotations can prevent build-time context imbalance warnings.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-xg6846b-bcmbca-xrdp-reset-v1-0-daf5d177e113@kernel.org?part=4

[PATCH RFC 5/5] reset: Add BCMBCA block resets through the PMB

[Linus Walleij]

Support BCM6846 XRDP, PCIe and PVTMON resets through the Processor
Monitor Bus (PMB) of the third generation Platform Management
Controller (PMC3).

Use per-SoC reset groups and sequences with transport serialization.
Reset XRDP and its Runner cores together to keep their shared SRAM
Buffer Pool Manager (SBPM) state consistent. Consumers must quiesce
all affected users, including firmware voltage monitoring for PVTMON.

Power-cycle resets for USB2, WAN, PERIPH, MEMC, BIU and BIU PLL were
considered but left out. These belong in power-domain and platform
power-management abstractions, which can coordinate affected users.

Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/reset/Kconfig            |  15 +++
 drivers/reset/Makefile           |   1 +
 drivers/reset/reset-bcmbca-pmb.c | 281 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 297 insertions(+)

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 3168ab1eea88..fb2b20deb050 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -49,6 +49,21 @@ config RESET_BCM6345
 	help
 	  This enables the reset controller driver for BCM6345 SoCs.
 
+config RESET_BCMBCA_PMB
+	tristate "Broadcom BCMBCA PMB reset controller"
+	depends on ARCH_BCMBCA || COMPILE_TEST
+	depends on OF && HAS_IOMEM
+	select BCMBCA_PMB
+	help
+	  Reset BCMBCA blocks through the Processor Monitor Bus (PMB) of
+	  the third generation Platform Management Controller (PMC3).
+	  Supports the BCM6846 XRDP packet engine and its Runner cores,
+	  PCIe, and analog acquisition in the process, voltage and temperature
+	  monitor. Blocks must be powered and all affected users quiesced
+	  before reset.
+
+	  The module will be called reset-bcmbca-pmb.
+
 config RESET_BERLIN
 	tristate "Berlin Reset Driver"
 	depends on ARCH_BERLIN || COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 45ad17062462..d0d618cc6495 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_RESET_ASPEED) += reset-aspeed.o
 obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
 obj-$(CONFIG_RESET_AXS10X) += reset-axs10x.o
 obj-$(CONFIG_RESET_BCM6345) += reset-bcm6345.o
+obj-$(CONFIG_RESET_BCMBCA_PMB) += reset-bcmbca-pmb.o
 obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
 obj-$(CONFIG_RESET_BRCMSTB) += reset-brcmstb.o
 obj-$(CONFIG_RESET_BRCMSTB_RESCAL) += reset-brcmstb-rescal.o
diff --git a/drivers/reset/reset-bcmbca-pmb.c b/drivers/reset/reset-bcmbca-pmb.c
new file mode 100644
index 000000000000..d3b247a48574
--- /dev/null
+++ b/drivers/reset/reset-bcmbca-pmb.c
@@ -0,0 +1,281 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * BCMBCA block resets through the Processor Monitor Bus (PMB) of the
+ * third generation Platform Management Controller (PMC3).
+ *
+ * SoC match data maps reset IDs to groups of blocks and their sequences.
+ * XRDP and its Runner cores share SRAM Buffer Pool Manager (SBPM) state
+ * and must be reset together, preserving their bus interfaces.
+ * PCIe uses soft-reset pulses; PVTMON resets analog acquisition.
+ */
+
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/reset-controller.h>
+#include <linux/soc/bcm/bcmbca-pmb.h>
+
+#include <dt-bindings/reset/brcm,bcm6846-pmb.h>
+
+struct bcmbca_pmb_reset_block {
+	u8 bus;
+	u8 device;
+	u16 offset; /* PMB word offset of the reset register. */
+	u32 mask;
+	u32 assert_value;
+	u32 deassert_value;
+};
+
+struct bcmbca_pmb_reset;
+
+struct bcmbca_pmb_reset_group {
+	const char *name;
+	int (*reset)(struct bcmbca_pmb_reset *rst,
+		     const struct bcmbca_pmb_reset_group *group);
+	const struct bcmbca_pmb_reset_block *blocks;
+	unsigned int num_blocks;
+	unsigned int assert_us;
+};
+
+struct bcmbca_pmb_reset_variant {
+	const struct bcmbca_pmb_reset_group *groups;
+	unsigned int num_groups;
+};
+
+struct bcmbca_pmb_reset {
+	struct reset_controller_dev rcdev;
+	const struct bcmbca_pmb_reset_variant *variant;
+	struct bcmbca_pmb *pmb;
+};
+
+static int bcmbca_pmb_reset_sequence(struct bcmbca_pmb_reset *rst,
+				     const struct bcmbca_pmb_reset_group *group);
+
+static const struct bcmbca_pmb_reset_block bcm6846_xrdp_resets[] = {
+	/* Keep the XRDP bus interfaces out of reset. */
+	{
+		.bus = 1,
+		.device = 12,
+		.offset = 5,
+		.mask = GENMASK(27, 0),
+		.assert_value = GENMASK(2, 0),
+		.deassert_value = GENMASK(27, 0),
+	},
+	{
+		.bus = 1,
+		.device = 14,
+		.offset = 5,
+		.mask = GENMASK(27, 0),
+		.deassert_value = GENMASK(27, 0),
+	},
+	{
+		.bus = 1,
+		.device = 15,
+		.offset = 5,
+		.mask = GENMASK(27, 0),
+		.deassert_value = GENMASK(27, 0),
+	},
+	{
+		.bus = 1,
+		.device = 16,
+		.offset = 5,
+		.mask = GENMASK(27, 0),
+		.deassert_value = GENMASK(27, 0),
+	},
+};
+
+/* Bits 6 and 7 select the lane configuration and must remain unchanged. */
+static const struct bcmbca_pmb_reset_block bcm6846_pcie_resets[] = {
+	{
+		.bus = 0,
+		.device = 18,
+		.offset = 5,
+		.mask = GENMASK(5, 0),
+		.assert_value = GENMASK(5, 0),
+	},
+	{
+		.bus = 0,
+		.device = 19,
+		.offset = 5,
+		.mask = GENMASK(5, 0),
+		.assert_value = GENMASK(5, 0),
+	},
+};
+
+/* Analog acquisition reset, independent of clock and power configuration. */
+static const struct bcmbca_pmb_reset_block bcm6846_pvtmon_reset = {
+	.bus = 1,
+	.device = 6,
+	.offset = 17,
+	.mask = BIT(0),
+	.deassert_value = BIT(0),
+};
+
+static const struct bcmbca_pmb_reset_group bcm6846_pmb_reset_groups[] = {
+	[BCM6846_RESET_XRDP] = {
+		.name = "XRDP",
+		.reset = bcmbca_pmb_reset_sequence,
+		.blocks = bcm6846_xrdp_resets,
+		.num_blocks = ARRAY_SIZE(bcm6846_xrdp_resets),
+		.assert_us = 10,
+	},
+	[BCM6846_RESET_PCIE0] = {
+		.name = "PCIe0",
+		.reset = bcmbca_pmb_reset_sequence,
+		.blocks = &bcm6846_pcie_resets[0],
+		.num_blocks = 1,
+		.assert_us = 10000,
+	},
+	[BCM6846_RESET_PCIE1] = {
+		.name = "PCIe1",
+		.reset = bcmbca_pmb_reset_sequence,
+		.blocks = &bcm6846_pcie_resets[1],
+		.num_blocks = 1,
+		.assert_us = 10000,
+	},
+	[BCM6846_RESET_PVTMON] = {
+		.name = "PVTMON",
+		.reset = bcmbca_pmb_reset_sequence,
+		.blocks = &bcm6846_pvtmon_reset,
+		.num_blocks = 1,
+		.assert_us = 1000,
+	},
+};
+
+static const struct bcmbca_pmb_reset_variant bcm6846_pmb_variant = {
+	.groups = bcm6846_pmb_reset_groups,
+	.num_groups = ARRAY_SIZE(bcm6846_pmb_reset_groups),
+};
+
+/* Best effort with both locks held; never restart a partially reset group. */
+static void bcmbca_pmb_reset_cleanup(struct bcmbca_pmb_reset *rst,
+				     const struct bcmbca_pmb_reset_group *group)
+{
+	const struct bcmbca_pmb_reset_block *block;
+	unsigned int i;
+	u32 value;
+	int ret;
+
+	for (i = 0; i < group->num_blocks; i++) {
+		block = &group->blocks[i];
+		ret = bcmbca_pmb_read(rst->pmb, block->bus, block->device,
+				      block->offset, &value);
+		if (!ret) {
+			value &= ~block->mask;
+			value |= block->assert_value;
+			ret = bcmbca_pmb_write(rst->pmb, block->bus, block->device,
+					       block->offset, value);
+		}
+		if (ret)
+			dev_err(rst->rcdev.dev,
+				"failed to hold PMB bus %u device %u in reset: %d\n",
+				block->bus, block->device, ret);
+	}
+}
+
+/* Called with both PMB locks held, including during error cleanup. */
+static int bcmbca_pmb_reset_sequence(struct bcmbca_pmb_reset *rst,
+				     const struct bcmbca_pmb_reset_group *group)
+{
+	const struct bcmbca_pmb_reset_block *block;
+	unsigned int i;
+	u32 value;
+	int ret, pass;
+
+	if (!group->num_blocks)
+		return -EINVAL;
+
+	/* Assert every block before releasing any of them. */
+	for (pass = 0; pass < 2; pass++) {
+		for (i = 0; i < group->num_blocks; i++) {
+			block = &group->blocks[i];
+			ret = bcmbca_pmb_read(rst->pmb, block->bus, block->device,
+					      block->offset, &value);
+			if (ret)
+				goto release;
+			value &= ~block->mask;
+			value |= pass ? block->deassert_value : block->assert_value;
+			ret = bcmbca_pmb_write(rst->pmb, block->bus, block->device,
+					       block->offset, value);
+			if (ret)
+				goto release;
+		}
+		if (!pass && group->assert_us)
+			usleep_range(group->assert_us, group->assert_us * 2);
+	}
+release:
+	if (ret) {
+		dev_err(rst->rcdev.dev, "%s %s failed at PMB bus %u device %u: %d\n",
+			group->name, pass ? "release" : "assert",
+			block->bus, block->device, ret);
+		bcmbca_pmb_reset_cleanup(rst, group);
+	}
+	return ret;
+}
+
+static int bcmbca_pmb_reset(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct bcmbca_pmb_reset *rst = container_of(rcdev, struct bcmbca_pmb_reset, rcdev);
+	const struct bcmbca_pmb_reset_group *group;
+	int ret;
+
+	if (id >= rst->variant->num_groups)
+		return -EINVAL;
+	group = &rst->variant->groups[id];
+	if (!group->reset)
+		return -EINVAL;
+
+	ret = bcmbca_pmb_lock(rst->pmb);
+	if (ret)
+		return ret;
+	ret = group->reset(rst, group);
+	bcmbca_pmb_unlock(rst->pmb);
+	return ret;
+}
+
+static const struct reset_control_ops bcmbca_pmb_reset_ops = {
+	.reset = bcmbca_pmb_reset,
+};
+
+static int bcmbca_pmb_reset_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct bcmbca_pmb_reset *rst;
+
+	rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
+	if (!rst)
+		return -ENOMEM;
+	rst->variant = device_get_match_data(dev);
+	if (!rst->variant)
+		return -EINVAL;
+	rst->pmb = bcmbca_pmb_get(dev);
+	if (IS_ERR(rst->pmb))
+		return dev_err_probe(dev, PTR_ERR(rst->pmb), "cannot get PMB transport\n");
+	rst->rcdev.owner = THIS_MODULE;
+	rst->rcdev.dev = dev;
+	rst->rcdev.ops = &bcmbca_pmb_reset_ops;
+	rst->rcdev.of_node = dev->of_node;
+	rst->rcdev.of_reset_n_cells = 1;
+	rst->rcdev.nr_resets = rst->variant->num_groups;
+	return devm_reset_controller_register(dev, &rst->rcdev);
+}
+
+static const struct of_device_id bcmbca_pmb_reset_match[] = {
+	{ .compatible = "brcm,bcm6846-pmb-reset", .data = &bcm6846_pmb_variant },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, bcmbca_pmb_reset_match);
+
+static struct platform_driver bcmbca_pmb_reset_driver = {
+	.probe = bcmbca_pmb_reset_probe,
+	.driver = {
+		.name = "bcmbca-pmb-reset",
+		.of_match_table = bcmbca_pmb_reset_match,
+	},
+};
+module_platform_driver(bcmbca_pmb_reset_driver);
+
+MODULE_DESCRIPTION("Broadcom BCMBCA PMB reset controller");
+MODULE_LICENSE("GPL");

-- 
2.56.0

Re: [PATCH RFC 5/5] reset: Add BCMBCA block resets through the PMB

[Philipp Zabel]

On Mi, 2026-10-14 at 11:48 +0200, Linus Walleij wrote:
> Support BCM6846 XRDP, PCIe and PVTMON resets through the Processor
> Monitor Bus (PMB) of the third generation Platform Management
> Controller (PMC3).
> 
> Use per-SoC reset groups and sequences with transport serialization.
> Reset XRDP and its Runner cores together to keep their shared SRAM
> Buffer Pool Manager (SBPM) state consistent. Consumers must quiesce
> all affected users, including firmware voltage monitoring for PVTMON.
> 
> Power-cycle resets for USB2, WAN, PERIPH, MEMC, BIU and BIU PLL were
> considered but left out. These belong in power-domain and platform
> power-management abstractions, which can coordinate affected users.
> 
> Assisted-by: LLM
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  drivers/reset/Kconfig            |  15 +++
>  drivers/reset/Makefile           |   1 +
>  drivers/reset/reset-bcmbca-pmb.c | 281 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 297 insertions(+)
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 3168ab1eea88..fb2b20deb050 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -49,6 +49,21 @@ config RESET_BCM6345
>  	help
>  	  This enables the reset controller driver for BCM6345 SoCs.
>  
> +config RESET_BCMBCA_PMB
> +	tristate "Broadcom BCMBCA PMB reset controller"
> +	depends on ARCH_BCMBCA || COMPILE_TEST
> +	depends on OF && HAS_IOMEM
> +	select BCMBCA_PMB
> +	help
> +	  Reset BCMBCA blocks through the Processor Monitor Bus (PMB) of
> +	  the third generation Platform Management Controller (PMC3).
> +	  Supports the BCM6846 XRDP packet engine and its Runner cores,
> +	  PCIe, and analog acquisition in the process, voltage and temperature
> +	  monitor. Blocks must be powered and all affected users quiesced
> +	  before reset.
> +
> +	  The module will be called reset-bcmbca-pmb.
> +
>  config RESET_BERLIN
>  	tristate "Berlin Reset Driver"
>  	depends on ARCH_BERLIN || COMPILE_TEST
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 45ad17062462..d0d618cc6495 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_RESET_ASPEED) += reset-aspeed.o
>  obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
>  obj-$(CONFIG_RESET_AXS10X) += reset-axs10x.o
>  obj-$(CONFIG_RESET_BCM6345) += reset-bcm6345.o
> +obj-$(CONFIG_RESET_BCMBCA_PMB) += reset-bcmbca-pmb.o
>  obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
>  obj-$(CONFIG_RESET_BRCMSTB) += reset-brcmstb.o
>  obj-$(CONFIG_RESET_BRCMSTB_RESCAL) += reset-brcmstb-rescal.o
> diff --git a/drivers/reset/reset-bcmbca-pmb.c b/drivers/reset/reset-bcmbca-pmb.c
> new file mode 100644
> index 000000000000..d3b247a48574
> --- /dev/null
> +++ b/drivers/reset/reset-bcmbca-pmb.c
> @@ -0,0 +1,281 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * BCMBCA block resets through the Processor Monitor Bus (PMB) of the
> + * third generation Platform Management Controller (PMC3).
> + *
> + * SoC match data maps reset IDs to groups of blocks and their sequences.
> + * XRDP and its Runner cores share SRAM Buffer Pool Manager (SBPM) state
> + * and must be reset together, preserving their bus interfaces.
> + * PCIe uses soft-reset pulses; PVTMON resets analog acquisition.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/reset-controller.h>
> +#include <linux/soc/bcm/bcmbca-pmb.h>
> +
> +#include <dt-bindings/reset/brcm,bcm6846-pmb.h>
> +
> +struct bcmbca_pmb_reset_block {
> +	u8 bus;
> +	u8 device;
> +	u16 offset; /* PMB word offset of the reset register. */
> +	u32 mask;
> +	u32 assert_value;
> +	u32 deassert_value;
> +};
> +
> +struct bcmbca_pmb_reset;
> +
> +struct bcmbca_pmb_reset_group {
> +	const char *name;
> +	int (*reset)(struct bcmbca_pmb_reset *rst,
> +		     const struct bcmbca_pmb_reset_group *group);

What is the point of this indirection?

There is only a single array of struct bcmbca_pmb_reset_group and all
entries use the same bcmbca_pmb_reset_sequence().

> +	const struct bcmbca_pmb_reset_block *blocks;
> +	unsigned int num_blocks;
> +	unsigned int assert_us;
> +};
> +
> +struct bcmbca_pmb_reset_variant {
> +	const struct bcmbca_pmb_reset_group *groups;
> +	unsigned int num_groups;
> +};
> +
> +struct bcmbca_pmb_reset {
> +	struct reset_controller_dev rcdev;
> +	const struct bcmbca_pmb_reset_variant *variant;
> +	struct bcmbca_pmb *pmb;
> +};
> +
> +static int bcmbca_pmb_reset_sequence(struct bcmbca_pmb_reset *rst,
> +				     const struct bcmbca_pmb_reset_group *group);
> +
> +static const struct bcmbca_pmb_reset_block bcm6846_xrdp_resets[] = {
> +	/* Keep the XRDP bus interfaces out of reset. */

What does this comment refer to? assert_value being GENMASK(2, 0)
instead of 0?

> +	{
> +		.bus = 1,
> +		.device = 12,
> +		.offset = 5,
> +		.mask = GENMASK(27, 0),
> +		.assert_value = GENMASK(2, 0),
> +		.deassert_value = GENMASK(27, 0),

Are the four different devices the packet engine and the tree runner
cores mentioned in the dt-bindings patch?

Do the 28 bits refer to 28 different resets?

> +	},
> +	{
> +		.bus = 1,
> +		.device = 14,
> +		.offset = 5,
> +		.mask = GENMASK(27, 0),
> +		.deassert_value = GENMASK(27, 0),
> +	},
> +	{
> +		.bus = 1,
> +		.device = 15,
> +		.offset = 5,
> +		.mask = GENMASK(27, 0),
> +		.deassert_value = GENMASK(27, 0),
> +	},
> +	{
> +		.bus = 1,
> +		.device = 16,
> +		.offset = 5,
> +		.mask = GENMASK(27, 0),
> +		.deassert_value = GENMASK(27, 0),
> +	},
> +};
> +
> +/* Bits 6 and 7 select the lane configuration and must remain unchanged. */
> +static const struct bcmbca_pmb_reset_block bcm6846_pcie_resets[] = {
> +	{
> +		.bus = 0,
> +		.device = 18,
> +		.offset = 5,
> +		.mask = GENMASK(5, 0),
> +		.assert_value = GENMASK(5, 0),

Same here, are these 6 different resets? Does the hardware require them
to be asserted/deasserted at the same time?

> +	},
> +	{
> +		.bus = 0,
> +		.device = 19,
> +		.offset = 5,
> +		.mask = GENMASK(5, 0),
> +		.assert_value = GENMASK(5, 0),
> +	},
> +};

Why put these in an array?

> +
> +/* Analog acquisition reset, independent of clock and power configuration. */
> +static const struct bcmbca_pmb_reset_block bcm6846_pvtmon_reset = {
> +	.bus = 1,
> +	.device = 6,
> +	.offset = 17,
> +	.mask = BIT(0),
> +	.deassert_value = BIT(0),
> +};
> +
> +static const struct bcmbca_pmb_reset_group bcm6846_pmb_reset_groups[] = {
> +	[BCM6846_RESET_XRDP] = {
> +		.name = "XRDP",
> +		.reset = bcmbca_pmb_reset_sequence,
> +		.blocks = bcm6846_xrdp_resets,
> +		.num_blocks = ARRAY_SIZE(bcm6846_xrdp_resets),
> +		.assert_us = 10,
> +	},
> +	[BCM6846_RESET_PCIE0] = {
> +		.name = "PCIe0",
> +		.reset = bcmbca_pmb_reset_sequence,
> +		.blocks = &bcm6846_pcie_resets[0],
> +		.num_blocks = 1,
> +		.assert_us = 10000,
> +	},
> +	[BCM6846_RESET_PCIE1] = {
> +		.name = "PCIe1",
> +		.reset = bcmbca_pmb_reset_sequence,
> +		.blocks = &bcm6846_pcie_resets[1],
> +		.num_blocks = 1,
> +		.assert_us = 10000,
> +	},
> +	[BCM6846_RESET_PVTMON] = {
> +		.name = "PVTMON",
> +		.reset = bcmbca_pmb_reset_sequence,
> +		.blocks = &bcm6846_pvtmon_reset,
> +		.num_blocks = 1,
> +		.assert_us = 1000,
> +	},
> +};
> +
> +static const struct bcmbca_pmb_reset_variant bcm6846_pmb_variant = {
> +	.groups = bcm6846_pmb_reset_groups,
> +	.num_groups = ARRAY_SIZE(bcm6846_pmb_reset_groups),
> +};
> +
> +/* Best effort with both locks held; never restart a partially reset group. */
> +static void bcmbca_pmb_reset_cleanup(struct bcmbca_pmb_reset *rst,
> +				     const struct bcmbca_pmb_reset_group *group)
> +{
> +	const struct bcmbca_pmb_reset_block *block;
> +	unsigned int i;
> +	u32 value;
> +	int ret;
> +
> +	for (i = 0; i < group->num_blocks; i++) {
> +		block = &group->blocks[i];
> +		ret = bcmbca_pmb_read(rst->pmb, block->bus, block->device,
> +				      block->offset, &value);
> +		if (!ret) {
> +			value &= ~block->mask;
> +			value |= block->assert_value;
> +			ret = bcmbca_pmb_write(rst->pmb, block->bus, block->device,
> +					       block->offset, value);
> +		}

Could the PMB bus driver provide an _update_bits() function for these
read-modify-write sequences? It is the one handling the locking anyway.

This whole driver feels a bit like if PMB could just export a regmap,
it would collapse into a few regmap_multi_reg_write()s.

> +		if (ret)
> +			dev_err(rst->rcdev.dev,
> +				"failed to hold PMB bus %u device %u in reset: %d\n",
> +				block->bus, block->device, ret);
> +	}
> +}
> +
> +/* Called with both PMB locks held, including during error cleanup. */

Which two locks are "both"? pmb->lock mutex and the PMB_MUTEX hardware
lock?

> +static int bcmbca_pmb_reset_sequence(struct bcmbca_pmb_reset *rst,
> +				     const struct bcmbca_pmb_reset_group *group)
> +{
> +	const struct bcmbca_pmb_reset_block *block;
> +	unsigned int i;
> +	u32 value;
> +	int ret, pass;
> +
> +	if (!group->num_blocks)
> +		return -EINVAL;
> +
> +	/* Assert every block before releasing any of them. */
> +	for (pass = 0; pass < 2; pass++) {

Maybe   for (pass = PASS_ASSERT; pass <= PASS_DEASSERT

> +		for (i = 0; i < group->num_blocks; i++) {
> +			block = &group->blocks[i];
> +			ret = bcmbca_pmb_read(rst->pmb, block->bus, block->device,
> +					      block->offset, &value);
> +			if (ret)
> +				goto release;
> +			value &= ~block->mask;
> +			value |= pass ? block->deassert_value : block->assert_value;
> +			ret = bcmbca_pmb_write(rst->pmb, block->bus, block->device,
> +					       block->offset, value);
> +			if (ret)
> +				goto release;
> +		}
> +		if (!pass && group->assert_us)
> +			usleep_range(group->assert_us, group->assert_us * 2);

Is that assertion delay the time the reset needs to stay asserted or is
this really a delay that needs to be observed between successive
assertions in a group?

> +	}
> +release:
> +	if (ret) {
> +		dev_err(rst->rcdev.dev, "%s %s failed at PMB bus %u device %u: %d\n",
> +			group->name, pass ? "release" : "assert",

Please use "deassert" to avoid possible confusion with the
reset_control_release() meaning of "release".

> +			block->bus, block->device, ret);
> +		bcmbca_pmb_reset_cleanup(rst, group);
> +	}
> +	return ret;
> +}
[...]

regards
Philipp