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

[Philipp Zabel]

On Di, 2026-11-03 at 18:43 -0600, Alex Elder wrote:
> Define a new platform driver that manages reset signals implemented by
> the Toshiba TC9564 SoC.  There are 13 reset signals available, but
> currently only the resets that are (or will soon be) used are defined
> to be controllable.
> 
> Two registers manage the state of the resets.  The registers are
> accessed via a regmap supplied by a system controller.  Access to
> the memory region is provided via a BAR on a PCIe endpoint function
> embedded in the TC9564 SoC.
> 
> Co-developed-by: Daniel Thompson <daniel@xxxxxxxxxxxx>
> Signed-off-by: Daniel Thompson <daniel@xxxxxxxxxxxx>
> Signed-off-by: Alex Elder <elder@xxxxxxxxxxxx>
> ---
>  MAINTAINERS                  |   7 ++
>  drivers/reset/Kconfig        |  12 ++++
>  drivers/reset/Makefile       |   1 +
>  drivers/reset/reset-tc9564.c | 135 +++++++++++++++++++++++++++++++++++
>  4 files changed, 155 insertions(+)
>  create mode 100644 drivers/reset/reset-tc9564.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ee01aed56508d..8389b11fe6e1a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -28355,6 +28355,13 @@ F:	Documentation/devicetree/bindings/soc/toshiba/toshiba,tc9564-config-syscon.ya
>  F:	drivers/soc/toshiba/tc9564-syscon.c
>  F:	include/dt-bindings/soc/toshiba,tc9564.h
>  
> +TOSHIBA TC9564 RESET DRIVER
> +M:	Lorenzo Bianconi <lorenzo.bianconi@xxxxxxxxxxxxxxxx>
> +M:	Alex Elder <elder@xxxxxxxxxx>
> +M:	Daniel Thompson <danielt@xxxxxxxxxx>
> +S:	Maintained
> +F:	drivers/reset/reset-tc9564.c
> +
>  TOSHIBA WMI HOTKEYS DRIVER
>  M:	Azael Avalos <coproscefalo@xxxxxxxxx>
>  L:	platform-driver-x86@xxxxxxxxxxxxxxx
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index eda9c698a95f3..8c1b08f4eebe6 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 SOC_TOSHIBA_TC9564_SYSCON
> +	select MFD_SYSCON
> +	default SOC_TOSHIBA_TC9564_SYSCON
> +	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.

Drop the second sentence, explaining driver details to a user that just
needs to decide whether to enable the driver or not is not helpful.

> +
>  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..fcd58d5f1962f
> --- /dev/null
> +++ b/drivers/reset/reset-tc9564.c
> @@ -0,0 +1,135 @@
[...]
> +static void reset_manage(struct reset_controller_dev *rcdev,
> +			 unsigned long id, bool assert)
> +{
> +	const struct tc9564_reset *reset = &tc9564_reset[id];
> +	struct regmap *regmap = dev_get_drvdata(rcdev->dev);
> +	u32 mask = reset->mask;
> +
> +	regmap_update_bits(regmap, reset->offset, mask, assert ? mask : 0);

Just propagating the error value ...

> +}
> +
> +static int reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	reset_manage(rcdev, id, true);
> +
> +	return 0;

... makes this even shorter.

> +}
> +
> +static int reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	reset_manage(rcdev, id, false);
> +
> +	return 0;
> +}
> +
> +static int reset_status(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	const struct tc9564_reset *reset = &tc9564_reset[id];
> +	struct regmap *regmap = dev_get_drvdata(rcdev->dev);
> +	u32 val;
> +
> +	regmap_read(regmap, reset->offset, &val);
> +
> +	return val & reset->mask ? 1 : 0;

Consider using regmap_test_bits() here.

> +}
> +
> +static const struct reset_control_ops reset_control_ops = {
> +	.assert = reset_assert,
> +	.deassert = reset_deassert,
> +	.status = reset_status,
> +};
> +
> +static int tc9564_reset_probe(struct platform_device *pdev)
> +{
> +	struct reset_controller_dev *rcdev;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *parent_np;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	parent_np = dev_of_node(dev->parent);
> +	if (!parent_np)
> +		return dev_err_probe(dev, -EINVAL, "no devicetree node\n");
> +
> +	/* Note: MMIO regmap (no error checking needed when used) */

I suggest you just add the unneeded error checks and drop this comment.

> +	regmap = device_node_to_regmap(parent_np);

Can't you just use "dev_get_regmap(dev->parent, NULL)" ?

regards
Philipp