linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH V4 1/6] misc/pvpanic: preparing for pvpanic driver framework
  2019-01-24 16:40 ` [PATCH V4 1/6] misc/pvpanic: preparing for " Peng Hao
@ 2019-01-24 13:46   ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2019-01-24 13:46 UTC (permalink / raw)
  To: Peng Hao; +Cc: Greg Kroah-Hartman, Arnd Bergmann, Linux Kernel Mailing List

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <peng.hao2@zte.com.cn> 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 <peng.hao2@zte.com.cn>
> ---
>  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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V4 2/6] misc/pvpanic: Add pvpanic driver framework
  2019-01-24 16:40 ` [PATCH V4 2/6] misc/pvpanic: Add " Peng Hao
@ 2019-01-24 13:47   ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2019-01-24 13:47 UTC (permalink / raw)
  To: Peng Hao; +Cc: Greg Kroah-Hartman, Arnd Bergmann, Linux Kernel Mailing List

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <peng.hao2@zte.com.cn> 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 <peng.hao2@zte.com.cn>
> ---
>  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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V4 3/6] misc/pvpanic: add API for pvpanic driver framework
  2019-01-24 16:40 ` [PATCH V4 3/6] misc/pvpanic: add API for " Peng Hao
@ 2019-01-24 13:54   ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2019-01-24 13:54 UTC (permalink / raw)
  To: Peng Hao; +Cc: Greg Kroah-Hartman, Arnd Bergmann, Linux Kernel Mailing List

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <peng.hao2@zte.com.cn> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V4 4/6] misc/pvpanic: add pvpanic acpi driver
  2019-01-24 16:40 ` [PATCH V4 4/6] misc/pvpanic: add pvpanic acpi driver Peng Hao
@ 2019-01-24 13:54   ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2019-01-24 13:54 UTC (permalink / raw)
  To: Peng Hao; +Cc: Greg Kroah-Hartman, Arnd Bergmann, Linux Kernel Mailing List

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <peng.hao2@zte.com.cn> 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 <peng.hao2@zte.com.cn>
> ---
>  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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V4 5/6] misc/pvpanic: add pvpanic mmio driver
  2019-01-24 16:40 ` [PATCH V4 5/6] misc/pvpanic: add pvpanic mmio driver Peng Hao
@ 2019-01-24 13:55   ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2019-01-24 13:55 UTC (permalink / raw)
  To: Peng Hao; +Cc: Greg Kroah-Hartman, Arnd Bergmann, Linux Kernel Mailing List

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <peng.hao2@zte.com.cn> 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 <peng.hao2@zte.com.cn>
> ---
>  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 <peng.hao2@zte.com.cn>
> + */
> +
> +#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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V4 0/6] add pvpanic driver framework
  2019-01-24 16:40 [PATCH V4 0/6] add pvpanic driver framework Peng Hao
@ 2019-01-24 14:00 ` Andy Shevchenko
       [not found]   ` <201902031723170231588@zte.com.cn>
  2019-01-24 16:40 ` [PATCH V4 1/6] misc/pvpanic: preparing for " Peng Hao
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2019-01-24 14:00 UTC (permalink / raw)
  To: Peng Hao; +Cc: Greg Kroah-Hartman, Arnd Bergmann, Linux Kernel Mailing List

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <peng.hao2@zte.com.cn> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V4 0/6] add pvpanic driver framework
@ 2019-01-24 16:40 Peng Hao
  2019-01-24 14:00 ` Andy Shevchenko
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Peng Hao @ 2019-01-24 16:40 UTC (permalink / raw)
  To: gregkh, arnd, andy.shevchenko; +Cc: linux-kernel, Peng Hao

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V4 1/6]  misc/pvpanic: preparing for pvpanic driver framework
  2019-01-24 16:40 [PATCH V4 0/6] add pvpanic driver framework Peng Hao
  2019-01-24 14:00 ` Andy Shevchenko
@ 2019-01-24 16:40 ` Peng Hao
  2019-01-24 13:46   ` Andy Shevchenko
  2019-01-24 16:40 ` [PATCH V4 2/6] misc/pvpanic: Add " Peng Hao
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Peng Hao @ 2019-01-24 16:40 UTC (permalink / raw)
  To: gregkh, arnd, andy.shevchenko; +Cc: linux-kernel, Peng Hao

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

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V4 2/6]  misc/pvpanic: Add pvpanic driver framework
  2019-01-24 16:40 [PATCH V4 0/6] add pvpanic driver framework Peng Hao
  2019-01-24 14:00 ` Andy Shevchenko
  2019-01-24 16:40 ` [PATCH V4 1/6] misc/pvpanic: preparing for " Peng Hao
@ 2019-01-24 16:40 ` Peng Hao
  2019-01-24 13:47   ` Andy Shevchenko
  2019-01-24 16:40 ` [PATCH V4 3/6] misc/pvpanic: add API for " Peng Hao
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Peng Hao @ 2019-01-24 16:40 UTC (permalink / raw)
  To: gregkh, arnd, andy.shevchenko; +Cc: linux-kernel, Peng Hao

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 <peng.hao2@zte.com.cn>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V4 3/6]  misc/pvpanic: add API for pvpanic driver framework
  2019-01-24 16:40 [PATCH V4 0/6] add pvpanic driver framework Peng Hao
                   ` (2 preceding siblings ...)
  2019-01-24 16:40 ` [PATCH V4 2/6] misc/pvpanic: Add " Peng Hao
@ 2019-01-24 16:40 ` Peng Hao
  2019-01-24 13:54   ` Andy Shevchenko
  2019-01-24 16:40 ` [PATCH V4 4/6] misc/pvpanic: add pvpanic acpi driver Peng Hao
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Peng Hao @ 2019-01-24 16:40 UTC (permalink / raw)
  To: gregkh, arnd, andy.shevchenko; +Cc: linux-kernel, Peng Hao

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

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 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 <peng.hao2@zte.com.cn>
+ */
+
+#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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V4 4/6]  misc/pvpanic: add pvpanic acpi driver
  2019-01-24 16:40 [PATCH V4 0/6] add pvpanic driver framework Peng Hao
                   ` (3 preceding siblings ...)
  2019-01-24 16:40 ` [PATCH V4 3/6] misc/pvpanic: add API for " Peng Hao
@ 2019-01-24 16:40 ` Peng Hao
  2019-01-24 13:54   ` Andy Shevchenko
  2019-01-24 16:40 ` [PATCH V4 5/6] misc/pvpanic: add pvpanic mmio driver Peng Hao
  2019-01-24 16:40 ` [PATCH V4 6/6] misc/pvpanic: add new pvpanic pci driver Peng Hao
  6 siblings, 1 reply; 14+ messages in thread
From: Peng Hao @ 2019-01-24 16:40 UTC (permalink / raw)
  To: gregkh, arnd, andy.shevchenko; +Cc: linux-kernel, Peng Hao

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

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V4 5/6]  misc/pvpanic: add pvpanic mmio driver
  2019-01-24 16:40 [PATCH V4 0/6] add pvpanic driver framework Peng Hao
                   ` (4 preceding siblings ...)
  2019-01-24 16:40 ` [PATCH V4 4/6] misc/pvpanic: add pvpanic acpi driver Peng Hao
@ 2019-01-24 16:40 ` Peng Hao
  2019-01-24 13:55   ` Andy Shevchenko
  2019-01-24 16:40 ` [PATCH V4 6/6] misc/pvpanic: add new pvpanic pci driver Peng Hao
  6 siblings, 1 reply; 14+ messages in thread
From: Peng Hao @ 2019-01-24 16:40 UTC (permalink / raw)
  To: gregkh, arnd, andy.shevchenko; +Cc: linux-kernel, Peng Hao

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

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 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 <peng.hao2@zte.com.cn>
+ */
+
+#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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V4 6/6]  misc/pvpanic: add new pvpanic pci driver
  2019-01-24 16:40 [PATCH V4 0/6] add pvpanic driver framework Peng Hao
                   ` (5 preceding siblings ...)
  2019-01-24 16:40 ` [PATCH V4 5/6] misc/pvpanic: add pvpanic mmio driver Peng Hao
@ 2019-01-24 16:40 ` Peng Hao
  6 siblings, 0 replies; 14+ messages in thread
From: Peng Hao @ 2019-01-24 16:40 UTC (permalink / raw)
  To: gregkh, arnd, andy.shevchenko; +Cc: linux-kernel, Peng Hao

Add new pvpanic pci driver to pvpanic driver framework.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 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 <peng.hao2@zte.com.cn>
+ */
+
+#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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH V4 0/6] add pvpanic driver framework
       [not found]   ` <201902031723170231588@zte.com.cn>
@ 2019-02-05 15:31     ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2019-02-05 15:31 UTC (permalink / raw)
  To: Peng Hao; +Cc: Greg Kroah-Hartman, Arnd Bergmann, Linux Kernel Mailing List

On Sun, Feb 3, 2019 at 11:23 AM <peng.hao2@zte.com.cn> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-02-05 15:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-24 16:40 [PATCH V4 0/6] add pvpanic driver framework Peng Hao
2019-01-24 14:00 ` Andy Shevchenko
     [not found]   ` <201902031723170231588@zte.com.cn>
2019-02-05 15:31     ` Andy Shevchenko
2019-01-24 16:40 ` [PATCH V4 1/6] misc/pvpanic: preparing for " Peng Hao
2019-01-24 13:46   ` Andy Shevchenko
2019-01-24 16:40 ` [PATCH V4 2/6] misc/pvpanic: Add " Peng Hao
2019-01-24 13:47   ` Andy Shevchenko
2019-01-24 16:40 ` [PATCH V4 3/6] misc/pvpanic: add API for " Peng Hao
2019-01-24 13:54   ` Andy Shevchenko
2019-01-24 16:40 ` [PATCH V4 4/6] misc/pvpanic: add pvpanic acpi driver Peng Hao
2019-01-24 13:54   ` Andy Shevchenko
2019-01-24 16:40 ` [PATCH V4 5/6] misc/pvpanic: add pvpanic mmio driver Peng Hao
2019-01-24 13:55   ` Andy Shevchenko
2019-01-24 16:40 ` [PATCH V4 6/6] misc/pvpanic: add new pvpanic pci driver Peng Hao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).