Re: [PATCH] add some EFI device smarts

Matt Domsch
2004-04-20T23:50:38Z

[-- Attachment #1: Type: text/plain, Size: 2121 bytes --]

On Tue, Apr 20, 2004 at 04:00:26PM -0600, Bjorn Helgaas wrote:
> (Like much of the EFI stuff, this really isn't ia64-specific. Maybe
> it's time to move some of it under drivers/efi? If there's interest,
> I can look at doing that.)

Matt T. had done the work to move it under drivers/efi, though now
that there's a drivers/firmware, that's more appropraite. It also
converted it to use sysfs instead of proc. There was a bug in
efivars_exit() where it was removing stuff (which could sleep) while
holding a spinlock which wasn't good, but that was about the only
issue anyone had with it.

> + /* Convert Unicode to normal chars */
> + for (i = 0; i < (name_size/sizeof(name_unicode[0])); i++)
> + name_utf8[i] = name_unicode[i] & 0xff;
> + name_utf8[i] = 0;

I've never had a clear understanding of this. It's not really UTF8
(else straight ASCII text could be used), but more like UCS2. (Yeah,
I'm sure I named it wrong myself too in the rest of the file...)

> +
> + if (strcmp(name_utf8, name))
> + continue;

This ignores the fact that someone could create a variable with the
same name but a different vendor GUID, and it would return the first
one found. Unfortunately, you need to request both pieces
specifically -

+int
+efi_get_variable(char *name, efi_variable_t *guid, unsigned char *data, unsigned long *size)

and do a guidcmp() on them as well as the strcmp() on the name.

> +int __init
> +efi_uart_console_only(void)

So to be useful, efivars can't be build modular anymore, right? Then
Kconfig needs to change as well. It's module_init(), is that early
enough to be used? Where is efi_uart_console_only() called from?
It's not in this patch.

> +typedef struct {
> + u8 type;
> + u8 sub_type;
> + u16 length;
> +} efi_generic_dev_path_t;

No typedefs, just struct efi_generic_dev_path, and
__attribute__((packed)) please just to be safe.

Thanks,
Matt

--
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]