From: Brian Masney <bmasney@redhat.com>
To: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com>
Cc: Drew Fustini <dfustini@oss.tenstorrent.com>,
Joel Stanley <jms@oss.tenstorrent.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>,
Philipp Zabel <p.zabel@pengutronix.de>,
linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
joel@jms.id.au, fustini@kernel.org, mpe@kernel.org,
mpe@oss.tenstorrent.com, npiggin@oss.tenstorrent.com,
agross@kernel.org, agross@oss.tenstorrent.com
Subject: Re: [PATCH v6 3/3] clk: tenstorrent: Add Atlantis clock controller driver
Date: Tue, 17 Feb 2026 19:35:09 -0500 [thread overview]
Message-ID: <aZUJPb2jQIx2evWN@redhat.com> (raw)
In-Reply-To: <CAEev2e8hwN1FBR6yMr_NeZrFx3BXNz88RHfHLhSM=GrpExsUyw@mail.gmail.com>
On Tue, Feb 17, 2026 at 05:29:56PM -0600, Anirudh Srinivasan wrote:
> Hi Brian,
>
> On Tue, Feb 17, 2026 at 5:22 PM Brian Masney <bmasney@redhat.com> wrote:
> > >
> > > We have a group of gate clocks that have a single enable bit shared
> > > among them (instead of individual enable bits for each clock). We need
> > > to keep track of the number of clocks within a group that have
> > > requested an enable, and only unset the bit if all the clocks are
> > > disabled. share_count is used to keep track of this. It gets updated
> > > by each clock. Hence it's a pointer (and the mutexes around access to
> > > it).
> >
> > The code currently has:
> >
> > struct atlantis_clk_gate_shared_config {
> > ...
> > unsigned int *share_count;
> > }
> >
> > That pointer is dereferenced like this in several places:
> >
> > need_enable = (*gate->config.share_count)++ == 0;
> >
> > I don't see why the pointer is needed. Can you drop the pointer
> > and the dereference like this?
> >
> > struct atlantis_clk_gate_shared_config {
> > ...
> > unsigned int share_count;
> > }
> >
> > need_enable = gate->config.share_count++ == 0;
> >
>
> In this case, wouldn't each atlantis_clk_gate_shared end up getting
> its own copy of share_count? Which is not what we want. Or maybe I'm
> not quite understanding what you're saying.
>
> Every time we create a group of these shared gate clks, we create a
> refcnt variable like this and pass the var to all clks that share it.
OK, I see now. Thanks for the explanation.
Brian
WARNING: multiple messages have this Message-ID (diff)
From: Brian Masney <bmasney@redhat.com>
To: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com>
Cc: Drew Fustini <dfustini@oss.tenstorrent.com>,
Joel Stanley <jms@oss.tenstorrent.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>,
Philipp Zabel <p.zabel@pengutronix.de>,
linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
joel@jms.id.au, fustini@kernel.org, mpe@kernel.org,
mpe@oss.tenstorrent.com, npiggin@oss.tenstorrent.com,
agross@kernel.org, agross@oss.tenstorrent.com
Subject: Re: [PATCH v6 3/3] clk: tenstorrent: Add Atlantis clock controller driver
Date: Tue, 17 Feb 2026 19:35:09 -0500 [thread overview]
Message-ID: <aZUJPb2jQIx2evWN@redhat.com> (raw)
In-Reply-To: <CAEev2e8hwN1FBR6yMr_NeZrFx3BXNz88RHfHLhSM=GrpExsUyw@mail.gmail.com>
On Tue, Feb 17, 2026 at 05:29:56PM -0600, Anirudh Srinivasan wrote:
> Hi Brian,
>
> On Tue, Feb 17, 2026 at 5:22 PM Brian Masney <bmasney@redhat.com> wrote:
> > >
> > > We have a group of gate clocks that have a single enable bit shared
> > > among them (instead of individual enable bits for each clock). We need
> > > to keep track of the number of clocks within a group that have
> > > requested an enable, and only unset the bit if all the clocks are
> > > disabled. share_count is used to keep track of this. It gets updated
> > > by each clock. Hence it's a pointer (and the mutexes around access to
> > > it).
> >
> > The code currently has:
> >
> > struct atlantis_clk_gate_shared_config {
> > ...
> > unsigned int *share_count;
> > }
> >
> > That pointer is dereferenced like this in several places:
> >
> > need_enable = (*gate->config.share_count)++ == 0;
> >
> > I don't see why the pointer is needed. Can you drop the pointer
> > and the dereference like this?
> >
> > struct atlantis_clk_gate_shared_config {
> > ...
> > unsigned int share_count;
> > }
> >
> > need_enable = gate->config.share_count++ == 0;
> >
>
> In this case, wouldn't each atlantis_clk_gate_shared end up getting
> its own copy of share_count? Which is not what we want. Or maybe I'm
> not quite understanding what you're saying.
>
> Every time we create a group of these shared gate clks, we create a
> refcnt variable like this and pass the var to all clks that share it.
OK, I see now. Thanks for the explanation.
Brian
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2026-02-18 0:35 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-16 22:16 [PATCH v6 0/3] Add Tenstorrent Atlantis Clock/Reset Controller Anirudh Srinivasan
2026-02-16 22:16 ` Anirudh Srinivasan
2026-02-16 22:16 ` [PATCH v6 1/3] dt-bindings: clk: tenstorrent: Add tenstorrent,atlantis-prcm-rcpu Anirudh Srinivasan
2026-02-16 22:16 ` Anirudh Srinivasan
2026-02-17 7:49 ` Krzysztof Kozlowski
2026-02-17 7:49 ` Krzysztof Kozlowski
2026-02-16 22:16 ` [PATCH v6 2/3] reset: tenstorrent: Add reset controller for Atlantis Anirudh Srinivasan
2026-02-16 22:16 ` Anirudh Srinivasan
2026-02-17 11:59 ` Philipp Zabel
2026-02-17 11:59 ` Philipp Zabel
2026-02-17 22:49 ` Anirudh Srinivasan
2026-02-17 22:49 ` Anirudh Srinivasan
2026-02-18 8:54 ` Philipp Zabel
2026-02-18 8:54 ` Philipp Zabel
2026-02-16 22:16 ` [PATCH v6 3/3] clk: tenstorrent: Add Atlantis clock controller driver Anirudh Srinivasan
2026-02-16 22:16 ` Anirudh Srinivasan
2026-02-17 16:14 ` Brian Masney
2026-02-17 16:14 ` Brian Masney
2026-02-17 23:12 ` Anirudh Srinivasan
2026-02-17 23:12 ` Anirudh Srinivasan
2026-02-17 23:22 ` Brian Masney
2026-02-17 23:22 ` Brian Masney
2026-02-17 23:29 ` Anirudh Srinivasan
2026-02-17 23:29 ` Anirudh Srinivasan
2026-02-18 0:35 ` Brian Masney [this message]
2026-02-18 0:35 ` Brian Masney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aZUJPb2jQIx2evWN@redhat.com \
--to=bmasney@redhat.com \
--cc=agross@kernel.org \
--cc=agross@oss.tenstorrent.com \
--cc=asrinivasan@oss.tenstorrent.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dfustini@oss.tenstorrent.com \
--cc=fustini@kernel.org \
--cc=jms@oss.tenstorrent.com \
--cc=joel@jms.id.au \
--cc=krzk+dt@kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=mpe@kernel.org \
--cc=mpe@oss.tenstorrent.com \
--cc=mturquette@baylibre.com \
--cc=npiggin@oss.tenstorrent.com \
--cc=p.zabel@pengutronix.de \
--cc=robh@kernel.org \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.