linux-kernel - Re: [PATCH v4 09/18] reset: thead: Add TH1520 reset controller driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f79109bad50ed926af7ffc06bc3ab155fe6224ed.camel@pengutronix.de>
Date: Wed, 29 Jan 2025 13:04:53 +0100
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Michal Wilczynski <m.wilczynski@...sung.com>, mturquette@...libre.com, 
 sboyd@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
  drew@...7.com, guoren@...nel.org, wefu@...hat.com,
 jassisinghbrar@...il.com,  paul.walmsley@...ive.com, palmer@...belt.com,
 aou@...s.berkeley.edu,  frank.binns@...tec.com, matt.coster@...tec.com, 
 maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
  airlied@...il.com, simona@...ll.ch, ulf.hansson@...aro.org,
 jszhang@...nel.org,  m.szyprowski@...sung.com
Cc: linux-clk@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org, 
	dri-devel@...ts.freedesktop.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v4 09/18] reset: thead: Add TH1520 reset controller
 driver

On Di, 2025-01-28 at 20:48 +0100, Michal Wilczynski wrote:
> Add reset controller driver for the T-HEAD TH1520 SoC that manages
> hardware reset lines for various subsystems. The driver currently
> implements support for GPU reset control, with infrastructure in place
> to extend support for NPU and Watchdog Timer resets in future updates.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@...sung.com>
> ---
>  MAINTAINERS | 1 +
>  drivers/reset/Kconfig | 10 ++
>  drivers/reset/Makefile | 1 +
>  drivers/reset/reset-th1520.c | 178 +++++++++++++++++++++++++++++++++++
>  4 files changed, 190 insertions(+)
>  create mode 100644 drivers/reset/reset-th1520.c
> 
[...]
> diff --git a/drivers/reset/reset-th1520.c b/drivers/reset/reset-th1520.c
> new file mode 100644
> index 000000000000..48afbc9f1cdd
> --- /dev/null
> +++ b/drivers/reset/reset-th1520.c
> @@ -0,0 +1,178 @@
[...]
> +static int th1520_reset_xlate(struct reset_controller_dev *rcdev,
> + const struct of_phandle_args *reset_spec)
> +{
> + unsigned int index = reset_spec->args[0];
> +
> + /* currently, only GPU reset is implemented in this driver */
> + if (index == TH1520_RESET_ID_GPU)
> + return index;
> +
> + return -EOPNOTSUPP;

It is customary to return -EINVAL for unsupported resets.

Further, you don't have to implement a custom .of_xlate at all.
With nr_resets being set to 1, and because TH1520_RESET_ID_GPU happens
to be 0, the of_reset_simple_xlate() default implementation will do
exactly the same.

[...]
> +static int th1520_reset_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct th1520_reset_priv *priv;
> + void __iomem *base;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + priv->map = devm_regmap_init_mmio(dev, base,
> + &th1520_reset_regmap_config);
> + if (IS_ERR(priv->map))
> + return PTR_ERR(priv->map);
> +
> + mutex_init(&priv->gpu_seq_lock);
> +
> + priv->rcdev.owner = THIS_MODULE;
> + priv->rcdev.nr_resets = 1;
> + priv->rcdev.ops = &th1520_reset_ops;
> + priv->rcdev.of_node = dev->of_node;
>    A. > + priv->rcdev.of_xlate = th1520_reset_xlate;
> + priv->rcdev.of_reset_n_cells = 1;

You could just drop these two lines again. With that,

Reviewed-by: Philipp Zabel <p.zabel@...gutronix.de>

regards
Philipp