[PATCH RFC 5/5] reset: Add BCMBCA block resets through the PMB
Philipp Zabel
p.zabel at pengutronix.de
Wed Oct 14 04:13:58 PDT 2026
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 at 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
More information about the linux-arm-kernel
mailing list