[RFC PATCH v6 6/9] accel/rocket: select the per-core clock and reset counts from match data
Diederik de Haas
diederik at cknow-tech.com
Fri Aug 7 01:10:08 PDT 2026
Hi Jiaxing,
On Thu Aug 6, 2026 at 8:34 AM CEST, Jiaxing Hu wrote:
> The RK3576 carries the same RKNN block with a different set of clocks and
nit: same RKNN block as what? And if it has a different set of clocks/resets,
is it still the same (HW) block?
> resets, so the counts cannot stay compile-time constants. Add a soc_data
> struct to the of_device_id match data and take the bulk counts from it.
> RK3588 keeps four clocks and two resets, so nothing changes for it.
>
> rocket_core_reset() is switched over as well. It is the same array, and
> leaving it on ARRAY_SIZE() would walk entries that were never acquired
> once a SoC asks for fewer.
but now comes the point I actually want to make:
> While moving through this path, take the two register writes in
> rocket_job_handle_irq() under job_lock. rocket_job_hw_submit() writes
> OPERATION_ENABLE from inside the lock, so a completion handled outside it
> can write the zero after that one and stop a task that has just started.
This should be its own separate commit (with a Fixes tag) ... which
probably doesn't have/should be part of this (RFC) series?
>
> Signed-off-by: Jiaxing Hu <gahing at gahingwoo.com>
> ---
> drivers/accel/rocket/rocket_core.c | 8 ++++----
> drivers/accel/rocket/rocket_core.h | 9 ++++++++-
> drivers/accel/rocket/rocket_drv.c | 12 +++++++++---
> drivers/accel/rocket/rocket_job.c | 12 +++++++++---
> 4 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> index 5dd260bac..b202d1581 100644
> --- a/drivers/accel/rocket/rocket_core.c
> +++ b/drivers/accel/rocket/rocket_core.c
> @@ -23,7 +23,7 @@ int rocket_core_init(struct rocket_core *core)
>
> core->resets[0].id = "srst_a";
> core->resets[1].id = "srst_h";
> - err = devm_reset_control_bulk_get_exclusive(&pdev->dev, ARRAY_SIZE(core->resets),
> + err = devm_reset_control_bulk_get_exclusive(&pdev->dev, core->soc->num_resets,
> core->resets);
> if (err)
> return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
> @@ -32,7 +32,7 @@ int rocket_core_init(struct rocket_core *core)
> core->clks[1].id = "hclk";
> core->clks[2].id = "npu";
> core->clks[3].id = "pclk";
> - err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
> + err = devm_clk_bulk_get(dev, core->soc->num_clks, core->clks);
> if (err)
> return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
>
> @@ -109,9 +109,9 @@ void rocket_core_fini(struct rocket_core *core)
>
> void rocket_core_reset(struct rocket_core *core)
> {
> - reset_control_bulk_assert(ARRAY_SIZE(core->resets), core->resets);
> + reset_control_bulk_assert(core->soc->num_resets, core->resets);
>
> udelay(10);
>
> - reset_control_bulk_deassert(ARRAY_SIZE(core->resets), core->resets);
> + reset_control_bulk_deassert(core->soc->num_resets, core->resets);
> }
> diff --git a/drivers/accel/rocket/rocket_core.h b/drivers/accel/rocket/rocket_core.h
> index f6d738285..0f424bb86 100644
> --- a/drivers/accel/rocket/rocket_core.h
> +++ b/drivers/accel/rocket/rocket_core.h
> @@ -27,16 +27,23 @@
> #define rocket_core_writel(core, reg, value) \
> writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
>
> +/* Per-SoC differences, selected by the of_device_id match data. */
> +struct rocket_soc_data {
> + unsigned int num_clks; /* clk_bulk count */
> + unsigned int num_resets; /* reset_bulk count */
> +};
> +
> struct rocket_core {
> struct device *dev;
> struct rocket_device *rdev;
> + const struct rocket_soc_data *soc;
> unsigned int index;
>
> int irq;
> void __iomem *pc_iomem;
> void __iomem *cna_iomem;
> void __iomem *core_iomem;
> - struct clk_bulk_data clks[4];
> + struct clk_bulk_data clks[6];
> struct reset_control_bulk_data resets[2];
>
> struct iommu_group *iommu_group;
> diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
> index 8bbbce594..6e7dc91c5 100644
> --- a/drivers/accel/rocket/rocket_drv.c
> +++ b/drivers/accel/rocket/rocket_drv.c
> @@ -176,6 +176,7 @@ static int rocket_probe(struct platform_device *pdev)
>
> rdev->cores[core].rdev = rdev;
> rdev->cores[core].dev = &pdev->dev;
> + rdev->cores[core].soc = of_device_get_match_data(&pdev->dev);
> rdev->cores[core].index = core;
>
> rdev->num_cores++;
> @@ -213,8 +214,13 @@ static void rocket_remove(struct platform_device *pdev)
> }
> }
>
> +static const struct rocket_soc_data rk3588_soc_data = {
> + .num_clks = 4,
> + .num_resets = 2,
> +};
> +
> static const struct of_device_id dt_match[] = {
> - { .compatible = "rockchip,rk3588-rknn-core" },
> + { .compatible = "rockchip,rk3588-rknn-core", .data = &rk3588_soc_data },
While it technically fits on 1 line, putting the ``.data = `` part on its
own line is easier to read and likely more future proof.
Cheers,
Diederik
> {}
> };
> MODULE_DEVICE_TABLE(of, dt_match);
> @@ -240,7 +246,7 @@ static int rocket_device_runtime_resume(struct device *dev)
> if (core < 0)
> return -ENODEV;
>
> - err = clk_bulk_prepare_enable(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> + err = clk_bulk_prepare_enable(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
> if (err) {
> dev_err(dev, "failed to enable (%d) clocks for core %d\n", err, core);
> return err;
> @@ -260,7 +266,7 @@ static int rocket_device_runtime_suspend(struct device *dev)
> if (!rocket_job_is_idle(&rdev->cores[core]))
> return -EBUSY;
>
> - clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> + clk_bulk_disable_unprepare(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
>
> return 0;
> }
> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index bb77b6bf0..aa26e2977 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -345,10 +345,15 @@ static void rocket_job_handle_irq(struct rocket_core *core)
> {
> pm_runtime_mark_last_busy(core->dev);
>
> - rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> - rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> + scoped_guard(mutex, &core->job_lock) {
> + /*
> + * Stopping the block belongs under the lock. hw_submit() writes
> + * OPERATION_ENABLE too, and outside the lock this zero can land
> + * after that one and kill a task that has only just started.
> + */
> + rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> + rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
>
> - scoped_guard(mutex, &core->job_lock)
> if (core->in_flight_job) {
> if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> rocket_job_hw_submit(core, core->in_flight_job);
> @@ -360,6 +365,7 @@ static void rocket_job_handle_irq(struct rocket_core *core)
> pm_runtime_put_autosuspend(core->dev);
> core->in_flight_job = NULL;
> }
> + }
> }
>
> static void
More information about the linux-arm-kernel
mailing list