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@xxxxxxxxxxxxxxx>
>> Cc: Jose Angel Pastrana <japp0005@xxxxxxxxxxxx>
>> Signed-off-by: Nikita Kravets <teackot@xxxxxxxxx>
>> ---
>> 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");
>>
>
