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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] reset: bcmbca-pmb: redundant reset callback in
  bcmbca_pmb_reset_group
--

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

reset: Add BCMBCA block resets through the PMB

This commit adds a reset controller driver for Broadcom BCMBCA blocks
controlled through the PMC3 Processor Monitor Bus regmap, implementing
the BCM6846 XRDP reset group and PVTMON acquisition reset.

> diff --git a/drivers/reset/reset-bcmbca-pmb.c b/drivers/reset/reset-bcmbca-pmb.c
> new file mode 100644
> index 0000000000000..e8e079c9ccb56
> --- /dev/null
> +++ b/drivers/reset/reset-bcmbca-pmb.c
> @@ -0,0 +1,243 @@

[ ... ]

> +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;
> +	/* Minimum reset assertion hold time in microseconds. */
> +	unsigned int assert_hold_us;
> +};

[ ... ]

> +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_hold_us = 10,
> +	},
> +	[BCM6846_RESET_PVTMON] = {
> +		.name = "PVTMON",
> +		.reset = bcmbca_pmb_reset_sequence,
> +		.blocks = &bcm6846_pvtmon_reset,
> +		.num_blocks = 1,
> +		.assert_hold_us = 1000,
> +	},
> +};

[ ... ]

> +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;
> +
> +	if (id >= rst->variant->num_groups)
> +		return -EINVAL;
> +	group = &rst->variant->groups[id];
> +	if (!group->reset)
> +		return -EINVAL;
> +
> +	guard(mutex)(&rst->lock);
> +	return group->reset(rst, group);
> +}

[Severity: Low]
As reported by Philipp Zabel on RFC [1], is the reset callback pointer in
struct bcmbca_pmb_reset_group still needed now that XRDP zone power
transitions live in the PMB power domain driver?

Every entry in bcm6846_pmb_reset_groups[] sets .reset to
bcmbca_pmb_reset_sequence, so bcmbca_pmb_reset() could call
bcmbca_pmb_reset_sequence(rst, group) directly and drop the callback field
along with the forward declarations of struct bcmbca_pmb_reset and
bcmbca_pmb_reset_sequence().

[1] https://lore.kernel.org/r/90c5efa22600044657e67b9e1297bda2fb53f60c.camel@pengutronix.de

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-xg6846b-bcmbca-xrdp-reset-v3-0-d6adeb71afc9@kernel.org?part=5