I have no objection to this patch.
I'm concerned that Al is not happy, but I'm happy for Leif to make any decisions about self contained ACPI changes for ARM Ltd platforms until further notice, so long as it still builds and boots.
On 10 March 2016 at 11:41, Evan Lloyd Evan.Lloyd@arm.com wrote:
-----Original Message----- From: Al Stone [mailto:al.stone@linaro.org] Sent: 10 March 2016 02:17 To: Evan Lloyd; linaro-uefi@lists.linaro.org Subject: Re: [Linaro-uefi] [PATCH] Platforms/ARM: Juno: Acpi - update SPCR to aslc.
On 03/09/2016 10:21 AM, evan.lloyd@arm.com wrote:
From: Sami Mujawar sami.mujawar@arm.com
Amend the Serial Port Console Redirection Table to enable (i.a.) Microsoft Windows Emergency Management Services (EMS) over the serial port.
This patch replaces the Spcr.asl file which described the SPCR table in TDL format with Spcr.aslc which allows more flexibility by way of permitting the use of PCDs. This means the serial port usage can be modified at build.
Please don't use ASL C files. IMHO, these are actually less flexible and more prone to error over time. There is a preprocessor in iasl that can likely be used to do exactly the same thing, and is probably why it exists in the first place. Worst case, look at the macro defining how to use iasl -- it may make sense to use cpp or something else instead.
- iasl is not the only ASL/TDL compiler. In the case of a specification change, you have to wait for all of them to catch up before submitting a patch.
2.The compilers do not all support pre-processing. 3.The ASL compilers do not much like some C code as present in AutoGen.h (e.g. extern const UINT32), making it difficult to use PCDs.
(As an aside, I have no idea why the UEFI build system doesn't just use the C pre-processor for ASL, TDL. etc. Trim is an unfortunate halfway house. Maybe, were it to strip the C code from AutoGen.h, there would be some point to it beyond minor error localisation.)
The issue in my mind is that the C structs will and do change -- it is not a matter of "if it changes" but "*when* it changes". In particular, changes to reserved entries often happen. The thing that also happens is the addition of fields so that the structure defined here can get out of sync with the actual table
I absolutely agree that things change. They normally change after something new has been tried out. A benefit of aslC is that you are not prevented from trying a change by some well-intentioned semantic check that doesn't know what the next release of the spec might look like yet.
It is possible in TDL to specify new fields with generic data types. However, that introduces exactly the problem you complain of in C, but in an arcane language familiar to far fewer than C is. Further, I am not sure that all TDL compilers take kindly to redefinitions of "known" tables. (I haven't tried, but I'd bet a pint on it failing.)
definition -- and can cause problems in the kernel later on (ACPICA may rely on those new fields in odd ways, for example). Further, iasl provides semantic checks on the content that GCC may or may not see; it is possible, and likely, based on past history, that incorrect tables are provided to the kernel that could have been noticed by iasl. In one case, the table signature was wrong, so the kernel never even saw the table, but of course GCC had no idea that the value of the signature was incorrect.
That's my $0.02 worth. I'm not the UEFI maintainer, so I can't NAK these, but I'd really rather aslc *not* be used.
In case it is not obvious from my comments above, I'd really rather TDL not be used. Each to his own. One has to assume they both have reasons for existing, however ill-advised we may each think them.
...
ciao, al
Al Stone Software Engineer Linaro Enterprise Group al.stone@linaro.org
Regards, Evan
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ Linaro-uefi mailing list Linaro-uefi@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-uefi