Messages in this thread | | | Date | Tue, 22 Jan 2019 10:37:32 +0100 | From | Greg KH <> | Subject | Re: [PATCH 2/6] misc/pvpanic: Add pvpanic driver framework |
| |
On Tue, Jan 22, 2019 at 03:25:07AM +0800, Peng Hao wrote: > Add pvpanic driver framework. >
You need a lot more description of what you did here than this, as I can not understand from this text, what the patch does, or more importantly, why it is doing this, at all.
> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn> > --- > drivers/misc/pvpanic/pvpanic.c | 171 ++++++++++------------------------------- > 1 file changed, 39 insertions(+), 132 deletions(-) > > diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c > index 595ac06..6380540 100644 > --- a/drivers/misc/pvpanic/pvpanic.c > +++ b/drivers/misc/pvpanic/pvpanic.c > @@ -8,15 +8,20 @@ > > #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> > > -static void __iomem *base; > +static struct { > + struct platform_device *pdev; > + void __iomem *base; > + bool is_ioport; > +} pvpanic_data = { > + .pdev = NULL, > + .is_ioport = false,
You do not need to initialize variables to 0 specifically like this.
> +}; > > #define PVPANIC_PANICKED (1 << 0) > > @@ -27,7 +32,7 @@ > static void > pvpanic_send_event(unsigned int event) > { > - iowrite8(event, base); > + iowrite8(event, pvpanic_data.base);
Why did you convert a single global variable into a single global structure? Why not, if you really need to pass this value around, do that at the same time as you will end up touching these same functions again, right?
thanks,
greg k-h
| |