| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGb2v64DmDXaduf7RrYynb+v7TCFA+ni6xPHfyCrwduYW0g=HA@mail.gmail.com>
Date: Fri, 5 Sep 2025 23:19:34 +0800
From: Chen-Yu Tsai <wens@...nel.org>
To: Andre Przywara <andre.przywara@....com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Stephen Boyd <sboyd@...nel.org>,
Jernej Skrabec <jernej@...nel.org>, Samuel Holland <samuel@...lland.org>, linux-sunxi@...ts.linux.dev,
linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/8] clk: sunxi-ng: sun55i-a523-ccu: Add missing NPU
module clock
On Fri, Sep 5, 2025 at 11:14 PM Andre Przywara <andre.przywara@....com> wrote:
>
> On Sun, 31 Aug 2025 01:08:57 +0800
> Chen-Yu Tsai <wens@...nel.org> wrote:
>
> Hi,
>
> > From: Chen-Yu Tsai <wens@...e.org>
> >
> > The main clock controller on the A523/T527 has the NPU's module clock.
> > It was missing from the original submission, likely because that was
> > based on the A523 user manual; the A523 is marketed without the NPU.
>
> Ah, sorry, I missed that one. I think I spotted writable bits in that
> register, but didn't find a clue what this clock was about. Anyway, checked
> the bits against the T527 manual, they match up.
>
> > Also, merge the private header back into the driver code itself. The
> > header only contains a macro containing the total number of clocks.
> > This has to be updated every time a missing clock gets added. Having
> > it in a separate file doesn't help the process. Instead just drop the
> > macro, and thus the header no longer has any reason to exist.
>
> Interesting, looks nice, and solves Krzysztof's complaint the other
> day about the binding header inclusion missing from the driver as well.
> Just one thought:
>
> > Signed-off-by: Chen-Yu Tsai <wens@...e.org>
> > ---
> > drivers/clk/sunxi-ng/ccu-sun55i-a523.c | 21 ++++++++++++++++++---
> > drivers/clk/sunxi-ng/ccu-sun55i-a523.h | 14 --------------
> > 2 files changed, 18 insertions(+), 17 deletions(-)
> > delete mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523.h
> >
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > index 1a9a1cb869e2..88405b624dc5 100644
> > --- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > +++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > @@ -11,6 +11,9 @@
> > #include <linux/module.h>
> > #include <linux/platform_device.h>
> >
> > +#include <dt-bindings/clock/sun55i-a523-ccu.h>
> > +#include <dt-bindings/reset/sun55i-a523-ccu.h>
> > +
>
> Should we have the number #define here, at a more central location? Seems a
> bit buried down in there. And then use a plural name while at it:
>
> #define NUM_CLOCKS CLK_NPU + 1
>
> Alternatively, put .num behind .hws below, so that the last clock and the
> number definition are close together?
I think this works better. One less place to look at.
ChenYu
> Cheers,
> Andre
>
> > #include "../clk.h"
> >
> > #include "ccu_common.h"
> > @@ -25,8 +28,6 @@
> > #include "ccu_nkmp.h"
> > #include "ccu_nm.h"
> >
> > -#include "ccu-sun55i-a523.h"
> > -
> > /*
> > * The 24 MHz oscillator, the root of most of the clock tree.
> > * .fw_name is the string used in the DT "clock-names" property, used to
> > @@ -486,6 +487,18 @@ static SUNXI_CCU_M_HW_WITH_MUX_GATE(ve_clk, "ve", ve_parents, 0x690,
> >
> > static SUNXI_CCU_GATE_HWS(bus_ve_clk, "bus-ve", ahb_hws, 0x69c, BIT(0), 0);
> >
> > +static const struct clk_hw *npu_parents[] = {
> > + &pll_periph0_480M_clk.common.hw,
> > + &pll_periph0_600M_clk.hw,
> > + &pll_periph0_800M_clk.common.hw,
> > + &pll_npu_2x_clk.hw,
> > +};
> > +static SUNXI_CCU_M_HW_WITH_MUX_GATE(npu_clk, "npu", npu_parents, 0x6e0,
> > + 0, 5, /* M */
> > + 24, 3, /* mux */
> > + BIT(31), /* gate */
> > + CLK_SET_RATE_PARENT);
> > +
> > static SUNXI_CCU_GATE_HWS(bus_dma_clk, "bus-dma", ahb_hws, 0x70c, BIT(0), 0);
> >
> > static SUNXI_CCU_GATE_HWS(bus_msgbox_clk, "bus-msgbox", ahb_hws, 0x71c,
> > @@ -1217,6 +1230,7 @@ static struct ccu_common *sun55i_a523_ccu_clks[] = {
> > &bus_ce_sys_clk.common,
> > &ve_clk.common,
> > &bus_ve_clk.common,
> > + &npu_clk.common,
> > &bus_dma_clk.common,
> > &bus_msgbox_clk.common,
> > &bus_spinlock_clk.common,
> > @@ -1343,7 +1357,7 @@ static struct ccu_common *sun55i_a523_ccu_clks[] = {
> > };
> >
> > static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
> > - .num = CLK_NUMBER,
> > + .num = CLK_NPU + 1,
> > .hws = {
> > [CLK_PLL_DDR0] = &pll_ddr_clk.common.hw,
> > [CLK_PLL_PERIPH0_4X] = &pll_periph0_4x_clk.common.hw,
> > @@ -1524,6 +1538,7 @@ static struct clk_hw_onecell_data sun55i_a523_hw_clks = {
> > [CLK_FANOUT0] = &fanout0_clk.common.hw,
> > [CLK_FANOUT1] = &fanout1_clk.common.hw,
> > [CLK_FANOUT2] = &fanout2_clk.common.hw,
> > + [CLK_NPU] = &npu_clk.common.hw,
> > },
> > };
> >
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.h b/drivers/clk/sunxi-ng/ccu-sun55i-a523.h
> > deleted file mode 100644
> > index fc8dd42f1b47..000000000000
> > --- a/drivers/clk/sunxi-ng/ccu-sun55i-a523.h
> > +++ /dev/null
> > @@ -1,14 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0 */
> > -/*
> > - * Copyright 2024 Arm Ltd.
> > - */
> > -
> > -#ifndef _CCU_SUN55I_A523_H
> > -#define _CCU_SUN55I_A523_H
> > -
> > -#include <dt-bindings/clock/sun55i-a523-ccu.h>
> > -#include <dt-bindings/reset/sun55i-a523-ccu.h>
> > -
> > -#define CLK_NUMBER (CLK_FANOUT2 + 1)
> > -
> > -#endif /* _CCU_SUN55I_A523_H */
>
Powered by blists - more mailing lists