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

Ethan Chen via posted 4 patches 1 year ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
There is a newer version of this series
[PATCH v3 1/4] hw/core: Add config stream
Posted by Ethan Chen via 1 year ago
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
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'))
 
 system_ss.add(files(
   'cpu-sysemu.c',
-- 
2.34.1
Re: [PATCH v3 1/4] hw/core: Add config stream
Posted by Alistair Francis 1 year ago
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

> 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

Alistair

>
>  system_ss.add(files(
>    'cpu-sysemu.c',
> --
> 2.34.1
>
>
Re: [PATCH v3 1/4] hw/core: Add config stream
Posted by Alistair Francis 1 year ago
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
>
> > 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

Alistair
Re: [PATCH v3 1/4] hw/core: Add config stream
Posted by Ethan Chen via 1 year ago
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