| Date | Tue, 16 Feb 2016 16:17:39 +0100 | From | Pali Rohár <> | Subject | Re: [PATCH v3 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 |
| |
On Tuesday 16 February 2016 15:50:28 Michał Kępień wrote: > + if (wmi_requires_smbios_request) { > + buffer = dell_smbios_get_buffer(); > + buffer->input[0] = 0x10000; > + buffer->input[1] = 0x51534554; > + buffer->input[3] = 0x1; > + dell_smbios_send_request(17, 3); > + err = buffer->output[0]; > + dell_smbios_release_buffer(); > + if (err) { > + pr_err("Failed to enable WMI (error %d)\n", err); > + wmi_remove_notify_handler(DELL_EVENT_GUID); > + dell_wmi_input_destroy(); > + return dell_smbios_error(err); > + } > + } > + > return 0; > } > module_init(dell_wmi_init); > > static void __exit dell_wmi_exit(void) > { > + struct calling_interface_buffer *buffer; > + > + if (wmi_requires_smbios_request) { > + buffer = dell_smbios_get_buffer(); > + buffer->input[0] = 0x10000; > + buffer->input[1] = 0x51534554; > + dell_smbios_send_request(17, 3); > + dell_smbios_release_buffer(); > + } > + > wmi_remove_notify_handler(DELL_EVENT_GUID); > dell_wmi_input_destroy(); > }
Hi! I would propose moving this get_buffer, send, release code into own function with boolean argument on/off. This de-duplicate same code plus decrease level of indentation in _init function. And maybe adding ascii string comment representation for that 0x5153... number can be useful.
-- Pali Rohár pali.rohar@gmail.com
|