On Fri, Apr 15, 2016 at 10:41:02AM -0600, Al Stone wrote:
On 04/15/2016 08:37 AM, Lorenzo Pieralisi wrote:
Hi Al,
On Mon, Mar 28, 2016 at 06:06:42PM -0600, Al Stone wrote:
The ACPI 6.1 specification was recently released at the end of January 2016, but the arm64 kernel documentation for the use of ACPI was written for the 5.1 version of the spec. There were significant additions to the spec that had not yet been mentioned -- for example, the 6.0 mechanisms added to make it easier to define processors and low power idle states, as well as the 6.1 addition allowing regular interrupts (not just from GPIO) be used to signal ACPI general purpose events.
This patch reflects going back through and examining the specs in detail and updating content appropriately. Whilst there, a few odds and ends of typos were caught as well. This brings the documentation up to date with ACPI 6.1 for arm64.
Changes for v3: -- Clarify use of _LPI/_RDI (Vikas Sajjan) -- Whitespace cleanup as pointed out by checkpatch
Changes for v2: -- Clean up white space (Harb Abdulhahmid) -- Clarification on _CCA usage (Harb Abdulhamid) -- IORT moved to required from recommended (Hanjun Guo) -- Clarify IORT description (Hanjun Guo)
Signed-off-by: Al Stone al.stone@linaro.org Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will.deacon@arm.com Cc: Jonathan Corbet corbet@lwn.net
Documentation/arm64/acpi_object_usage.txt | 446 ++++++++++++++++++++++-------- Documentation/arm64/arm-acpi.txt | 28 +- 2 files changed, 357 insertions(+), 117 deletions(-)
I went through this patch twice and before posting my review comments I have some questions to _ASK ;-):
- Do we really need acpi_object_usage.txt to list all possible ACPI methods in the ACPI specs ("Use as needed") and update them as the specs evolve ? IMO that's what the ACPI specs are for and that's what AML developers will refer to, I do not see the point in listing all methods in that file (can't it become an ARM addendum to the ACPI specs at least to deprecate methods/tables that are obsolete in ARM's world) ?
The original intent was to provide guidance to those unfamiliar with ACPI, and in particular provide some details on what usage makes sense on ARM. In writing it, the objective was that arm-acpi.txt be primarily an overall view, but that acpi_object_usage.txt answered specific questions about specific objects, which we got asked about frequently since APCI was very new on ARM at the time. That being said, however, it is possible that acpi_object_usage.txt has outlived its usefulness, as those that need to have become familiar with ACPI.
It doesn't help in the ACPI specs since the idea was to try to document recommended Linux-specific usage, which may or may not be OS-agnostic, but would at least be in the open to encourage common usage.
Understood, the point I wanted to make is that adding a list of methods in acpi_object_usage.txt ("Use as needed") is not necessarily additional information, you can add a pointer at ACPI specs (for that specific purpose - as I said there are parts of the patch that add additional information Linux related) for that purpose instead of having to list all of them in acpi_object_usage.txt again.
How do we keep acpi_object_usage.txt in sync with ACPI specs from now onwards ? Is that what we really want/need ?
How do we keep arm-acpi.txt in sync with kernel supported ARM64 ACPI features (if - given that this document is part of the Linux kernel docs - its aim is to describe what bits of ACPI are supported on arm64 (?)) ?
Well, maintenance will be necessary as new spec revisions come out, just like any other part of the kernel code. I don't see anything unique about these documents versus any other; is there something else in the question that I'm not seeing?
No, see above.
I guess I just assumed that since I wrote these, I'd be responsible for keeping them up to date. If you're volunteering to do so, I would not object :-).
I asked because it is kernel documentation and it has to be reviewed as such, some updates I found them necessary, adding a list of new ACPI methods that came up with ACPI 6.1, maybe, but that's already in the specs, so I question why they should be listed in that file, unless there is something kernel people really have to know, I will comment on the specific methods.
So, agreed with fixing the typos, agreed with arm-acpi.txt (and with updating it) which describes how the ARM64 kernel is using ACPI methods/tables, but acpi_object_usage.txt and in particular describing in there what methods are _useful_ and what are not, honestly I think we should ask ourselves what that file is really meant to be.
Happy to send my review comments as a follow-up since overall the patch is OK, I wanted to ask the basic questions above first.
Thanks, Lorenzo [snip...]
Does that help clarify?
Yes, I will send my few minor remarks next week and ACK accordingly, it was just for me to understand, as I mentioned.
Thanks, Lorenzo