QEMU RISC-V virt machine poweroff driver

QEMU RISC-V virt machine poweroff driver

[Christoph Hellwig]

Hi all,

this patch add a driver for the test device in the Qemu RISC-V
virt machine which allows properly shutting down the VM.
It also is added to the riscv defconfig given that qemu-virt
is the most popular riscv platform.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/2] dt-bindings: power: reset: document the QEMU RISC-V virt machine poweroff device

[Christoph Hellwig]

Add the binding for the trivial Qemu RISC-V poweroff mechanism, which is
just a single MMIO register exposed through the DT.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 .../power/reset/qemu-riscv-virt-poweroff.txt     | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt

diff --git a/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt b/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt
new file mode 100644
index 000000000000..80ff6fd4e3b7
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt
@@ -0,0 +1,16 @@
+QEMU RISC-V virt machine poweroff device
+
+This is a device in Qemu that can signal successful or error exit
+by writing two magic numbers to a trivial mmio register.
+A Linux poweroff is implemented as successful exit.
+
+Required Properties:
+-compatible: "sifive,test0"
+-reg: Specifies the physical address of the register
+
+Example:
+
+	test@100000 {
+		compatible = "sifive,test0";
+		reg = <0x100000 0x1000>;
+	};
-- 
2.20.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/2] power: reset: add a QEMU RISC-V virt machine poweroff driver

[Christoph Hellwig]

Add a trivial poweroff driver for the qemu-virt test device that
allows an oderly shutdown of the VM.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/riscv/configs/defconfig                  |  2 +
 drivers/power/reset/Kconfig                   |  8 ++++
 drivers/power/reset/Makefile                  |  1 +
 .../power/reset/qemu-riscv-virt-poweroff.c    | 47 +++++++++++++++++++
 4 files changed, 58 insertions(+)
 create mode 100644 drivers/power/reset/qemu-riscv-virt-poweroff.c

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 420a0dbef386..47da87725b5e 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -63,6 +63,8 @@ CONFIG_HW_RANDOM_VIRTIO=y
 CONFIG_SPI=y
 CONFIG_SPI_SIFIVE=y
 # CONFIG_PTP_1588_CLOCK is not set
+CONFIG_POWER_RESET=y
+CONFIG_QEMU_RISCV_VIRT_POWEROFF=y
 CONFIG_DRM=y
 CONFIG_DRM_RADEON=y
 CONFIG_DRM_VIRTIO_GPU=y
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index a564237278ff..56cb18520850 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -256,5 +256,13 @@ config NVMEM_REBOOT_MODE
 	  then the bootloader can read it and take different
 	  action according to the mode.
 
+config QEMU_RISCV_VIRT_POWEROFF
+	tristate "QEMU RISC-V virt machine poweroff driver"
+	depends on OF
+	depends on RISCV || COMPILE_TEST
+	help
+	  Say y here to allow RISC-V Qemu VMs to be shut down by
+	  the kernel.
+
 endif
 
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index 85da3198e4e0..b3094016b634 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o
 obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o
 obj-$(CONFIG_POWER_RESET_SC27XX) += sc27xx-poweroff.o
 obj-$(CONFIG_NVMEM_REBOOT_MODE) += nvmem-reboot-mode.o
+obj-$(CONFIG_QEMU_RISCV_VIRT_POWEROFF) += qemu-riscv-virt-poweroff.o
diff --git a/drivers/power/reset/qemu-riscv-virt-poweroff.c b/drivers/power/reset/qemu-riscv-virt-poweroff.c
new file mode 100644
index 000000000000..5b9a12dd853b
--- /dev/null
+++ b/drivers/power/reset/qemu-riscv-virt-poweroff.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 Christoph Hellwig.
+ */
+
+#include <linux/reboot.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/module.h>
+
+#define VIRT_TEST_FINISHER_PASS         0x5555
+
+static u16 __iomem *test_addr;
+
+static void qemu_virt_power_off(void)
+{
+	writew(VIRT_TEST_FINISHER_PASS, test_addr);
+}
+
+static int sifive_test_probe(struct platform_device *pdev)
+{
+	/* there can only be a single instance */
+	if (WARN_ON_ONCE(test_addr))
+		return -EINVAL;
+
+	test_addr = of_iomap(pdev->dev.of_node, 0);
+	if (!test_addr)
+		return -EINVAL;
+	pm_power_off = qemu_virt_power_off;
+	return 0;
+}
+
+static const struct of_device_id sifive_test_of_match[] = {
+	{ .compatible = "sifive,test0", },
+	{},
+};
+
+static struct platform_driver sifive_test_driver = {
+	.probe			= sifive_test_probe,
+	.driver = {
+		.name		= "sifive_test",
+		.of_match_table = sifive_test_of_match,
+	},
+};
+module_platform_driver(sifive_test_driver);
-- 
2.20.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] dt-bindings: power: reset: document the QEMU RISC-V virt machine poweroff device

[Palmer Dabbelt]

On Thu, 07 Nov 2019 13:24:07 PST (-0800), Christoph Hellwig wrote:
> Add the binding for the trivial Qemu RISC-V poweroff mechanism, which is
> just a single MMIO register exposed through the DT.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  .../power/reset/qemu-riscv-virt-poweroff.txt     | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt
>
> diff --git a/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt b/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt
> new file mode 100644
> index 000000000000..80ff6fd4e3b7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt
> @@ -0,0 +1,16 @@
> +QEMU RISC-V virt machine poweroff device
> +
> +This is a device in Qemu that can signal successful or error exit
> +by writing two magic numbers to a trivial mmio register.
> +A Linux poweroff is implemented as successful exit.

There's a third value that reboots the system, but it's only implemented in 
qemu-4.2 (not released yet) and above.  It'll be 'compatible = "sifive,test1", 
"sifive,test0";' (or at least will be when I merge my patch to do so).

> +
> +Required Properties:
> +-compatible: "sifive,test0"
> +-reg: Specifies the physical address of the register
> +
> +Example:
> +
> +	test@100000 {
> +		compatible = "sifive,test0";
> +		reg = <0x100000 0x1000>;
> +	};

Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] power: reset: add a QEMU RISC-V virt machine poweroff driver

[Palmer Dabbelt]

On Thu, 07 Nov 2019 13:24:08 PST (-0800), Christoph Hellwig wrote:
> Add a trivial poweroff driver for the qemu-virt test device that
> allows an oderly shutdown of the VM.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/riscv/configs/defconfig                  |  2 +
>  drivers/power/reset/Kconfig                   |  8 ++++
>  drivers/power/reset/Makefile                  |  1 +
>  .../power/reset/qemu-riscv-virt-poweroff.c    | 47 +++++++++++++++++++
>  4 files changed, 58 insertions(+)
>  create mode 100644 drivers/power/reset/qemu-riscv-virt-poweroff.c
>
> diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
> index 420a0dbef386..47da87725b5e 100644
> --- a/arch/riscv/configs/defconfig
> +++ b/arch/riscv/configs/defconfig
> @@ -63,6 +63,8 @@ CONFIG_HW_RANDOM_VIRTIO=y
>  CONFIG_SPI=y
>  CONFIG_SPI_SIFIVE=y
>  # CONFIG_PTP_1588_CLOCK is not set
> +CONFIG_POWER_RESET=y
> +CONFIG_QEMU_RISCV_VIRT_POWEROFF=y
>  CONFIG_DRM=y
>  CONFIG_DRM_RADEON=y
>  CONFIG_DRM_VIRTIO_GPU=y
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index a564237278ff..56cb18520850 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -256,5 +256,13 @@ config NVMEM_REBOOT_MODE
>  	  then the bootloader can read it and take different
>  	  action according to the mode.
>
> +config QEMU_RISCV_VIRT_POWEROFF
> +	tristate "QEMU RISC-V virt machine poweroff driver"
> +	depends on OF
> +	depends on RISCV || COMPILE_TEST
> +	help
> +	  Say y here to allow RISC-V Qemu VMs to be shut down by
> +	  the kernel.
> +
>  endif
>
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index 85da3198e4e0..b3094016b634 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -30,3 +30,4 @@ obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o
>  obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o
>  obj-$(CONFIG_POWER_RESET_SC27XX) += sc27xx-poweroff.o
>  obj-$(CONFIG_NVMEM_REBOOT_MODE) += nvmem-reboot-mode.o
> +obj-$(CONFIG_QEMU_RISCV_VIRT_POWEROFF) += qemu-riscv-virt-poweroff.o
> diff --git a/drivers/power/reset/qemu-riscv-virt-poweroff.c b/drivers/power/reset/qemu-riscv-virt-poweroff.c
> new file mode 100644
> index 000000000000..5b9a12dd853b
> --- /dev/null
> +++ b/drivers/power/reset/qemu-riscv-virt-poweroff.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 Christoph Hellwig.
> + */
> +
> +#include <linux/reboot.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_address.h>
> +#include <linux/module.h>
> +
> +#define VIRT_TEST_FINISHER_PASS         0x5555
> +
> +static u16 __iomem *test_addr;
> +
> +static void qemu_virt_power_off(void)
> +{
> +	writew(VIRT_TEST_FINISHER_PASS, test_addr);
> +}
> +
> +static int sifive_test_probe(struct platform_device *pdev)
> +{
> +	/* there can only be a single instance */
> +	if (WARN_ON_ONCE(test_addr))
> +		return -EINVAL;
> +
> +	test_addr = of_iomap(pdev->dev.of_node, 0);
> +	if (!test_addr)
> +		return -EINVAL;
> +	pm_power_off = qemu_virt_power_off;
> +	return 0;
> +}
> +
> +static const struct of_device_id sifive_test_of_match[] = {
> +	{ .compatible = "sifive,test0", },
> +	{},
> +};
> +
> +static struct platform_driver sifive_test_driver = {
> +	.probe			= sifive_test_probe,
> +	.driver = {
> +		.name		= "sifive_test",
> +		.of_match_table = sifive_test_of_match,
> +	},
> +};
> +module_platform_driver(sifive_test_driver);

Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: QEMU RISC-V virt machine poweroff driver

[Palmer Dabbelt]

On Thu, 07 Nov 2019 13:24:06 PST (-0800), Christoph Hellwig wrote:
> Hi all,
>
> this patch add a driver for the test device in the Qemu RISC-V
> virt machine which allows properly shutting down the VM.
> It also is added to the riscv defconfig given that qemu-virt
> is the most popular riscv platform.

Thanks!  I'm assuming this is going in through some other tree, so

Acked-by: Palmer Dabbelt <palmer@dabbelt.com>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] dt-bindings: power: reset: document the QEMU RISC-V virt machine poweroff device

[Paul Walmsley]

On Thu, 7 Nov 2019, Christoph Hellwig wrote:

> Add the binding for the trivial Qemu RISC-V poweroff mechanism, which is

There's nothing RISC-V specific here.  This IP isn't defined in the RISC-V 
specifications, or anything like that.  

Apparently it's a SiFive IP block which now has a virtual IP 
implementation in QEMU in hw/riscv/sifive_test.c.  But since there's 
nothing RISC-V specific about this IP block, any QEMU system, with any CPU 
implementation, should be able to use this virtual IP, and this Linux 
driver.

For these reasons, it's better if "RISC-V" is just removed from everywhere 
in this driver.  If something needs to go in its place, "SiFive" may be 
better.


- Paul


> just a single MMIO register exposed through the DT.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  .../power/reset/qemu-riscv-virt-poweroff.txt     | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt b/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt
> new file mode 100644
> index 000000000000..80ff6fd4e3b7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt
> @@ -0,0 +1,16 @@
> +QEMU RISC-V virt machine poweroff device
> +
> +This is a device in Qemu that can signal successful or error exit
> +by writing two magic numbers to a trivial mmio register.
> +A Linux poweroff is implemented as successful exit.
> +
> +Required Properties:
> +-compatible: "sifive,test0"
> +-reg: Specifies the physical address of the register
> +
> +Example:
> +
> +	test@100000 {
> +		compatible = "sifive,test0";
> +		reg = <0x100000 0x1000>;
> +	};
> -- 
> 2.20.1
> 
> 


- Paul

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] power: reset: add a QEMU RISC-V virt machine poweroff driver

[Paul Walmsley]

On Thu, 7 Nov 2019, Christoph Hellwig wrote:

> Add a trivial poweroff driver for the qemu-virt test device that
> allows an oderly shutdown of the VM.

"orderly"

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/riscv/configs/defconfig                  |  2 +
>  drivers/power/reset/Kconfig                   |  8 ++++
>  drivers/power/reset/Makefile                  |  1 +
>  .../power/reset/qemu-riscv-virt-poweroff.c    | 47 +++++++++++++++++++
>  4 files changed, 58 insertions(+)
>  create mode 100644 drivers/power/reset/qemu-riscv-virt-poweroff.c
> 
> diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
> index 420a0dbef386..47da87725b5e 100644
> --- a/arch/riscv/configs/defconfig
> +++ b/arch/riscv/configs/defconfig
> @@ -63,6 +63,8 @@ CONFIG_HW_RANDOM_VIRTIO=y
>  CONFIG_SPI=y
>  CONFIG_SPI_SIFIVE=y
>  # CONFIG_PTP_1588_CLOCK is not set
> +CONFIG_POWER_RESET=y
> +CONFIG_QEMU_RISCV_VIRT_POWEROFF=y
>  CONFIG_DRM=y
>  CONFIG_DRM_RADEON=y
>  CONFIG_DRM_VIRTIO_GPU=y
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index a564237278ff..56cb18520850 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -256,5 +256,13 @@ config NVMEM_REBOOT_MODE
>  	  then the bootloader can read it and take different
>  	  action according to the mode.
>  
> +config QEMU_RISCV_VIRT_POWEROFF
> +	tristate "QEMU RISC-V virt machine poweroff driver"
> +	depends on OF
> +	depends on RISCV || COMPILE_TEST
> +	help
> +	  Say y here to allow RISC-V Qemu VMs to be shut down by
> +	  the kernel.

Same comments here and below as 

https://lore.kernel.org/linux-riscv/mhng-0dedc685-73d2-4e7f-b608-69385a6e7a99@palmer-si-x1c4/T/#t

there's nothing that should be RISC-V specific here.

> +
>  endif
>  
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index 85da3198e4e0..b3094016b634 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -30,3 +30,4 @@ obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o
>  obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o
>  obj-$(CONFIG_POWER_RESET_SC27XX) += sc27xx-poweroff.o
>  obj-$(CONFIG_NVMEM_REBOOT_MODE) += nvmem-reboot-mode.o
> +obj-$(CONFIG_QEMU_RISCV_VIRT_POWEROFF) += qemu-riscv-virt-poweroff.o
> diff --git a/drivers/power/reset/qemu-riscv-virt-poweroff.c b/drivers/power/reset/qemu-riscv-virt-poweroff.c
> new file mode 100644
> index 000000000000..5b9a12dd853b
> --- /dev/null
> +++ b/drivers/power/reset/qemu-riscv-virt-poweroff.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 Christoph Hellwig.
> + */
> +
> +#include <linux/reboot.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_address.h>
> +#include <linux/module.h>
> +
> +#define VIRT_TEST_FINISHER_PASS         0x5555
> +
> +static u16 __iomem *test_addr;
> +
> +static void qemu_virt_power_off(void)
> +{
> +	writew(VIRT_TEST_FINISHER_PASS, test_addr);
> +}
> +
> +static int sifive_test_probe(struct platform_device *pdev)
> +{
> +	/* there can only be a single instance */
> +	if (WARN_ON_ONCE(test_addr))
> +		return -EINVAL;
> +
> +	test_addr = of_iomap(pdev->dev.of_node, 0);
> +	if (!test_addr)
> +		return -EINVAL;
> +	pm_power_off = qemu_virt_power_off;
> +	return 0;
> +}
> +
> +static const struct of_device_id sifive_test_of_match[] = {
> +	{ .compatible = "sifive,test0", },
> +	{},
> +};
> +
> +static struct platform_driver sifive_test_driver = {
> +	.probe			= sifive_test_probe,
> +	.driver = {
> +		.name		= "sifive_test",
> +		.of_match_table = sifive_test_of_match,
> +	},
> +};
> +module_platform_driver(sifive_test_driver);
> -- 
> 2.20.1
> 
> 


- Paul

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: QEMU RISC-V virt machine poweroff driver

[Anup Patel]

On Fri, Nov 8, 2019 at 2:54 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Hi all,
>
> this patch add a driver for the test device in the Qemu RISC-V
> virt machine which allows properly shutting down the VM.
> It also is added to the riscv defconfig given that qemu-virt
> is the most popular riscv platform.

We really don't need this driver. Instead, we can simply re-use
following drivers:
mfd/syscon
power/reset/syscon-reboot
power/reset/syscon-poweroff

Just enable following to your defconfig:
CONFIG_POWER_RESET=y
CONFIG_POWER_RESET_SYSCON=y
CONFIG_POWER_RESET_SYSCON_POWEROFF=y
CONFIG_SYSCON_REBOOT_MODE=y


Once above drivers are enabled in your defconfig, make sure
test device DT nodes are described in the following way for virt machine:

testdev: test@100000 {
    compatible = "syscon";
    reg = <0x100000 0x1000>;
};

reboot {
    compatible = "syscon-reboot";
    regmap = <&testdev>;
    offset = <0x0>;
    value = <0x7777>;
};

poweroff {
    compatible = "syscon-poweroff";
    regmap = <&testdev>;
    offset = <0x0>;
    value = <0x5555>;
};


Here's the QEMU changes I used for above DT nodes:

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index d7c5d630eb..7f8206c726 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -186,7 +186,7 @@ static void create_fdt(RISCVVirtState *s, const
struct MemmapEntry *memmap,
     int cpu;
     uint32_t *cells;
     char *nodename;
-    uint32_t plic_phandle, phandle = 1;
+    uint32_t plic_phandle, test_phandle, phandle = 1;
     int i;
     hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
     hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
@@ -357,13 +357,32 @@ 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_string(fdt, nodename, "compatible", "syscon");
     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",


Regards,
Anup


>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: QEMU RISC-V virt machine poweroff driver

[Christoph Hellwig]

On Mon, Nov 11, 2019 at 05:06:24PM +0530, Anup Patel wrote:
> We really don't need this driver. Instead, we can simply re-use
> following drivers:
> mfd/syscon
> power/reset/syscon-reboot
> power/reset/syscon-poweroff
> 
> Just enable following to your defconfig:
> CONFIG_POWER_RESET=y
> CONFIG_POWER_RESET_SYSCON=y
> CONFIG_POWER_RESET_SYSCON_POWEROFF=y
> CONFIG_SYSCON_REBOOT_MODE=y
> 
> 
> Once above drivers are enabled in your defconfig, make sure
> test device DT nodes are described in the following way for virt machine:

Oh well, that is a lot more churn than a just works driver, and
will also pull it dependencies like regmap which quite blow up the
kernel size.  But I guess that is where modern Linux drivers are
heading, so I'm not going to complain too loud..

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: QEMU RISC-V virt machine poweroff driver

[Paul Walmsley]

On Mon, 11 Nov 2019, Christoph Hellwig wrote:

> On Mon, Nov 11, 2019 at 05:06:24PM +0530, Anup Patel wrote:
> > We really don't need this driver. Instead, we can simply re-use
> > following drivers:
> > mfd/syscon
> > power/reset/syscon-reboot
> > power/reset/syscon-poweroff
> > 
> > Just enable following to your defconfig:
> > CONFIG_POWER_RESET=y
> > CONFIG_POWER_RESET_SYSCON=y
> > CONFIG_POWER_RESET_SYSCON_POWEROFF=y
> > CONFIG_SYSCON_REBOOT_MODE=y
> > 
> > 
> > Once above drivers are enabled in your defconfig, make sure
> > test device DT nodes are described in the following way for virt machine:
> 
> Oh well, that is a lot more churn than a just works driver, and
> will also pull it dependencies like regmap which quite blow up the
> kernel size.  But I guess that is where modern Linux drivers are
> heading, so I'm not going to complain too loud..

The core issue is that putting random register writes in DT doesn't match 
the hardware.  And the doctrine with DT has always been that it's supposed 
to represent the actual hardware.  On FPGA bitstreams or ASICs that have 
the teststatus/testfinisher IP block, there really is an IP block out 
there - it's not just a bare register.

If you update your driver to note that this is a SiFive IP block rather 
than a "RISC-V" IP block, I'll ack it.


- Paul

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: QEMU RISC-V virt machine poweroff driver

[Anup Patel]

On Mon, Nov 11, 2019 at 10:50 PM Paul Walmsley <paul@pwsan.com> wrote:
>
> On Mon, 11 Nov 2019, Christoph Hellwig wrote:
>
> > On Mon, Nov 11, 2019 at 05:06:24PM +0530, Anup Patel wrote:
> > > We really don't need this driver. Instead, we can simply re-use
> > > following drivers:
> > > mfd/syscon
> > > power/reset/syscon-reboot
> > > power/reset/syscon-poweroff
> > >
> > > Just enable following to your defconfig:
> > > CONFIG_POWER_RESET=y
> > > CONFIG_POWER_RESET_SYSCON=y
> > > CONFIG_POWER_RESET_SYSCON_POWEROFF=y
> > > CONFIG_SYSCON_REBOOT_MODE=y
> > >
> > >
> > > Once above drivers are enabled in your defconfig, make sure
> > > test device DT nodes are described in the following way for virt machine:
> >
> > Oh well, that is a lot more churn than a just works driver, and
> > will also pull it dependencies like regmap which quite blow up the
> > kernel size.  But I guess that is where modern Linux drivers are
> > heading, so I'm not going to complain too loud..
>
> The core issue is that putting random register writes in DT doesn't match
> the hardware.  And the doctrine with DT has always been that it's supposed
> to represent the actual hardware.  On FPGA bitstreams or ASICs that have
> the teststatus/testfinisher IP block, there really is an IP block out
> there - it's not just a bare register.
>
> If you update your driver to note that this is a SiFive IP block rather
> than a "RISC-V" IP block, I'll ack it.
>

The SiFive Test device has only one register at offset 0x0 and three
possible magic values (0x3333, 0x5555, and 0x7777).

The SYSCON based Reboot and Poweroff driver do exactly the same
thing what Christop's virt machine poweroff driver does so we are not
doing "random register writes" via DT.

In fact, using SYSCON based Reboot and Poweroff we are actually
describing the Reboot and Poweroff mechanism directly in DT without
adding a complete driver for just one register write. This means we
are totally aligned with "DT doctrine" and over here we going one-step
more by describing Reboot and Poweroff mechanism in DT.

A quick GREP shows that the SYSCON Reboot and Poweroff drivers
are quite widely used in ARM, ARM64 and MIPS architectures. Some of
the  SOCs using these drivers are: Samsung Exynos, HiSilicon Hi3660,
HiSilicon Hi6220, Rockchip RK3xxx, AppliedMicro XGene, Broadcom
BCM33xx, Broadcom BCM63xx, etc. Majority of ARM/ARM64 SOCs
these days use the PSCI based SYSTEM RESET and SHUTDOWN
methods so we might not see more Reboot and Poweroff drivers for
ARM world.

IMHO, we should definitely avoid adding a driver to Linux when there
a generic driver already available. This helps in kernel maintenance
in long-term.

Regards,
Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] dt-bindings: power: reset: document the QEMU RISC-V virt machine poweroff device

[Rob Herring]

On Thu, Nov 07, 2019 at 02:32:01PM -0800, Paul Walmsley wrote:
> On Thu, 7 Nov 2019, Christoph Hellwig wrote:
> 
> > Add the binding for the trivial Qemu RISC-V poweroff mechanism, which is
>
> There's nothing RISC-V specific here.  This IP isn't defined in the RISC-V 
> specifications, or anything like that.  

Does the VM have a spec or anything? Would be nice if it did. Would be 
nicer still to just copy Arm's virt machine.

> Apparently it's a SiFive IP block which now has a virtual IP 
> implementation in QEMU in hw/riscv/sifive_test.c.  But since there's 
> nothing RISC-V specific about this IP block, any QEMU system, with any CPU 
> implementation, should be able to use this virtual IP, and this Linux 
> driver.
> 
> For these reasons, it's better if "RISC-V" is just removed from everywhere 
> in this driver.  If something needs to go in its place, "SiFive" may be 
> better.

IIRC, there's already syscon poweroff binding. You could probably use 
that.

> 
> 
> - Paul
> 
> 
> > just a single MMIO register exposed through the DT.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  .../power/reset/qemu-riscv-virt-poweroff.txt     | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt b/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt
> > new file mode 100644
> > index 000000000000..80ff6fd4e3b7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/reset/qemu-riscv-virt-poweroff.txt
> > @@ -0,0 +1,16 @@
> > +QEMU RISC-V virt machine poweroff device
> > +
> > +This is a device in Qemu that can signal successful or error exit
> > +by writing two magic numbers to a trivial mmio register.
> > +A Linux poweroff is implemented as successful exit.
> > +
> > +Required Properties:
> > +-compatible: "sifive,test0"
> > +-reg: Specifies the physical address of the register
> > +
> > +Example:
> > +
> > +	test@100000 {
> > +		compatible = "sifive,test0";
> > +		reg = <0x100000 0x1000>;
> > +	};
> > -- 
> > 2.20.1
> > 
> > 
> 
> 
> - Paul

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: QEMU RISC-V virt machine poweroff driver

[Rob Herring]

On Tue, Nov 12, 2019 at 09:46:33AM +0530, Anup Patel wrote:
> On Mon, Nov 11, 2019 at 10:50 PM Paul Walmsley <paul@pwsan.com> wrote:
> >
> > On Mon, 11 Nov 2019, Christoph Hellwig wrote:
> >
> > > On Mon, Nov 11, 2019 at 05:06:24PM +0530, Anup Patel wrote:
> > > > We really don't need this driver. Instead, we can simply re-use
> > > > following drivers:
> > > > mfd/syscon
> > > > power/reset/syscon-reboot
> > > > power/reset/syscon-poweroff
> > > >
> > > > Just enable following to your defconfig:
> > > > CONFIG_POWER_RESET=y
> > > > CONFIG_POWER_RESET_SYSCON=y
> > > > CONFIG_POWER_RESET_SYSCON_POWEROFF=y
> > > > CONFIG_SYSCON_REBOOT_MODE=y
> > > >
> > > >
> > > > Once above drivers are enabled in your defconfig, make sure
> > > > test device DT nodes are described in the following way for virt machine:
> > >
> > > Oh well, that is a lot more churn than a just works driver, and
> > > will also pull it dependencies like regmap which quite blow up the
> > > kernel size.  But I guess that is where modern Linux drivers are
> > > heading, so I'm not going to complain too loud..
> >
> > The core issue is that putting random register writes in DT doesn't match
> > the hardware.  And the doctrine with DT has always been that it's supposed
> > to represent the actual hardware.  On FPGA bitstreams or ASICs that have
> > the teststatus/testfinisher IP block, there really is an IP block out
> > there - it's not just a bare register.
> >
> > If you update your driver to note that this is a SiFive IP block rather
> > than a "RISC-V" IP block, I'll ack it.
> >
> 
> The SiFive Test device has only one register at offset 0x0 and three
> possible magic values (0x3333, 0x5555, and 0x7777).
> 
> The SYSCON based Reboot and Poweroff driver do exactly the same
> thing what Christop's virt machine poweroff driver does so we are not
> doing "random register writes" via DT.
> 
> In fact, using SYSCON based Reboot and Poweroff we are actually
> describing the Reboot and Poweroff mechanism directly in DT without
> adding a complete driver for just one register write. This means we
> are totally aligned with "DT doctrine" and over here we going one-step
> more by describing Reboot and Poweroff mechanism in DT.
> 
> A quick GREP shows that the SYSCON Reboot and Poweroff drivers
> are quite widely used in ARM, ARM64 and MIPS architectures. Some of
> the  SOCs using these drivers are: Samsung Exynos, HiSilicon Hi3660,
> HiSilicon Hi6220, Rockchip RK3xxx, AppliedMicro XGene, Broadcom
> BCM33xx, Broadcom BCM63xx, etc. Majority of ARM/ARM64 SOCs
> these days use the PSCI based SYSTEM RESET and SHUTDOWN
> methods so we might not see more Reboot and Poweroff drivers for
> ARM world.
> 
> IMHO, we should definitely avoid adding a driver to Linux when there
> a generic driver already available. This helps in kernel maintenance
> in long-term.

I guess I should have finished reading the thread...

I agree with both of you. :) The DT binding should match the h/w as Paul 
says. However, a h/w specific binding can easily map to a generic driver 
if a given client OS has one. That probably hasn't been done yet for 
syscon-poweroff, but should.

Rob

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: QEMU RISC-V virt machine poweroff driver

[Nick Kossifidis]

Στις 2019-11-07 23:24, Christoph Hellwig έγραψε:
> Hi all,
> 
> this patch add a driver for the test device in the Qemu RISC-V
> virt machine which allows properly shutting down the VM.
> It also is added to the riscv defconfig given that qemu-virt
> is the most popular riscv platform.
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

We do this already through OpenSBI, I contributed a patch for this some 
time ago.

https://github.com/riscv/opensbi/commit/51e543511a7425da1a5378e149de0b45928c7111#diff-36ecc47313ff13e406c53b99471f294e

Last time I checked I could just halt and the system would shut down 
properly.

Regards,
Nick

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: QEMU RISC-V virt machine poweroff driver

[Anup Patel]

On Sat, Nov 16, 2019 at 3:45 AM Nick Kossifidis <mick@ics.forth.gr> wrote:
>
> Στις 2019-11-07 23:24, Christoph Hellwig έγραψε:
> > Hi all,
> >
> > this patch add a driver for the test device in the Qemu RISC-V
> > virt machine which allows properly shutting down the VM.
> > It also is added to the riscv defconfig given that qemu-virt
> > is the most popular riscv platform.
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> We do this already through OpenSBI, I contributed a patch for this some
> time ago.
>
> https://github.com/riscv/opensbi/commit/51e543511a7425da1a5378e149de0b45928c7111#diff-36ecc47313ff13e406c53b99471f294e
>
> Last time I checked I could just halt and the system would shut down
> properly.

We only have poweroff SBI call in SBI v0.1 spec.

Also, we had proposed SBI v0.2 SRST extension having both
Poweroff and Reboot SBI calls but before we can have this SBI
extension we need to clarify scope of SBI interface in RISC-V
privilege spec so that we can have system-level SBI calls defined
in SBI spec. In future, we will bring back the proposal for SBI v0.2
SRST extension.

There are two cases in which we might need SOC specific
Poweroff and Reboot drivers in Linux:
1. SBI v0.2 onwards all SBI calls are optional so SOC vendor
can choose to provide explicit driver instead of implementing
SBI calls for Poweroff and Reboot.
2. The M-mode NOMMU Linux does not have SBI interface
so for this case SOC vendors will have to provide explicit
drivers.

If SOC Poweroff and Reboot mechanisms are simple enough
then SOC vendors can straight away use SYSCON Poweroff
and SYSCON Reboot drivers instead of writing it from scratch.

Regards,
Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv