On Mon, May 19, 2025 at 03:43:37PM GMT, Tomeu Vizoso wrote:
> +#endif
> diff --git a/drivers/accel/rocket/rocket_device.c
> b/drivers/accel/rocket/rocket_device.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..bb469ac87d36249157f4ba9d9f7106ad558309e4
> --- /dev/null
> +++ b/drivers/accel/rocket/rocket_device.c
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright 2024-2025 Tomeu Vizoso <to...@tomeuvizoso.net> */
> +
> +#include <linux/clk.h>
> +#include <linux/dev_printk.h>
> +
> +#include "rocket_device.h"
> +
> +int rocket_device_init(struct rocket_device *rdev)
> +{
> + struct device *dev = rdev->cores[0].dev;
> + int err;
> +
> + rdev->clk_npu = devm_clk_get(dev, "npu");
> + if (IS_ERR(rdev->clk_npu)) {
> + err = PTR_ERR(rdev->clk_npu);
> + dev_err(dev, "devm_clk_get failed %d for clock npu\n", err);
> + return err;
> + }
That's probe path? so use standard syntax:
return dev_err_probe(). One line instead of four.
> +
> + rdev->pclk = devm_clk_get(dev, "pclk");
> + if (IS_ERR(rdev->pclk)) {
> + err = PTR_ERR(rdev->pclk);
> + dev_err(dev, "devm_clk_get failed %d for clock pclk\n", err);
> + return err;
Same here... except that this should be blk API and entire function gets
smaller.
> + }
> +
> + /* Initialize core 0 (top) */
> + err = rocket_core_init(&rdev->cores[0]);
> + if (err)
> + return err;
> +
> + return 0;
> +}
...
> +static int rocket_device_runtime_resume(struct device *dev)
> +{
> + struct rocket_device *rdev = dev_get_drvdata(dev);
> + int core = find_core_for_dev(dev);
> + int err = 0;
> +
> + if (core < 0)
> + return -ENODEV;
> +
> + if (core == 0) {
> + err = clk_prepare_enable(rdev->clk_npu);
> + if (err) {
> + dev_err(dev, "clk_prepare_enable failed %d for clock
> npu\n", err);
> + return err;
> + }
> +
> + err = clk_prepare_enable(rdev->pclk);
> + if (err) {
> + dev_err(dev, "clk_prepare_enable failed %d for clock
> pclk\n", err);
> + goto error_clk_npu;
> + }
> + }
> +
> + err = clk_prepare_enable(rdev->cores[core].a_clk);
> + if (err) {
> + dev_err(dev, "clk_prepare_enable failed %d for a_clk in core
> %d\n", err, core);
> + goto error_pclk;
> + }
> +
> + err = clk_prepare_enable(rdev->cores[core].h_clk);
> + if (err) {
> + dev_err(dev, "clk_prepare_enable failed %d for h_clk in core
> %d\n", err, core);
> + goto error_a_clk;
> + }
All four above calls could be just one call with bulk API.
> +
> + return 0;
> +
> +error_a_clk:
> + clk_disable_unprepare(rdev->cores[core].a_clk);
> +
> +error_pclk:
> + if (core == 0)
> + clk_disable_unprepare(rdev->pclk);
> +
> +error_clk_npu:
> + if (core == 0)
> + clk_disable_unprepare(rdev->clk_npu);
And all this would be gone...
> +
> + return err;
Best regards,
Krzysztof
