[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6c05cc9e-815d-7a94-8b2d-f17fd5d47354@redhat.com>
Date: Tue, 25 Apr 2023 12:34:05 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Armin Wolf <W_Armin@....de>, Jorge Lopez <jorgealtxwork@...il.com>,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
thomas@...ch.de
Subject: Re: [PATCH v10 03/14] HP BIOSCFG driver - bioscfg
Hi All,
On 4/19/23 20:04, Armin Wolf wrote:
> Am 19.04.23 um 17:13 schrieb Jorge Lopez:
<snip>
>> +static int __init bioscfg_init(void)
>> +{
>> + int ret = 0;
>> + int bios_capable = wmi_has_guid(HP_WMI_BIOS_GUID);
>> +
>> + if (!bios_capable) {
>> + pr_err("Unable to run on non-HP system\n");
>> + return -ENODEV;
>> + }
>> +
>
> Currently, this driver will no get automatically loaded on supported hardware,
> something which would be quite beneficial for users to have.
> Since the HP_WMI_BIOS_GUID is already handled by the hp-wmi driver, maybe this
> driver (which also already implements a function similar to hp_wmi_perform_query())
> could register a platform device which is then used by this driver? This together
> with MODULE_DEVICE_TABLE() would allow for automatically loading the module on supported hardware.
Both drivers can already co-exist since the old hp-wmi driver uses the old
wmi kernel functions and is not a "wmi_driver" so there is no need for
a platform_device for this driver to bind to since the wmi_device is
still free for it to bind to.
This does indeed need a MODULE_DEVICE_TABLE() statement for
the bios_attr_pass_interface_id_table[] id-table. Note only for that
table, because the HP_WMI_BIOS_GUID is present on models which do
not support this and we don't want the module to auto-load there.
Regards,
Hans
>
> Armin Wolf
>
>> + ret = init_bios_attr_set_interface();
>> + if (ret)
>> + return ret;
>> +
>> + ret = init_bios_attr_pass_interface();
>> + if (ret)
>> + goto err_exit_bios_attr_set_interface;
>> +
>> + if (!bioscfg_drv.bios_attr_wdev || !bioscfg_drv.password_attr_wdev) {
>> + pr_debug("Failed to find set or pass interface\n");
>> + ret = -ENODEV;
>> + goto err_exit_bios_attr_pass_interface;
>> + }
>> +
>> + ret = fw_attributes_class_get(&fw_attr_class);
>> + if (ret)
>> + goto err_exit_bios_attr_pass_interface;
>> +
>> + bioscfg_drv.class_dev = device_create(fw_attr_class, NULL, MKDEV(0, 0),
>> + NULL, "%s", DRIVER_NAME);
>> + if (IS_ERR(bioscfg_drv.class_dev)) {
>> + ret = PTR_ERR(bioscfg_drv.class_dev);
>> + goto err_unregister_class;
>> + }
>> +
>> + bioscfg_drv.main_dir_kset = kset_create_and_add("attributes", NULL,
>> + &bioscfg_drv.class_dev->kobj);
>> + if (!bioscfg_drv.main_dir_kset) {
>> + ret = -ENOMEM;
>> + pr_debug("Failed to create and add attributes\n");
>> + goto err_destroy_classdev;
>> + }
>> +
>> + bioscfg_drv.authentication_dir_kset = kset_create_and_add("authentication", NULL,
>> + &bioscfg_drv.class_dev->kobj);
>> + if (!bioscfg_drv.authentication_dir_kset) {
>> + ret = -ENOMEM;
>> + pr_debug("Failed to create and add authentication\n");
>> + goto err_release_attributes_data;
>> + }
>> +
>> + /*
>> + * sysfs level attributes.
>> + * - pending_reboot
>> + */
>> + ret = create_attributes_level_sysfs_files();
>> + if (ret)
>> + pr_debug("Failed to create sysfs level attributes\n");
>> +
>> + ret = hp_init_bios_attributes(HPWMI_STRING_TYPE, HP_WMI_BIOS_STRING_GUID);
>> + if (ret)
>> + pr_debug("Failed to populate string type attributes\n");
>> +
>> + ret = hp_init_bios_attributes(HPWMI_INTEGER_TYPE, HP_WMI_BIOS_INTEGER_GUID);
>> + if (ret)
>> + pr_debug("Failed to populate integer type attributes\n");
>> +
>> + ret = hp_init_bios_attributes(HPWMI_ENUMERATION_TYPE, HP_WMI_BIOS_ENUMERATION_GUID);
>> + if (ret)
>> + pr_debug("Failed to populate enumeration type attributes\n");
>> +
>> + ret = hp_init_bios_attributes(HPWMI_ORDERED_LIST_TYPE, HP_WMI_BIOS_ORDERED_LIST_GUID);
>> + if (ret)
>> + pr_debug("Failed to populate ordered list object type attributes\n");
>> +
>> + ret = hp_init_bios_attributes(HPWMI_PASSWORD_TYPE, HP_WMI_BIOS_PASSWORD_GUID);
>> + if (ret)
>> + pr_debug("Failed to populate password object type attributes\n");
>> +
>> + bioscfg_drv.spm_data.attr_name_kobj = NULL;
>> + ret = hp_add_other_attributes(HPWMI_SECURE_PLATFORM_TYPE);
>> + if (ret)
>> + pr_debug("Failed to populate secure platform object type attribute\n");
>> +
>> + bioscfg_drv.sure_start_attr_kobj = NULL;
>> + ret = hp_add_other_attributes(HPWMI_SURE_START_TYPE);
>> + if (ret)
>> + pr_debug("Failed to populate sure start object type attribute\n");
>> +
>> + return 0;
>> +
>> +err_release_attributes_data:
>> + release_attributes_data();
>> +
>> +err_destroy_classdev:
>> + device_destroy(fw_attr_class, MKDEV(0, 0));
>> +
>> +err_unregister_class:
>> + fw_attributes_class_put();
>> +
>> +err_exit_bios_attr_pass_interface:
>> + exit_bios_attr_pass_interface();
>> +
>> +err_exit_bios_attr_set_interface:
>> + exit_bios_attr_set_interface();
>> +
>> + return ret;
>> +}
>> +
>> +static void __exit bioscfg_exit(void)
>> +{
>> + release_attributes_data();
>> + device_destroy(fw_attr_class, MKDEV(0, 0));
>> +
>> + fw_attributes_class_put();
>> + exit_bios_attr_set_interface();
>> + exit_bios_attr_pass_interface();
>> +}
>> +
>> +module_init(bioscfg_init);
>> +module_exit(bioscfg_exit);
>
Powered by blists - more mailing lists