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.shevche...@linux.intel.com>
---

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to