Re: [PATCH] Fix ACPI halt for certain DSDTs

[Andrey Borzenkov]

В Tue, 1 Jul 2014 01:55:22 +0200
Valentin Dornauer <address@hidden> пишет:

> Hello!
> 
> GRUB2 currently seems to be unable to shut down my Fujitsu Lifebook
> E744 laptop using ACPI. After some debugging, I nailed the problem
> down to two problems within GRUB’s ‘acpihalt’ command. Please see
> the attached patch for my solution.
> 

See below

> +     in skip_ext_op(), correctly parse OpRegionOp (TermArgs aren't always
> +     simply strings!); add function to skip TermArgs.

It does not really parse every possible value of Type2Opcode, so commit
message should probably reflect it.

> +    case GRUB_ACPI_OPCODE_ADD:
> +    case GRUB_ACPI_OPCODE_CONCAT:
> +    case GRUB_ACPI_OPCODE_SUBTRACT:
> +    case GRUB_ACPI_OPCODE_MULTIPLY:
> ...

Sort by name?

> +      if (add == 0)
> +        return 0;

You use if (!add) in another places. Let's be consistent :)

>      GRUB_ACPI_OPCODE_NAME = 8, GRUB_ACPI_OPCODE_BYTE_CONST = 0x0a,
> +    GRUB_ACPI_OPCODE_ALIAS = 0x06,

Looks like list is ordered by opcode value, so it should go between
GRUB_ACPI_OPCODE_NAME and GRUB_ACPI_OPCODE_BYTE_CONST. Same also next hunk.

> +    GRUB_ACPI_OPCODE_CONCATRES = 0x80,

In my copy of ACPI spec ConcatRes has opcode 0x84.

Otherwise looks good. Vladimir, is it OK to commit?

signature.asc
Description: PGP signature