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

[Conor Dooley]

On Wed, Jan 28, 2026 at 09:42:42AM -0600, Anirudh Srinivasan wrote:
> Hi Conor,
> 
> On Wed, Jan 28, 2026 at 9:02 AM Conor Dooley <conor at kernel.org> wrote:
> >
> > 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 at 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 at oss.tenstorrent.com>
> > > > > ---
> > > > 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.
> 
> Yes, it seems like that example actually needs one regmap for
> configuring and another for checking lock status of the PLL, but in
> our case only one is enough.
> 
> > > 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.
> 
> Understood.
> 
> >
> > 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?
> 
> I haven't been able to completely test the clock driver for the other
> subsystems yet, so that hasn't been posted yet. One of the comments on
> the previous versions of this series was to document the complete
> bindings, so I've tried to document all the 4 PRCMs.

Right. Looking at the mail from Krzysztof, I suspect he meant to
completely document and explain the rcpu prcm, not all of the prcms (he
couldn't really know they existed, based on your v1, right?).
I'd suggest you drop the !rcpu stuff for now, and submit it when you
have the driver for them ready to go. That's typically what's done to
avoid introducing bindings that need to be changed once the driver
actually turns up, since as you say you've not actually tested the
driver for those prcms.

> 
> >
> > > 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.
> 
> In the clock driver, you'd end up having something like this
> 
> static const struct clk_parent_data osc_24m_clk[] = {
>         { .index = 0 },
>  };
> .
> .
> static const struct clk_parent_data pcie_pll[] = {
>         { .index = 0 },
> };
> 
> Both point to index 0, but one is for the rcpu prcm and another is for
> the pcie prcm. To me, this felt like it might confuse someone looking
> at the driver. But it doesn't affect functionality in any manner.

If you think it is going to be confusing, then move the bits common to
!rcpu and rcpu prcms to a file, with the unique bits in dedicated files
perhaps? It seems like they'd be fairly different even with your current
scheme and keeping them apart would aid readability of the driver in
either case?
You can do this extraction as part of adding the !rcpu code, that
doesn't need to be done for the rcpu stuff since the concept of "common"
wouldn't exist in upstream until the !rcpu stuff arrives.

Cheers,
Conor.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20260128/9ff8593b/attachment.sig>