[U-Boot] [UBOOT PATCH v5 0/3] Add ACPI table support

[Saket Sinha]

Hi Bin ,

Please find my response inline -



On Tue, Aug 18, 2015 at 2:53 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Saket,
>
> On Tue, Aug 18, 2015 at 3:10 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Hi Saket,
>>
>> On Tue, Aug 18, 2015 at 9:25 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Hi Saket,
>>>
>>> On Tue, Aug 18, 2015 at 3:29 AM, Saket Sinha <saket.sinha89 at gmail.com> wrote:
>>>>
>>>> ACPI(Advanced Configuration and Power Interface), is a Power Management and
>>>> configuration standard allowing the operating system to control the amount of
>>>> power each device is given (allowing it to put certain devices on standby or
>>>> power-off for example). It is also used to control and/or check thermal zones
>>>> (temperature sensors, fan speeds, etc), battery levels, PCI IRQ routing, CPUs,
>>>> NUMA domains and many other things.
>>>>
>>>> Changes in v5:
>>>> - Incoporated Simon's review comments.
>>>
>>> Please wait until a day or two before you post new version. I haven't
>>> got a chance to review v4, but you've already posted v5. Also when you
>>> reply our review comments, please keep the U-Boot mailing list cc'ed.
>>>
>>
>> I finished reviewing v5, but still see lots of coding convention
>> issues :( I suggest you read your codes line by line and make sure you
>> do not catch those obvious issues before you send the v6.
>>
>
> Some additional comments:
>
> 1). When I applied the patch in my git repo, I got:
>
> $ git am U-Boot-v5-1-3-x86-Generate-a-valid-ACPI-table.patch
> Applying: x86: Generate a valid ACPI table
> ~/work/git/u-boot/.git/rebase-apply/patch:428: new blank line at EOF.
> +
> ~/git/u-boot/.git/rebase-apply/patch:879: new blank line at EOF.
> +
> warning: 2 lines add whitespace errors.
> $ git am U-Boot-v5-2-3-x86-Add-ACPI-table-support-to-QEMU.patch
> Applying: x86: Add ACPI table support to QEMU
> ~/work/git/u-boot/.git/rebase-apply/patch:202: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
>
> Please fix them too.
>

Addressed in patchset series v6.

> 2). When I build qemu-x86, I noticed that:
>
> Intel ACPI Component Architecture
> ASL Optimizing Compiler version 20140828-64 [Sep 18 2014]
> Copyright (c) 2000 - 2014 Intel Corporation
>
> ASL Input:     arch/x86/cpu/qemu/dsdt.asl.tmp - 443 lines, 25927
> bytes, 342 keywords
> Hex Dump:      arch/x86/cpu/qemu/dsdt.hex - 65515 bytes
>
> Compilation complete. 1 Errors, 0 Warnings, 1 Remarks, 432 Optimizations
>
> Looks that there is 1 Error? Could you please investigate this?
>

Addressed in patchset series v6.

> 3). Testing shows that: the Linux kernel graphics console does not
> work with this series.
> I've tested that with PIRQ table and MP table, the same kernel image
> and rootfs does have a working graphics console.
> Please investigate this.
>

Yes sadly it does not. What I could debug here is that some values in
the qemu dsdt table needs to be fixed to make it working.
I can only put it as a TODO now and will dig into the issue deeper to
investigate this.


> Regards,
> Bin

Regards,
Saket Sinha