Message-ID: <54A3B526.9040903@linaro.org>
Date:	Wed, 31 Dec 2014 16:34:46 +0800
From:	Hanjun Guo <hanjun.guo@...aro.org>
To:	ashwinc@...eaurora.org, catalin.marinas@....com, rjw@...ysocki.net,
	mark.rutland@....com, olof@...om.net, grant.likely@...aro.org,
	will.deacon@....com
CC:	linaro-acpi@...ts.linaro.org, Liviu.Dudau@....com,
	lv.zheng@...el.com, robh@...nel.org, Lorenzo.Pieralisi@....com,
	al.stone@...aro.org, daniel.lezcano@...aro.org,
	robert.moore@...el.com, linux-acpi@...r.kernel.org,
	Charles.Garcia-Tobin@....com, rric@...nel.org,
	jason@...edaemon.net, arnd@...db.de, marc.zyngier@....com,
	jcm@...hat.com, broonie@...nel.org,
	"bhelgaas@...gle.com." <linux-arm-kernel@...ts.infradead.org>,
	graeme.gregory@...aro.org, Kangkang.Shen@...wei.com,
	rdunlap@...radead.org, linux-kernel@...r.kernel.org,
	Sudeep.Holla@....com
Subject: Re: [PATCH v5 18/18] Documentation: ACPI for ARM64

On 2014年12月31日 04:13, ashwinc@...eaurora.org wrote:
> Hi Hanjun,
>
> Overall the document looks good to us. Some minor clarifications below.
>
>> ---------- Forwarded message ----------
>> From: Graeme Gregory <graeme.gregory@...aro.org>
>>
>> Add documentation for the guidelines of how to use ACPI
>> on ARM64.
>>
>> Signed-off-by: Graeme Gregory <graeme.gregory@...aro.org>
>> Signed-off-by: Al Stone <al.stone@...aro.org>
>> Signed-off-by: Hanjun Guo <hanjun.guo@...aro.org>
>> ---
>>   Documentation/arm64/arm-acpi.txt |  323
>> ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 323 insertions(+)
>>   create mode 100644 Documentation/arm64/arm-acpi.txt
>>
>
> [..]
>
>> +Relationship with Device Tree
>> +-----------------------------
>
> [..]
>
>> +When booting using ACPI tables, the /chosen node in DT will still be
>> parsed
>> +to extract the kernel command line and initrd path.  No other section of
>> the
>> +DT will be used.
>
> Is this still true?

No, we can booting the ACPI system in EFI stub without dtb. Catalin
also pointed out this issue, I will remove this paragraph.

>
>
>> +Programmable Power Control Resources
>> +------------------------------------
>> +Programmable power control resources include such resources as
>> voltage/current
>> +providers (regulators) and clock sources.
>> +
>> +The kernel assumes that power control of these resources is represented
>> with
>> +Power Resource Objects (ACPI section 7.1).  The ACPI core will then
>> handle
>> +correctly enabling and disabling resources as they are needed.  In order
>> to
>> +get that to work, ACPI assumes each device has defined D-states and that
>> these
>> +can be controlled through the optional ACPI methods _PS0, _PS1, _PS2, and
>> _PS3;
>> +in ACPI, _PS0 is the method to invoke to turn a device full on, and _PS3
>> is for
>> +turning a device full off.
>> +
>> +The kernel ACPI code will also assume that the _PS? methods follow the
>> normal
>> +ACPI rules for such methods:
>> +
>> +   -- If either _PS0 or _PS3 is implemented, then the other method must
>> also
>> +      be implemented.
>> +
>> +   -- If a device requires usage or setup of a power resource when on,
>> the ASL
>> +      should organize that it is allocated/enabled using the _PS0 method.
>> +
>> +   -- Resources allocated or enabled in the _PS0 method should be
>> disabled
>> +      or de-allocated in the _PS3 method.
>> +
>> +   -- Firmware will leave the resources in a reasonable state before
>> handing
>> +      over control to the kernel.
>> +
>
> We found this section could be improved a bit by explicitly calling out
> the options for handling device PM. Platform vendor has two choices.
> Resources can be managed in _PSx routine which gets called on entry to Dx.
>   Or they can be declared separately as power resources with their own _ON
> and _OFF methods.  They are then tied back to D-states for a particular
> device via _PRx which specifies which power resources a device needs to be
> on while in Dx.  Kernel then tracks number of devices using a power
> resource and calls _ON/_OFF as needed.

Good point, this exactly what ACPI spec says, we need to update this
paragraph a little bit.

>
>> +Such code in _PS? methods will of course be very platform specific.  But,
>> +this allows the driver to abstract out the interface for operating the
>> device
>> +and avoid having to read special non-standard values from ACPI tables.
>> Further,
>> +abstracting the use of these resources allows the hardware to change over
>> time
>> +without requiring updates to the driver.
>> +
>
> I think its been mentioned in the past and you planned to add it here: we
> should explicitly state that with ACPI, the kernel clock/vreg framework
> are not expected to be used at all.
>
>> +
>> +Clocks
>> +------
>> +ACPI makes the assumption that clocks are initialized by the firmware --
>> +UEFI, in this case -- to some working value before control is handed over
>> +to the kernel.  This has implications for devices such as UARTs, or SoC
>> +driven LCD displays, for example.
>> +
>> +When the kernel boots, the clock is assumed to be set to reasonable
>> +working value.  If for some reason the frequency needs to change -- e.g.,
>> +throttling for power management -- the device driver should expect that
>> +process to be abstracted out into some ACPI method that can be invoked
>
> Exception to this is CPU clocks where CPPC provides a much richer
> interface than just blindly invoking some method.
>
>> +(please see the ACPI specification for further recommendations on
>> standard
>> +methods to be expected).  If is not, there is no direct way for ACPI to
>> +control the clocks.
>> +
>> +
>
> [..]
>
>> +ASWG
>> +----
>> +The following areas are not yet fully defined for ARM in the 5.1 version
>> +of the ACPI specification and are expected to be worked through in the
>> +UEFI ACPI Specification Working Group (ASWG):
>> +
>> +   -- ACPI based CPU topology
>> +   -- ACPI based Power management
>
> Should clarify this to idle management rather than generic power management.

and I think it is CPU idle specific here, right?

>
>> +   -- CPU idle control based on PSCI
>> +   -- CPU performance control (CPPC)
>
> There is no ongoing work on CPPC. Additional enhancements may be explored
> in the future, but spec is viable as is.

will remove it.

Thanks for reviewing it!

Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists