| Message ID | 20231010172037.611063-5-teackot@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | platform/x86: msi-ec: Add the first platform device attributes | expand |
On Tue, 10 Oct 2023, Nikita Kravets wrote: > Register a platform driver for the future features. > > Cc: Aakash Singh <mail@singhaakash.dev> > Cc: Jose Angel Pastrana <japp0005@red.ujaen.es> > Signed-off-by: Nikita Kravets <teackot@gmail.com> > --- > drivers/platform/x86/msi-ec.c | 44 +++++++++++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c > index f26a3121092f..12c559c9eac4 100644 > --- a/drivers/platform/x86/msi-ec.c > +++ b/drivers/platform/x86/msi-ec.c > @@ -818,6 +818,30 @@ static struct acpi_battery_hook battery_hook = { > .name = MSI_EC_DRIVER_NAME, > }; > > +/* > + * Sysfs platform driver > + */ > + > +static int msi_platform_probe(struct platform_device *pdev) > +{ > + return 0; > +} > + > +static int msi_platform_remove(struct platform_device *pdev) > +{ > + return 0; > +} No need to provide empty .probe() or .remove(). > +static struct platform_device *msi_platform_device; > + > +static struct platform_driver msi_platform_driver = { > + .driver = { > + .name = MSI_EC_DRIVER_NAME, > + }, > + .probe = msi_platform_probe, > + .remove = msi_platform_remove, > +}; > + > /* > * Module load/unload > */ > @@ -878,6 +902,23 @@ static int __init msi_ec_init(void) > if (result < 0) > return result; > > + result = platform_driver_register(&msi_platform_driver); > + if (result < 0) > + return result; > + > + msi_platform_device = platform_device_alloc(MSI_EC_DRIVER_NAME, -1); > + if (msi_platform_device == NULL) { > + platform_driver_unregister(&msi_platform_driver); > + return -ENOMEM; > + } > + > + result = platform_device_add(msi_platform_device); > + if (result < 0) { > + platform_device_del(msi_platform_device); > + platform_driver_unregister(&msi_platform_driver); > + return result; Instead of duplicating error handling, make a proper rollback with goto and labels, or better yet, use the cleanup.h if you know how it works. > + } > + > battery_hook_register(&battery_hook); > return 0; > } > @@ -885,6 +926,9 @@ static int __init msi_ec_init(void) > static void __exit msi_ec_exit(void) > { > battery_hook_unregister(&battery_hook); > + > + platform_driver_unregister(&msi_platform_driver); > + platform_device_del(msi_platform_device); > } > > MODULE_LICENSE("GPL"); >
Hi, On 10/11/23 15:00, Ilpo Järvinen wrote: > On Tue, 10 Oct 2023, Nikita Kravets wrote: > >> Register a platform driver for the future features. >> >> Cc: Aakash Singh <mail@singhaakash.dev> >> Cc: Jose Angel Pastrana <japp0005@red.ujaen.es> >> Signed-off-by: Nikita Kravets <teackot@gmail.com> >> --- >> drivers/platform/x86/msi-ec.c | 44 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 44 insertions(+) >> >> diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c >> index f26a3121092f..12c559c9eac4 100644 >> --- a/drivers/platform/x86/msi-ec.c >> +++ b/drivers/platform/x86/msi-ec.c >> @@ -818,6 +818,30 @@ static struct acpi_battery_hook battery_hook = { >> .name = MSI_EC_DRIVER_NAME, >> }; >> >> +/* >> + * Sysfs platform driver >> + */ >> + >> +static int msi_platform_probe(struct platform_device *pdev) >> +{ >> + return 0; >> +} >> + >> +static int msi_platform_remove(struct platform_device *pdev) >> +{ >> + return 0; >> +} > > No need to provide empty .probe() or .remove(). > >> +static struct platform_device *msi_platform_device; >> + >> +static struct platform_driver msi_platform_driver = { >> + .driver = { >> + .name = MSI_EC_DRIVER_NAME, >> + }, >> + .probe = msi_platform_probe, >> + .remove = msi_platform_remove, >> +}; >> + >> /* >> * Module load/unload >> */ >> @@ -878,6 +902,23 @@ static int __init msi_ec_init(void) >> if (result < 0) >> return result; >> >> + result = platform_driver_register(&msi_platform_driver); >> + if (result < 0) >> + return result; >> + >> + msi_platform_device = platform_device_alloc(MSI_EC_DRIVER_NAME, -1); >> + if (msi_platform_device == NULL) { >> + platform_driver_unregister(&msi_platform_driver); >> + return -ENOMEM; >> + } >> + >> + result = platform_device_add(msi_platform_device); >> + if (result < 0) { >> + platform_device_del(msi_platform_device); >> + platform_driver_unregister(&msi_platform_driver); >> + return result; > > Instead of duplicating error handling, make a proper rollback with goto > and labels, or better yet, use the cleanup.h if you know how it works. Actually it would be better for a driver like this to use platform_create_bundle(), see e.g. the last couple of lines from: drivers/platform/x86/x86-android-tablets/core.c This will do both the platform_device registration as well as the driver registration in one go avoiding the need for rollback on error and it will also allow probe() and all functions only used by probe() to be marked as __init so that they can be free-ed from memory once msi_ec_init() has completed running. Regards, Hans > >> + } >> + >> battery_hook_register(&battery_hook); >> return 0; >> } >> @@ -885,6 +926,9 @@ static int __init msi_ec_init(void) >> static void __exit msi_ec_exit(void) >> { >> battery_hook_unregister(&battery_hook); >> + >> + platform_driver_unregister(&msi_platform_driver); >> + platform_device_del(msi_platform_device); >> } >> >> MODULE_LICENSE("GPL"); >> >
diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c index f26a3121092f..12c559c9eac4 100644 --- a/drivers/platform/x86/msi-ec.c +++ b/drivers/platform/x86/msi-ec.c @@ -818,6 +818,30 @@ static struct acpi_battery_hook battery_hook = { .name = MSI_EC_DRIVER_NAME, }; +/* + * Sysfs platform driver + */ + +static int msi_platform_probe(struct platform_device *pdev) +{ + return 0; +} + +static int msi_platform_remove(struct platform_device *pdev) +{ + return 0; +} + +static struct platform_device *msi_platform_device; + +static struct platform_driver msi_platform_driver = { + .driver = { + .name = MSI_EC_DRIVER_NAME, + }, + .probe = msi_platform_probe, + .remove = msi_platform_remove, +}; + /* * Module load/unload */ @@ -878,6 +902,23 @@ static int __init msi_ec_init(void) if (result < 0) return result; + result = platform_driver_register(&msi_platform_driver); + if (result < 0) + return result; + + msi_platform_device = platform_device_alloc(MSI_EC_DRIVER_NAME, -1); + if (msi_platform_device == NULL) { + platform_driver_unregister(&msi_platform_driver); + return -ENOMEM; + } + + result = platform_device_add(msi_platform_device); + if (result < 0) { + platform_device_del(msi_platform_device); + platform_driver_unregister(&msi_platform_driver); + return result; + } + battery_hook_register(&battery_hook); return 0; } @@ -885,6 +926,9 @@ static int __init msi_ec_init(void) static void __exit msi_ec_exit(void) { battery_hook_unregister(&battery_hook); + + platform_driver_unregister(&msi_platform_driver); + platform_device_del(msi_platform_device); } MODULE_LICENSE("GPL");
Register a platform driver for the future features. Cc: Aakash Singh <mail@singhaakash.dev> Cc: Jose Angel Pastrana <japp0005@red.ujaen.es> Signed-off-by: Nikita Kravets <teackot@gmail.com> --- drivers/platform/x86/msi-ec.c | 44 +++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)