Re: [PATCH v2 2/5] dt-bindings: reset: thead,th1520-reset: Add controllers for more subsys

[Drew Fustini <fustini@kernel.org>]

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@disroot.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@thelio:~/linux$ rm drivers/reset/reset-th1520.o
pdp7@thelio:~/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@thelio:~/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