From: Brian Masney <masneyb@kernel.org>
To: Rosen Penev <rosenp@gmail.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
Stephen Boyd <sboyd@kernel.org>,
Brian Masney <bmasney+clk@redhat.com>,
Jerome Brunet <jbrunet+clk@baylibre.com>,
Alban Bedel <albeu@free.fr>,
Antony Pavlov <antonynpavlov@gmail.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org,
linux-clk@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 0/6] clk: add a driver for the ath79 PLL controller
Date: Mon, 9 Nov 2026 21:25:10 -0500 [thread overview]
Message-ID: <avKAhjtbi85dpGoV@x1g10> (raw)
In-Reply-To: <20261110013646.1582970-1-rosenp@gmail.com>
Hi Rosen,
On Mon, Nov 09, 2026 at 05:36:40PM -0800, Rosen Penev wrote:
> The clocks of the Atheros/Qualcomm ath79 MIPS SoCs are set up by
> arch/mips/ath79/clock.c, a CLK_OF_DECLARE driver that registers every
> clock before the driver core is up. This series replaces it with a new
> driver in drivers/clk.
>
> Only the reference and CPU clocks are registered early, with
> CLK_OF_DECLARE_DRIVER(), because the MIPS timer needs the CPU rate from
> plat_time_init(). A platform driver bound to the same node registers the
> DDR, AHB, MDIO, UART1 and PCIe clocks. Until it does, the early provider
> returns -EPROBE_DEFER for them. clk-mt7621 uses the same scheme.
>
> Patches 1 and 2 move the two remaining non-clock duties out of the arch
> clock code, so that it only uses its own registers and the clk API: the
> QCA956x timer interrupt workaround, and the reference clock that is
> read from the bootstrap pins when the device tree has no rate. Patches 3
> and 4 add the UART1 and PCIe PLL clock IDs. Patch 5 adds the new driver,
> and patch 6 removes the old code.
>
> Tested on a TP-Link Archer C7 v2 (QCA9558) with an LLVM build. The CPU,
> DDR, AHB and reference rates (960, 800, 266.67 and 40 MHz) match those
> of the old code. The serial console, both GMACs, the PCIe PLL with a
> QCA9880 behind it, and the WMAC all work, and no devices are left
> deferred. The other SoCs are not tested on hardware. For each SoC the
> rate calculation was compiled in userspace together with the old code
> and fed 2 million random register sets. The results are identical,
> except where the old code overflowed 32 bits with register values that
> no real setup uses.
>
> Changes in v2:
> - Post the new driver as one patch, instead of moving
> arch/mips/ath79/clock.c to drivers/clk and cleaning it up over six
> patches (review feedback).
> - Register only the reference and CPU clocks with CLK_OF_DECLARE, and
> the others from a platform driver (review feedback).
> - Describe the SoCs with match data, and read the CPU, DDR and AHB
> selection of AR934x and later with one function.
> - Keep mapping the AR934x SRIF block from its fixed address. It lies
> inside the wireless MAC registers, which ath9k claims, so it can't be
> a reg entry of the PLL node.
> - Drop the clkdev registrations, which have no users.
>
> Claude Code (Claude Opus 5.5) was used while preparing this series. It
> wrote most of the new driver from the old one, together with the
> userspace test of the rate calculation. I reviewed and tested every
> change and take responsibility for it.
40 minutes elapsed from when I asked for changes to when a fully formed
new driver was posted with major changes. Your cover letter says this
v2 was tested on real hardware, however I am honestly a bit skeptical
given the short time frame. On top of that, that's not enough time to
also go through what all Claude did for you. Please slow down!!
I'm going to pass on reviewing this version. I want you to take some
time to test this on real hardware, and understand the code. If after
that you truly have no additional changes, then I'll reconsider
reviewing this version.
Also, if you want to help, then consider helping with reviews on patches
from other people, in particular with the large series for the new SoC
vendors. It's a lot of work reviewing the flood of patches from
everyone. You can see the backlog for the clk core on the kernel.org
patchwork server.
Brian
next prev parent reply other threads:[~2026-11-10 2:25 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-10 1:36 Rosen Penev
2026-11-10 1:36 ` [PATCH v2 1/6] MIPS: ath79: apply the QCA956x timer workaround in setup Rosen Penev
2026-11-10 1:36 ` [PATCH v2 2/6] MIPS: ath79: move the bootstrap reference clock to setup Rosen Penev
2026-11-10 1:36 ` [PATCH v2 3/6] dt-bindings: clock: ath79: add the UART1 clock Rosen Penev
2026-11-10 1:36 ` [PATCH v2 4/6] dt-bindings: clock: ath79: add the PCIe PLL clock Rosen Penev
2026-11-10 1:36 ` [PATCH v2 5/6] clk: add a driver for the Atheros/Qualcomm ath79 PLL controller Rosen Penev
2026-11-10 1:36 ` [PATCH v2 6/6] MIPS: ath79: remove the old clock code Rosen Penev
2026-11-10 2:25 ` Brian Masney [this message]
2026-11-10 2:35 ` [PATCH v2 0/6] clk: add a driver for the ath79 PLL controller Rosen Penev
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=avKAhjtbi85dpGoV@x1g10 \
--to=masneyb@kernel.org \
--cc=albeu@free.fr \
--cc=antonynpavlov@gmail.com \
--cc=bmasney+clk@redhat.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=jbrunet+clk@baylibre.com \
--cc=krzk+dt@kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=robh@kernel.org \
--cc=rosenp@gmail.com \
--cc=sboyd@kernel.org \
--cc=tsbogend@alpha.franken.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®