2019-01-24 08:20:56

by Peng Hao

[permalink] [raw]
Subject: [PATCH V4 0/6] add pvpanic driver framework

QEMU community requires additional PCI devices to simulate PVPANIC
devices so that some architectures can not occupy precious less than 4G
of memory space.
Previously, I added PCI driver directly to the original version of the driver,
which made the whole driver file look a bit cluttered. So Andy Shevchenko suggests:
"I would recommend to split it in a way how it's done for ChipIdea USB driver,
for example. (drivers/usb/chipidea if I'm not mistaken)".

v3 ---> v4 : add help text info in Konfig from patch_0004 to
patch_0006
adjust structure definition position in patch_0002
and patch_0003
v2 ---> v3 : add change infomation.

v1 ---> v2 : add patch 0000 to descript the whole patch series.
modify text infomation from patch_0002 to patch_0006.
modify "SPDX-License-Identifier: GPL-2.0-or-later"
to "SPDX-License-Identifier: GPL-2.0+"

Peng Hao (6):
misc/pvpanic: preparing for pvpanic driver framework
misc/pvpanic: Add pvpanic driver framework
misc/pvpanic: add API for pvpanic driver framework
misc/pvpanic: add pvpanic acpi driver
misc/pvpanic: add pvpanic mmio driver
misc/pvpanic: add new pvpanic pci driver

drivers/misc/Kconfig | 9 +-
drivers/misc/Makefile | 2 +-
drivers/misc/pvpanic.c | 192 ------------------------------------
drivers/misc/pvpanic/Kconfig | 34 ++++++
drivers/misc/pvpanic/Makefile | 8 ++
drivers/misc/pvpanic/pvpanic-acpi.c | 77 +++++++++++++++
drivers/misc/pvpanic/pvpanic-of.c | 53 ++++++++++
drivers/misc/pvpanic/pvpanic-pci.c | 56 +++++++++++
drivers/misc/pvpanic/pvpanic.c | 131 ++++++++++++++++++++++++
drivers/misc/pvpanic/pvpanic.h | 14 +++
10 files changed, 366 insertions(+), 201 deletions(-)
delete mode 100644 drivers/misc/pvpanic.c
create mode 100644 drivers/misc/pvpanic/Kconfig
create mode 100644 drivers/misc/pvpanic/Makefile
create mode 100644 drivers/misc/pvpanic/pvpanic-acpi.c
create mode 100644 drivers/misc/pvpanic/pvpanic-of.c
create mode 100644 drivers/misc/pvpanic/pvpanic-pci.c
create mode 100644 drivers/misc/pvpanic/pvpanic.c
create mode 100644 drivers/misc/pvpanic/pvpanic.h

--
1.8.3.1



2019-01-24 08:18:04

by Peng Hao

[permalink] [raw]
Subject: [PATCH V4 1/6] misc/pvpanic: preparing for pvpanic driver framework

Preparing for pvpanic driver framework. Create a pvpanic driver
directory and move current driver file to new directory.

Signed-off-by: Peng Hao <[email protected]>
---
drivers/misc/Kconfig | 9 +--------
drivers/misc/Makefile | 2 +-
drivers/misc/pvpanic/Kconfig | 7 +++++++
drivers/misc/pvpanic/Makefile | 5 +++++
drivers/misc/{ => pvpanic}/pvpanic.c | 0
5 files changed, 14 insertions(+), 9 deletions(-)
create mode 100644 drivers/misc/pvpanic/Kconfig
create mode 100644 drivers/misc/pvpanic/Makefile
rename drivers/misc/{ => pvpanic}/pvpanic.c (100%)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index f417b06..aa3a805 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -513,14 +513,7 @@ config MISC_RTSX
tristate
default MISC_RTSX_PCI || MISC_RTSX_USB

-config PVPANIC
- tristate "pvpanic device support"
- depends on HAS_IOMEM && (ACPI || OF)
- help
- This driver provides support for the pvpanic device. pvpanic is
- a paravirtualized device provided by QEMU; it lets a virtual machine
- (guest) communicate panic events to the host.
-
+source "drivers/misc/pvpanic/Kconfig"
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index e39ccbb..cfe20b3 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -58,4 +58,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
obj-$(CONFIG_OCXL) += ocxl/
obj-y += cardreader/
-obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_PVPANIC) += pvpanic/
diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
new file mode 100644
index 0000000..3e612c6
--- /dev/null
+++ b/drivers/misc/pvpanic/Kconfig
@@ -0,0 +1,7 @@
+config PVPANIC
+ tristate "pvpanic device support"
+ depends on HAS_IOMEM && (ACPI || OF)
+ help
+ This driver provides support for the pvpanic device. pvpanic is
+ a paravirtualized device provided by QEMU; it lets a virtual machine
+ (guest) communicate panic events to the host.
diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
new file mode 100644
index 0000000..6394224
--- /dev/null
+++ b/drivers/misc/pvpanic/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# Copyright (c) 2018 ZTE Ltd.
+
+obj-$(CONFIG_PVPANIC) += pvpanic.o
diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c
similarity index 100%
rename from drivers/misc/pvpanic.c
rename to drivers/misc/pvpanic/pvpanic.c
--
1.8.3.1


2019-01-24 08:18:20

by Peng Hao

[permalink] [raw]
Subject: [PATCH V4 5/6] misc/pvpanic: add pvpanic mmio driver

Make pvpanic mmio driver as separate file and modify code
in order to adapt the framework.

Signed-off-by: Peng Hao <[email protected]>
---
drivers/misc/pvpanic/Kconfig | 7 ++++++
drivers/misc/pvpanic/Makefile | 1 +
drivers/misc/pvpanic/pvpanic-of.c | 53 +++++++++++++++++++++++++++++++++++++++
3 files changed, 61 insertions(+)
create mode 100644 drivers/misc/pvpanic/pvpanic-of.c

diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
index 1dcfe20..14074af 100644
--- a/drivers/misc/pvpanic/Kconfig
+++ b/drivers/misc/pvpanic/Kconfig
@@ -16,5 +16,12 @@ config PVPANIC_ACPI
This driver is one specific driver for pvpanic driver framework.
It provides an acpi device as pvpanic device.

+config PVPANIC_OF
+ tristate "pvpanic mmio driver"
+ depends on OF
+ help
+ This driver is one specific driver for pvpanic driver framework.
+ It provides a mmio device as pvpanic device.
+
endif

diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
index c5b73ca..63ef0db 100644
--- a/drivers/misc/pvpanic/Makefile
+++ b/drivers/misc/pvpanic/Makefile
@@ -4,3 +4,4 @@

obj-$(CONFIG_PVPANIC) += pvpanic.o
obj-$(CONFIG_PVPANIC_ACPI) += pvpanic-acpi.o
+obj-$(CONFIG_PVPANIC_OF) += pvpanic-of.o
diff --git a/drivers/misc/pvpanic/pvpanic-of.c b/drivers/misc/pvpanic/pvpanic-of.c
new file mode 100644
index 0000000..73ca5f3
--- /dev/null
+++ b/drivers/misc/pvpanic/pvpanic-of.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * pvpanic of driver.
+ *
+ * Copyright (C) 2019 ZTE Ltd.
+ * Author: Peng Hao <[email protected]>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include "pvpanic.h"
+
+static int pvpanic_mmio_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ int ret;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EINVAL;
+
+ ret = pvpanic_add_device(&pdev->dev, res);
+ if (ret)
+ return -ENODEV;
+
+ return 0;
+}
+
+static int pvpanic_mmio_remove(struct platform_device *pdev)
+{
+ pvpanic_remove_device();
+ return 0;
+}
+
+static const struct of_device_id pvpanic_mmio_match[] = {
+ { .compatible = "qemu,pvpanic-mmio", },
+ {}
+};
+
+static struct platform_driver pvpanic_mmio_driver = {
+ .driver = {
+ .name = "pvpanic-mmio",
+ .of_match_table = pvpanic_mmio_match,
+ },
+ .probe = pvpanic_mmio_probe,
+ .remove = pvpanic_mmio_remove,
+};
+
+module_platform_driver(pvpanic_mmio_driver);
--
1.8.3.1


2019-01-24 08:18:25

by Peng Hao

[permalink] [raw]
Subject: [PATCH V4 4/6] misc/pvpanic: add pvpanic acpi driver

Make pvpanic acpi driver as separate file and modify code
in order to adapt the framework.

Signed-off-by: Peng Hao <[email protected]>
---
drivers/misc/pvpanic/Kconfig | 13 +++++++
drivers/misc/pvpanic/Makefile | 1 +
drivers/misc/pvpanic/pvpanic-acpi.c | 77 +++++++++++++++++++++++++++++++++++++
3 files changed, 91 insertions(+)
create mode 100644 drivers/misc/pvpanic/pvpanic-acpi.c

diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
index 3e612c6..1dcfe20 100644
--- a/drivers/misc/pvpanic/Kconfig
+++ b/drivers/misc/pvpanic/Kconfig
@@ -5,3 +5,16 @@ config PVPANIC
This driver provides support for the pvpanic device. pvpanic is
a paravirtualized device provided by QEMU; it lets a virtual machine
(guest) communicate panic events to the host.
+
+if PVPANIC
+
+config PVPANIC_ACPI
+ tristate "pvpanic acpi driver"
+ depends on ACPI
+ default PVPANIC
+ help
+ This driver is one specific driver for pvpanic driver framework.
+ It provides an acpi device as pvpanic device.
+
+endif
+
diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
index 6394224..c5b73ca 100644
--- a/drivers/misc/pvpanic/Makefile
+++ b/drivers/misc/pvpanic/Makefile
@@ -3,3 +3,4 @@
# Copyright (c) 2018 ZTE Ltd.

obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-$(CONFIG_PVPANIC_ACPI) += pvpanic-acpi.o
diff --git a/drivers/misc/pvpanic/pvpanic-acpi.c b/drivers/misc/pvpanic/pvpanic-acpi.c
new file mode 100644
index 0000000..8d10924
--- /dev/null
+++ b/drivers/misc/pvpanic/pvpanic-acpi.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * pvpanic acpi driver.
+ *
+ * Copyright (C) 2019 ZTE Ltd.
+ * Author: Peng Hao
+ */
+#include <linux/acpi.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include "pvpanic.h"
+
+static int pvpanic_add(struct acpi_device *device);
+static int pvpanic_remove(struct acpi_device *device);
+
+static const struct acpi_device_id pvpanic_device_ids[] = {
+ { "QEMU0001", 0 },
+ { "", 0 }
+};
+MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
+
+static struct acpi_driver pvpanic_driver = {
+ .name = "pvpanic",
+ .class = "QEMU",
+ .ids = pvpanic_device_ids,
+ .ops = {
+ .add = pvpanic_add,
+ .remove = pvpanic_remove,
+ },
+ .owner = THIS_MODULE,
+};
+
+static acpi_status
+pvpanic_walk_resources(struct acpi_resource *res, void *context)
+{
+ struct resource r;
+ int ret = 0;
+ struct device *dev = context;
+
+ memset(&r, 0, sizeof(r));
+ if (acpi_dev_resource_io(res, &r) || acpi_dev_resource_memory(res, &r))
+ ret = pvpanic_add_device(dev, &r);
+
+ if (!ret)
+ return AE_OK;
+
+ return AE_ERROR;
+}
+static int pvpanic_add(struct acpi_device *device)
+{
+ int ret;
+ acpi_status status;
+
+ ret = acpi_bus_get_status(device);
+ if (ret < 0)
+ return ret;
+
+ if (!device->status.enabled || !device->status.functional)
+ return -ENODEV;
+
+ status = acpi_walk_resources(device->handle, METHOD_NAME__CRS,
+ pvpanic_walk_resources, &device->dev);
+
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ return 0;
+}
+
+static int pvpanic_remove(struct acpi_device *device)
+{
+ pvpanic_remove_device();
+ return 0;
+}
+
+module_acpi_driver(pvpanic_driver);
--
1.8.3.1


2019-01-24 08:19:06

by Peng Hao

[permalink] [raw]
Subject: [PATCH V4 3/6] misc/pvpanic: add API for pvpanic driver framework

Add pvpanic_add/remove_device API. Follow-up patches will use them to
add/remove specific drivers into framework.

Signed-off-by: Peng Hao <[email protected]>
---
drivers/misc/pvpanic/pvpanic.c | 47 ++++++++++++++++++++++++++++++++++++++----
drivers/misc/pvpanic/pvpanic.h | 15 ++++++++++++++
2 files changed, 58 insertions(+), 4 deletions(-)
create mode 100644 drivers/misc/pvpanic/pvpanic.h

diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c
index f44a884..6225dfb 100644
--- a/drivers/misc/pvpanic/pvpanic.c
+++ b/drivers/misc/pvpanic/pvpanic.c
@@ -14,7 +14,11 @@
#include <linux/platform_device.h>
#include <linux/types.h>

-static void __iomem *base;
+static struct {
+ struct platform_device *pdev;
+ void __iomem *base;
+ bool is_ioport;
+} pvpanic_data;

#define PVPANIC_PANICKED (1 << 0)

@@ -25,7 +29,7 @@
static void
pvpanic_send_event(unsigned int event)
{
- iowrite8(event, base);
+ iowrite8(event, pvpanic_data.base);
}

static int
@@ -41,10 +45,43 @@
.priority = 1, /* let this called before broken drm_fb_helper */
};

+int pvpanic_add_device(struct device *dev, struct resource *res)
+{
+ struct platform_device *pdev;
+ int ret;
+
+ pdev = platform_device_alloc("pvpanic", -1);
+ if (!pdev)
+ return -ENOMEM;
+
+ pdev->dev.parent = dev;
+
+ ret = platform_device_add_resources(pdev, res, 1);
+ if (ret)
+ goto err;
+
+ ret = platform_device_add(pdev);
+ if (ret)
+ goto err;
+ pvpanic_data.pdev = pdev;
+
+ return 0;
+err:
+ platform_device_put(pdev);
+ return -1;
+}
+
+void pvpanic_remove_device(void)
+{
+ platform_device_unregister(pvpanic_data.pdev);
+ pvpanic_data.pdev = NULL;
+}
+
static int pvpanic_platform_probe (struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct resource *res;
+ void __iomem *base;

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (res) {
@@ -59,8 +96,10 @@ static int pvpanic_platform_probe (struct platform_device *pdev)
base = ioport_map(res->start, resource_size(res));
if (!base)
return -ENODEV;
+ pvpanic_data.is_ioport = true;
}

+ pvpanic_data.base = base;
atomic_notifier_chain_register(&panic_notifier_list,
&pvpanic_panic_nb);

@@ -71,8 +110,8 @@ static int pvpanic_platform_remove(struct platform_device *pdev)
{
atomic_notifier_chain_unregister(&panic_notifier_list,
&pvpanic_panic_nb);
-
- iounmap(base);
+ if (pvpanic_data.is_ioport)
+ iounmap(pvpanic_data.base);

return 0;
}
diff --git a/drivers/misc/pvpanic/pvpanic.h b/drivers/misc/pvpanic/pvpanic.h
new file mode 100644
index 0000000..7fb6bb2
--- /dev/null
+++ b/drivers/misc/pvpanic/pvpanic.h
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* pvpanic driver framework header file
+ *
+ * Copyright (C) 2019 ZTE Ltd.
+ *
+ * Author: Peng Hao <[email protected]>
+ */
+
+#ifndef __DRIVERS_MISC_PVPANIC_H
+#define __DRIVERS_MISC_PVPANIC_H
+
+extern int pvpanic_add_device(struct device *dev, struct resource *res);
+extern void pvpanic_remove_device(void);
+
+#endif
--
1.8.3.1


2019-01-24 08:19:49

by Peng Hao

[permalink] [raw]
Subject: [PATCH V4 6/6] misc/pvpanic: add new pvpanic pci driver

Add new pvpanic pci driver to pvpanic driver framework.

Signed-off-by: Peng Hao <[email protected]>
---
drivers/misc/pvpanic/Kconfig | 10 ++++++-
drivers/misc/pvpanic/Makefile | 1 +
drivers/misc/pvpanic/pvpanic-pci.c | 56 ++++++++++++++++++++++++++++++++++++++
3 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 drivers/misc/pvpanic/pvpanic-pci.c

diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
index 14074af..f24c488 100644
--- a/drivers/misc/pvpanic/Kconfig
+++ b/drivers/misc/pvpanic/Kconfig
@@ -1,6 +1,6 @@
config PVPANIC
tristate "pvpanic device support"
- depends on HAS_IOMEM && (ACPI || OF)
+ depends on HAS_IOMEM && (ACPI || OF || PCI)
help
This driver provides support for the pvpanic device. pvpanic is
a paravirtualized device provided by QEMU; it lets a virtual machine
@@ -23,5 +23,13 @@ config PVPANIC_OF
This driver is one specific driver for pvpanic driver framework.
It provides a mmio device as pvpanic device.

+config PVPANIC_PCI
+ tristate "pvpanic pci driver"
+ depends on PCI
+ default PVPANIC
+ help
+ This driver is one specific driver for pvpanic driver framework.
+ It provides a pci device as pvpanic device.
+
endif

diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
index 63ef0db..7c71f85 100644
--- a/drivers/misc/pvpanic/Makefile
+++ b/drivers/misc/pvpanic/Makefile
@@ -5,3 +5,4 @@
obj-$(CONFIG_PVPANIC) += pvpanic.o
obj-$(CONFIG_PVPANIC_ACPI) += pvpanic-acpi.o
obj-$(CONFIG_PVPANIC_OF) += pvpanic-of.o
+obj-$(CONFIG_PVPANIC_PCI) += pvpanic-pci.o
diff --git a/drivers/misc/pvpanic/pvpanic-pci.c b/drivers/misc/pvpanic/pvpanic-pci.c
new file mode 100644
index 0000000..1261710
--- /dev/null
+++ b/drivers/misc/pvpanic/pvpanic-pci.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * pvpanic acpi driver.
+ *
+ * Copyright (C) 2019 ZTE Ltd.
+ * Author: Peng Hao <[email protected]>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/types.h>
+#include "pvpanic.h"
+
+#define PCI_VENDOR_ID_REDHAT 0x1b36
+#define PCI_DEVICE_ID_REDHAT_PVPANIC 0x0101
+
+static const struct pci_device_id pvpanic_pci_id_tbl[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_PVPANIC),},
+ {}
+};
+
+static int pvpanic_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ int ret;
+ struct resource res;
+
+ ret = pcim_enable_device(pdev);
+ if (ret < 0)
+ return ret;
+
+ memset(&res, 0, sizeof(res));
+ res.start = pci_resource_start(pdev, 0);
+ res.end = pci_resource_end(pdev, 0);
+ res.flags = IORESOURCE_MEM;
+ ret = pvpanic_add_device(&pdev->dev, &res);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static void pvpanic_pci_remove(struct pci_dev *pdev)
+{
+ pvpanic_remove_device();
+}
+
+static struct pci_driver pvpanic_pci_driver = {
+ .name = "pvpanic-pci",
+ .id_table = pvpanic_pci_id_tbl,
+ .probe = pvpanic_pci_probe,
+ .remove = pvpanic_pci_remove,
+};
+
+module_pci_driver(pvpanic_pci_driver);
--
1.8.3.1


2019-01-24 08:20:08

by Peng Hao

[permalink] [raw]
Subject: [PATCH V4 2/6] misc/pvpanic: Add pvpanic driver framework

Add pvpanic driver framework. Follow-up patches will split the original
pvpanic acpi/of driver as the two separate files and modify code to
adapt the framework.

Signed-off-by: Peng Hao <[email protected]>
---
drivers/misc/pvpanic/pvpanic.c | 158 +++++++----------------------------------
1 file changed, 27 insertions(+), 131 deletions(-)

diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c
index 595ac06..f44a884 100644
--- a/drivers/misc/pvpanic/pvpanic.c
+++ b/drivers/misc/pvpanic/pvpanic.c
@@ -8,11 +8,9 @@

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

-#include <linux/acpi.h>
+#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/types.h>

@@ -43,59 +41,25 @@
.priority = 1, /* let this called before broken drm_fb_helper */
};

-#ifdef CONFIG_ACPI
-static int pvpanic_add(struct acpi_device *device);
-static int pvpanic_remove(struct acpi_device *device);
-
-static const struct acpi_device_id pvpanic_device_ids[] = {
- { "QEMU0001", 0 },
- { "", 0 }
-};
-MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
-
-static struct acpi_driver pvpanic_driver = {
- .name = "pvpanic",
- .class = "QEMU",
- .ids = pvpanic_device_ids,
- .ops = {
- .add = pvpanic_add,
- .remove = pvpanic_remove,
- },
- .owner = THIS_MODULE,
-};
-
-static acpi_status
-pvpanic_walk_resources(struct acpi_resource *res, void *context)
+static int pvpanic_platform_probe(struct platform_device *pdev)
{
- struct resource r;
-
- if (acpi_dev_resource_io(res, &r)) {
- base = ioport_map(r.start, resource_size(&r));
- return AE_OK;
- } else if (acpi_dev_resource_memory(res, &r)) {
- base = ioremap(r.start, resource_size(&r));
- return AE_OK;
- }
-
- return AE_ERROR;
-}
-
-static int pvpanic_add(struct acpi_device *device)
-{
- int ret;
-
- ret = acpi_bus_get_status(device);
- if (ret < 0)
- return ret;
-
- if (!device->status.enabled || !device->status.functional)
- return -ENODEV;
-
- acpi_walk_resources(device->handle, METHOD_NAME__CRS,
- pvpanic_walk_resources, NULL);
-
- if (!base)
- return -ENODEV;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (res) {
+ base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(base))
+ return -ENODEV;
+ } else {
+ res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ if (!res)
+ return -ENODEV;
+
+ base = ioport_map(res->start, resource_size(res));
+ if (!base)
+ return -ENODEV;
+ }

atomic_notifier_chain_register(&panic_notifier_list,
&pvpanic_panic_nb);
@@ -103,90 +67,22 @@ static int pvpanic_add(struct acpi_device *device)
return 0;
}

-static int pvpanic_remove(struct acpi_device *device)
+static int pvpanic_platform_remove(struct platform_device *pdev)
{
-
atomic_notifier_chain_unregister(&panic_notifier_list,
&pvpanic_panic_nb);
- iounmap(base);
-
- return 0;
-}
-
-static int pvpanic_register_acpi_driver(void)
-{
- return acpi_bus_register_driver(&pvpanic_driver);
-}
-
-static void pvpanic_unregister_acpi_driver(void)
-{
- acpi_bus_unregister_driver(&pvpanic_driver);
-}
-#else
-static int pvpanic_register_acpi_driver(void)
-{
- return -ENODEV;
-}

-static void pvpanic_unregister_acpi_driver(void) {}
-#endif
-
-static int pvpanic_mmio_probe(struct platform_device *pdev)
-{
- struct resource *mem;
-
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem)
- return -EINVAL;
-
- base = devm_ioremap_resource(&pdev->dev, mem);
- if (IS_ERR(base))
- return PTR_ERR(base);
-
- atomic_notifier_chain_register(&panic_notifier_list,
- &pvpanic_panic_nb);
-
- return 0;
-}
-
-static int pvpanic_mmio_remove(struct platform_device *pdev)
-{
-
- atomic_notifier_chain_unregister(&panic_notifier_list,
- &pvpanic_panic_nb);
+ iounmap(base);

return 0;
}

-static const struct of_device_id pvpanic_mmio_match[] = {
- { .compatible = "qemu,pvpanic-mmio", },
- {}
-};
-
-static struct platform_driver pvpanic_mmio_driver = {
+static struct platform_driver pvpanic_driver = {
+ .probe = pvpanic_platform_probe,
+ .remove = pvpanic_platform_remove,
.driver = {
- .name = "pvpanic-mmio",
- .of_match_table = pvpanic_mmio_match,
- },
- .probe = pvpanic_mmio_probe,
- .remove = pvpanic_mmio_remove,
+ .name = "pvpanic",
+ }
};

-static int __init pvpanic_mmio_init(void)
-{
- if (acpi_disabled)
- return platform_driver_register(&pvpanic_mmio_driver);
- else
- return pvpanic_register_acpi_driver();
-}
-
-static void __exit pvpanic_mmio_exit(void)
-{
- if (acpi_disabled)
- platform_driver_unregister(&pvpanic_mmio_driver);
- else
- pvpanic_unregister_acpi_driver();
-}
-
-module_init(pvpanic_mmio_init);
-module_exit(pvpanic_mmio_exit);
+module_platform_driver(pvpanic_driver);
--
1.8.3.1


2019-01-24 13:48:43

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH V4 1/6] misc/pvpanic: preparing for pvpanic driver framework

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <[email protected]> wrote:
>
> Preparing for pvpanic driver framework. Create a pvpanic driver
> directory and move current driver file to new directory.
>

This one is okay.

> Signed-off-by: Peng Hao <[email protected]>
> ---
> drivers/misc/Kconfig | 9 +--------
> drivers/misc/Makefile | 2 +-
> drivers/misc/pvpanic/Kconfig | 7 +++++++
> drivers/misc/pvpanic/Makefile | 5 +++++
> drivers/misc/{ => pvpanic}/pvpanic.c | 0
> 5 files changed, 14 insertions(+), 9 deletions(-)
> create mode 100644 drivers/misc/pvpanic/Kconfig
> create mode 100644 drivers/misc/pvpanic/Makefile
> rename drivers/misc/{ => pvpanic}/pvpanic.c (100%)
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f417b06..aa3a805 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -513,14 +513,7 @@ config MISC_RTSX
> tristate
> default MISC_RTSX_PCI || MISC_RTSX_USB
>
> -config PVPANIC
> - tristate "pvpanic device support"
> - depends on HAS_IOMEM && (ACPI || OF)
> - help
> - This driver provides support for the pvpanic device. pvpanic is
> - a paravirtualized device provided by QEMU; it lets a virtual machine
> - (guest) communicate panic events to the host.
> -
> +source "drivers/misc/pvpanic/Kconfig"
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index e39ccbb..cfe20b3 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -58,4 +58,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
> obj-$(CONFIG_OCXL) += ocxl/
> obj-y += cardreader/
> -obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-$(CONFIG_PVPANIC) += pvpanic/
> diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
> new file mode 100644
> index 0000000..3e612c6
> --- /dev/null
> +++ b/drivers/misc/pvpanic/Kconfig
> @@ -0,0 +1,7 @@
> +config PVPANIC
> + tristate "pvpanic device support"
> + depends on HAS_IOMEM && (ACPI || OF)
> + help
> + This driver provides support for the pvpanic device. pvpanic is
> + a paravirtualized device provided by QEMU; it lets a virtual machine
> + (guest) communicate panic events to the host.
> diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
> new file mode 100644
> index 0000000..6394224
> --- /dev/null
> +++ b/drivers/misc/pvpanic/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +#
> +# Copyright (c) 2018 ZTE Ltd.
> +
> +obj-$(CONFIG_PVPANIC) += pvpanic.o
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c
> similarity index 100%
> rename from drivers/misc/pvpanic.c
> rename to drivers/misc/pvpanic/pvpanic.c
> --
> 1.8.3.1
>


--
With Best Regards,
Andy Shevchenko

2019-01-24 13:49:40

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH V4 2/6] misc/pvpanic: Add pvpanic driver framework

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <[email protected]> wrote:
>
> Add pvpanic driver framework. Follow-up patches will split the original
> pvpanic acpi/of driver as the two separate files and modify code to
> adapt the framework.
>

This one has run-time bisectability issues.

> Signed-off-by: Peng Hao <[email protected]>
> ---
> drivers/misc/pvpanic/pvpanic.c | 158 +++++++----------------------------------
> 1 file changed, 27 insertions(+), 131 deletions(-)
>
> diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c
> index 595ac06..f44a884 100644
> --- a/drivers/misc/pvpanic/pvpanic.c
> +++ b/drivers/misc/pvpanic/pvpanic.c
> @@ -8,11 +8,9 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> -#include <linux/acpi.h>
> +#include <linux/io.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> -#include <linux/of.h>
> -#include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/types.h>
>
> @@ -43,59 +41,25 @@
> .priority = 1, /* let this called before broken drm_fb_helper */
> };
>
> -#ifdef CONFIG_ACPI
> -static int pvpanic_add(struct acpi_device *device);
> -static int pvpanic_remove(struct acpi_device *device);
> -
> -static const struct acpi_device_id pvpanic_device_ids[] = {
> - { "QEMU0001", 0 },
> - { "", 0 }
> -};
> -MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
> -
> -static struct acpi_driver pvpanic_driver = {
> - .name = "pvpanic",
> - .class = "QEMU",
> - .ids = pvpanic_device_ids,
> - .ops = {
> - .add = pvpanic_add,
> - .remove = pvpanic_remove,
> - },
> - .owner = THIS_MODULE,
> -};
> -
> -static acpi_status
> -pvpanic_walk_resources(struct acpi_resource *res, void *context)
> +static int pvpanic_platform_probe(struct platform_device *pdev)
> {
> - struct resource r;
> -
> - if (acpi_dev_resource_io(res, &r)) {
> - base = ioport_map(r.start, resource_size(&r));
> - return AE_OK;
> - } else if (acpi_dev_resource_memory(res, &r)) {
> - base = ioremap(r.start, resource_size(&r));
> - return AE_OK;
> - }
> -
> - return AE_ERROR;
> -}
> -
> -static int pvpanic_add(struct acpi_device *device)
> -{
> - int ret;
> -
> - ret = acpi_bus_get_status(device);
> - if (ret < 0)
> - return ret;
> -
> - if (!device->status.enabled || !device->status.functional)
> - return -ENODEV;
> -
> - acpi_walk_resources(device->handle, METHOD_NAME__CRS,
> - pvpanic_walk_resources, NULL);
> -
> - if (!base)
> - return -ENODEV;
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (res) {
> + base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base))
> + return -ENODEV;
> + } else {
> + res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> + if (!res)
> + return -ENODEV;
> +
> + base = ioport_map(res->start, resource_size(res));
> + if (!base)
> + return -ENODEV;
> + }
>
> atomic_notifier_chain_register(&panic_notifier_list,
> &pvpanic_panic_nb);
> @@ -103,90 +67,22 @@ static int pvpanic_add(struct acpi_device *device)
> return 0;
> }
>
> -static int pvpanic_remove(struct acpi_device *device)
> +static int pvpanic_platform_remove(struct platform_device *pdev)
> {
> -
> atomic_notifier_chain_unregister(&panic_notifier_list,
> &pvpanic_panic_nb);
> - iounmap(base);
> -
> - return 0;
> -}
> -
> -static int pvpanic_register_acpi_driver(void)
> -{
> - return acpi_bus_register_driver(&pvpanic_driver);
> -}
> -
> -static void pvpanic_unregister_acpi_driver(void)
> -{
> - acpi_bus_unregister_driver(&pvpanic_driver);
> -}
> -#else
> -static int pvpanic_register_acpi_driver(void)
> -{
> - return -ENODEV;
> -}
>
> -static void pvpanic_unregister_acpi_driver(void) {}
> -#endif
> -
> -static int pvpanic_mmio_probe(struct platform_device *pdev)
> -{
> - struct resource *mem;
> -
> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!mem)
> - return -EINVAL;
> -
> - base = devm_ioremap_resource(&pdev->dev, mem);
> - if (IS_ERR(base))
> - return PTR_ERR(base);
> -
> - atomic_notifier_chain_register(&panic_notifier_list,
> - &pvpanic_panic_nb);
> -
> - return 0;
> -}
> -
> -static int pvpanic_mmio_remove(struct platform_device *pdev)
> -{
> -
> - atomic_notifier_chain_unregister(&panic_notifier_list,
> - &pvpanic_panic_nb);
> + iounmap(base);
>
> return 0;
> }
>
> -static const struct of_device_id pvpanic_mmio_match[] = {
> - { .compatible = "qemu,pvpanic-mmio", },
> - {}
> -};
> -
> -static struct platform_driver pvpanic_mmio_driver = {
> +static struct platform_driver pvpanic_driver = {
> + .probe = pvpanic_platform_probe,
> + .remove = pvpanic_platform_remove,
> .driver = {
> - .name = "pvpanic-mmio",
> - .of_match_table = pvpanic_mmio_match,
> - },
> - .probe = pvpanic_mmio_probe,
> - .remove = pvpanic_mmio_remove,
> + .name = "pvpanic",
> + }
> };
>
> -static int __init pvpanic_mmio_init(void)
> -{
> - if (acpi_disabled)
> - return platform_driver_register(&pvpanic_mmio_driver);
> - else
> - return pvpanic_register_acpi_driver();
> -}
> -
> -static void __exit pvpanic_mmio_exit(void)
> -{
> - if (acpi_disabled)
> - platform_driver_unregister(&pvpanic_mmio_driver);
> - else
> - pvpanic_unregister_acpi_driver();
> -}
> -
> -module_init(pvpanic_mmio_init);
> -module_exit(pvpanic_mmio_exit);
> +module_platform_driver(pvpanic_driver);
> --
> 1.8.3.1
>


--
With Best Regards,
Andy Shevchenko

2019-01-24 13:54:45

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH V4 3/6] misc/pvpanic: add API for pvpanic driver framework

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <[email protected]> wrote:
>
> Add pvpanic_add/remove_device API. Follow-up patches will use them to
> add/remove specific drivers into framework.

I'm not sure this is the best way to instantiate the drivers.

Code with platfrom_device_add*() is related to platform which
instantiates the necessary set of the drivers.
In case of OF it should be done in Device Tree, in ACPI case you would
have some device description for that in DSDT table.

> +int pvpanic_add_device(struct device *dev, struct resource *res)
> +{
> + struct platform_device *pdev;
> + int ret;
> +
> + pdev = platform_device_alloc("pvpanic", -1);
> + if (!pdev)
> + return -ENOMEM;
> +
> + pdev->dev.parent = dev;
> +
> + ret = platform_device_add_resources(pdev, res, 1);
> + if (ret)
> + goto err;
> +
> + ret = platform_device_add(pdev);
> + if (ret)
> + goto err;
> + pvpanic_data.pdev = pdev;
> +
> + return 0;
> +err:
> + platform_device_put(pdev);

> + return -1;

It should return proper %-ERRNO code. I think to achieve this the
following can be used

return ret;

> +}

> static int pvpanic_platform_probe (struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct resource *res;
> + void __iomem *base;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (res) {
> @@ -59,8 +96,10 @@ static int pvpanic_platform_probe (struct platform_device *pdev)
> base = ioport_map(res->start, resource_size(res));
> if (!base)
> return -ENODEV;

> + pvpanic_data.is_ioport = true;

You better allocate this on a heap and put as a pointer to the device
driver private data.

> }
>
> + pvpanic_data.base = base;

Ditto for entire pvpanic_data instance.

--
With Best Regards,
Andy Shevchenko

2019-01-24 13:56:10

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH V4 4/6] misc/pvpanic: add pvpanic acpi driver

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <[email protected]> wrote:
>
> Make pvpanic acpi driver as separate file and modify code
> in order to adapt the framework.

This should be patch 2 in this series to avoid run-time bisectability issues.

>
> Signed-off-by: Peng Hao <[email protected]>
> ---
> drivers/misc/pvpanic/Kconfig | 13 +++++++
> drivers/misc/pvpanic/Makefile | 1 +
> drivers/misc/pvpanic/pvpanic-acpi.c | 77 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 91 insertions(+)
> create mode 100644 drivers/misc/pvpanic/pvpanic-acpi.c
>
> diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
> index 3e612c6..1dcfe20 100644
> --- a/drivers/misc/pvpanic/Kconfig
> +++ b/drivers/misc/pvpanic/Kconfig
> @@ -5,3 +5,16 @@ config PVPANIC
> This driver provides support for the pvpanic device. pvpanic is
> a paravirtualized device provided by QEMU; it lets a virtual machine
> (guest) communicate panic events to the host.
> +
> +if PVPANIC
> +
> +config PVPANIC_ACPI
> + tristate "pvpanic acpi driver"
> + depends on ACPI
> + default PVPANIC
> + help
> + This driver is one specific driver for pvpanic driver framework.
> + It provides an acpi device as pvpanic device.
> +
> +endif
> +
> diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
> index 6394224..c5b73ca 100644
> --- a/drivers/misc/pvpanic/Makefile
> +++ b/drivers/misc/pvpanic/Makefile
> @@ -3,3 +3,4 @@
> # Copyright (c) 2018 ZTE Ltd.
>
> obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-$(CONFIG_PVPANIC_ACPI) += pvpanic-acpi.o
> diff --git a/drivers/misc/pvpanic/pvpanic-acpi.c b/drivers/misc/pvpanic/pvpanic-acpi.c
> new file mode 100644
> index 0000000..8d10924
> --- /dev/null
> +++ b/drivers/misc/pvpanic/pvpanic-acpi.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * pvpanic acpi driver.
> + *
> + * Copyright (C) 2019 ZTE Ltd.
> + * Author: Peng Hao
> + */
> +#include <linux/acpi.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include "pvpanic.h"
> +
> +static int pvpanic_add(struct acpi_device *device);
> +static int pvpanic_remove(struct acpi_device *device);
> +
> +static const struct acpi_device_id pvpanic_device_ids[] = {
> + { "QEMU0001", 0 },
> + { "", 0 }
> +};
> +MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
> +
> +static struct acpi_driver pvpanic_driver = {
> + .name = "pvpanic",
> + .class = "QEMU",
> + .ids = pvpanic_device_ids,
> + .ops = {
> + .add = pvpanic_add,
> + .remove = pvpanic_remove,
> + },
> + .owner = THIS_MODULE,
> +};
> +
> +static acpi_status
> +pvpanic_walk_resources(struct acpi_resource *res, void *context)
> +{
> + struct resource r;
> + int ret = 0;
> + struct device *dev = context;
> +
> + memset(&r, 0, sizeof(r));
> + if (acpi_dev_resource_io(res, &r) || acpi_dev_resource_memory(res, &r))
> + ret = pvpanic_add_device(dev, &r);
> +
> + if (!ret)
> + return AE_OK;
> +
> + return AE_ERROR;
> +}
> +static int pvpanic_add(struct acpi_device *device)
> +{
> + int ret;
> + acpi_status status;
> +
> + ret = acpi_bus_get_status(device);
> + if (ret < 0)
> + return ret;
> +
> + if (!device->status.enabled || !device->status.functional)
> + return -ENODEV;
> +
> + status = acpi_walk_resources(device->handle, METHOD_NAME__CRS,
> + pvpanic_walk_resources, &device->dev);
> +
> + if (ACPI_FAILURE(status))
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static int pvpanic_remove(struct acpi_device *device)
> +{
> + pvpanic_remove_device();
> + return 0;
> +}
> +
> +module_acpi_driver(pvpanic_driver);
> --
> 1.8.3.1
>


--
With Best Regards,
Andy Shevchenko

2019-01-24 13:58:00

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH V4 5/6] misc/pvpanic: add pvpanic mmio driver

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <[email protected]> wrote:
>
> Make pvpanic mmio driver as separate file and modify code
> in order to adapt the framework.

This should go as patch 3

>
> Signed-off-by: Peng Hao <[email protected]>
> ---
> drivers/misc/pvpanic/Kconfig | 7 ++++++
> drivers/misc/pvpanic/Makefile | 1 +
> drivers/misc/pvpanic/pvpanic-of.c | 53 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 61 insertions(+)
> create mode 100644 drivers/misc/pvpanic/pvpanic-of.c
>
> diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
> index 1dcfe20..14074af 100644
> --- a/drivers/misc/pvpanic/Kconfig
> +++ b/drivers/misc/pvpanic/Kconfig
> @@ -16,5 +16,12 @@ config PVPANIC_ACPI
> This driver is one specific driver for pvpanic driver framework.
> It provides an acpi device as pvpanic device.
>
> +config PVPANIC_OF
> + tristate "pvpanic mmio driver"
> + depends on OF
> + help
> + This driver is one specific driver for pvpanic driver framework.
> + It provides a mmio device as pvpanic device.
> +
> endif
>
> diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
> index c5b73ca..63ef0db 100644
> --- a/drivers/misc/pvpanic/Makefile
> +++ b/drivers/misc/pvpanic/Makefile
> @@ -4,3 +4,4 @@
>
> obj-$(CONFIG_PVPANIC) += pvpanic.o
> obj-$(CONFIG_PVPANIC_ACPI) += pvpanic-acpi.o
> +obj-$(CONFIG_PVPANIC_OF) += pvpanic-of.o
> diff --git a/drivers/misc/pvpanic/pvpanic-of.c b/drivers/misc/pvpanic/pvpanic-of.c
> new file mode 100644
> index 0000000..73ca5f3
> --- /dev/null
> +++ b/drivers/misc/pvpanic/pvpanic-of.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * pvpanic of driver.
> + *
> + * Copyright (C) 2019 ZTE Ltd.
> + * Author: Peng Hao <[email protected]>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +#include "pvpanic.h"
> +
> +static int pvpanic_mmio_probe(struct platform_device *pdev)
> +{
> + struct resource *res;
> + int ret;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -EINVAL;
> +
> + ret = pvpanic_add_device(&pdev->dev, res);
> + if (ret)
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static int pvpanic_mmio_remove(struct platform_device *pdev)
> +{
> + pvpanic_remove_device();
> + return 0;
> +}
> +
> +static const struct of_device_id pvpanic_mmio_match[] = {
> + { .compatible = "qemu,pvpanic-mmio", },
> + {}
> +};
> +
> +static struct platform_driver pvpanic_mmio_driver = {
> + .driver = {
> + .name = "pvpanic-mmio",
> + .of_match_table = pvpanic_mmio_match,
> + },
> + .probe = pvpanic_mmio_probe,
> + .remove = pvpanic_mmio_remove,
> +};
> +
> +module_platform_driver(pvpanic_mmio_driver);
> --
> 1.8.3.1
>


--
With Best Regards,
Andy Shevchenko

2019-01-24 14:01:08

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH V4 0/6] add pvpanic driver framework

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <[email protected]> wrote:
>
> QEMU community requires additional PCI devices to simulate PVPANIC
> devices so that some architectures can not occupy precious less than 4G
> of memory space.
> Previously, I added PCI driver directly to the original version of the driver,
> which made the whole driver file look a bit cluttered. So Andy Shevchenko suggests:
> "I would recommend to split it in a way how it's done for ChipIdea USB driver,
> for example. (drivers/usb/chipidea if I'm not mistaken)".
>

Thank you for an update.

The series has the bisectability issues.

Try to use the following approach (if I got your idea right):
1. Convert to use common probe() etc functions inplace
2. Split ACPI part
3. Split OF part
4. Add PCI part

In Kconfig and Makefile follow the example like present under
drivers/usb/chipidea/.

> v3 ---> v4 : add help text info in Konfig from patch_0004 to
> patch_0006
> adjust structure definition position in patch_0002
> and patch_0003
> v2 ---> v3 : add change infomation.
>
> v1 ---> v2 : add patch 0000 to descript the whole patch series.
> modify text infomation from patch_0002 to patch_0006.
> modify "SPDX-License-Identifier: GPL-2.0-or-later"
> to "SPDX-License-Identifier: GPL-2.0+"
>
> Peng Hao (6):
> misc/pvpanic: preparing for pvpanic driver framework
> misc/pvpanic: Add pvpanic driver framework
> misc/pvpanic: add API for pvpanic driver framework
> misc/pvpanic: add pvpanic acpi driver
> misc/pvpanic: add pvpanic mmio driver
> misc/pvpanic: add new pvpanic pci driver
>
> drivers/misc/Kconfig | 9 +-
> drivers/misc/Makefile | 2 +-
> drivers/misc/pvpanic.c | 192 ------------------------------------
> drivers/misc/pvpanic/Kconfig | 34 ++++++
> drivers/misc/pvpanic/Makefile | 8 ++
> drivers/misc/pvpanic/pvpanic-acpi.c | 77 +++++++++++++++
> drivers/misc/pvpanic/pvpanic-of.c | 53 ++++++++++
> drivers/misc/pvpanic/pvpanic-pci.c | 56 +++++++++++
> drivers/misc/pvpanic/pvpanic.c | 131 ++++++++++++++++++++++++
> drivers/misc/pvpanic/pvpanic.h | 14 +++
> 10 files changed, 366 insertions(+), 201 deletions(-)
> delete mode 100644 drivers/misc/pvpanic.c
> create mode 100644 drivers/misc/pvpanic/Kconfig
> create mode 100644 drivers/misc/pvpanic/Makefile
> create mode 100644 drivers/misc/pvpanic/pvpanic-acpi.c
> create mode 100644 drivers/misc/pvpanic/pvpanic-of.c
> create mode 100644 drivers/misc/pvpanic/pvpanic-pci.c
> create mode 100644 drivers/misc/pvpanic/pvpanic.c
> create mode 100644 drivers/misc/pvpanic/pvpanic.h
>
> --
> 1.8.3.1
>


--
With Best Regards,
Andy Shevchenko

2019-02-05 15:33:12

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH V4 0/6] add pvpanic driver framework

On Sun, Feb 3, 2019 at 11:23 AM <[email protected]> wrote:
> >> QEMU community requires additional PCI devices to simulate PVPANIC
> >> devices so that some architectures can not occupy precious less than 4G
> >> of memory space.
> >> Previously, I added PCI driver directly to the original version of the driver,
> >> which made the whole driver file look a bit cluttered. So Andy Shevchenko suggests:
> >> "I would recommend to split it in a way how it's done for ChipIdea USB driver,
> >> for example. (drivers/usb/chipidea if I'm not mistaken)".

> >Thank you for an update.
> >
> >The series has the bisectability issues.
> >
> >Try to use the following approach (if I got your idea right):
> >1. Convert to use common probe() etc functions inplace

> I think there is no "common probe" for acpi/OF/PCI .

In that case a careful split would be preferred.

> In this driver framework (like chipidea driver framework), acpi/OF/PCI
> driver's probe trigger the driver framework's probe. In the driver framework
> it create a new platform device as the acpi/OF/PCI device's child device.

> I don't understand how the bisectability issues happen.

IIRC you have a gap in between of couple of patches where in first
some functionality is removed and reappeared later on.


--
With Best Regards,
Andy Shevchenko