Re: [PATCH 2/2] reset: toshiba: introduce a TC9564 SoC reset driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] reset: tc9564: hardware state initialized after reset controller registration
- [High] reset: tc9564: active consumers forcibly reset during driver removal
--

commit 504236856b96e78b9064b4d58c3ef7ba166341e3
Author: Alex Elder <elder@riscstar.com>

reset: toshiba: introduce a TC9564 SoC reset driver

Defines a new platform driver that manages reset signals implemented by the
Toshiba TC9564 SoC. The state of the resets is controlled via MMIO to a
region managed by a system controller.

> diff --git a/drivers/reset/reset-tc9564.c b/drivers/reset/reset-tc9564.c
> new file mode 100644
> index 0000000000000..0243c2e0efd54
> --- /dev/null
> +++ b/drivers/reset/reset-tc9564.c

[ ... ]

> +static int tc9564_reset_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct tc9564_resets *resets;
> +
> +	if (!dev_of_node(dev))
> +		return dev_err_probe(dev, -EINVAL, "no devicetree node\n");
> +
> +	resets = tc9564_reset_init(dev);
> +	if (IS_ERR(resets))
> +		return dev_err_probe(dev, PTR_ERR(resets),
> +				     "failed to initialize resets\n");
> +
> +	/* Force all resets to be initially asserted */
> +	tc9564_reset_assert_all(resets);

[Severity: High]
Does this cause a race condition with consumer drivers?

tc9564_reset_init() globally registers the reset controller via
devm_reset_controller_register(). Once registered, consumers can
immediately probe and deassert their reset lines.

If a consumer driver deasserts its reset right after registration,
will this subsequent call to tc9564_reset_assert_all() overwrite the
consumer's hardware state and unexpectedly yank running hardware back
into reset?

> +
> +	platform_set_drvdata(pdev, resets);
> +
> +	return 0;
> +}
> +
> +static void tc9564_reset_remove(struct platform_device *pdev)
> +{
> +	struct tc9564_resets *resets = platform_get_drvdata(pdev);
> +
> +	/* Leave all resets asserted when done */
> +	tc9564_reset_assert_all(resets);

[Severity: High]
Does this unexpectedly assert resets for active consumers?

Since the reset controller is registered using devm_reset_controller_register(),
the devres subsystem will unregister the controller only after this remove
callback completes.

This means the controller is still active and consumers might be actively
using their deasserted reset lines when tc9564_reset_assert_all() is called
here. Should the driver coordinate with consumers or unregister itself first
before forcing all lines into reset?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012164354.3872326-1-elder@riscstar.com?part=2