Re: [RFC PATCH v6 7/9] accel/rocket: add RK3576 NPU (RKNN) support

[Igor Paunovic <royalnet026@gmail.com>]

Hi Jiaxing,

No need to apologise - you found it, and you found it properly.  Diffing
an ordered trace of every register write against the vendor driver on
the same board is the right tool for exactly this class of problem, and
a 12 bit versus 16 bit field in a header derived from another SoC is not
something a reviewer was going to catch by reading.

I had gone through v6 with the RK3588 side in mind and had a list of
comments on the poll path in 7/9.  Most of it goes away with the
polling, so I will not spend your time on it.

One item outlives it, because it is not part of the poll machinery.  In
rocket_core_init(), the new multi-power-domain attach returns without
unwinding rocket_job_init():

> +	if (core->soc->multi_power_domain) {
> +		struct dev_pm_domain_list *pd_list;
> +
> +		err = devm_pm_domain_attach_list(dev, NULL, &pd_list);
> +		if (err < 0)
> +			return dev_err_probe(dev, err,
> +					     "failed to attach NPU power domains\n");
> +	}

The path immediately above it shows what is missing: rocket_job_init()'s
own failure path puts the iommu_group reference back before returning.
If the attach fails here, the scheduler, the ordered workqueue and that
iommu_group reference all stay behind.  Since RK3576 still needs the
attach in v7, I expect the same shape to survive the rewrite.

Smaller, and it may disappear anyway now that you are splitting 6/9: the
commit message says nothing changes for RK3588, but struct rocket_core's
clks[] grows from 4 to 6 there while the two extra names only arrive in
7/9.  Either a line in the message or moving the growth to the patch
that uses it.

For v7 on my side: once the poll is gone, the only change my hardware
executes is the job_lock move, which you are taking out of the series
anyway.  I am happy to run the series on all three cores here - probe,
multi-task jobs, all cores in parallel, a forced timeout and reset, and
runtime-PM cycling checked against a bit-exact oracle - and report what
I see.  I would rather send you results than a tag that covers less than
it looks like it does.

Igor