[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YCZVvxxHI8mMEwTC@kroah.com>
Date: Fri, 12 Feb 2021 11:17:35 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Mihai Carabas <mihai.carabas@...cle.com>
Cc: linux-kernel@...r.kernel.org, arnd@...db.de, rdunlap@...radead.org,
bobo.shaobowang@...wei.com
Subject: Re: [PATCH v2 1/3] misc/pvpanic: split-up generic and platform
dependent code
On Fri, Feb 12, 2021 at 11:17:04AM +0200, Mihai Carabas wrote:
> Split-up generic and platform dependent code in order to be able to re-use
> generic event handling code in pvpanic PCI device driver in the next patch.
>
> The code from pvpanic.c was split in two new files:
> - pvpanic-common.c: generic code that handles pvpanic events
> - mmio.c: platform/bus dependent code
>
> Signed-off-by: Mihai Carabas <mihai.carabas@...cle.com>
> ---
> drivers/misc/Kconfig | 9 +--
> drivers/misc/Makefile | 2 +-
> drivers/misc/pvpanic.c | 111 ----------------------------------
> drivers/misc/pvpanic/Kconfig | 12 ++++
> drivers/misc/pvpanic/Makefile | 2 +
> drivers/misc/pvpanic/mmio.c | 83 +++++++++++++++++++++++++
> drivers/misc/pvpanic/pvpanic-common.c | 60 ++++++++++++++++++
> drivers/misc/pvpanic/pvpanic.h | 10 +++
> 8 files changed, 169 insertions(+), 120 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/mmio.c
> create mode 100644 drivers/misc/pvpanic/pvpanic-common.c
> create mode 100644 drivers/misc/pvpanic/pvpanic.h
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index fafa8b0..0273ecb 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -448,14 +448,6 @@ 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.
> -
> config HISI_HIKEY_USB
> tristate "USB GPIO Hub on HiSilicon Hikey 960/970 Platform"
> depends on (OF && GPIOLIB) || COMPILE_TEST
> @@ -481,4 +473,5 @@ source "drivers/misc/ocxl/Kconfig"
> source "drivers/misc/cardreader/Kconfig"
> source "drivers/misc/habanalabs/Kconfig"
> source "drivers/misc/uacce/Kconfig"
> +source "drivers/misc/pvpanic/Kconfig"
> endmenu
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index d23231e..9f411b8 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -52,7 +52,7 @@ obj-$(CONFIG_CXL_BASE) += cxl/
> 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/
> obj-$(CONFIG_HABANA_AI) += habanalabs/
> obj-$(CONFIG_UACCE) += uacce/
> obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> deleted file mode 100644
> index 41cab29..00000000
> --- a/drivers/misc/pvpanic.c
> +++ /dev/null
> @@ -1,111 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+
> -/*
> - * Pvpanic Device Support
> - *
> - * Copyright (C) 2013 Fujitsu.
> - * Copyright (C) 2018 ZTE.
> - */
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/io.h>
> -#include <linux/kernel.h>
> -#include <linux/kexec.h>
> -#include <linux/mod_devicetable.h>
> -#include <linux/module.h>
> -#include <linux/platform_device.h>
> -#include <linux/types.h>
> -
> -#include <uapi/misc/pvpanic.h>
> -
> -static void __iomem *base;
> -
> -MODULE_AUTHOR("Hu Tao <hutao@...fujitsu.com>");
> -MODULE_DESCRIPTION("pvpanic device driver");
> -MODULE_LICENSE("GPL");
> -
> -static void
> -pvpanic_send_event(unsigned int event)
> -{
> - iowrite8(event, base);
> -}
> -
> -static int
> -pvpanic_panic_notify(struct notifier_block *nb, unsigned long code,
> - void *unused)
> -{
> - unsigned int event = PVPANIC_PANICKED;
> -
> - if (kexec_crash_loaded())
> - event = PVPANIC_CRASH_LOADED;
> -
> - pvpanic_send_event(event);
> -
> - return NOTIFY_DONE;
> -}
> -
> -static struct notifier_block pvpanic_panic_nb = {
> - .notifier_call = pvpanic_panic_notify,
> - .priority = 1, /* let this called before broken drm_fb_helper */
> -};
> -
> -static int pvpanic_mmio_probe(struct platform_device *pdev)
> -{
> - struct device *dev = &pdev->dev;
> - struct resource *res;
> -
> - res = platform_get_mem_or_io(pdev, 0);
> - if (!res)
> - return -EINVAL;
> -
> - switch (resource_type(res)) {
> - case IORESOURCE_IO:
> - base = devm_ioport_map(dev, res->start, resource_size(res));
> - if (!base)
> - return -ENOMEM;
> - break;
> - case IORESOURCE_MEM:
> - base = devm_ioremap_resource(dev, res);
> - if (IS_ERR(base))
> - return PTR_ERR(base);
> - break;
> - default:
> - return -EINVAL;
> - }
> -
> - 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);
> -
> - return 0;
> -}
> -
> -static const struct of_device_id pvpanic_mmio_match[] = {
> - { .compatible = "qemu,pvpanic-mmio", },
> - {}
> -};
> -
> -static const struct acpi_device_id pvpanic_device_ids[] = {
> - { "QEMU0001", 0 },
> - { "", 0 }
> -};
> -MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
> -
> -static struct platform_driver pvpanic_mmio_driver = {
> - .driver = {
> - .name = "pvpanic-mmio",
> - .of_match_table = pvpanic_mmio_match,
> - .acpi_match_table = pvpanic_device_ids,
> - },
> - .probe = pvpanic_mmio_probe,
> - .remove = pvpanic_mmio_remove,
> -};
> -module_platform_driver(pvpanic_mmio_driver);
> diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
> new file mode 100644
> index 00000000..0dce6ef
> --- /dev/null
> +++ b/drivers/misc/pvpanic/Kconfig
> @@ -0,0 +1,12 @@
> +config PVPANIC
> + bool "pvpanic device support"
> + help
> + This option enables pvpanic device driver.
No it doesn't, it allows you to _select_ a specific pvpanic driver, on
its own, it is not a driver, right?
> +
> +config PVPANIC_MMIO
> + tristate "pvpanic MMIO device support"
> + depends on HAS_IOMEM && (ACPI || OF) && PVPANIC
> + 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 00000000..9ea3355
> --- /dev/null
> +++ b/drivers/misc/pvpanic/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_PVPANIC_MMIO) += pvpanic-mmio.o
> +pvpanic-mmio-objs := pvpanic-common.o mmio.o
You put the "common" logic in the mmio driver? How is that going to
work for the PCI driver?
Why is there not a pvpanic.ko that contains the "core" code only?
thanks,
greg k-h
Powered by blists - more mailing lists