| 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: <aOCEt7UGpSyWTury@pie>
Date: Sat, 4 Oct 2025 02:21:43 +0000
From: Yao Zi <ziyao@...root.org>
To: Drew Fustini <fustini@...nel.org>
Cc: Rob Herring <robh@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Albert Ou <aou@...s.berkeley.edu>,
Michal Wilczynski <m.wilczynski@...sung.com>,
Alexandre Ghiti <alex@...ti.fr>, devicetree@...r.kernel.org,
Han Gao <gaohan@...as.ac.cn>, Han Gao <rabenda.cn@...il.com>,
linux-kernel@...r.kernel.org, Guo Ren <guoren@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
linux-riscv@...ts.infradead.org, Fu Wei <wefu@...hat.com>
Subject: Re: [PATCH v2 2/5] dt-bindings: reset: thead,th1520-reset: Add
controllers for more subsys
On Fri, Oct 03, 2025 at 03:46:17PM -0700, Drew Fustini wrote:
> On Mon, Sep 15, 2025 at 09:53:28AM +0000, Yao Zi wrote:
> > TH1520 SoC is divided into several subsystems, most of them have
> > distinct reset controllers. Let's document reset controllers other than
> > the one for VO subsystem and IDs for their reset signals.
> >
> > Signed-off-by: Yao Zi <ziyao@...root.org>
>
> Thanks for sending these patches.
>
> > ---
> > .../bindings/reset/thead,th1520-reset.yaml | 8 +-
> > .../dt-bindings/reset/thead,th1520-reset.h | 216 ++++++++++++++++++
> > 2 files changed, 223 insertions(+), 1 deletion(-)
> >
> [snip]
> > diff --git a/include/dt-bindings/reset/thead,th1520-reset.h b/include/dt-bindings/reset/thead,th1520-reset.h
> > index e51d6314d131..68ac52ed69de 100644
> > --- a/include/dt-bindings/reset/thead,th1520-reset.h
> > +++ b/include/dt-bindings/reset/thead,th1520-reset.h
> > @@ -7,6 +7,200 @@
> > #ifndef _DT_BINDINGS_TH1520_RESET_H
> > #define _DT_BINDINGS_TH1520_RESET_H
> [snip]
> > +/* DSP Subsystem */
> > +#define TH1520_RESET_ID_X2X_DSP1 0
> > +#define TH1520_RESET_ID_X2X_DSP0 1
> > +#define TH1520_RESET_ID_X2X_SLAVE_DSP1 2
> > +#define TH1520_RESET_ID_X2X_SLAVE_DSP0 3
> > +#define TH1520_RESET_ID_DSP0_CORE 4
> > +#define TH1520_RESET_ID_DSP0_DEBUG 5
> > +#define TH1520_RESET_ID_DSP0_APB 6
> > +#define TH1520_RESET_ID_DSP1_CORE 4
> > +#define TH1520_RESET_ID_DSP1_DEBUG 5
> > +#define TH1520_RESET_ID_DSP1_APB 6
> > +#define TH1520_RESET_ID_DSPSYS_APB 7
> > +#define TH1520_RESET_ID_AXI4_DSPSYS_SLV 8
> > +#define TH1520_RESET_ID_AXI4_DSPSYS 9
> > +#define TH1520_RESET_ID_AXI4_DSP_RS 10
>
> This doesn't seem right. The numbers for each subsystem should not
> repeat. Here the DSP0 and DSP1 items have the same numbers: 4, 5, 6.
>
> This causes both clang and sparse to complain. I think you can just
> change this so that TH1520_RESET_ID_DSP1_CORE is 7 and so on. The
> indexes don't really have any concrete meaning other than how they are
> used as unique keys.
You're correct, it's a copy-paste error, just like the one spotted in v1
of the series...
I'm not sure why either my GCC or sparse yielded no warning about them.
Will figure it out and send v3 with this fixed. Much sorry for these
stupid mistakes.
Best regards,
Yao Zi
> The warnings:
>
> AR kernel/built-in.a
> CC drivers/reset/reset-th1520.o
> drivers/reset/reset-th1520.c:655:32: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
> 655 | [TH1520_RESET_ID_DSP1_CORE] = {
> | ^
> 656 | .bit = BIT(12),
> | ~~~~~~~~~~~~~~~
> 657 | .reg = TH1520_DSPSYS_RST_CFG,
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 658 | },
> | ~
> drivers/reset/reset-th1520.c:643:32: note: previous initialization is here
> 643 | [TH1520_RESET_ID_DSP0_CORE] = {
> | ^
> 644 | .bit = BIT(8),
> | ~~~~~~~~~~~~~~
> 645 | .reg = TH1520_DSPSYS_RST_CFG,
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 646 | },
> | ~
> drivers/reset/reset-th1520.c:659:33: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
> 659 | [TH1520_RESET_ID_DSP1_DEBUG] = {
> | ^
> 660 | .bit = BIT(13),
> | ~~~~~~~~~~~~~~~
> 661 | .reg = TH1520_DSPSYS_RST_CFG,
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 662 | },
> | ~
> drivers/reset/reset-th1520.c:647:33: note: previous initialization is here
> 647 | [TH1520_RESET_ID_DSP0_DEBUG] = {
> | ^
> 648 | .bit = BIT(9),
> | ~~~~~~~~~~~~~~
> 649 | .reg = TH1520_DSPSYS_RST_CFG,
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 650 | },
> | ~
> drivers/reset/reset-th1520.c:663:31: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
> 663 | [TH1520_RESET_ID_DSP1_APB] = {
> | ^
> 664 | .bit = BIT(14),
> | ~~~~~~~~~~~~~~~
> 665 | .reg = TH1520_DSPSYS_RST_CFG,
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 666 | },
> | ~
> drivers/reset/reset-th1520.c:651:31: note: previous initialization is here
> 651 | [TH1520_RESET_ID_DSP0_APB] = {
> | ^
> 652 | .bit = BIT(10),
> | ~~~~~~~~~~~~~~~
> 653 | .reg = TH1520_DSPSYS_RST_CFG,
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 654 | },
> | ~
> 3 warnings generated.
> CHECK drivers/reset/reset-th1520.c
> drivers/reset/reset-th1520.c:643:10: warning: Initializer entry defined twice
> drivers/reset/reset-th1520.c:655:10: also defined here
>
>
> Thanks,
> Drew
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Powered by blists - more mailing lists