Messages in this thread | | | Subject | Re: [PATCH] platform/x86: intel_pmc_core: do not create a static struct device | From | Hans de Goede <> | Date | Thu, 24 Sep 2020 10:39:07 +0200 |
| |
Hi,
On 9/23/20 8:48 PM, Greg Kroah-Hartman wrote: > A struct device is a dynamic structure, with reference counting. > "Tricking" the kernel to make a dynamic structure static, by working > around the driver core release detection logic, is not nice. > > Because of this, this code has been used as an example for others on > "how to do things", which is just about the worst thing possible to have > happen. > > Fix this all up by making the platform device dynamic and providing a > real release function. > > Cc: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com> > Cc: Vishwanath Somayaji <vishwanath.somayaji@intel.com> > Cc: Darren Hart <dvhart@infradead.org> > Cc: Andy Shevchenko <andy@infradead.org> > Cc: Rajat Jain <rajatja@google.com> > Cc: platform-driver-x86@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Reported-by: Maximilian Luz <luzmaximilian@gmail.com> > Fixes: b02f6a2ef0a1 ("platform/x86: intel_pmc_core: Attach using APCI HID "INT33A1"") > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> --- > drivers/platform/x86/intel_pmc_core_pltdrv.c | 26 +++++++++++++------- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/drivers/platform/x86/intel_pmc_core_pltdrv.c b/drivers/platform/x86/intel_pmc_core_pltdrv.c > index 731281855cc8..73797680b895 100644 > --- a/drivers/platform/x86/intel_pmc_core_pltdrv.c > +++ b/drivers/platform/x86/intel_pmc_core_pltdrv.c > @@ -20,15 +20,10 @@ > > static void intel_pmc_core_release(struct device *dev) > { > - /* Nothing to do. */ > + kfree(dev); > } > > -static struct platform_device pmc_core_device = { > - .name = "intel_pmc_core", > - .dev = { > - .release = intel_pmc_core_release, > - }, > -}; > +static struct platform_device *pmc_core_device; > > /* > * intel_pmc_core_platform_ids is the list of platforms where we want to > @@ -52,6 +47,8 @@ MODULE_DEVICE_TABLE(x86cpu, intel_pmc_core_platform_ids); > > static int __init pmc_core_platform_init(void) > { > + int retval; > + > /* Skip creating the platform device if ACPI already has a device */ > if (acpi_dev_present("INT33A1", NULL, -1)) > return -ENODEV; > @@ -59,12 +56,23 @@ static int __init pmc_core_platform_init(void) > if (!x86_match_cpu(intel_pmc_core_platform_ids)) > return -ENODEV; > > - return platform_device_register(&pmc_core_device); > + pmc_core_device = kzalloc(sizeof(*pmc_core_device), GFP_KERNEL); > + if (!pmc_core_device) > + return -ENOMEM; > + > + pmc_core_device->name = "intel_pmc_core"; > + pmc_core_device->dev.release = intel_pmc_core_release; > + > + retval = platform_device_register(pmc_core_device); > + if (retval) > + kfree(pmc_core_device); > + > + return retval; > } > > static void __exit pmc_core_platform_exit(void) > { > - platform_device_unregister(&pmc_core_device); > + platform_device_unregister(pmc_core_device); > } > > module_init(pmc_core_platform_init); >
| |