Linux-Rockchip Archive on lore.kernel.org
 help / color / mirror / Atom feed
* accel/rocket: clks[].id fix - standalone patch, or would you prefer to split it out?
@ 2026-07-29  7:20 Igor Paunovic
       [not found] ` <CAPsqS2STG3ejiYe5AaYM2gODdDtkcvDvKuOerJZ3CJko1PZ=Hg@mail.gmail.com>
  0 siblings, 1 reply; 2+ messages in thread
From: Igor Paunovic @ 2026-07-29  7:20 UTC (permalink / raw)
  To: Jiaxing Hu; +Cc: linux-rockchip, Tomeu Vizoso, Igor Paunovic

Hi Jiaxing,

I have been working with the rocket NPU driver on RK3588 (Orange Pi 5
Plus) and ran into the missing core->clks[].id initialisation in
rocket_core_init().

I noticed it while reading the live clock tree rather than the source:
/sys/kernel/debug/clk/clk_summary shows four "fdab0000.npu" consumer
handles on aclk_npu0 - and likewise on aclk_npu1/aclk_npu2 for the other
two cores - while hclk_npu0, pclk_npu_root and scmi_clk_npu have no
"fdab0000.npu" consumer at all. The driver believes it owns four clocks
but really holds four handles to the AXI clock.

Before writing anything up I went through the lists, and I see you
already fix exactly this as part of

  [RFC PATCH v2 6/8] accel/rocket: add RK3576 NPU (RKNN) support
  Message-Id: <20260718031146.3368811-7-gahing@gahingwoo.com>

so I would rather ask you first than send something that looks like a
duplicate.

The reason I care: I would like to look into OPP/devfreq support for
rocket later on - Jonas Karlman mentioned it as future work in the
RK3568 thread - and today the driver holds no reference at all to the
clock that actually feeds the NPU ("npu", SCMI_CLK_NPU on RK3588). That
is in the way of any frequency scaling work. The NPU still runs anyway -
clk_summary shows scmi_clk_npu with the hardware enable bit set but no
Linux consumer, i.e. firmware keeps it running - so the bug is latent
rather than fatal.

Timing-wise this also seems like a natural moment to ask: Will has
already picked up the two iommu patches from your v2, and after
Alexey's review you mentioned a v3 respin of the accel patch. Since the
RK3576 inference issue is still being debugged, the clks[].id fix would
otherwise sit inside an RFC for a while, even though it is independent
of that work and the bug it fixes is in mainline today.

Would you prefer to:

 (a) split the clks[].id fix out of your series and send it standalone
     with a Fixes: tag? That way it can land independently of the RK3576
     enablement work and be backported. I would be glad to give it a
     Tested-by on RK3588 / Orange Pi 5 Plus.

 (b) or, if you would rather keep your series as it is, I can send the
     standalone fix with you on Cc and a note under the --- pointing at
     your patch.

Either works for me. I mostly want to avoid us duplicating effort, and
you posted it first.

Thanks,
Igor Paunovic

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: accel/rocket: clks[].id fix - standalone patch, or would you prefer to split it out?
       [not found] ` <CAPsqS2STG3ejiYe5AaYM2gODdDtkcvDvKuOerJZ3CJko1PZ=Hg@mail.gmail.com>
@ 2026-07-29  9:38   ` Igor Paunovic
  0 siblings, 0 replies; 2+ messages in thread
From: Igor Paunovic @ 2026-07-29  9:38 UTC (permalink / raw)
  To: Tomeu Vizoso, linux-rockchip, Jiaxing Hu

Hi Tomeu

(re-adding linux-rockchip to Cc, I think it got dropped by accident)

Will do - I just sent the clks[].id fix as a standalone patch:

[PATCH] accel/rocket: request the core clocks by name
https://lore.kernel.org/linux-rockchip/20260729092939.118779-1-royalnet026@gmail.com/

It is intentionally minimal (no per-SoC match data, ARRAY_SIZE and
clks[4] untouched) so it should rebase trivially under both the RK3576
v3 and the RK3568 series.

Jiaxing: your version in 6/8 of course stays correct - with this landed
first, that hunk in v3 just shrinks to the two new CBUF ids.
Thanks,

Igor

On Wed, Jul 29, 2026 at 10:22 AM Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>
> Hi Igor and Jiaxing,
>
> On Wed, Jul 29, 2026 at 9:20 AM Igor Paunovic <royalnet026@gmail.com> wrote:
> >
> > Hi Jiaxing,
> >
> > I have been working with the rocket NPU driver on RK3588 (Orange Pi 5
> > Plus) and ran into the missing core->clks[].id initialisation in
> > rocket_core_init().
> >
> > I noticed it while reading the live clock tree rather than the source:
> > /sys/kernel/debug/clk/clk_summary shows four "fdab0000.npu" consumer
> > handles on aclk_npu0 - and likewise on aclk_npu1/aclk_npu2 for the other
> > two cores - while hclk_npu0, pclk_npu_root and scmi_clk_npu have no
> > "fdab0000.npu" consumer at all. The driver believes it owns four clocks
> > but really holds four handles to the AXI clock.
> >
> > Before writing anything up I went through the lists, and I see you
> > already fix exactly this as part of
> >
> >   [RFC PATCH v2 6/8] accel/rocket: add RK3576 NPU (RKNN) support
> >   Message-Id: <20260718031146.3368811-7-gahing@gahingwoo.com>
> >
> > so I would rather ask you first than send something that looks like a
> > duplicate.
> >
> > The reason I care: I would like to look into OPP/devfreq support for
> > rocket later on - Jonas Karlman mentioned it as future work in the
> > RK3568 thread - and today the driver holds no reference at all to the
> > clock that actually feeds the NPU ("npu", SCMI_CLK_NPU on RK3588). That
> > is in the way of any frequency scaling work. The NPU still runs anyway -
> > clk_summary shows scmi_clk_npu with the hardware enable bit set but no
> > Linux consumer, i.e. firmware keeps it running - so the bug is latent
> > rather than fatal.
> >
> > Timing-wise this also seems like a natural moment to ask: Will has
> > already picked up the two iommu patches from your v2, and after
> > Alexey's review you mentioned a v3 respin of the accel patch. Since the
> > RK3576 inference issue is still being debugged, the clks[].id fix would
> > otherwise sit inside an RFC for a while, even though it is independent
> > of that work and the bug it fixes is in mainline today.
> >
> > Would you prefer to:
> >
> >  (a) split the clks[].id fix out of your series and send it standalone
> >      with a Fixes: tag? That way it can land independently of the RK3576
> >      enablement work and be backported. I would be glad to give it a
> >      Tested-by on RK3588 / Orange Pi 5 Plus.
> >
> >  (b) or, if you would rather keep your series as it is, I can send the
> >      standalone fix with you on Cc and a note under the --- pointing at
> >      your patch.
> >
> > Either works for me. I mostly want to avoid us duplicating effort, and
> > you posted it first.
>
> I would be glad if you could send any fixes upfront, specially if
> getting more SoCs supported will still take some time.
>
> Thanks,
>
> Tomeu
>
> > Thanks,
> > Igor Paunovic

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-29  9:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29  7:20 accel/rocket: clks[].id fix - standalone patch, or would you prefer to split it out? Igor Paunovic
     [not found] ` <CAPsqS2STG3ejiYe5AaYM2gODdDtkcvDvKuOerJZ3CJko1PZ=Hg@mail.gmail.com>
2026-07-29  9:38   ` Igor Paunovic

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox