Re: [PATCH v2 0/6] clk: add a driver for the ath79 PLL controller

[Rosen Penev]

On Mon, Nov 9, 2026 at 6:54 PM Brian Masney <masneyb@xxxxxxxxxx> wrote:
>
> Hi Rosen,
>
> On Mon, Nov 09, 2026 at 06:35:32PM -0800, Rosen Penev wrote:
> > On Mon, Nov 9, 2026 at 6:25 PM Brian Masney <masneyb@xxxxxxxxxx> wrote:
> > > 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!!
> >
> > This was tested with a linux-next kernel on an Archer C7 v2 hooked up
> > through serial and two ethernet cables (LAN and WAN). There's no
> > difference that I can see between the old and new driver.
>
> The clk_summary debugfs entries would be a good place to compare as
> well.
>
> > That being said, I only have two pieces of hardware on which testing
> > can be done. Both are rather new relatively speaking. Older version of
> > this series is currently live on OpenWrt with seemingly no ill
> > consequences.
> > >
> > > 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.
> >
> > Sashiko had two complaints. I'll fix those in v3.
> > >
> > > 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.
> >
> > I agree. It's quite unfortunate and have no solution to it.
>
> Just some friendly advice: If you keep churning out AI code as quick as
> you can generate it, then your stuff will end up at the very bottom of
> the review queue. There's a long backlog of other changes that need to
> be reviewed.
Sure. I'll wait a bit.
>
> I'm not saying that you need to go review patches, but that goes a
> really long way to building goodwill in the community. Maybe if you post
> a large series like this go help review 2 or 3 patch sets that's in the
> backlog from other people.
>
> But seriously, the main ask from me is to please slow down before you
> post new patches. Provide some more details about how this was tested on
> real hardware.
Sure.
>
> Brian