Re: [PATCH v3 1/3] dt-bindings: clk: tenstorrent: Add tenstorrent,atlantis-prcm

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3044 bytes --]

On Tue, Jan 27, 2026 at 05:39:33PM -0600, Anirudh Srinivasan wrote:
> Hi Conor,
> 
> On Tue, Jan 27, 2026 at 1:58 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Mon, Jan 26, 2026 at 03:07:14PM -0600, Anirudh Srinivasan wrote:
> > > Document bindings for Tenstorrent Atlantis PRCM that manages clocks
> > > and resets. This block is instantiated 4 times in the SoC.
> > > This commit documents the clocks from the RCPU PRCM block.
> > >
> > > Signed-off-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com>
> > > ---
> > >  .../bindings/clock/tenstorrent,atlantis-prcm.yaml  |  82 ++++++++++++++++
> > >  MAINTAINERS                                        |   2 +
> > >  .../dt-bindings/clock/tenstorrent,atlantis-prcm.h  | 103 +++++++++++++++++++++
> > >  3 files changed, 187 insertions(+)
> > >
> > > +
> > > +  tenstorrent,prcm-rcpu:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description:
> > > +      Phandle reference to RCPU prcm, needed by other 3 prcms (PCIe, MM, HSIO)
> > > +      as the control registers for the PLLs that drive these subsystems are in
> > > +      RCPU prcm's range
> >
> > This is pretty suspect sounding, if the PLLs for !rcpu are controlled in
> > the rcpu register region, why is it not a clock parent for the !rcpu
> > prcms?
> 
> I saw another clock driver doing it in the manner I did [1], and

The example is using it just to check lock status, which I think is
different than what you've got here? What you wrote implies that the
whole configuration for these PLLs is in that register region.

> thought that it would make writing the bindings and the clock driver
> simpler. Each prcm node would have a single input clock (otherwise
> there would be a differing number of input clocks for each prcm node).
> 
> What would you suggest that I do?

I suggest that you model the clock tree correctly in devicetree, even if
that makes things more complicated. One prcm node having more input
clocks isn't something to be afraid of, it should be pretty
straightforward to handle in both devicetree and driver, and is not any
more complicated than having to deal with the syscon phandle that you
use at the moment.

btw, where is the code for the !rcpu clock controllers? AFAICT, this
series only has the rcpu portion and I can't find the code that actually
uses the phandle. Why is the patch documenting stuff that has no user?

> This would also avoid having the clock tree in the driver contain
> multiple entries for some of the PLLs (one in the rcpu subsystem where
> it is defined and another where the same clock is referred with {
> .index = 0 }) which could become confusing.

I don't really understand what you mean by this. Can you elaborate?
If you mean that multiple clocks produced by the prcm would all use
index = 0 as their parent, that does not sound abnormal to me. Without
being able to see the !rcpu driver implementations, I can't even make
guesses as to what the clock tree looks like.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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