Re: [PATCH v3 1/4] hw/core: Add config stream

[Ethan Chen via <qemu-riscv@nongnu.org>]

On Tue, Nov 21, 2023 at 03:28:13PM +1000, Alistair Francis wrote:
> On Tue, Nov 21, 2023 at 3:24 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, Nov 14, 2023 at 7:49 PM Ethan Chen via <qemu-devel@nongnu.org> wrote:
> > >
> > > Make other device can use /hw/core/stream.c by select this config.
> > >
> > > Signed-off-by: Ethan Chen <ethan84@andestech.com>
> > > ---
> > >  hw/core/Kconfig     | 3 +++
> > >  hw/core/meson.build | 1 +
> > >  2 files changed, 4 insertions(+)
> > >
> > > diff --git a/hw/core/Kconfig b/hw/core/Kconfig
> > > index 9397503656..628dc3d883 100644
> > > --- a/hw/core/Kconfig
> > > +++ b/hw/core/Kconfig
> > > @@ -27,3 +27,6 @@ config REGISTER
> > >
> > >  config SPLIT_IRQ
> > >      bool
> > > +
> > > +config STREAM
> > > +    bool
> > > \ No newline at end of file
> >
> > You are missing a newline here. I think checkpatch should catch this,
> > make sure you run it on all of your patches

Sorry for that. It is wired that this was not catched by checkpatch.

> >
> > > diff --git a/hw/core/meson.build b/hw/core/meson.build
> > > index 67dad04de5..d6ce14d5ce 100644
> > > --- a/hw/core/meson.build
> > > +++ b/hw/core/meson.build
> > > @@ -34,6 +34,7 @@ system_ss.add(when: 'CONFIG_REGISTER', if_true: files('register.c'))
> > >  system_ss.add(when: 'CONFIG_SPLIT_IRQ', if_true: files('split-irq.c'))
> > >  system_ss.add(when: 'CONFIG_XILINX_AXI', if_true: files('stream.c'))
> > >  system_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true: files('sysbus-fdt.c'))
> > > +system_ss.add(when: 'CONFIG_STREAM', if_true: files('stream.c'))
> >
> > You have added the build but not the file. This will fail to compile.
> >
> > Each patch must compile and run when applied individually in order.
> > That way we maintain git bisectability. Can you please make sure that
> > the build is not broken as your patches are applied
> 
> Whoops! The file already exists.
> 
> We should only include the file stream.c once. So we should change the
> CONFIG_XILINX_AXI to select CONFIG_STREAM in this patch
> 

I will fix that in next revision.

Thanks,
Ethan Chen