[PATCH] riscv/virt: Add syscon reboot and poweroff DT nodes

Anup Patel posted 1 patch 6 years ago
Test asan passed
Test checkpatch passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191111133804.14454-1-anup.patel@wdc.com
Maintainers: Alistair Francis <Alistair.Francis@wdc.com>, Palmer Dabbelt <palmer@dabbelt.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
There is a newer version of this series
hw/riscv/virt.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
[PATCH] riscv/virt: Add syscon reboot and poweroff DT nodes
Posted by Anup Patel 6 years ago
The SiFive test device found on virt machine can be used by
generic syscon reboot and poweroff drivers available in Linux
kernel.

This patch updates FDT generation in virt machine so that
Linux kernel can probe and use generic syscon drivers.

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 hw/riscv/virt.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index cc8f311e6b..fdfa359713 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -182,11 +182,11 @@ static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
     uint64_t mem_size, const char *cmdline)
 {
     void *fdt;
-    int cpu;
+    int cpu, i;
     uint32_t *cells;
     char *nodename;
-    uint32_t plic_phandle, phandle = 1;
-    int i;
+    const char test_compat[] = "sifive,test0\0syscon";
+    uint32_t plic_phandle, test_phandle, phandle = 1;
     hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
     hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
 
@@ -356,13 +356,33 @@ static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
     create_pcie_irq_map(fdt, nodename, plic_phandle);
     g_free(nodename);
 
+    test_phandle = phandle++;
     nodename = g_strdup_printf("/test@%lx",
         (long)memmap[VIRT_TEST].base);
     qemu_fdt_add_subnode(fdt, nodename);
-    qemu_fdt_setprop_string(fdt, nodename, "compatible", "sifive,test0");
+    qemu_fdt_setprop(fdt, nodename, "compatible",
+                     test_compat, sizeof(test_compat));
     qemu_fdt_setprop_cells(fdt, nodename, "reg",
         0x0, memmap[VIRT_TEST].base,
         0x0, memmap[VIRT_TEST].size);
+    qemu_fdt_setprop_cell(fdt, nodename, "phandle", test_phandle);
+    test_phandle = qemu_fdt_get_phandle(fdt, nodename);
+    g_free(nodename);
+
+    nodename = g_strdup_printf("/reboot");
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-reboot");
+    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
+    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
+    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_RESET);
+    g_free(nodename);
+
+    nodename = g_strdup_printf("/poweroff");
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-poweroff");
+    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
+    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
+    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_PASS);
     g_free(nodename);
 
     nodename = g_strdup_printf("/uart@%lx",
-- 
2.17.1


RE: [PATCH] riscv/virt: Add syscon reboot and poweroff DT nodes
Posted by Anup Patel 6 years ago
Correct Palmer's email address.

> -----Original Message-----
> From: Anup Patel
> Sent: Monday, November 11, 2019 7:08 PM
> To: Peter Maydell <peter.maydell@linaro.org>; Palmer Dabbelt
> <palmer@sifive.com>; Alistair Francis <Alistair.Francis@wdc.com>; Sagar
> Karandikar <sagark@eecs.berkeley.edu>
> Cc: Atish Patra <Atish.Patra@wdc.com>; Christoph Hellwig <hch@lst.de>;
> Anup Patel <anup@brainfault.org>; qemu-riscv@nongnu.org; qemu-
> devel@nongnu.org; Anup Patel <Anup.Patel@wdc.com>
> Subject: [PATCH] riscv/virt: Add syscon reboot and poweroff DT nodes
> 
> The SiFive test device found on virt machine can be used by generic syscon
> reboot and poweroff drivers available in Linux kernel.
> 
> This patch updates FDT generation in virt machine so that Linux kernel can
> probe and use generic syscon drivers.
> 
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  hw/riscv/virt.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index cc8f311e6b..fdfa359713
> 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -182,11 +182,11 @@ static void create_fdt(RISCVVirtState *s, const
> struct MemmapEntry *memmap,
>      uint64_t mem_size, const char *cmdline)  {
>      void *fdt;
> -    int cpu;
> +    int cpu, i;
>      uint32_t *cells;
>      char *nodename;
> -    uint32_t plic_phandle, phandle = 1;
> -    int i;
> +    const char test_compat[] = "sifive,test0\0syscon";
> +    uint32_t plic_phandle, test_phandle, phandle = 1;
>      hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
>      hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
> 
> @@ -356,13 +356,33 @@ static void create_fdt(RISCVVirtState *s, const
> struct MemmapEntry *memmap,
>      create_pcie_irq_map(fdt, nodename, plic_phandle);
>      g_free(nodename);
> 
> +    test_phandle = phandle++;
>      nodename = g_strdup_printf("/test@%lx",
>          (long)memmap[VIRT_TEST].base);
>      qemu_fdt_add_subnode(fdt, nodename);
> -    qemu_fdt_setprop_string(fdt, nodename, "compatible", "sifive,test0");
> +    qemu_fdt_setprop(fdt, nodename, "compatible",
> +                     test_compat, sizeof(test_compat));
>      qemu_fdt_setprop_cells(fdt, nodename, "reg",
>          0x0, memmap[VIRT_TEST].base,
>          0x0, memmap[VIRT_TEST].size);
> +    qemu_fdt_setprop_cell(fdt, nodename, "phandle", test_phandle);
> +    test_phandle = qemu_fdt_get_phandle(fdt, nodename);
> +    g_free(nodename);
> +
> +    nodename = g_strdup_printf("/reboot");
> +    qemu_fdt_add_subnode(fdt, nodename);
> +    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-
> reboot");
> +    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
> +    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
> +    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_RESET);
> +    g_free(nodename);
> +
> +    nodename = g_strdup_printf("/poweroff");
> +    qemu_fdt_add_subnode(fdt, nodename);
> +    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-
> poweroff");
> +    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
> +    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
> +    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_PASS);
>      g_free(nodename);
> 
>      nodename = g_strdup_printf("/uart@%lx",
> --
> 2.17.1


Re: [PATCH] riscv/virt: Add syscon reboot and poweroff DT nodes
Posted by Bin Meng 6 years ago
On Mon, Nov 11, 2019 at 9:42 PM Anup Patel <Anup.Patel@wdc.com> wrote:
>
> Correct Palmer's email address.
>
> > -----Original Message-----
> > From: Anup Patel
> > Sent: Monday, November 11, 2019 7:08 PM
> > To: Peter Maydell <peter.maydell@linaro.org>; Palmer Dabbelt
> > <palmer@sifive.com>; Alistair Francis <Alistair.Francis@wdc.com>; Sagar
> > Karandikar <sagark@eecs.berkeley.edu>
> > Cc: Atish Patra <Atish.Patra@wdc.com>; Christoph Hellwig <hch@lst.de>;
> > Anup Patel <anup@brainfault.org>; qemu-riscv@nongnu.org; qemu-
> > devel@nongnu.org; Anup Patel <Anup.Patel@wdc.com>
> > Subject: [PATCH] riscv/virt: Add syscon reboot and poweroff DT nodes
> >
> > The SiFive test device found on virt machine can be used by generic syscon
> > reboot and poweroff drivers available in Linux kernel.
> >
> > This patch updates FDT generation in virt machine so that Linux kernel can
> > probe and use generic syscon drivers.
> >
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> >  hw/riscv/virt.c | 28 ++++++++++++++++++++++++----
> >  1 file changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index cc8f311e6b..fdfa359713
> > 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -182,11 +182,11 @@ static void create_fdt(RISCVVirtState *s, const
> > struct MemmapEntry *memmap,
> >      uint64_t mem_size, const char *cmdline)  {
> >      void *fdt;
> > -    int cpu;
> > +    int cpu, i;
> >      uint32_t *cells;
> >      char *nodename;
> > -    uint32_t plic_phandle, phandle = 1;
> > -    int i;
> > +    const char test_compat[] = "sifive,test0\0syscon";
> > +    uint32_t plic_phandle, test_phandle, phandle = 1;
> >      hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
> >      hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
> >
> > @@ -356,13 +356,33 @@ static void create_fdt(RISCVVirtState *s, const
> > struct MemmapEntry *memmap,
> >      create_pcie_irq_map(fdt, nodename, plic_phandle);
> >      g_free(nodename);
> >
> > +    test_phandle = phandle++;
> >      nodename = g_strdup_printf("/test@%lx",
> >          (long)memmap[VIRT_TEST].base);
> >      qemu_fdt_add_subnode(fdt, nodename);
> > -    qemu_fdt_setprop_string(fdt, nodename, "compatible", "sifive,test0");
> > +    qemu_fdt_setprop(fdt, nodename, "compatible",
> > +                     test_compat, sizeof(test_compat));
> >      qemu_fdt_setprop_cells(fdt, nodename, "reg",
> >          0x0, memmap[VIRT_TEST].base,
> >          0x0, memmap[VIRT_TEST].size);
> > +    qemu_fdt_setprop_cell(fdt, nodename, "phandle", test_phandle);
> > +    test_phandle = qemu_fdt_get_phandle(fdt, nodename);

Is this necessary?

> > +    g_free(nodename);
> > +
> > +    nodename = g_strdup_printf("/reboot");
> > +    qemu_fdt_add_subnode(fdt, nodename);
> > +    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-
> > reboot");
> > +    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
> > +    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
> > +    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_RESET);
> > +    g_free(nodename);
> > +
> > +    nodename = g_strdup_printf("/poweroff");
> > +    qemu_fdt_add_subnode(fdt, nodename);
> > +    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-
> > poweroff");
> > +    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
> > +    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
> > +    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_PASS);
> >      g_free(nodename);
> >
> >      nodename = g_strdup_printf("/uart@%lx",
> > --

Regards,
Bin

RE: [PATCH] riscv/virt: Add syscon reboot and poweroff DT nodes
Posted by Anup Patel 6 years ago

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: Monday, November 11, 2019 8:58 PM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>; Alistair Francis
> <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> Palmer Dabbelt <palmer@dabbelt.com>; qemu-devel@nongnu.org; Atish
> Patra <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; Christoph Hellwig
> <hch@lst.de>; Anup Patel <anup@brainfault.org>
> Subject: Re: [PATCH] riscv/virt: Add syscon reboot and poweroff DT nodes
> 
> On Mon, Nov 11, 2019 at 9:42 PM Anup Patel <Anup.Patel@wdc.com> wrote:
> >
> > Correct Palmer's email address.
> >
> > > -----Original Message-----
> > > From: Anup Patel
> > > Sent: Monday, November 11, 2019 7:08 PM
> > > To: Peter Maydell <peter.maydell@linaro.org>; Palmer Dabbelt
> > > <palmer@sifive.com>; Alistair Francis <Alistair.Francis@wdc.com>;
> > > Sagar Karandikar <sagark@eecs.berkeley.edu>
> > > Cc: Atish Patra <Atish.Patra@wdc.com>; Christoph Hellwig
> > > <hch@lst.de>; Anup Patel <anup@brainfault.org>;
> > > qemu-riscv@nongnu.org; qemu- devel@nongnu.org; Anup Patel
> > > <Anup.Patel@wdc.com>
> > > Subject: [PATCH] riscv/virt: Add syscon reboot and poweroff DT nodes
> > >
> > > The SiFive test device found on virt machine can be used by generic
> > > syscon reboot and poweroff drivers available in Linux kernel.
> > >
> > > This patch updates FDT generation in virt machine so that Linux
> > > kernel can probe and use generic syscon drivers.
> > >
> > > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > > ---
> > >  hw/riscv/virt.c | 28 ++++++++++++++++++++++++----
> > >  1 file changed, 24 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index
> > > cc8f311e6b..fdfa359713
> > > 100644
> > > --- a/hw/riscv/virt.c
> > > +++ b/hw/riscv/virt.c
> > > @@ -182,11 +182,11 @@ static void create_fdt(RISCVVirtState *s,
> > > const struct MemmapEntry *memmap,
> > >      uint64_t mem_size, const char *cmdline)  {
> > >      void *fdt;
> > > -    int cpu;
> > > +    int cpu, i;
> > >      uint32_t *cells;
> > >      char *nodename;
> > > -    uint32_t plic_phandle, phandle = 1;
> > > -    int i;
> > > +    const char test_compat[] = "sifive,test0\0syscon";
> > > +    uint32_t plic_phandle, test_phandle, phandle = 1;
> > >      hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
> > >      hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
> > >
> > > @@ -356,13 +356,33 @@ static void create_fdt(RISCVVirtState *s,
> > > const struct MemmapEntry *memmap,
> > >      create_pcie_irq_map(fdt, nodename, plic_phandle);
> > >      g_free(nodename);
> > >
> > > +    test_phandle = phandle++;
> > >      nodename = g_strdup_printf("/test@%lx",
> > >          (long)memmap[VIRT_TEST].base);
> > >      qemu_fdt_add_subnode(fdt, nodename);
> > > -    qemu_fdt_setprop_string(fdt, nodename, "compatible",
> "sifive,test0");
> > > +    qemu_fdt_setprop(fdt, nodename, "compatible",
> > > +                     test_compat, sizeof(test_compat));
> > >      qemu_fdt_setprop_cells(fdt, nodename, "reg",
> > >          0x0, memmap[VIRT_TEST].base,
> > >          0x0, memmap[VIRT_TEST].size);
> > > +    qemu_fdt_setprop_cell(fdt, nodename, "phandle", test_phandle);
> > > +    test_phandle = qemu_fdt_get_phandle(fdt, nodename);
> 
> Is this necessary?

I was doubtful about this line myself but I tried to keep code
consistent with other DT nodes (such as PLIC).

Regards,
Anup

> 
> > > +    g_free(nodename);
> > > +
> > > +    nodename = g_strdup_printf("/reboot");
> > > +    qemu_fdt_add_subnode(fdt, nodename);
> > > +    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-
> > > reboot");
> > > +    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
> > > +    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
> > > +    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_RESET);
> > > +    g_free(nodename);
> > > +
> > > +    nodename = g_strdup_printf("/poweroff");
> > > +    qemu_fdt_add_subnode(fdt, nodename);
> > > +    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-
> > > poweroff");
> > > +    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
> > > +    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
> > > +    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_PASS);
> > >      g_free(nodename);
> > >
> > >      nodename = g_strdup_printf("/uart@%lx",
> > > --
> 
> Regards,
> Bin
Re: [PATCH] riscv/virt: Add syscon reboot and poweroff DT nodes
Posted by Alistair Francis 6 years ago
On Mon, Nov 11, 2019 at 5:38 AM Anup Patel <Anup.Patel@wdc.com> wrote:
>
> The SiFive test device found on virt machine can be used by
> generic syscon reboot and poweroff drivers available in Linux
> kernel.
>
> This patch updates FDT generation in virt machine so that
> Linux kernel can probe and use generic syscon drivers.
>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>

Overall this looks fine. Palmer currently has a patch on list changing
the sifive test string as well. It's probably best to rebase this on
that patch.

We probably also need to make sure this is accepted in the RISC-V
kernel tree first.

Alistair

> ---
>  hw/riscv/virt.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index cc8f311e6b..fdfa359713 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -182,11 +182,11 @@ static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
>      uint64_t mem_size, const char *cmdline)
>  {
>      void *fdt;
> -    int cpu;
> +    int cpu, i;
>      uint32_t *cells;
>      char *nodename;
> -    uint32_t plic_phandle, phandle = 1;
> -    int i;
> +    const char test_compat[] = "sifive,test0\0syscon";
> +    uint32_t plic_phandle, test_phandle, phandle = 1;
>      hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
>      hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
>
> @@ -356,13 +356,33 @@ static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
>      create_pcie_irq_map(fdt, nodename, plic_phandle);
>      g_free(nodename);
>
> +    test_phandle = phandle++;
>      nodename = g_strdup_printf("/test@%lx",
>          (long)memmap[VIRT_TEST].base);
>      qemu_fdt_add_subnode(fdt, nodename);
> -    qemu_fdt_setprop_string(fdt, nodename, "compatible", "sifive,test0");
> +    qemu_fdt_setprop(fdt, nodename, "compatible",
> +                     test_compat, sizeof(test_compat));
>      qemu_fdt_setprop_cells(fdt, nodename, "reg",
>          0x0, memmap[VIRT_TEST].base,
>          0x0, memmap[VIRT_TEST].size);
> +    qemu_fdt_setprop_cell(fdt, nodename, "phandle", test_phandle);
> +    test_phandle = qemu_fdt_get_phandle(fdt, nodename);
> +    g_free(nodename);
> +
> +    nodename = g_strdup_printf("/reboot");
> +    qemu_fdt_add_subnode(fdt, nodename);
> +    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-reboot");
> +    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
> +    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
> +    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_RESET);
> +    g_free(nodename);
> +
> +    nodename = g_strdup_printf("/poweroff");
> +    qemu_fdt_add_subnode(fdt, nodename);
> +    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-poweroff");
> +    qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
> +    qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
> +    qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_PASS);
>      g_free(nodename);
>
>      nodename = g_strdup_printf("/uart@%lx",
> --
> 2.17.1
>
>