Re: [PATCH 1/2] efi: register efivars with efi or gsmi from module_init

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

 



On Mon, 2013-02-18 at 02:01 +0100, Tom Gundersen wrote:
> The generic efi code or the gsmi code used to request efivars to register itself
> with them. That would only work if efivars was compiled into the kernel, and not
> when built as a module.
> 
> I.e., CONFIG_EFI_VARS_GENERIC_OPS=y and CONFIG_EFI_VARS_SYSFS=m gives:
> 
> drivers/built-in.o: In function `generic_register':
> drivers/firmware/efi/generic-ops.c:41: undefined reference
> to `efivars_sysfs_init'.
> 
> This patch reverses the logic and does the registration from
> efivars_sysfs_init().
> 
> Cc: Matt Fleming <matt.fleming@xxxxxxxxx>
> Signed-off-by: Tom Gundersen <teg@xxxxxxx>
> ---

Guh, yes. The way I implemented this is rather silly. Thanks for the
report! I was trying to avoid having a struct kobject * in struct
efivars, because that inherently ties the efivar_entry API to the sysfs
code, and the whole point of this series is that you can have EFI
variable support without the sysfs stuff.

Instead of doing the #ifdef-ery how does this look? It keeps everything
contained within struct efivars just like the old code. Unfortunately
it's applied against some changes I've got locally, so the context might
look a bit weird, but you can still get the idea.

---

diff --git a/drivers/firmware/efi/generic-ops.c b/drivers/firmware/efi/generic-ops.c
index 7f51ebd..1a0fc96 100644
--- a/drivers/firmware/efi/generic-ops.c
+++ b/drivers/firmware/efi/generic-ops.c
@@ -26,8 +26,6 @@ static struct efivar_operations generic_ops;
 
 int generic_register(void)
 {
-	int error;
-
 	if (!efi_enabled(EFI_RUNTIME_SERVICES))
 		return 0;
 
@@ -36,14 +34,7 @@ int generic_register(void)
 	generic_ops.get_next_variable = efi.get_next_variable;
 	generic_ops.query_variable_info = efi.query_variable_info;
 
-	error = efivars_register(&generic_efivars, &generic_ops);
-	if (error)
-		return error;
-
-#if defined(CONFIG_EFI_VARS_SYSFS) || defined(CONFIG_EFI_VARS_SYSFS_MODULE)
-	efivars_sysfs_init(efi_kobj);
-#endif
-	return 0;
+	return efivars_register(&generic_efivars, &generic_ops, efi_kobj);
 }
 
 void generic_unregister(void)
diff --git a/drivers/firmware/efi/sysfs.c b/drivers/firmware/efi/sysfs.c
index 797e451..8a42d08 100644
--- a/drivers/firmware/efi/sysfs.c
+++ b/drivers/firmware/efi/sysfs.c
@@ -525,8 +525,9 @@ void efivars_sysfs_exit(void)
 	kset_unregister(efivars_kset);
 }
 
-int efivars_sysfs_init(struct kobject *parent_kobj)
+int efivars_sysfs_init(void)
 {
+	struct kobject *parent_kobj;
 	int error = 0;
 
 	printk(KERN_INFO "EFI Variables Facility v%s %s\n", EFIVARS_VERSION,
@@ -535,6 +536,12 @@ int efivars_sysfs_init(struct kobject *parent_kobj)
 	if (!efi_enabled(EFI_RUNTIME_SERVICES))
 		return 0;
 
+	parent_kobj = efivars_kobject();
+	if (!parent_kobj) {
+		printk(KERN_ERR "efivars: No kobject registered.\n");
+		return -EINVAL;
+	}
+
 	efivars_kset = kset_create_and_add("vars", NULL, parent_kobj);
 	if (!efivars_kset) {
 		printk(KERN_ERR "efivars: Subsystem registration failed.\n");
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index ef216b4..81f70db 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -855,17 +855,28 @@ int efivar_entry_iter(int (*func)(struct efivar_entry *, void *), void *data)
 }
 EXPORT_SYMBOL_GPL(efivar_entry_iter);
 
+/**
+ * efivars_kobject - get the kobject for the registered efivars
+ */
+struct kobject *efivars_kobject(void)
+{
+	return __efivars->kobject;
+}
+EXPORT_SYMBOL_GPL(efivars_kobject);
+
 static DEFINE_MUTEX(efivars_mutex);
 
 /**
  * efivars_register - register an efivars
  * @efivars: efivars to register
  * @ops: efivars operations
+ * @kobject: @efivars-specific kobject
  *
  * Only a single efivars can be registered at any time.
  */
 int efivars_register(struct efivars *efivars,
-		     const struct efivar_operations *ops)
+		     const struct efivar_operations *ops,
+		     struct kobject *kobject)
 {
 	int rv;
 
@@ -877,6 +888,7 @@ int efivars_register(struct efivars *efivars,
 
 	spin_lock_init(&efivars->lock);
 	INIT_LIST_HEAD(&efivars->list);
+	efivars->kobject = kobject;
 	efivars->ops = ops;
 
 	__efivars = efivars;
@@ -926,7 +938,7 @@ out:
 }
 EXPORT_SYMBOL_GPL(efivars_unregister);
 
-static struct kobject *efivars_kobject;
+static struct kobject *efivars_kobj;
 
 static int __init
 efivars_init(void)
@@ -943,8 +955,8 @@ efivars_init(void)
 		return -ENOMEM;
 	}
 
-	efivars_kobject = kobject_create_and_add("efivars", efi_kobj);
-	if (!efivars_kobject) {
+	efivars_kobj = kobject_create_and_add("efivars", efi_kobj);
+	if (!efivars_kobj) {
 		pr_err("efivars: Subsystem registration failed.\n");
 		kobject_put(efi_kobj);
 		return -ENOMEM;
@@ -965,7 +977,7 @@ static void __exit
 efivars_exit(void)
 {
 	if (efi_enabled(EFI_RUNTIME_SERVICES)) {
-		kobject_put(efivars_kobject);
+		kobject_put(efivars_kobj);
 		kobject_put(efi_kobj);
 	}
 }
diff --git a/drivers/firmware/google/gsmi.c b/drivers/firmware/google/gsmi.c
index e902f21..d5ac00c 100644
--- a/drivers/firmware/google/gsmi.c
+++ b/drivers/firmware/google/gsmi.c
@@ -882,7 +882,7 @@ static __init int gsmi_init(void)
 		goto out_remove_bin_file;
 	}
 
-	ret = efivars_register(&efivars, &efivar_ops);
+	ret = efivars_register(&efivars, &efivar_ops, gsmi_kobj);
 	if (ret) {
 		printk(KERN_INFO "gsmi: Failed to register efivars\n");
 		goto out_remove_sysfs_files;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index b81928b..ab3934d 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -825,8 +825,10 @@ struct efivar_entry {
 };
 
 int efivars_register(struct efivars *efivars,
-		     const struct efivar_operations *ops);
+		     const struct efivar_operations *ops,
+		     struct kobject *kobject);
 int efivars_unregister(struct efivars *efivars);
+struct kobject *efivars_kobject(void);
 
 int efivar_init(int (*func)(struct efivar_entry *, void *), void *data);
 
@@ -861,7 +863,7 @@ int efivar_query_info(u32 attributes, u64 *storage_size,
 		      u64 *remaining_size, u64 *max_size);
 
 #if defined(CONFIG_EFI_VARS_SYSFS) || defined(CONFIG_EFI_VARS_SYSFS_MODULE)
-int efivars_sysfs_init(struct kobject *parent);
+int efivars_sysfs_init(void);
 int efivar_create_sysfs_entry(struct efivar_entry *new_var);
 #endif
 


--
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