Instead of mapping to built-in device properties, implement a full property provider. This is needed due to an architectural differences between built-in device property data structures and ones that are used on Apple machines. Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> --- Hi, Lukas. This is a skeleton of proof-of-concept conversion of apple-properties to be a full featured property provider. While it compiles, I only drafted it, so, main work should be still done. Consider to take this and finish the conversion. This is pretty much needed to avoid union aliasing I mistakenly introduced in built-in device property data structures. The patch is based on top of recently sent patches against this file and the one, which is kept in https://bitbucket.org/andy-shev/linux/branch/topic/eds-acpi (last 5 commits related to the subject). To the rest of Cc'ed people. I would like your opinion on the idea so far and perhaps valuable input. drivers/firmware/efi/apple-properties.c | 149 ++++++++++++++++++++++---------- 1 file changed, 105 insertions(+), 44 deletions(-) diff --git a/drivers/firmware/efi/apple-properties.c b/drivers/firmware/efi/apple-properties.c index beb4255e12bf..2902fae1a1ac 100644 --- a/drivers/firmware/efi/apple-properties.c +++ b/drivers/firmware/efi/apple-properties.c @@ -36,6 +36,38 @@ static int __init dump_properties_enable(char *arg) __setup("dump_apple_properties", dump_properties_enable); +struct apple_property { + const char *name; + size_t length; + const void *data; +}; + +struct apple_properties { + struct device *dev; + struct fwnode_handle fwnode; + const struct apple_property *properties; +}; + +// TODO: Keep them in linked list +// TODO: Add / Remove device notify handler? + +static const struct fwnode_operations apple_fwnode_ops; + +static inline bool is_apple_node(const struct fwnode_handle *fwnode) +{ + return !IS_ERR_OR_NULL(fwnode) && fwnode->ops == &apple_fwnode_ops; +} + +#define to_apple_node(__fwnode) \ +({ \ + typeof(__fwnode) __to_apple_node_fwnode = __fwnode; \ + \ + is_apple_node(__to_apple_node_fwnode) ? \ + container_of(__to_apple_node_fwnode, \ + struct apple_properties, fwnode) : \ + NULL; \ +}) + struct dev_header { u32 len; u32 prop_count; @@ -53,11 +85,16 @@ struct properties_header { struct dev_header dev_header[0]; }; -static void __init unmarshal_key_value_pairs(struct dev_header *dev_header, - struct device *dev, void *ptr, - struct property_entry entry[]) +static int __init unmarshal_key_value_pairs(struct dev_header *dev_header, + struct device *dev, void *ptr, + struct apple_properties *props) { - int i; + struct apple_property *property; + unsigned int i; + + property = kcalloc(dev_header->prop_count + 1, sizeof(*property), GFP_KERNEL); + if (!property) + return -ENOMEM; for (i = 0; i < dev_header->prop_count; i++) { int remaining = dev_header->len - (ptr - (void *)dev_header); @@ -77,8 +114,7 @@ static void __init unmarshal_key_value_pairs(struct dev_header *dev_header, } val_len = *(typeof(val_len) *)(ptr + key_len); - if (key_len + val_len > remaining || - val_len < sizeof(val_len)) { + if (key_len + val_len > remaining || val_len < sizeof(val_len)) { dev_err(dev, "invalid property val len at %#zx\n", ptr - (void *)dev_header + key_len); break; @@ -90,49 +126,48 @@ static void __init unmarshal_key_value_pairs(struct dev_header *dev_header, dev_err(dev, "cannot allocate property name\n"); break; } - ucs2_as_utf8(key, ptr + sizeof(key_len), - key_len - sizeof(key_len)); + ucs2_as_utf8(key, ptr + sizeof(key_len), key_len - sizeof(key_len)); - entry[i].name = key; - entry[i].length = val_len - sizeof(val_len); - entry[i].is_array = !!entry[i].length; - entry[i].type = DEV_PROP_U32; - entry[i].pointer.u32_data = ptr + key_len + sizeof(val_len); + property[i].name = key; + property[i].length = val_len - sizeof(val_len); + // TODO: Check! + property[i].data = kmemdup(ptr + key_len + sizeof(val_len), val_len - sizeof(val_len), GFP_KERNEL); if (dump_properties) { - dev_info(dev, "property: %s\n", entry[i].name); + dev_info(dev, "property: %s\n", property[i].name); print_hex_dump(KERN_INFO, pr_fmt(), DUMP_PREFIX_OFFSET, - 16, 1, entry[i].pointer.u32_data, - entry[i].length, true); + 16, 1, property[i].data, property[i].length, true); } ptr += key_len + val_len; } - if (i != dev_header->prop_count) { + if (i < dev_header->prop_count) { dev_err(dev, "got %d device properties, expected %u\n", i, dev_header->prop_count); print_hex_dump(KERN_ERR, pr_fmt(), DUMP_PREFIX_OFFSET, 16, 1, dev_header, dev_header->len, true); - return; + // TODO: Free allocated resources? + return -EINVAL; } dev_info(dev, "assigning %d device properties\n", i); + return 0; } -static int __init unmarshal_devices(struct properties_header *properties) +static int __init unmarshal_devices(struct properties_header *properties_header) { size_t offset = offsetof(struct properties_header, dev_header[0]); - while (offset + sizeof(struct dev_header) < properties->len) { - struct dev_header *dev_header = (void *)properties + offset; - struct property_entry *entry = NULL; + while (offset + sizeof(struct dev_header) < properties_header->len) { + struct dev_header *dev_header = (void *)properties_header + offset; + struct apple_properties *properties = NULL; struct device *dev; size_t len; - int ret, i; void *ptr; + int ret; - if (offset + dev_header->len > properties->len || + if (offset + dev_header->len > properties_header->len || dev_header->len <= sizeof(*dev_header)) { pr_err("invalid len in dev_header at %#zx\n", offset); return -EINVAL; @@ -146,31 +181,27 @@ static int __init unmarshal_devices(struct properties_header *properties) pr_err("device path parse error %ld at %#zx:\n", PTR_ERR(dev), ptr - (void *)dev_header); print_hex_dump(KERN_ERR, pr_fmt(), DUMP_PREFIX_OFFSET, - 16, 1, dev_header, dev_header->len, true); + 16, 1, dev_header, dev_header->len, true); dev = NULL; goto skip_device; } - entry = kcalloc(dev_header->prop_count + 1, sizeof(*entry), - GFP_KERNEL); - if (!entry) { + properties = kzalloc(sizeof(*properties), GFP_KERNEL); + if (!properties) { dev_err(dev, "cannot allocate properties\n"); goto skip_device; } - unmarshal_key_value_pairs(dev_header, dev, ptr, entry); - if (!entry[0].name) - goto skip_device; - - ret = device_add_properties(dev, entry); /* makes deep copy */ + ret = unmarshal_key_value_pairs(dev_header, dev, ptr, properties); if (ret) - dev_err(dev, "error %d assigning properties\n", ret); + goto skip_device; - for (i = 0; entry[i].name; i++) - kfree(entry[i].name); + properties->dev = dev; + properties->fwnode.ops = &apple_fwnode_ops; + set_secondary_fwnode(dev, &properties->fwnode); skip_device: - kfree(entry); + kfree(properties); put_device(dev); offset += dev_header->len; } @@ -180,7 +211,7 @@ static int __init unmarshal_devices(struct properties_header *properties) static int __init map_properties(void) { - struct properties_header *properties; + struct properties_header *properties_header; struct setup_data *data; u32 data_len; u64 pa_data; @@ -212,19 +243,19 @@ static int __init map_properties(void) return -ENOMEM; } - properties = (struct properties_header *)data->data; - if (properties->version != 1) { + properties_header = (struct properties_header *)data->data; + if (properties_header->version != 1) { pr_err("unsupported version:\n"); print_hex_dump(KERN_ERR, pr_fmt(), DUMP_PREFIX_OFFSET, - 16, 1, properties, data_len, true); + 16, 1, properties_header, data_len, true); ret = -ENOTSUPP; - } else if (properties->len != data_len) { + } else if (properties_header->len != data_len) { pr_err("length mismatch, expected %u\n", data_len); print_hex_dump(KERN_ERR, pr_fmt(), DUMP_PREFIX_OFFSET, - 16, 1, properties, data_len, true); + 16, 1, properties_header, data_len, true); ret = -EINVAL; } else - ret = unmarshal_devices(properties); + ret = unmarshal_devices(properties_header); /* * Can only free the setup_data payload but not its header @@ -240,3 +271,33 @@ static int __init map_properties(void) } fs_initcall(map_properties); + +static bool +apple_fwnode_property_present(const struct fwnode_handle *fwnode, + const char *propname) +{ + return -ENOTSUPP; +} + +static int +apple_fwnode_property_read_int_array(const struct fwnode_handle *fwnode, + const char *propname, + unsigned int elem_size, + void *val, size_t nval) +{ + return -ENOTSUPP; +} + +static int +apple_fwnode_property_read_string_array(const struct fwnode_handle *fwnode, + const char *propname, + const char **val, size_t nval) +{ + return -ENOTSUPP; +} + +static const struct fwnode_operations apple_fwnode_ops = { + .property_present = apple_fwnode_property_present, + .property_read_int_array = apple_fwnode_property_read_int_array, + .property_read_string_array = apple_fwnode_property_read_string_array, +}; -- 2.16.1 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html