| 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: <aOGckdJcY44hD4Hn@x1>
Date: Sat, 4 Oct 2025 15:15:45 -0700
From: Drew Fustini <fustini@...nel.org>
To: Yao Zi <ziyao@...root.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 Sat, Oct 04, 2025 at 02:21:43AM +0000, Yao Zi wrote:
> 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
Have you tried using W=1?
I do see the warning in gcc with that:
pdp7@...lio:~/linux$ rm drivers/reset/reset-th1520.o
pdp7@...lio:~/linux$ make W=1 CROSS_COMPILE=riscv64-linux-gnu- ARCH=riscv C=1 -j16
CALL scripts/checksyscalls.sh
Documentation/.renames.txt: warning: ignored by one of the .gitignore files
CC drivers/reset/reset-th1520.o
drivers/reset/reset-th1520.c:655:39: warning: initialized field overwritten [-Woverride-init]
655 | [TH1520_RESET_ID_DSP1_CORE] = {
| ^
drivers/reset/reset-th1520.c:655:39: note: (near initialization for ‘th1520_dsp_resets[4]’)
drivers/reset/reset-th1520.c:659:40: warning: initialized field overwritten [-Woverride-init]
659 | [TH1520_RESET_ID_DSP1_DEBUG] = {
| ^
drivers/reset/reset-th1520.c:659:40: note: (near initialization for ‘th1520_dsp_resets[5]’)
drivers/reset/reset-th1520.c:663:38: warning: initialized field overwritten [-Woverride-init]
663 | [TH1520_RESET_ID_DSP1_APB] = {
| ^
drivers/reset/reset-th1520.c:663:38: note: (near initialization for ‘th1520_dsp_resets[6]’)
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
AR drivers/reset/built-in.a
AR drivers/built-in.a
AR built-in.a
AR vmlinux.a
LD vmlinux.o
MODPOST Module.symvers
UPD include/generated/utsversion.h
CC init/version-timestamp.o
CHECK init/version-timestamp.c
KSYMS .tmp_vmlinux0.kallsyms.S
AS .tmp_vmlinux0.kallsyms.o
LD .tmp_vmlinux1
NM .tmp_vmlinux1.syms
KSYMS .tmp_vmlinux1.kallsyms.S
AS .tmp_vmlinux1.kallsyms.o
LD .tmp_vmlinux2
NM .tmp_vmlinux2.syms
KSYMS .tmp_vmlinux2.kallsyms.S
AS .tmp_vmlinux2.kallsyms.o
LD vmlinux.unstripped
NM System.map
SORTTAB vmlinux.unstripped
OBJCOPY vmlinux
OBJCOPY modules.builtin.modinfo
GEN modules.builtin
OBJCOPY arch/riscv/boot/Image
Kernel: arch/riscv/boot/Image is ready
GZIP arch/riscv/boot/Image.gz
Kernel: arch/riscv/boot/Image.gz is ready
pdp7@...lio:~/linux$ head .config
#
# Automatically generated file; DO NOT EDIT.
# Linux/riscv 6.17.0 Kernel Configuration
#
CONFIG_CC_VERSION_TEXT="riscv64-linux-gnu-gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0"
CONFIG_CC_IS_GCC=y
CONFIG_GCC_VERSION=130300
CONFIG_CLANG_VERSION=0
CONFIG_AS_IS_GNU=y
CONFIG_AS_VERSION=24200
Thanks,
Drew
Powered by blists - more mailing lists