[RFC/mock up/PoC/DRAFT PATCH v1] efi/apple-properties: Convert to be a property provider

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux