Re: [PATCH v2 1/3] misc/pvpanic: split-up generic and platform dependent code

[Greg KH <gregkh@linuxfoundation.org>]

On Fri, Feb 12, 2021 at 12:29:49PM +0200, Mihai Carabas wrote:
> ..snip
> > > -};
> > > -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?
> 
> Yes. I will update the comment.
> 
> 
> > > +
> > > +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?
> 
> My intention was to put the code in both drivers. There is no generic module
> as it will complicate things: the generic module would have to have its own
> state (e.g. pvpanic_probe would have to create a queue of base addresses).

And when you link both into the kernel image directly, you end up with
duplicate symbols that break the build :(

> Do you want to see a generic module pvpanic.ko with its own state?

As it is, it will just not work so you have to do something...

thanks,

greg k-h