On Mon, May 8, 2023 at 4:35 PM Thomas Weißschuh <thomas@xxxxxxxx> wrote: > > On 2023-05-05 17:00:35-0500, Jorge Lopez wrote: > > <snip> > > > --- > > Based on the latest platform-drivers-x86.git/for-next > > --- > > .../x86/hp/hp-bioscfg/ordered-attributes.c | 443 ++++++++++++++++++ > > 1 file changed, 443 insertions(+) > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/ordered-attributes.c > > Should be named order-list-attributes. > > > > > diff --git a/drivers/platform/x86/hp/hp-bioscfg/ordered-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/ordered-attributes.c > > new file mode 100644 > > index 000000000000..1d06fbefceca > > --- /dev/null > > +++ b/drivers/platform/x86/hp/hp-bioscfg/ordered-attributes.c > > @@ -0,0 +1,443 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Functions corresponding to ordered list type attributes under > > + * BIOS ORDERED LIST GUID for use with hp-bioscfg driver. > > + * > > + * Copyright (c) 2022 HP Development Company, L.P. > > + */ > > + > > +#include "bioscfg.h" > > + > > +GET_INSTANCE_ID(ordered_list); > > + > > +static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) > > +{ > > + int instance_id = get_ordered_list_instance_id(kobj); > > + > > + if (instance_id < 0) > > + return -EIO; > > + > > + return sysfs_emit(buf, "%s\n", > > + bioscfg_drv.ordered_list_data[instance_id].current_value); > > +} > > + > > +/** > > + * validate_ordered_list_input() - > > + * Validate input of current_value against possible values > > + * > > + * @instance_id: The instance on which input is validated > > + * @buf: Input value > > + */ > > +static int validate_ordered_list_input(int instance_id, const char *buf) > > +{ > > + struct ordered_list_data *ordered_list_data = &bioscfg_drv.ordered_list_data[instance_id]; > > + > > + if (ordered_list_data->common.requires_physical_presence) > > + set_reboot_and_signal_event(); > > I think the block above can actually be pulled up into > ATTRIBUTE_PROPERTY_STORE() and removed from all the attributes. > Done! > > + > > + return 0; > > +} > > + <snip> > > +static struct attribute *ordered_list_attrs[] = { > > + &ordered_list_display_langcode.attr, > > + &ordered_list_display_name.attr, > > + &ordered_list_current_val.attr, > > + &ordered_list_elements_val.attr, > > + &ordered_list_type.attr, > > + NULL, > > No comma. Done! > > > +}; > > + > > +static const struct attribute_group ordered_list_attr_group = { > > + .attrs = ordered_list_attrs, > > +}; > > + > > +int alloc_ordered_list_data(void) > > +{ > > + bioscfg_drv.ordered_list_instances_count = > > + get_instance_count(HP_WMI_BIOS_ORDERED_LIST_GUID); > > + bioscfg_drv.ordered_list_data = kcalloc(bioscfg_drv.ordered_list_instances_count, > > + sizeof(struct ordered_list_data), GFP_KERNEL); > > + if (!bioscfg_drv.ordered_list_data) { > > + bioscfg_drv.ordered_list_instances_count = 0; > > + return -ENOMEM; > > + } > > + return 0; > > +} > > + > > +/** > > + * populate_ordered_list_package_data() - > > + * Populate all properties of an instance under ordered_list attribute > > + * > > + * @order_obj: ACPI object with ordered_list data > > + * @instance_id: The instance to enumerate > > + * @attr_name_kobj: The parent kernel object > > + */ > > +int populate_ordered_list_package_data(union acpi_object *order_obj, int instance_id, > > + struct kobject *attr_name_kobj) > > +{ > > + struct ordered_list_data *ordered_list_data = &bioscfg_drv.ordered_list_data[instance_id]; > > + > > + ordered_list_data->attr_name_kobj = attr_name_kobj; > > + > > + populate_ordered_list_elements_from_package(order_obj, > > + order_obj->package.count, > > + instance_id); > > + update_attribute_permissions(ordered_list_data->common.is_readonly, > > + &ordered_list_current_val); > > + friendly_user_name_update(ordered_list_data->common.path, > > + attr_name_kobj->name, > > + ordered_list_data->common.display_name, > > + sizeof(ordered_list_data->common.display_name)); > > + return sysfs_create_group(attr_name_kobj, &ordered_list_attr_group); > > +} > > + > > +/* Expected Values types associated with each element */ > > +static const acpi_object_type expected_order_types[] = { > > + [NAME] = ACPI_TYPE_STRING, > > + [VALUE] = ACPI_TYPE_STRING, > > + [PATH] = ACPI_TYPE_STRING, > > + [IS_READONLY] = ACPI_TYPE_INTEGER, > > + [DISPLAY_IN_UI] = ACPI_TYPE_INTEGER, > > + [REQUIRES_PHYSICAL_PRESENCE] = ACPI_TYPE_INTEGER, > > + [SEQUENCE] = ACPI_TYPE_INTEGER, > > + [PREREQUISITES_SIZE] = ACPI_TYPE_INTEGER, > > + [PREREQUISITES] = ACPI_TYPE_STRING, > > + [SECURITY_LEVEL] = ACPI_TYPE_INTEGER, > > + [ORD_LIST_SIZE] = ACPI_TYPE_INTEGER, > > + [ORD_LIST_ELEMENTS] = ACPI_TYPE_STRING, > > +}; > > + > > +int populate_ordered_list_elements_from_package(union acpi_object *order_obj, > > + int order_obj_count, > > + int instance_id) > > Can be static. Same for the other attributes. Done! > > > +{ <snip> > > + for (elem = 1; elem < MAX_ELEMENTS_SIZE && part; elem++) { > > + strscpy(ordered_list_data->elements[elem], > > + part, > > + sizeof(ordered_list_data->elements[elem])); > > + part = strsep(&part_tmp, ","); > > + } > > + > > + kfree(tmpstr); > > This will be freed below anyways. > > > + break; > > + default: > > + pr_warn("Invalid element: %d found in Ordered_List attribute or data may be malformed\n", elem); > > + break; > > + } > > + kfree(tmpstr); > > + kfree(str_value); > > + } > > + > > +exit_list_package: > > No need for function name in jump label. Done! > > > + kfree(tmpstr); > > + kfree(str_value); > > + return 0; > > +} > > + > > +/** > > + * populate_ordered_list_buffer_data() - Populate all properties of an > > + * instance under ordered list attribute > > + * > > + * @buffer_ptr: Buffer pointer > > + * @buffer_size: Buffer size > > + * @instance_id: The instance to enumerate > > + * @attr_name_kobj: The parent kernel object > > + */ > > +int populate_ordered_list_buffer_data(u8 *buffer_ptr, u32 *buffer_size, int instance_id, > > + struct kobject *attr_name_kobj) > > +{ > > + struct ordered_list_data *ordered_list_data = &bioscfg_drv.ordered_list_data[instance_id]; > > + > > + ordered_list_data->attr_name_kobj = attr_name_kobj; > > + > > + /* Populate ordered list elements */ > > + populate_ordered_list_elements_from_buffer(buffer_ptr, buffer_size, > > + instance_id); > > + update_attribute_permissions(ordered_list_data->common.is_readonly, > > + &ordered_list_current_val); > > + friendly_user_name_update(ordered_list_data->common.path, > > + attr_name_kobj->name, > > + ordered_list_data->common.display_name, > > + sizeof(ordered_list_data->common.display_name)); > > + > > + return sysfs_create_group(attr_name_kobj, &ordered_list_attr_group); > > +} > > + > > +int populate_ordered_list_elements_from_buffer(u8 *buffer_ptr, u32 *buffer_size, > > + int instance_id) > > Can be static. Same for the other attributes. Done! > > > +{ > > + int reqs; > > + int values; > > + struct ordered_list_data *ordered_list_data = &bioscfg_drv.ordered_list_data[instance_id]; > > + > > + strscpy(ordered_list_data->common.display_name_language_code, > > + LANG_CODE_STR, > > + sizeof(ordered_list_data->common.display_name_language_code)); > > + > > + // VALUE: > > + get_string_from_buffer(&buffer_ptr, buffer_size, ordered_list_data->current_value, > > + sizeof(ordered_list_data->current_value)); > > + > > + // PATH: > > + get_string_from_buffer(&buffer_ptr, buffer_size, ordered_list_data->common.path, > > + sizeof(ordered_list_data->common.path)); > > + > > + // IS_READONLY: > > + get_integer_from_buffer(&buffer_ptr, buffer_size, > > + &ordered_list_data->common.is_readonly); > > + > > + //DISPLAY_IN_UI: > > + get_integer_from_buffer(&buffer_ptr, buffer_size, > > + &ordered_list_data->common.display_in_ui); > > + > > + // REQUIRES_PHYSICAL_PRESENCE: > > + get_integer_from_buffer(&buffer_ptr, buffer_size, > > + &ordered_list_data->common.requires_physical_presence); > > + > > + // SEQUENCE: > > + get_integer_from_buffer(&buffer_ptr, buffer_size, > > + &ordered_list_data->common.sequence); > > + > > + // PREREQUISITES_SIZE: > > + get_integer_from_buffer(&buffer_ptr, buffer_size, > > + &ordered_list_data->common.prerequisites_size); > > + > > + if (ordered_list_data->common.prerequisites_size > MAX_PREREQUISITES_SIZE) { > > + /* Report a message and limit prerequisite size to maximum value */ > > + pr_warn("String Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n"); > > + ordered_list_data->common.prerequisites_size = MAX_PREREQUISITES_SIZE; > > + } > > + > > + // PREREQUISITES: > > + for (reqs = 0; > > + reqs < ordered_list_data->common.prerequisites_size && reqs < MAX_PREREQUISITES_SIZE; > > + reqs++) > > + get_string_from_buffer(&buffer_ptr, buffer_size, > > + ordered_list_data->common.prerequisites[reqs], > > + sizeof(ordered_list_data->common.prerequisites[reqs])); > > + > > + // SECURITY_LEVEL: > > + get_integer_from_buffer(&buffer_ptr, buffer_size, > > + &ordered_list_data->common.security_level); > > + > > + // ORD_LIST_SIZE: > > + get_integer_from_buffer(&buffer_ptr, buffer_size, > > + &ordered_list_data->elements_size); > > + > > + if (ordered_list_data->elements_size > MAX_ELEMENTS_SIZE) { > > + /* Report a message and limit elements size to maximum value */ > > + pr_warn("Ordered List size value exceeded the maximum number of elements supported or data may be malformed\n"); > > + ordered_list_data->elements_size = MAX_ELEMENTS_SIZE; > > + } > > + > > + // ORD_LIST_ELEMENTS: > > + for (values = 0; values < ordered_list_data->elements_size && values < MAX_ELEMENTS_SIZE; > > values < min(ordered_list_data->elements_size, MAX_ELEMENTS_SIZE) > > Also elements_size can actually never be > MAX_ELEMENTS_SIZE here. > Same for PREREQUISITES. > But as discussed before this limiting to a max amount of elements is > invalid, as following data will be misinterpreted. > Done! > > + values++) > > + get_string_from_buffer(&buffer_ptr, buffer_size, > > + ordered_list_data->elements[values], > > + sizeof(ordered_list_data->elements[values])); > > + > > + return 0; > > +} > > + > > +/** > > + * exit_ordered_list_attributes() - Clear all attribute data > > + * > > + * Clears all data allocated for this group of attributes > > + */ > > +void exit_ordered_list_attributes(void) > > +{ > > + int instance_id; > > + > > + for (instance_id = 0; instance_id < bioscfg_drv.ordered_list_instances_count; > > + instance_id++) { > > + struct kobject *attr_name_kobj = > > + bioscfg_drv.ordered_list_data[instance_id].attr_name_kobj; > > + > > + if (attr_name_kobj) > > + sysfs_remove_group(attr_name_kobj, > > + &ordered_list_attr_group); > > + } > > + bioscfg_drv.ordered_list_instances_count = 0; > > + > > + kfree(bioscfg_drv.ordered_list_data); > > + bioscfg_drv.ordered_list_data = NULL; > > +} > > -- > > 2.34.1 > >