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

[Philipp Zabel]

On Mo, 2026-10-12 at 11:43 -0500, Alex Elder wrote:
> Define a new platform driver that manages reset signals implemented by
> the Toshiba TC9564 SoC.  There are 13 reset signals available, although
> only 11 can be controlled.
> 
> Two registers manage the state of the resets.  The registers are
> accessed via a regmap supplied by a system controller, because the
> region of memory accessed is shared with other drivers.
> 
> Access to the memory region is provided via a BAR on a PCIe endpoint
> function embedded in the TC9564 SoC.  For that reason the PCIe reset
> cannot be manipulated by this driver (it is assumed to be deasserted).
> Similarly, control is not available for the I2C reset, because the
> PCIe subsystem on the TC9564 relies on I2C.
> 
> Co-developed-by: Daniel Thompson <daniel@xxxxxxxxxxxx>
> Signed-off-by: Daniel Thompson <daniel@xxxxxxxxxxxx>
> Signed-off-by: Alex Elder <elder@xxxxxxxxxxxx>
> ---
>  MAINTAINERS                  |   1 +
>  drivers/reset/Kconfig        |  12 +++
>  drivers/reset/Makefile       |   1 +
>  drivers/reset/reset-tc9564.c | 192 +++++++++++++++++++++++++++++++++++
>  4 files changed, 206 insertions(+)
>  create mode 100644 drivers/reset/reset-tc9564.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d0eb44079fe17..a8f4982ed1a43 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -28185,6 +28185,7 @@ M:	Alex Elder <elder@xxxxxxxxxx>
>  M:	Daniel Thompson <danielt@xxxxxxxxxx>
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/reset/toshiba,tc9564-reset.yaml
> +F:	drivers/reset/reset-tc9564.c
>  F:	include/dt-bindings/reset/toshiba,tc9564.h
>  
>  TOSHIBA WMI HOTKEYS DRIVER
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index eda9c698a95f3..b40736368894c 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -358,6 +358,18 @@ config RESET_SUNXI
>  	help
>  	  This enables the reset driver for Allwinner SoCs.
>  
> +config RESET_TC9564
> +	tristate "Toshiba TC9564 reset support"
> +	depends on TC9564_PCI
> +	select MFD_SYSCON
> +	default TC9564_PCI
> +	help
> +	  This enables support for the reset controller embedded in the
> +	  Toshiba TC9564 SoC.  The state of reset lines is controlled
> +	  via MMIO to a region managed by a system controller; this
> +	  ensures access to the region is coordinated between this and
> +	  the clock and XGMAC (stmmac) drivers.
> +
>  config RESET_TENSTORRENT_ATLANTIS
>  	tristate "Tenstorrent atlantis reset driver"
>  	depends on ARCH_TENSTORRENT || COMPILE_TEST
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 489fafd6bbe3f..6ddded26309ef 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -45,6 +45,7 @@ obj-$(CONFIG_RESET_SKY1_AUDSS) += reset-sky1-audss.o
>  obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
>  obj-$(CONFIG_RESET_SUNPLUS) += reset-sunplus.o
>  obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> +obj-$(CONFIG_RESET_TC9564) += reset-tc9564.o
>  obj-$(CONFIG_RESET_TENSTORRENT_ATLANTIS) += reset-tenstorrent-atlantis.o
>  obj-$(CONFIG_RESET_TH1520) += reset-th1520.o
>  obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> 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
> @@ -0,0 +1,192 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/device-id/of.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +
> +#include <dt-bindings/reset/toshiba,tc9564.h>
> +
> +#define RST_CTRL0_OFFSET	0x1008
> +#define RST_CTRL1_OFFSET	0x1010
> +
> +struct tc9564_reset {
> +	u32 offset;		/* RST_CTRL0_OFFSET or RST_CTRL1_OFFSET */
> +	u32 mask;		/* Zero means undefined reset */
> +};
> +
> +struct tc9564_resets {
> +	struct regmap *regmap;
> +	struct reset_controller_dev rcdev;
> +};
> +
> +#define TC9564_RESET_INIT0(_name, _bit)	__TC9564_RESET_INIT(_name, 0, _bit)
> +#define TC9564_RESET_INIT1(_name, _bit)	__TC9564_RESET_INIT(_name, 1, _bit)
> +
> +#define __TC9564_RESET_INIT(_name, _reg, _bit)		\
> +	[RESET_##_name] = {				\
> +		.offset	= RST_CTRL ## _reg ## _OFFSET,	\
> +		.mask = BIT(_bit),			\
> +	}
> +
> +/*
> + * The PCIe and I2C resets are controllable, but we depend on PCIe
> + * and that relies on I2C, so we don't allow them to be asserted.
> + */
> +static const struct tc9564_reset tc9564_reset[] = {
> +	TC9564_RESET_INIT0(MCU, 0),
> +	TC9564_RESET_INIT0(MCU1, 1),
> +	TC9564_RESET_INIT0(INTC, 4),
> +	/* TC9564_RESET_INIT0(PCIE, 9), */
> +	/* TC9564_RESET_INIT0(I2C, 12), */
> +	TC9564_RESET_INIT0(UART, 16),
> +	TC9564_RESET_INIT0(MSIGEN, 18),
> +
> +	TC9564_RESET_INIT0(MAC0_MAC, 7),
> +	TC9564_RESET_INIT0(MAC0_PMA, 30),
> +	TC9564_RESET_INIT0(MAC0_XPCS, 31),
> +
> +	TC9564_RESET_INIT1(MAC1_MAC, 7),
> +	TC9564_RESET_INIT1(MAC1_PMA, 30),
> +	TC9564_RESET_INIT1(MAC1_XPCS, 31),
> +};
> +#define TC9564_RESET_COUNT	ARRAY_SIZE(tc9564_reset)
> +
> +static const struct tc9564_resets *
> +rcdev_to_tc9564_resets(struct reset_controller_dev *rcdev)
> +{
> +	return container_of_const(rcdev, struct tc9564_resets, rcdev);
> +}
> +
> +static int tc9564_reset_manage(struct reset_controller_dev *rcdev,
> +			       unsigned long id, bool assert)
> +{
> +	const struct tc9564_resets *resets = rcdev_to_tc9564_resets(rcdev);
> +
> +	if (id < rcdev->nr_resets) {

This check is superfluous.

fwnode_reset_simple_xlate() already checked this before a reset control
was even created.

> +		const struct tc9564_reset *reset = &tc9564_reset[id];
> +		u32 mask = reset->mask;
> +
> +		if (mask)

This check is superfluous, as every element in the tc9564_reset array
has mask set.

> +			return regmap_update_bits(resets->regmap,
> +						  reset->offset, mask,
> +						  assert ? mask : 0);
> +	}
> +
> +	dev_err(rcdev->dev, "invalid reset (%s id %lu)\n",
> +		assert ? "assert" : "deassert", id);
> +
> +	return -ENXIO;

This branch is never taken.

> +}
> +
[...]
> +static struct tc9564_resets *tc9564_reset_init(struct device *dev)
> +{
> +	struct reset_controller_dev *rcdev;
> +	struct tc9564_resets *resets;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	regmap = syscon_node_to_regmap(dev_of_node(dev->parent));
> +	if (IS_ERR(regmap)) {
> +		dev_err(dev, "failed to get config regmap\n");
> +		return ERR_CAST(regmap);
> +	}
> +
> +	resets = devm_kzalloc(dev, sizeof(*resets), GFP_KERNEL);
> +	if (!resets)
> +		return ERR_PTR(-ENOMEM);
> +	resets->regmap = regmap;
> +
> +	rcdev = &resets->rcdev;
> +	rcdev->ops = &tc9564_reset_control_ops;
> +	rcdev->owner = THIS_MODULE;
> +	rcdev->dev = dev;
> +	rcdev->of_node = dev_of_node(dev);
> +	rcdev->nr_resets = TC9564_RESET_COUNT;
> +
> +	ret = devm_reset_controller_register(dev, rcdev);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return resets;
> +}
> +
> +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);
> +
> +	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);

Should this be done by the individual drivers?
What happens if the reset controller driver is unbound while e.g. one
of the MACs is active?

regards
Philipp