| Message ID | 1406206825-15590-20-git-send-email-hanjun.guo@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 07/24/2014 06:00 AM, Hanjun Guo wrote: > From: Graeme Gregory <graeme.gregory@linaro.org> > > Add documentation for the guidelines of how to use ACPI > on ARM64. > > Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org> > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > --- > Documentation/arm64/arm-acpi.txt | 240 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 240 insertions(+) > create mode 100644 Documentation/arm64/arm-acpi.txt > > diff --git a/Documentation/arm64/arm-acpi.txt b/Documentation/arm64/arm-acpi.txt > new file mode 100644 > index 0000000..12cd550 > --- /dev/null > +++ b/Documentation/arm64/arm-acpi.txt > @@ -0,0 +1,240 @@ > +ACPI on ARMv8 Servers > +--------------------- > + > +ACPI will be used for ARMv8 general purpose servers designed to follow > +the SBSA specification (currently available to people with an ARM login at > +http://silver.arm.com) .com). > + > +The implemented ACPI version is 5.1 + errata as released by the UEFI Forum, > +which is available at <http://www.uefi.org/acpi/specs>. > + > +If the machine does not meet these requirements then it is likely that Device > +Tree (DT) is more suitable for the hardware. > + > +Relationship with Device Tree > +----------------------------- > + > +ACPI support in drivers and subsystems for ARMv8 should never be mutually > +exclusive with DT support at compile time. > + > +At boot time the kernel will only use one description method depending on > +parameters passed from the bootloader. > + > +Regardless of whether DT or ACPI is used, the kernel must always be capable > +of booting with either scheme. > + > +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. > + > +Booting using ACPI tables > +------------------------- > + > +Currently, the only defined method to pass ACPI tables to the kernel on ARMv8 > +is via the UEFI system configuration table. > + > +The UEFI implementation MUST set the ACPI_20_TABLE_GUID to point to the > +RSDP table (the table with the ACPI signature "RSD PTR "). > + > +The pointer to the RSDP table will be retrieved from EFI by the ACPI core. > + > +Processing of ACPI tables may be disabled by passing acpi=off on the kernel > +command line. > + > +DO use an XSDT, RSDTs are deprecated and should not be used on arm64. They XSDT; > +only allow for 32bit addresses. 32-bit > + > +DO NOT use the 32-bit address fields in the FADT, they are deprecated, the FADT; they are deprecated. The > +64-bit alternatives MUST be used. > + > +The minimum set of tables MUST include RSDP, XSDT, FACS, FADT, DSDT, MADT > +and GTDT. If PCI is used the MCFG table MUST also be present. > + > +ACPI Detection > +-------------- > + > +Drivers should determine their probe() type by checking for ACPI_HANDLE, > +or .of_node, or other information in the device structure. This is > +detailed further in the "Driver Recomendations" section. Recommendations > + > +If the presence of ACPI needs to be detected at runtime, then check the value > +of acpi_disabled. If CONFIG_ACPI not being set acpi_disabled will always be 1. If CONFIG_ACPI is not set, acpi_disabled will always be 1. > + > +Device Enumeration > +------------------ > + > +Device descriptions in ACPI should use standard recognised ACPI interfaces. > +These are far simpler than the information provided via Device Tree. Drivers > +should take into account this simplicity and work with sensible defaults. > + > +On no account should a Device Tree attempt to be replicated in ASL using such > +constructs as Name(KEY0, "Value1") type constructs. Additional driver specific > +data should be passed in the appropriate _DSM (ACPI Section 9.14.1) method or > +_DSD (ACPI Section 6.2.5). This data should be rare and not OS specific. > + > +Common _DSD bindings should be submitted to ASWG to be included in the > +document :- > + > +http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm > + > +TODO: Clarification and examples from Juno implementation. > + > +Programmable Power Control Resources > +------------------------------------ > + > +Programmable power control resources include such resources as voltage/current > +providers (regulators) and clock sources. > + > +For power control of these resources they should be represented with Power > +Resource Objects (ACPI Section 7.1). The ACPI core will then handle correctly > +enabling/disabling of resources as they are needed. > + > +There exists in the ACPI 5.1 specification no standard binding for these objects > +to enable programmable levels or rates so this should be avoid if possible and avoided > +the resources set to appropriate level by the firmware. If this is not possible levels > +then any manipulation should be abstracted in ASL. > + > +Each device in ACPI has D-states and these can be controlled through > +the optional methods _PS0..._PS3 where _PS0 is full on and _PS3 is full off. > + > +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 organise that it is allocated/enabled using the _PS0 method. > + > +Resources allocated/enabled in the _PS0 method should be disabled/de-allocated > +in the _PS3 method. > + > +Such code in _PS? methods will of course be very platform specific but > +should allow the driver to operate the device without special non standard non-standard > +values being read from ASL. Further, abstracting the use of these resources > +allows hardware revisions without requiring updates to the kernel. > + > +TODO: Clarification and examples from Juno implementation. > + > +Clocks > +------ > + > +Like clocks that are part of the power resources there is no standard way > +to represent a clock tree in ACPI 5.1 in a similar manner to how it is > +described in DT. > + > +Devices affected by this include things like UARTs, SoC driven LCD displays, > +etc. > + > +The firmware for example UEFI should initialise these clocks to fixed working (for example, UEFI) > +values before the kernel is executed. If a driver requires to know rates of > +clocks set by firmware then they can be passed to kernel using _DSD. > + > +example :- > + > +Device (CLK0) { > + ... > + > + Name (_DSD, Package() { > + ToUUID("XXXXX"), > + Package() { > + Package(2) {"#clock-cells", 0}, > + Package(2) {"clock-frequency", "10000"} > + } > + }) > + > + ... > +} > + > +Device (USR1) { > + ... > + > + Name (_DSD, Package() { > + ToUUID("XXXXX"), > + Package() { > + Package(2) {"clocks", Package() {1, ^CLK0}}}, > + } > + }) > + > + ... > +} > + > +Driver Recommendations > +---------------------- > + > +DO NOT remove any FDT handling when adding ACPI support for a driver, different driver. Different > +systems may use the same device. > + > +DO try and keep complex sections of ACPI and DT functionality seperate. This separate. > +may mean a patch to break out some complex DT to another function before > +the patch to add ACPI. This may happen in other functions but is most likely > +in probe function. This gives a clearer flow of data for reviewing driver > +source. > + > +probe() :- > + > +TODO: replace this with a specific real example from Juno? > + > +static int device_probe_dt(struct platform_device *pdev) > +{ > + /* DT specific functionality */ > + ... > +} > + > +static int device_probe_acpi(struct platform_device *pdev) > +{ > + /* ACPI specific functionality */ > + ... > +} > + > +static int device_probe(stuct platform_device *pdev) > +{ > + ... > + acpi_handle handle = ACPI_HANDLE(&pdev->dev); > + struct device_node node = pdev->dev.of_node; > + ... > + > + if (node) > + ret = device_probe_dt(pdev); > + else if (handle) > + ret = device_probe_acpi(pdev); > + else > + /* other initialisation */ > + ... > + /* Continue with any generic probe operations */ > + ... > +} > + > +DO keep the MODULE_DEVICE_TABLE entries together in the driver to make it clear > +the different names the driver is probed for, both from DT and from ACPI. > + > +module device tables :- > + > +static struct of_device_id virtio_mmio_match[] = { > + { .compatible = "virtio,mmio", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, virtio_mmio_match); > + > +static const struct acpi_device_id virtio_mmio_acpi_match[] = { > + { "LNRO0005", }, > + { } > +}; > +MODULE_DEVICE_TABLE(acpi, virtio_mmio_acpi_match); > + > +TODO: Add any other helpful rules that develop from Juno ACPI work. > + > +ASWG > +---- > + > +The following areas are not yet well defined for ARM in the current ACPI > +specification and are expected to be worked through in the UEFI ACPI > +Specification Working Group (ASWG) <http://www.uefi.org/workinggroups>. > +Participation in this group is open to all UEFI members. > + > + - ACPI based CPU topology > + - ACPI based Power management > + - CPU idle control based on PSCI > + - CPU performance control (CPPC) > + > +No code shall be accepted into the kernel unless it complies with the released > +standards from UEFI ASWG. If there are features missing from ACPI to make it > +function on a platform ECRs should be submitted to ASWG and go through the on a platform, ECRs > +approval process. >
On 07/24/2014 02:16 PM, Naresh Bhat wrote: > > On 24 July 2014 18:30, Hanjun Guo <hanjun.guo@linaro.org <mailto:hanjun.guo@linaro.org>> wrote: > > From: Graeme Gregory <graeme.gregory@linaro.org <mailto:graeme.gregory@linaro.org>> > > Add documentation for the guidelines of how to use ACPI > on ARM64. > > Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org <mailto:graeme.gregory@linaro.org>> > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org <mailto:hanjun.guo@linaro.org>> > --- > Documentation/arm64/arm-acpi.txt | 240 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 240 insertions(+) > create mode 100644 Documentation/arm64/arm-acpi.txt > > diff --git a/Documentation/arm64/arm-acpi.txt b/Documentation/arm64/arm-acpi.txt > new file mode 100644 > index 0000000..12cd550 > --- /dev/null > +++ b/Documentation/arm64/arm-acpi.txt > @@ -0,0 +1,240 @@ > +ACPI on ARMv8 Servers > +--------------------- > + > +ACPI will be used for ARMv8 general purpose servers designed to follow > +the SBSA specification (currently available to people with an ARM login at > +http://silver.arm.com) > + > +The implemented ACPI version is 5.1 + errata as released by the UEFI Forum, > +which is available at <http://www.uefi.org/acpi/specs>. > + > +If the machine does not meet these requirements then it is likely that Device > +Tree (DT) is more suitable for the hardware. > + > +Relationship with Device Tree > +----------------------------- > + > +ACPI support in drivers and subsystems for ARMv8 should never be mutually > +exclusive with DT support at compile time. > + > +At boot time the kernel will only use one description method depending on > +parameters passed from the bootloader. > + > +Regardless of whether DT or ACPI is used, the kernel must always be capable > +of booting with either scheme. > + > +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. > + > +Booting using ACPI tables > +------------------------- > + > +Currently, the only defined method to pass ACPI tables to the kernel on ARMv8 > +is via the UEFI system configuration table. > + > +The UEFI implementation MUST set the ACPI_20_TABLE_GUID to point to the > +RSDP table (the table with the ACPI signature "RSD PTR "). > + > +The pointer to the RSDP table will be retrieved from EFI by the ACPI core. > + > +Processing of ACPI tables may be disabled by passing acpi=off on the kernel > +command line. > + > +DO use an XSDT, RSDTs are deprecated and should not be used on arm64. They > +only allow for 32bit addresses. > + > +DO NOT use the 32-bit address fields in the FADT, they are deprecated, the > +64-bit alternatives MUST be used. > + > +The minimum set of tables MUST include RSDP, XSDT, FACS, FADT, DSDT, MADT > +and GTDT. If PCI is used the MCFG table MUST also be present. > + > +ACPI Detection > +-------------- > + > +Drivers should determine their probe() type by checking for ACPI_HANDLE, > +or .of_node, or other information in the device structure. This is > +detailed further in the "Driver Recomendations" section. > + > +If the presence of ACPI needs to be detected at runtime, then check the value > +of acpi_disabled. If CONFIG_ACPI not being set acpi_disabled will always be 1. > + > +Device Enumeration > +------------------ > + > +Device descriptions in ACPI should use standard recognised ACPI interfaces. > > > recognized Yeah, I saw all of these also, but we accept British or American spelling of these words. > > > +These are far simpler than the information provided via Device Tree. Drivers > +should take into account this simplicity and work with sensible defaults. > + > +On no account should a Device Tree attempt to be replicated in ASL using such > +constructs as Name(KEY0, "Value1") type constructs. Additional driver specific > +data should be passed in the appropriate _DSM (ACPI Section 9.14.1) method or > +_DSD (ACPI Section 6.2.5). This data should be rare and not OS specific. > + > +Common _DSD bindings should be submitted to ASWG to be included in the > +document :- > + > +http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm > + > +TODO: Clarification and examples from Juno implementation. > + > +Programmable Power Control Resources > +------------------------------------ > + > +Programmable power control resources include such resources as voltage/current > +providers (regulators) and clock sources. > + > +For power control of these resources they should be represented with Power > +Resource Objects (ACPI Section 7.1). The ACPI core will then handle correctly > +enabling/disabling of resources as they are needed. > + > +There exists in the ACPI 5.1 specification no standard binding for these objects > +to enable programmable levels or rates so this should be avoid if possible and > +the resources set to appropriate level by the firmware. If this is not possible > +then any manipulation should be abstracted in ASL. > + > +Each device in ACPI has D-states and these can be controlled through > +the optional methods _PS0..._PS3 where _PS0 is full on and _PS3 is full off. > + > +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 organise that it is allocated/enabled using the _PS0 method. > > > organize > > + > +Resources allocated/enabled in the _PS0 method should be disabled/de-allocated > +in the _PS3 method. > + > +Such code in _PS? methods will of course be very platform specific but > +should allow the driver to operate the device without special non standard > +values being read from ASL. Further, abstracting the use of these resources > +allows hardware revisions without requiring updates to the kernel. > + > +TODO: Clarification and examples from Juno implementation. > + > +Clocks > +------ > + > +Like clocks that are part of the power resources there is no standard way > +to represent a clock tree in ACPI 5.1 in a similar manner to how it is > +described in DT. > + > +Devices affected by this include things like UARTs, SoC driven LCD displays, > +etc. > + > +The firmware for example UEFI should initialise these clocks to fixed working > > > initialize > > > +values before the kernel is executed. If a driver requires to know rates of > +clocks set by firmware then they can be passed to kernel using _DSD. > + > +example :- > + > +Device (CLK0) { > + ... > + > + Name (_DSD, Package() { > + ToUUID("XXXXX"), > + Package() { > + Package(2) {"#clock-cells", 0}, > + Package(2) {"clock-frequency", "10000"} > + } > + }) > + > + ... > +} > + > +Device (USR1) { > + ... > + > + Name (_DSD, Package() { > + ToUUID("XXXXX"), > + Package() { > + Package(2) {"clocks", Package() {1, ^CLK0}}}, > + } > + }) > + > + ... > +} > + > +Driver Recommendations > +---------------------- > + > +DO NOT remove any FDT handling when adding ACPI support for a driver, different > +systems may use the same device. > + > +DO try and keep complex sections of ACPI and DT functionality seperate. This > > > separate > > > +may mean a patch to break out some complex DT to another function before > +the patch to add ACPI. This may happen in other functions but is most likely > +in probe function. This gives a clearer flow of data for reviewing driver > +source. > + > +probe() :- > + > +TODO: replace this with a specific real example from Juno? > + > +static int device_probe_dt(struct platform_device *pdev) > +{ > + /* DT specific functionality */ > + ... > +} > + > +static int device_probe_acpi(struct platform_device *pdev) > +{ > + /* ACPI specific functionality */ > + ... > +} > + > +static int device_probe(stuct platform_device *pdev) > +{ > + ... > + acpi_handle handle = ACPI_HANDLE(&pdev->dev); > + struct device_node node = pdev->dev.of_node; > + ... > + > + if (node) > + ret = device_probe_dt(pdev); > + else if (handle) > + ret = device_probe_acpi(pdev); > + else > + /* other initialisation */ > > > initialization > > > + ... > + /* Continue with any generic probe operations */ > + ... > +} > + > +DO keep the MODULE_DEVICE_TABLE entries together in the driver to make it clear > +the different names the driver is probed for, both from DT and from ACPI. > + > +module device tables :- > + > +static struct of_device_id virtio_mmio_match[] = { > + { .compatible = "virtio,mmio", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, virtio_mmio_match); > + > +static const struct acpi_device_id virtio_mmio_acpi_match[] = { > + { "LNRO0005", }, > + { } > +}; > +MODULE_DEVICE_TABLE(acpi, virtio_mmio_acpi_match); > + > +TODO: Add any other helpful rules that develop from Juno ACPI work. > + > +ASWG > +---- > + > +The following areas are not yet well defined for ARM in the current ACPI > +specification and are expected to be worked through in the UEFI ACPI > +Specification Working Group (ASWG) <http://www.uefi.org/workinggroups>. > +Participation in this group is open to all UEFI members. > + > + - ACPI based CPU topology > + - ACPI based Power management > + - CPU idle control based on PSCI > + - CPU performance control (CPPC) > + > +No code shall be accepted into the kernel unless it complies with the released > +standards from UEFI ASWG. If there are features missing from ACPI to make it > +function on a platform ECRs should be submitted to ASWG and go through the > +approval process. > -- > 1.7.9.5 > >
Hi Randy, Thank you for your careful review comments, I will update it in next version :) Best Regards Hanjun On 2014-7-25 4:42, Randy Dunlap wrote: > On 07/24/2014 06:00 AM, Hanjun Guo wrote: >> From: Graeme Gregory <graeme.gregory@linaro.org> >> >> Add documentation for the guidelines of how to use ACPI >> on ARM64. >> >> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org> >> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> >> --- >> Documentation/arm64/arm-acpi.txt | 240 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 240 insertions(+) >> create mode 100644 Documentation/arm64/arm-acpi.txt >> >> diff --git a/Documentation/arm64/arm-acpi.txt b/Documentation/arm64/arm-acpi.txt >> new file mode 100644 >> index 0000000..12cd550 >> --- /dev/null >> +++ b/Documentation/arm64/arm-acpi.txt >> @@ -0,0 +1,240 @@ >> +ACPI on ARMv8 Servers >> +--------------------- >> + >> +ACPI will be used for ARMv8 general purpose servers designed to follow >> +the SBSA specification (currently available to people with an ARM login at >> +http://silver.arm.com) > > .com). > >> + >> +The implemented ACPI version is 5.1 + errata as released by the UEFI Forum, >> +which is available at <http://www.uefi.org/acpi/specs>. >> + >> +If the machine does not meet these requirements then it is likely that Device >> +Tree (DT) is more suitable for the hardware. >> + >> +Relationship with Device Tree >> +----------------------------- >> + >> +ACPI support in drivers and subsystems for ARMv8 should never be mutually >> +exclusive with DT support at compile time. >> + >> +At boot time the kernel will only use one description method depending on >> +parameters passed from the bootloader. >> + >> +Regardless of whether DT or ACPI is used, the kernel must always be capable >> +of booting with either scheme. >> + >> +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. >> + >> +Booting using ACPI tables >> +------------------------- >> + >> +Currently, the only defined method to pass ACPI tables to the kernel on ARMv8 >> +is via the UEFI system configuration table. >> + >> +The UEFI implementation MUST set the ACPI_20_TABLE_GUID to point to the >> +RSDP table (the table with the ACPI signature "RSD PTR "). >> + >> +The pointer to the RSDP table will be retrieved from EFI by the ACPI core. >> + >> +Processing of ACPI tables may be disabled by passing acpi=off on the kernel >> +command line. >> + >> +DO use an XSDT, RSDTs are deprecated and should not be used on arm64. They > > XSDT; > >> +only allow for 32bit addresses. > > 32-bit > >> + >> +DO NOT use the 32-bit address fields in the FADT, they are deprecated, the > > FADT; they are deprecated. The > >> +64-bit alternatives MUST be used. >> + >> +The minimum set of tables MUST include RSDP, XSDT, FACS, FADT, DSDT, MADT >> +and GTDT. If PCI is used the MCFG table MUST also be present. >> + >> +ACPI Detection >> +-------------- >> + >> +Drivers should determine their probe() type by checking for ACPI_HANDLE, >> +or .of_node, or other information in the device structure. This is >> +detailed further in the "Driver Recomendations" section. > > Recommendations > >> + >> +If the presence of ACPI needs to be detected at runtime, then check the value >> +of acpi_disabled. If CONFIG_ACPI not being set acpi_disabled will always be 1. > > If CONFIG_ACPI is not set, acpi_disabled will always be 1. > >> + >> +Device Enumeration >> +------------------ >> + >> +Device descriptions in ACPI should use standard recognised ACPI interfaces. >> +These are far simpler than the information provided via Device Tree. Drivers >> +should take into account this simplicity and work with sensible defaults. >> + >> +On no account should a Device Tree attempt to be replicated in ASL using such >> +constructs as Name(KEY0, "Value1") type constructs. Additional driver specific >> +data should be passed in the appropriate _DSM (ACPI Section 9.14.1) method or >> +_DSD (ACPI Section 6.2.5). This data should be rare and not OS specific. >> + >> +Common _DSD bindings should be submitted to ASWG to be included in the >> +document :- >> + >> +http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm >> + >> +TODO: Clarification and examples from Juno implementation. >> + >> +Programmable Power Control Resources >> +------------------------------------ >> + >> +Programmable power control resources include such resources as voltage/current >> +providers (regulators) and clock sources. >> + >> +For power control of these resources they should be represented with Power >> +Resource Objects (ACPI Section 7.1). The ACPI core will then handle correctly >> +enabling/disabling of resources as they are needed. >> + >> +There exists in the ACPI 5.1 specification no standard binding for these objects >> +to enable programmable levels or rates so this should be avoid if possible and > > avoided > >> +the resources set to appropriate level by the firmware. If this is not possible > > levels > >> +then any manipulation should be abstracted in ASL. >> + >> +Each device in ACPI has D-states and these can be controlled through >> +the optional methods _PS0..._PS3 where _PS0 is full on and _PS3 is full off. >> + >> +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 organise that it is allocated/enabled using the _PS0 method. >> + >> +Resources allocated/enabled in the _PS0 method should be disabled/de-allocated >> +in the _PS3 method. >> + >> +Such code in _PS? methods will of course be very platform specific but >> +should allow the driver to operate the device without special non standard > > non-standard > >> +values being read from ASL. Further, abstracting the use of these resources >> +allows hardware revisions without requiring updates to the kernel. >> + >> +TODO: Clarification and examples from Juno implementation. >> + >> +Clocks >> +------ >> + >> +Like clocks that are part of the power resources there is no standard way >> +to represent a clock tree in ACPI 5.1 in a similar manner to how it is >> +described in DT. >> + >> +Devices affected by this include things like UARTs, SoC driven LCD displays, >> +etc. >> + >> +The firmware for example UEFI should initialise these clocks to fixed working > > (for example, UEFI) > >> +values before the kernel is executed. If a driver requires to know rates of >> +clocks set by firmware then they can be passed to kernel using _DSD. >> + >> +example :- >> + >> +Device (CLK0) { >> + ... >> + >> + Name (_DSD, Package() { >> + ToUUID("XXXXX"), >> + Package() { >> + Package(2) {"#clock-cells", 0}, >> + Package(2) {"clock-frequency", "10000"} >> + } >> + }) >> + >> + ... >> +} >> + >> +Device (USR1) { >> + ... >> + >> + Name (_DSD, Package() { >> + ToUUID("XXXXX"), >> + Package() { >> + Package(2) {"clocks", Package() {1, ^CLK0}}}, >> + } >> + }) >> + >> + ... >> +} >> + >> +Driver Recommendations >> +---------------------- >> + >> +DO NOT remove any FDT handling when adding ACPI support for a driver, different > > driver. Different > >> +systems may use the same device. >> + >> +DO try and keep complex sections of ACPI and DT functionality seperate. This > > separate. > >> +may mean a patch to break out some complex DT to another function before >> +the patch to add ACPI. This may happen in other functions but is most likely >> +in probe function. This gives a clearer flow of data for reviewing driver >> +source. >> + >> +probe() :- >> + >> +TODO: replace this with a specific real example from Juno? >> + >> +static int device_probe_dt(struct platform_device *pdev) >> +{ >> + /* DT specific functionality */ >> + ... >> +} >> + >> +static int device_probe_acpi(struct platform_device *pdev) >> +{ >> + /* ACPI specific functionality */ >> + ... >> +} >> + >> +static int device_probe(stuct platform_device *pdev) >> +{ >> + ... >> + acpi_handle handle = ACPI_HANDLE(&pdev->dev); >> + struct device_node node = pdev->dev.of_node; >> + ... >> + >> + if (node) >> + ret = device_probe_dt(pdev); >> + else if (handle) >> + ret = device_probe_acpi(pdev); >> + else >> + /* other initialisation */ >> + ... >> + /* Continue with any generic probe operations */ >> + ... >> +} >> + >> +DO keep the MODULE_DEVICE_TABLE entries together in the driver to make it clear >> +the different names the driver is probed for, both from DT and from ACPI. >> + >> +module device tables :- >> + >> +static struct of_device_id virtio_mmio_match[] = { >> + { .compatible = "virtio,mmio", }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, virtio_mmio_match); >> + >> +static const struct acpi_device_id virtio_mmio_acpi_match[] = { >> + { "LNRO0005", }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(acpi, virtio_mmio_acpi_match); >> + >> +TODO: Add any other helpful rules that develop from Juno ACPI work. >> + >> +ASWG >> +---- >> + >> +The following areas are not yet well defined for ARM in the current ACPI >> +specification and are expected to be worked through in the UEFI ACPI >> +Specification Working Group (ASWG) <http://www.uefi.org/workinggroups>. >> +Participation in this group is open to all UEFI members. >> + >> + - ACPI based CPU topology >> + - ACPI based Power management >> + - CPU idle control based on PSCI >> + - CPU performance control (CPPC) >> + >> +No code shall be accepted into the kernel unless it complies with the released >> +standards from UEFI ASWG. If there are features missing from ACPI to make it >> +function on a platform ECRs should be submitted to ASWG and go through the > > on a platform, ECRs > >> +approval process. >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27/07/2014 03:34, Olof Johansson wrote: > On Thu, Jul 24, 2014 at 6:00 AM, Hanjun Guo <hanjun.guo@linaro.org> wrote: >> From: Graeme Gregory <graeme.gregory@linaro.org> >> >> Add documentation for the guidelines of how to use ACPI >> on ARM64. > As the most vocal participant against ACPI being adopted, I would have > appreciated a cc on this patch set -- it's not like you were going for > a minimal set of cc recipients already. It makes it seem like you're > trying to sneak it past me for comments. Not cool. I know that's > probably not your intent, but still. > > Some comments below. Overall the doc looks pretty good, but the > details about _DSD and clocks are somewhat worrisome. > >> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org> >> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> >> --- >> Documentation/arm64/arm-acpi.txt | 240 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 240 insertions(+) >> create mode 100644 Documentation/arm64/arm-acpi.txt >> >> diff --git a/Documentation/arm64/arm-acpi.txt b/Documentation/arm64/arm-acpi.txt >> new file mode 100644 >> index 0000000..12cd550 >> --- /dev/null >> +++ b/Documentation/arm64/arm-acpi.txt >> @@ -0,0 +1,240 @@ >> +ACPI on ARMv8 Servers >> +--------------------- >> + >> +ACPI will be used for ARMv8 general purpose servers designed to follow > "ACPI might be used" or "can be used" > >> +the SBSA specification (currently available to people with an ARM login at >> +http://silver.arm.com) >> + >> +The implemented ACPI version is 5.1 + errata as released by the UEFI Forum, >> +which is available at <http://www.uefi.org/acpi/specs>. > The implemented version where? The kernel implements that version? > Worth clarifying. ok the tables passed must be acpi 5.1+, the kernel must then obviously implement the 5.1 features, will clarify. >> +If the machine does not meet these requirements then it is likely that Device >> +Tree (DT) is more suitable for the hardware. > This is should be a very clear statement that is currently vague > w.r.t. which requirements are met or not, especially based on the > sentence above. The SBSA is the set of requirements, will clarify. >> +Relationship with Device Tree >> +----------------------------- >> + >> +ACPI support in drivers and subsystems for ARMv8 should never be mutually >> +exclusive with DT support at compile time. >> + >> +At boot time the kernel will only use one description method depending on >> +parameters passed from the bootloader. > Possibly overriden by kernel bootargs. And as debated for quite a > while earlier this year, acpi should still default to off -- if a DT > and ACPI are both passed in, DT should at this time be given priority. This does not work due to DT being misused as the kernel/bootloader communication layer as well. So a DT is always passed to the kernel. We cannot tell whether this is a useful DT without unpacking it and trying to boot platform from it. There is an acpi=off parameter that can be passed to always disable acpi runtime. > (Where can I learn more about how the boot loaders currently handle > this? Do some of them pass in both DT and ACPI on some platforms, for > example?) Currently only one bootloader protocol is supported for ACPI and thats UEFI. As noted above due to abuse of DT in the /chosen/ node a DT is always passed to the kernel. >> +Regardless of whether DT or ACPI is used, the kernel must always be capable >> +of booting with either scheme. > It should always be possible to compile out ACPI. There will be plenty > of platforms that will not implement it, so disabling CONFIG_ACPI > needs to be possible. This will always be possible! >> +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. >> + >> +Booting using ACPI tables >> +------------------------- >> + >> +Currently, the only defined method to pass ACPI tables to the kernel on ARMv8 >> +is via the UEFI system configuration table. >> + >> +The UEFI implementation MUST set the ACPI_20_TABLE_GUID to point to the >> +RSDP table (the table with the ACPI signature "RSD PTR "). >> + >> +The pointer to the RSDP table will be retrieved from EFI by the ACPI core. >> + >> +Processing of ACPI tables may be disabled by passing acpi=off on the kernel >> +command line. >> + >> +DO use an XSDT, RSDTs are deprecated and should not be used on arm64. They >> +only allow for 32bit addresses. >> + >> +DO NOT use the 32-bit address fields in the FADT, they are deprecated, the >> +64-bit alternatives MUST be used. >> + >> +The minimum set of tables MUST include RSDP, XSDT, FACS, FADT, DSDT, MADT >> +and GTDT. If PCI is used the MCFG table MUST also be present. >> + >> +ACPI Detection >> +-------------- >> + >> +Drivers should determine their probe() type by checking for ACPI_HANDLE, >> +or .of_node, or other information in the device structure. This is >> +detailed further in the "Driver Recomendations" section. >> + >> +If the presence of ACPI needs to be detected at runtime, then check the value >> +of acpi_disabled. If CONFIG_ACPI not being set acpi_disabled will always be 1. > Just to make sure, if acpi_disabled is 0, then there will be no acpi > handle associated with the device, right? I.e. there should be no need > to have every single driver check for whether ACPI is disabled, the > handle check should just fail instead. I need to clarify this obviously, I meant for the second paragraph for that to be for code outside driver probing. Inside drivers they should only check for ACPI_HANDLE presence. But other bits of code espcially bits parsing tables for information in early boot should check for acpi_disabled before hunting for the tables etc. >> +Device Enumeration >> +------------------ >> + >> +Device descriptions in ACPI should use standard recognised ACPI interfaces. >> +These are far simpler than the information provided via Device Tree. Drivers >> +should take into account this simplicity and work with sensible defaults. >> + >> +On no account should a Device Tree attempt to be replicated in ASL using such >> +constructs as Name(KEY0, "Value1") type constructs. Additional driver specific >> +data should be passed in the appropriate _DSM (ACPI Section 9.14.1) method or >> +_DSD (ACPI Section 6.2.5). This data should be rare and not OS specific. > I see these two sentences as contradictory, given that the _DSD doc > linked below contains examples that mirror over several properties, > such as "linux,default-trigger" and other LED-specific properties. > (section 2.4.2 in the below doc). "default-state" seems to come from > DT too? > > Care to elaborate and explain what the intention here is? This could > worst case turn into quite a mess. > > Given that ACPI can present completely different data based on what OS > is running, it's quite common to indeed have OS specific data in > there. How does that relate to this document and these practices? OS specific data has traditionally not worked out well for ACPI, I would like to "persuade" people not to use it on ARM. The _DSD was quite late to the standards process and the supporting documentation is still catching up. We are working with ARM to bring these issues up and to define proper OS agnostic bindings for ARM. >> +Common _DSD bindings should be submitted to ASWG to be included in the >> +document :- >> + >> +http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm > So, for these that are a mirror of the device tree bindings, there > needs to be a wording here around reviewing the DT binding first so we > don't get diverging bindings. > >> +TODO: Clarification and examples from Juno implementation. >> + >> +Programmable Power Control Resources >> +------------------------------------ >> + >> +Programmable power control resources include such resources as voltage/current >> +providers (regulators) and clock sources. >> + >> +For power control of these resources they should be represented with Power >> +Resource Objects (ACPI Section 7.1). The ACPI core will then handle correctly >> +enabling/disabling of resources as they are needed. >> + >> +There exists in the ACPI 5.1 specification no standard binding for these objects >> +to enable programmable levels or rates so this should be avoid if possible and >> +the resources set to appropriate level by the firmware. If this is not possible >> +then any manipulation should be abstracted in ASL. >> + >> +Each device in ACPI has D-states and these can be controlled through >> +the optional methods _PS0..._PS3 where _PS0 is full on and _PS3 is full off. >> + >> +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 organise that it is allocated/enabled using the _PS0 method. >> + >> +Resources allocated/enabled in the _PS0 method should be disabled/de-allocated >> +in the _PS3 method. >> + >> +Such code in _PS? methods will of course be very platform specific but >> +should allow the driver to operate the device without special non standard >> +values being read from ASL. Further, abstracting the use of these resources >> +allows hardware revisions without requiring updates to the kernel. >> + >> +TODO: Clarification and examples from Juno implementation. >> + >> +Clocks >> +------ >> + >> +Like clocks that are part of the power resources there is no standard way >> +to represent a clock tree in ACPI 5.1 in a similar manner to how it is >> +described in DT. >> + >> +Devices affected by this include things like UARTs, SoC driven LCD displays, >> +etc. >> + >> +The firmware for example UEFI should initialise these clocks to fixed working > Odd wording. Maube "The firmware (for example UEFI) should..." agreed! >> +values before the kernel is executed. If a driver requires to know rates of >> +clocks set by firmware then they can be passed to kernel using _DSD. >> + >> +example :- >> + >> +Device (CLK0) { >> + ... >> + >> + Name (_DSD, Package() { >> + ToUUID("XXXXX"), >> + Package() { >> + Package(2) {"#clock-cells", 0}, > Clock-cells? What do they mean here? Is this specified in the ACPI > standards? I had to register to get access to it, and didn't feel like > doing that right now. I guess it's not _all_ that open a spec. :( > >> + Package(2) {"clock-frequency", "10000"} >> + } >> + }) >> + >> + ... >> +} >> + >> +Device (USR1) { >> + ... >> + >> + Name (_DSD, Package() { >> + ToUUID("XXXXX"), >> + Package() { >> + Package(2) {"clocks", Package() {1, ^CLK0}}}, > A clock is a device in the ACPI model? Why not just provide the rate > as data into the device here? You said you're not trying to model the > clock tree, so why reference an external node for it? This section is still a bit WIP due to the above noted issues with _DSD documentation catching up with the standards process. I will need to work with the clock maintainers to see if we can agree a proper set of bindings for this. #blah-cells always was my least favorite DT feature. >> + } >> + }) >> + >> + ... >> +} >> + >> +Driver Recommendations >> +---------------------- >> + >> +DO NOT remove any FDT handling when adding ACPI support for a driver, different >> +systems may use the same device. >> + >> +DO try and keep complex sections of ACPI and DT functionality seperate. This >> +may mean a patch to break out some complex DT to another function before >> +the patch to add ACPI. This may happen in other functions but is most likely >> +in probe function. This gives a clearer flow of data for reviewing driver >> +source. >> + >> +probe() :- >> + >> +TODO: replace this with a specific real example from Juno? >> + >> +static int device_probe_dt(struct platform_device *pdev) >> +{ >> + /* DT specific functionality */ >> + ... >> +} >> + >> +static int device_probe_acpi(struct platform_device *pdev) >> +{ >> + /* ACPI specific functionality */ >> + ... >> +} >> + >> +static int device_probe(stuct platform_device *pdev) >> +{ >> + ... >> + acpi_handle handle = ACPI_HANDLE(&pdev->dev); >> + struct device_node node = pdev->dev.of_node; >> + ... >> + >> + if (node) >> + ret = device_probe_dt(pdev); >> + else if (handle) >> + ret = device_probe_acpi(pdev); >> + else >> + /* other initialisation */ >> + ... >> + /* Continue with any generic probe operations */ >> + ... >> +} > This looks good to me, and it's also my preferred way of ACPI-enabling > drivers. I guess we might discuss this at KS since it was a proposed > topic there, and others will object. :) Hopefully someone can summarise the discussion at KS for me, I will not be there. >> + >> +DO keep the MODULE_DEVICE_TABLE entries together in the driver to make it clear >> +the different names the driver is probed for, both from DT and from ACPI. >> + >> +module device tables :- >> + >> +static struct of_device_id virtio_mmio_match[] = { >> + { .compatible = "virtio,mmio", }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, virtio_mmio_match); >> + >> +static const struct acpi_device_id virtio_mmio_acpi_match[] = { >> + { "LNRO0005", }, >> + { } > No comma here, but a comma on DT. Probably make them equivalent for > consistency (including space between the braces). ok >> +}; >> +MODULE_DEVICE_TABLE(acpi, virtio_mmio_acpi_match); >> + >> +TODO: Add any other helpful rules that develop from Juno ACPI work. > Looks like this should be fixed before the patch is merged, or this > TODO removed. The plan is to fix these TODOs with actual data from Juno as that will be in the UEFI for Juno. >> + >> +ASWG >> +---- >> + >> +The following areas are not yet well defined for ARM in the current ACPI >> +specification and are expected to be worked through in the UEFI ACPI >> +Specification Working Group (ASWG) <http://www.uefi.org/workinggroups>. >> +Participation in this group is open to all UEFI members. >> + >> + - ACPI based CPU topology >> + - ACPI based Power management >> + - CPU idle control based on PSCI >> + - CPU performance control (CPPC) >> + >> +No code shall be accepted into the kernel unless it complies with the released >> +standards from UEFI ASWG. If there are features missing from ACPI to make it >> +function on a platform ECRs should be submitted to ASWG and go through the >> +approval process. > Thanks for listing the things that are not in place yet. Please keep > this doc up to date as new areas are discovered. > > > -Olof Thanks for the feedback, we shall work to incorporate it into the document. Graeme -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Saturday 26 July 2014 19:34:48 Olof Johansson wrote: > On Thu, Jul 24, 2014 at 6:00 AM, Hanjun Guo <hanjun.guo@linaro.org> wrote: > > +Relationship with Device Tree > > +----------------------------- > > + > > +ACPI support in drivers and subsystems for ARMv8 should never be mutually > > +exclusive with DT support at compile time. > > + > > +At boot time the kernel will only use one description method depending on > > +parameters passed from the bootloader. > > Possibly overriden by kernel bootargs. And as debated for quite a > while earlier this year, acpi should still default to off -- if a DT > and ACPI are both passed in, DT should at this time be given priority. I think this would be harder to do with the way that ACPI is passed in to the kernel. IIRC, you always have a minimal DT information based on the ARM64 boot protocol, but in the case of ACPI, this contains pointers to the ACPI tables, which are then used for populating the Linux platform devices (unless acpi=disabled is set), while the other contents of the DTB may be present but we skip the of_platform_populate state. If this is correct, then replacing the firmware-generated dtb with a user-provided on would implicitly remove the ACPI tables from visibility, which is exactly what we want. It's possible that I'm misremembering it though, and it should be documented better. > > +Regardless of whether DT or ACPI is used, the kernel must always be capable > > +of booting with either scheme. > > It should always be possible to compile out ACPI. There will be plenty > of platforms that will not implement it, so disabling CONFIG_ACPI > needs to be possible. Right. > > +Clocks > > +------ > > + > > +Like clocks that are part of the power resources there is no standard way > > +to represent a clock tree in ACPI 5.1 in a similar manner to how it is > > +described in DT. > > + > > +Devices affected by this include things like UARTs, SoC driven LCD displays, > > +etc. > > + > > +The firmware for example UEFI should initialise these clocks to fixed working > > +values before the kernel is executed. If a driver requires to know rates of > > +clocks set by firmware then they can be passed to kernel using _DSD. > > + > > +example :- > > + > > +Device (CLK0) { > > + ... > > + > > + Name (_DSD, Package() { > > + ToUUID("XXXXX"), > > + Package() { > > + Package(2) {"#clock-cells", 0}, > > Clock-cells? What do they mean here? Is this specified in the ACPI > standards? I had to register to get access to it, and didn't feel like > doing that right now. I guess it's not _all_ that open a spec. :( ... > > + Package(2) {"clock-frequency", "10000"} > > + } > > + }) > > + > > + ... > > +} > > + > > +Device (USR1) { > > + ... > > + > > + Name (_DSD, Package() { > > + ToUUID("XXXXX"), > > + Package() { > > + Package(2) {"clocks", Package() {1, ^CLK0}}}, > > A clock is a device in the ACPI model? Why not just provide the rate > as data into the device here? You said you're not trying to model the > clock tree, so why reference an external node for it? Exactly. I think what is going on here is a conflict of interests between Intel's embedded ACPI uses and the ARM64 server requirements. The above closely resembles what we do in DT, and that makes perfect sense for Intel's machines so they can reuse a lot of the infrastructure we put in place for DT. I also suspect it will take a few more years before this actually gets accepted into both an ACPI specification and the common operating systems (no point doing it if only Linux is going to adopt it). For the servers, I don't see how it makes any sense at all, independent of the architecture, and relying on a feature like this would only serve to delay the adoption of ACPI (whether that is a good or bad thing may be a matter of perspective). Maybe Graeme or others can comment on where this is coming from. What kind of driver would actually need to find out the clock rate of a device on an arm64 server? The examples above list "UARTs, SoC driven LCD displays, etc.". For all I know, the UART is required to be PL01x (without DMA) compatible, which should be fully described in ACPI, and I don't see why a server would come with an LCD. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28/07/2014 10:07, Arnd Bergmann wrote: > On Saturday 26 July 2014 19:34:48 Olof Johansson wrote: >> On Thu, Jul 24, 2014 at 6:00 AM, Hanjun Guo <hanjun.guo@linaro.org> wrote: >>> +Relationship with Device Tree >>> +----------------------------- >>> + >>> +ACPI support in drivers and subsystems for ARMv8 should never be mutually >>> +exclusive with DT support at compile time. >>> + >>> +At boot time the kernel will only use one description method depending on >>> +parameters passed from the bootloader. >> Possibly overriden by kernel bootargs. And as debated for quite a >> while earlier this year, acpi should still default to off -- if a DT >> and ACPI are both passed in, DT should at this time be given priority. > I think this would be harder to do with the way that ACPI is passed in > to the kernel. IIRC, you always have a minimal DT information based on > the ARM64 boot protocol, but in the case of ACPI, this contains pointers > to the ACPI tables, which are then used for populating the Linux platform > devices (unless acpi=disabled is set), while the other contents of the > DTB may be present but we skip the of_platform_populate state. > > If this is correct, then replacing the firmware-generated dtb with a > user-provided on would implicitly remove the ACPI tables from visibility, > which is exactly what we want. > > It's possible that I'm misremembering it though, and it should be > documented better. > >>> +Regardless of whether DT or ACPI is used, the kernel must always be capable >>> +of booting with either scheme. >> It should always be possible to compile out ACPI. There will be plenty >> of platforms that will not implement it, so disabling CONFIG_ACPI >> needs to be possible. > Right. > >>> +Clocks >>> +------ >>> + >>> +Like clocks that are part of the power resources there is no standard way >>> +to represent a clock tree in ACPI 5.1 in a similar manner to how it is >>> +described in DT. >>> + >>> +Devices affected by this include things like UARTs, SoC driven LCD displays, >>> +etc. >>> + >>> +The firmware for example UEFI should initialise these clocks to fixed working >>> +values before the kernel is executed. If a driver requires to know rates of >>> +clocks set by firmware then they can be passed to kernel using _DSD. >>> + >>> +example :- >>> + >>> +Device (CLK0) { >>> + ... >>> + >>> + Name (_DSD, Package() { >>> + ToUUID("XXXXX"), >>> + Package() { >>> + Package(2) {"#clock-cells", 0}, >> Clock-cells? What do they mean here? Is this specified in the ACPI >> standards? I had to register to get access to it, and didn't feel like >> doing that right now. I guess it's not _all_ that open a spec. :( > ... >>> + Package(2) {"clock-frequency", "10000"} >>> + } >>> + }) >>> + >>> + ... >>> +} >>> + >>> +Device (USR1) { >>> + ... >>> + >>> + Name (_DSD, Package() { >>> + ToUUID("XXXXX"), >>> + Package() { >>> + Package(2) {"clocks", Package() {1, ^CLK0}}}, >> A clock is a device in the ACPI model? Why not just provide the rate >> as data into the device here? You said you're not trying to model the >> clock tree, so why reference an external node for it? > Exactly. I think what is going on here is a conflict of interests between > Intel's embedded ACPI uses and the ARM64 server requirements. The above > closely resembles what we do in DT, and that makes perfect sense for > Intel's machines so they can reuse a lot of the infrastructure we put > in place for DT. I also suspect it will take a few more years before > this actually gets accepted into both an ACPI specification and the > common operating systems (no point doing it if only Linux is going to > adopt it). > > For the servers, I don't see how it makes any sense at all, independent > of the architecture, and relying on a feature like this would only serve > to delay the adoption of ACPI (whether that is a good or bad thing > may be a matter of perspective). > > Maybe Graeme or others can comment on where this is coming from. What kind > of driver would actually need to find out the clock rate of a device on > an arm64 server? The examples above list "UARTs, SoC driven LCD displays, > etc.". For all I know, the UART is required to be PL01x (without DMA) > compatible, which should be fully described in ACPI, and I don't see why > a server would come with an LCD. > > The PL011 UART is the use-case I keep hitting, that IP block has a variable input clock on pretty much everything I have seen in the wild. I really hope that this use does not spread beyond a few essential devices like the UART. IMO all real hardware should be the other side of a PCIe bridge. Graeme -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 28 July 2014 10:23:57 Graeme Gregory wrote: > The PL011 UART is the use-case I keep hitting, that IP block has a > variable input clock on pretty much everything I have seen in the wild. Ok, I see. What does ACPI-5.1 say about pl011? Interestingly, the subset of pl011 that is specified by SBSA does not contain the IBRD/FBRD registers, effectively making it a fixed-rated UART (I guess that would be a ART, without the U then), and you consequently don't even need to know the clock rate. However, my guess is that most hardware in the real world contains an actual pl011 and it does make a lot of sense to allow setting the baud rate on it, which then requires knowing the input clock. If there is any hardware that implements just the SBSA-mandated subset rather than the full pl011, we should probably implement both in the kernel: a dumb driver that can only send and receive, and the more complex one that can set the bit rates and flow-control but that requires a standardized ACPI table with the input clock rate. Whether the two would belong into one file or two separate driver modules is something I can't tell, it would be up to the serial maintainers to decide. > I really hope that this use does not spread beyond a few essential > devices like the UART. IMO all real hardware should be the other side of > a PCIe bridge. I would definitely agree with that. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28/07/14 11:46, Arnd Bergmann wrote: > On Monday 28 July 2014 10:23:57 Graeme Gregory wrote: >> The PL011 UART is the use-case I keep hitting, that IP block has a >> variable input clock on pretty much everything I have seen in the wild. > > Ok, I see. What does ACPI-5.1 say about pl011? > > Interestingly, the subset of pl011 that is specified by SBSA does not > contain the IBRD/FBRD registers, effectively making it a fixed-rated > UART (I guess that would be a ART, without the U then), and you > consequently don't even need to know the clock rate. The idea of this was probably to let the baudrate set by some firmware code to the "right" value and the spec just didn't want to expose the details for the generic UART: "This specification does not cover registers needed to configure the UART as these are considered hardware-specific and will be set up by hardware-specific software." To me that reads like the SBSA UART is just for debugging, and you are expected just to access the data register. > However, my guess is that most hardware in the real world contains > an actual pl011 and it does make a lot of sense to allow setting > the baud rate on it, which then requires knowing the input clock. > > If there is any hardware that implements just the SBSA-mandated subset > rather than the full pl011, we should probably implement both > in the kernel: a dumb driver that can only send and receive, and the > more complex one that can set the bit rates and flow-control but that > requires a standardized ACPI table with the input clock rate. The fast model I use can be switched to use the SBSA restricted PL011, and as expected the Linux kernel crashes at the device doesn't support DMA (and a lot more stuff) - but the current code requires it. So I am about to implement a new driver for that SBSA subset. So far this will be a separate driver, starting from a copy of amba-pl011.c, but removing most of the code ;-) > Whether the two would belong into one file or two separate driver > modules is something I can't tell, it would be up to the serial > maintainers to decide. Sharing support for both devices in the same file doesn't seem to make too much sense to me so far (but I may amend this later). Will post a version as soon as I have it finished. Cheers, Andre. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 28 July 2014 15:20:06 Andre Przywara wrote: > On 28/07/14 11:46, Arnd Bergmann wrote: > > On Monday 28 July 2014 10:23:57 Graeme Gregory wrote: > >> The PL011 UART is the use-case I keep hitting, that IP block has a > >> variable input clock on pretty much everything I have seen in the wild. > > > > Ok, I see. What does ACPI-5.1 say about pl011? > > > > Interestingly, the subset of pl011 that is specified by SBSA does not > > contain the IBRD/FBRD registers, effectively making it a fixed-rated > > UART (I guess that would be a ART, without the U then), and you > > consequently don't even need to know the clock rate. > > The idea of this was probably to let the baudrate set by some firmware > code to the "right" value and the spec just didn't want to expose the > details for the generic UART: > "This specification does not cover registers needed to configure the > UART as these are considered hardware-specific and will be set up by > hardware-specific software." > To me that reads like the SBSA UART is just for debugging, and you are > expected just to access the data register. Right, makes sense. It also avoids the case where Linux for some reason ends up using a different line rate than the firmware, which can cause a lot of unnecessary pain. > > However, my guess is that most hardware in the real world contains > > an actual pl011 and it does make a lot of sense to allow setting > > the baud rate on it, which then requires knowing the input clock. > > > > If there is any hardware that implements just the SBSA-mandated subset > > rather than the full pl011, we should probably implement both > > in the kernel: a dumb driver that can only send and receive, and the > > more complex one that can set the bit rates and flow-control but that > > requires a standardized ACPI table with the input clock rate. > > The fast model I use can be switched to use the SBSA restricted PL011, > and as expected the Linux kernel crashes at the device doesn't support > DMA (and a lot more stuff) - but the current code requires it. It does? We have a lot of platforms that don't have DMA support for pl011. > So I am about to implement a new driver for that SBSA subset. So far > this will be a separate driver, starting from a copy of amba-pl011.c, > but removing most of the code ;-) Ok. You might want to consider starting from a different base though. IIRC, pl011 uses uart_port as the basic abstraction, while the new driver should probably use the raw tty_port instead. drivers/tty/goldfish.c is probably a good example to look at for that. You could also make it a hvc_driver like drivers/tty/hvc/hvc_vio.c, but I'm not sure if that model seen favorable by the tty maintainers. It would probably be the shortest driver though. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 28, 2014 at 09:42:57AM +0100, Graeme Gregory wrote: > > On 27/07/2014 03:34, Olof Johansson wrote: > >On Thu, Jul 24, 2014 at 6:00 AM, Hanjun Guo <hanjun.guo@linaro.org> wrote: > >>From: Graeme Gregory <graeme.gregory@linaro.org> > >> > >>Add documentation for the guidelines of how to use ACPI > >>on ARM64. > >As the most vocal participant against ACPI being adopted, I would have > >appreciated a cc on this patch set -- it's not like you were going for > >a minimal set of cc recipients already. It makes it seem like you're > >trying to sneak it past me for comments. Not cool. I know that's > >probably not your intent, but still. > > > >Some comments below. Overall the doc looks pretty good, but the > >details about _DSD and clocks are somewhat worrisome. > > > >>Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org> > >>Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > >>--- > >> Documentation/arm64/arm-acpi.txt | 240 ++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 240 insertions(+) > >> create mode 100644 Documentation/arm64/arm-acpi.txt > >> > >>diff --git a/Documentation/arm64/arm-acpi.txt b/Documentation/arm64/arm-acpi.txt > >>new file mode 100644 > >>index 0000000..12cd550 > >>--- /dev/null > >>+++ b/Documentation/arm64/arm-acpi.txt > >>@@ -0,0 +1,240 @@ > >>+ACPI on ARMv8 Servers > >>+--------------------- > >>+ > >>+ACPI will be used for ARMv8 general purpose servers designed to follow > >"ACPI might be used" or "can be used" > > > >>+the SBSA specification (currently available to people with an ARM login at > >>+http://silver.arm.com) > >>+ > >>+The implemented ACPI version is 5.1 + errata as released by the UEFI Forum, > >>+which is available at <http://www.uefi.org/acpi/specs>. > >The implemented version where? The kernel implements that version? > >Worth clarifying. > ok > > the tables passed must be acpi 5.1+, the kernel must then obviously > implement the 5.1 features, will clarify. > >>+If the machine does not meet these requirements then it is likely that Device > >>+Tree (DT) is more suitable for the hardware. > >This is should be a very clear statement that is currently vague > >w.r.t. which requirements are met or not, especially based on the > >sentence above. > The SBSA is the set of requirements, will clarify. > > >>+Relationship with Device Tree > >>+----------------------------- > >>+ > >>+ACPI support in drivers and subsystems for ARMv8 should never be mutually > >>+exclusive with DT support at compile time. > >>+ > >>+At boot time the kernel will only use one description method depending on > >>+parameters passed from the bootloader. > >Possibly overriden by kernel bootargs. And as debated for quite a > >while earlier this year, acpi should still default to off -- if a DT > >and ACPI are both passed in, DT should at this time be given priority. > This does not work due to DT being misused as the kernel/bootloader > communication layer as well. So a DT is always passed to the kernel. > We cannot tell whether this is a useful DT without unpacking it and > trying to boot platform from it. > > There is an acpi=off parameter that can be passed to always disable > acpi runtime. Right, but the agreement we had from earlier this year was to keep ACPI default off until we've seen at least the first generation of real hardware, since we have no confidence that the system vendors will do sane things with ACPI yet. If they completely mess it up, we at least will retain basic functionality without accuring huge technical debt dealing with the messed up ACPI tables forever. In other words, to boot with ACPI enabled, the requrement is that you have to pass 'acpi' or 'acpi=on' on the command line (or equivalent). Not the other way around -- to have it on by default and give an option to turn it off. If you need a refresher: http://www.secretlab.ca/archives/27 > >(Where can I learn more about how the boot loaders currently handle > >this? Do some of them pass in both DT and ACPI on some platforms, for > >example?) > Currently only one bootloader protocol is supported for ACPI and > thats UEFI. As noted above due to abuse of DT in the /chosen/ node a > DT is always passed to the kernel. The above is hard to understand. I suppose I should restate my question: I presume actual end users will use something like Grub2 to load a kernel/ramdisk/dtb. Where can I learn more about how it handles ACPI? > >>+Regardless of whether DT or ACPI is used, the kernel must always be capable > >>+of booting with either scheme. > >It should always be possible to compile out ACPI. There will be plenty > >of platforms that will not implement it, so disabling CONFIG_ACPI > >needs to be possible. > This will always be possible! Yes, I commented because it should also be clear in the doc. > >>+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. > >>+ > >>+Booting using ACPI tables > >>+------------------------- > >>+ > >>+Currently, the only defined method to pass ACPI tables to the kernel on ARMv8 > >>+is via the UEFI system configuration table. > >>+ > >>+The UEFI implementation MUST set the ACPI_20_TABLE_GUID to point to the > >>+RSDP table (the table with the ACPI signature "RSD PTR "). > >>+ > >>+The pointer to the RSDP table will be retrieved from EFI by the ACPI core. > >>+ > >>+Processing of ACPI tables may be disabled by passing acpi=off on the kernel > >>+command line. > >>+ > >>+DO use an XSDT, RSDTs are deprecated and should not be used on arm64. They > >>+only allow for 32bit addresses. > >>+ > >>+DO NOT use the 32-bit address fields in the FADT, they are deprecated, the > >>+64-bit alternatives MUST be used. > >>+ > >>+The minimum set of tables MUST include RSDP, XSDT, FACS, FADT, DSDT, MADT > >>+and GTDT. If PCI is used the MCFG table MUST also be present. > >>+ > >>+ACPI Detection > >>+-------------- > >>+ > >>+Drivers should determine their probe() type by checking for ACPI_HANDLE, > >>+or .of_node, or other information in the device structure. This is > >>+detailed further in the "Driver Recomendations" section. > >>+ > >>+If the presence of ACPI needs to be detected at runtime, then check the value > >>+of acpi_disabled. If CONFIG_ACPI not being set acpi_disabled will always be 1. > >Just to make sure, if acpi_disabled is 0, then there will be no acpi > >handle associated with the device, right? I.e. there should be no need > >to have every single driver check for whether ACPI is disabled, the > >handle check should just fail instead. > I need to clarify this obviously, I meant for the second paragraph > for that to be for code outside driver probing. Inside drivers they > should only check for ACPI_HANDLE presence. But other bits of code > espcially bits parsing tables for information in early boot should > check for acpi_disabled before hunting for the tables etc. Yep, that sounds sane. > >>+Device Enumeration > >>+------------------ > >>+ > >>+Device descriptions in ACPI should use standard recognised ACPI interfaces. > >>+These are far simpler than the information provided via Device Tree. Drivers > >>+should take into account this simplicity and work with sensible defaults. > >>+ > >>+On no account should a Device Tree attempt to be replicated in ASL using such > >>+constructs as Name(KEY0, "Value1") type constructs. Additional driver specific > >>+data should be passed in the appropriate _DSM (ACPI Section 9.14.1) method or > >>+_DSD (ACPI Section 6.2.5). This data should be rare and not OS specific. > >I see these two sentences as contradictory, given that the _DSD doc > >linked below contains examples that mirror over several properties, > >such as "linux,default-trigger" and other LED-specific properties. > >(section 2.4.2 in the below doc). "default-state" seems to come from > >DT too? > > > >Care to elaborate and explain what the intention here is? This could > >worst case turn into quite a mess. > > > >Given that ACPI can present completely different data based on what OS > >is running, it's quite common to indeed have OS specific data in > >there. How does that relate to this document and these practices? > OS specific data has traditionally not worked out well for ACPI, I > would like to "persuade" people not to use it on ARM. It hasn't? I think Microsoft disagrees. It's also how vendors have been able to present an older machine description to keep their newer hardware compatible with older software, isn't it? How do you expect to handle that if you can only present one table? It's the same challenge that DT has. > The _DSD was quite late to the standards process and the supporting > documentation is still catching up. We are working with ARM to bring > these issues up and to define proper OS agnostic bindings for ARM. I'm guessing that the first ARM should be ACPI? Or is ARM Ltd on critical path on this?! > >>+Common _DSD bindings should be submitted to ASWG to be included in the > >>+document :- > >>+ > >>+http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm > >So, for these that are a mirror of the device tree bindings, there > >needs to be a wording here around reviewing the DT binding first so we > >don't get diverging bindings. > > > >>+TODO: Clarification and examples from Juno implementation. > >>+ > >>+Programmable Power Control Resources > >>+------------------------------------ > >>+ > >>+Programmable power control resources include such resources as voltage/current > >>+providers (regulators) and clock sources. > >>+ > >>+For power control of these resources they should be represented with Power > >>+Resource Objects (ACPI Section 7.1). The ACPI core will then handle correctly > >>+enabling/disabling of resources as they are needed. > >>+ > >>+There exists in the ACPI 5.1 specification no standard binding for these objects > >>+to enable programmable levels or rates so this should be avoid if possible and > >>+the resources set to appropriate level by the firmware. If this is not possible > >>+then any manipulation should be abstracted in ASL. > >>+ > >>+Each device in ACPI has D-states and these can be controlled through > >>+the optional methods _PS0..._PS3 where _PS0 is full on and _PS3 is full off. > >>+ > >>+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 organise that it is allocated/enabled using the _PS0 method. > >>+ > >>+Resources allocated/enabled in the _PS0 method should be disabled/de-allocated > >>+in the _PS3 method. > >>+ > >>+Such code in _PS? methods will of course be very platform specific but > >>+should allow the driver to operate the device without special non standard > >>+values being read from ASL. Further, abstracting the use of these resources > >>+allows hardware revisions without requiring updates to the kernel. > >>+ > >>+TODO: Clarification and examples from Juno implementation. > >>+ > >>+Clocks > >>+------ > >>+ > >>+Like clocks that are part of the power resources there is no standard way > >>+to represent a clock tree in ACPI 5.1 in a similar manner to how it is > >>+described in DT. > >>+ > >>+Devices affected by this include things like UARTs, SoC driven LCD displays, > >>+etc. > >>+ > >>+The firmware for example UEFI should initialise these clocks to fixed working > >Odd wording. Maube "The firmware (for example UEFI) should..." > agreed! > >>+values before the kernel is executed. If a driver requires to know rates of > >>+clocks set by firmware then they can be passed to kernel using _DSD. > >>+ > >>+example :- > >>+ > >>+Device (CLK0) { > >>+ ... > >>+ > >>+ Name (_DSD, Package() { > >>+ ToUUID("XXXXX"), > >>+ Package() { > >>+ Package(2) {"#clock-cells", 0}, > >Clock-cells? What do they mean here? Is this specified in the ACPI > >standards? I had to register to get access to it, and didn't feel like > >doing that right now. I guess it's not _all_ that open a spec. :( > > > >>+ Package(2) {"clock-frequency", "10000"} > >>+ } > >>+ }) > >>+ > >>+ ... > >>+} > >>+ > >>+Device (USR1) { > >>+ ... > >>+ > >>+ Name (_DSD, Package() { > >>+ ToUUID("XXXXX"), > >>+ Package() { > >>+ Package(2) {"clocks", Package() {1, ^CLK0}}}, > >A clock is a device in the ACPI model? Why not just provide the rate > >as data into the device here? You said you're not trying to model the > >clock tree, so why reference an external node for it? > This section is still a bit WIP due to the above noted issues with > _DSD documentation catching up with the standards process. I will > need to work with the clock maintainers to see if we can agree a > proper set of bindings for this. #blah-cells always was my least > favorite DT feature. Ok, work in progress is fine with me. But it is the reason for why we need to continue defaulting to ACPI off for now -- just because we don't want to have to support it in case some vendor picks up and use something half-baked while it's still being worked on. > >>+ } > >>+ }) > >>+ > >>+ ... > >>+} > >>+ > >>+Driver Recommendations > >>+---------------------- > >>+ > >>+DO NOT remove any FDT handling when adding ACPI support for a driver, different > >>+systems may use the same device. > >>+ > >>+DO try and keep complex sections of ACPI and DT functionality seperate. This > >>+may mean a patch to break out some complex DT to another function before > >>+the patch to add ACPI. This may happen in other functions but is most likely > >>+in probe function. This gives a clearer flow of data for reviewing driver > >>+source. > >>+ > >>+probe() :- > >>+ > >>+TODO: replace this with a specific real example from Juno? > >>+ > >>+static int device_probe_dt(struct platform_device *pdev) > >>+{ > >>+ /* DT specific functionality */ > >>+ ... > >>+} > >>+ > >>+static int device_probe_acpi(struct platform_device *pdev) > >>+{ > >>+ /* ACPI specific functionality */ > >>+ ... > >>+} > >>+ > >>+static int device_probe(stuct platform_device *pdev) > >>+{ > >>+ ... > >>+ acpi_handle handle = ACPI_HANDLE(&pdev->dev); > >>+ struct device_node node = pdev->dev.of_node; > >>+ ... > >>+ > >>+ if (node) > >>+ ret = device_probe_dt(pdev); > >>+ else if (handle) > >>+ ret = device_probe_acpi(pdev); > >>+ else > >>+ /* other initialisation */ > >>+ ... > >>+ /* Continue with any generic probe operations */ > >>+ ... > >>+} > >This looks good to me, and it's also my preferred way of ACPI-enabling > >drivers. I guess we might discuss this at KS since it was a proposed > >topic there, and others will object. :) > Hopefully someone can summarise the discussion at KS for me, I will > not be there. LWN.net usually has good summaries. I hope they'll be in the room this time too. If not, we'll send out a summary separately. > >>+ > >>+DO keep the MODULE_DEVICE_TABLE entries together in the driver to make it clear > >>+the different names the driver is probed for, both from DT and from ACPI. > >>+ > >>+module device tables :- > >>+ > >>+static struct of_device_id virtio_mmio_match[] = { > >>+ { .compatible = "virtio,mmio", }, > >>+ {}, > >>+}; > >>+MODULE_DEVICE_TABLE(of, virtio_mmio_match); > >>+ > >>+static const struct acpi_device_id virtio_mmio_acpi_match[] = { > >>+ { "LNRO0005", }, > >>+ { } > >No comma here, but a comma on DT. Probably make them equivalent for > >consistency (including space between the braces). > ok > >>+}; > >>+MODULE_DEVICE_TABLE(acpi, virtio_mmio_acpi_match); > >>+ > >>+TODO: Add any other helpful rules that develop from Juno ACPI work. > >Looks like this should be fixed before the patch is merged, or this > >TODO removed. > The plan is to fix these TODOs with actual data from Juno as that > will be in the UEFI for Juno. > >>+ > >>+ASWG > >>+---- > >>+ > >>+The following areas are not yet well defined for ARM in the current ACPI > >>+specification and are expected to be worked through in the UEFI ACPI > >>+Specification Working Group (ASWG) <http://www.uefi.org/workinggroups>. > >>+Participation in this group is open to all UEFI members. > >>+ > >>+ - ACPI based CPU topology > >>+ - ACPI based Power management > >>+ - CPU idle control based on PSCI > >>+ - CPU performance control (CPPC) > >>+ > >>+No code shall be accepted into the kernel unless it complies with the released > >>+standards from UEFI ASWG. If there are features missing from ACPI to make it > >>+function on a platform ECRs should be submitted to ASWG and go through the > >>+approval process. > >Thanks for listing the things that are not in place yet. Please keep > >this doc up to date as new areas are discovered. > > > > > >-Olof > Thanks for the feedback, we shall work to incorporate it into the document. Happy to help. Besides readiness to flip ACPI on by default, I don't think we're in substantial technical disagreements here. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 28, 2014 at 11:07:50AM +0200, Arnd Bergmann wrote: > On Saturday 26 July 2014 19:34:48 Olof Johansson wrote: > > On Thu, Jul 24, 2014 at 6:00 AM, Hanjun Guo <hanjun.guo@linaro.org> wrote: > > > +Relationship with Device Tree > > > +----------------------------- > > > + > > > +ACPI support in drivers and subsystems for ARMv8 should never be mutually > > > +exclusive with DT support at compile time. > > > + > > > +At boot time the kernel will only use one description method depending on > > > +parameters passed from the bootloader. > > > > Possibly overriden by kernel bootargs. And as debated for quite a > > while earlier this year, acpi should still default to off -- if a DT > > and ACPI are both passed in, DT should at this time be given priority. > > I think this would be harder to do with the way that ACPI is passed in > to the kernel. IIRC, you always have a minimal DT information based on > the ARM64 boot protocol, but in the case of ACPI, this contains pointers > to the ACPI tables, which are then used for populating the Linux platform > devices (unless acpi=disabled is set), while the other contents of the > DTB may be present but we skip the of_platform_populate state. How can it be harder to do? If you support acpi=off, then you should support acpi=on. Another alternative would be to have an early fixup that stubs out the acpi properties from the DTB unless there's an 'acpi' or 'acpi=on' argument on the cmdline. Not quite as tidy a solution, though. > If this is correct, then replacing the firmware-generated dtb with a > user-provided on would implicitly remove the ACPI tables from visibility, > which is exactly what we want. I was of the impression that firmware patches in the ACPI entries into either device-tree before launching the kernel. Is that not the case? And what if some bootloader chooses to do it that way in the future? It's better to not assume that they get it right. > It's possible that I'm misremembering it though, and it should be > documented better. Yes, definitely needs to be documented to not leave room for random interpretation later on. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 28, 2014 at 09:23:40AM -0700, Olof Johansson wrote: > On Mon, Jul 28, 2014 at 09:42:57AM +0100, Graeme Gregory wrote: > > >>+On no account should a Device Tree attempt to be replicated in ASL using such > > >>+constructs as Name(KEY0, "Value1") type constructs. Additional driver specific > > >>+data should be passed in the appropriate _DSM (ACPI Section 9.14.1) method or > > >>+_DSD (ACPI Section 6.2.5). This data should be rare and not OS specific. ... > > >I see these two sentences as contradictory, given that the _DSD doc > > >worst case turn into quite a mess. > > >Given that ACPI can present completely different data based on what OS > > >is running, it's quite common to indeed have OS specific data in > > >there. How does that relate to this document and these practices? > > OS specific data has traditionally not worked out well for ACPI, I > > would like to "persuade" people not to use it on ARM. > It hasn't? I think Microsoft disagrees. It's also how vendors have been able to > present an older machine description to keep their newer hardware compatible > with older software, isn't it? How do you expect to handle that if you can > only present one table? It's the same challenge that DT has. It seems sensible to recommend against using OS specifics if possible if only from the point of view of improving the robustness of the system - the less paths there are to test in the BIOS the more likely it is that the active path is one that's been well tested. It's legal in the spec and you can do it but encouraging people not to do it will hopefully make life easier down the line. Similarly encouraging people to put as little as possible in there should reduce the opportunities they have to get things wrong. The best use case for OS testing is to enable a non-default workaround for older versions of the OS but in the case of Linux that's a bit tricky since we don't have clear versions to test against - even with the kernel version number it's never clear if it's been patched by a distro or something. Windows is a much more fixed target here.
On 2014-7-27 10:34, Olof Johansson wrote: > On Thu, Jul 24, 2014 at 6:00 AM, Hanjun Guo <hanjun.guo@linaro.org> wrote: >> From: Graeme Gregory <graeme.gregory@linaro.org> >> >> Add documentation for the guidelines of how to use ACPI >> on ARM64. > > As the most vocal participant against ACPI being adopted, I would have > appreciated a cc on this patch set -- it's not like you were going for > a minimal set of cc recipients already. It makes it seem like you're > trying to sneak it past me for comments. Not cool. I know that's > probably not your intent, but still. My bad, I'm sorry for that. Actually it was not my intention, I was not playing with a full deck when I sent this patch set out, I missed someone else (such as Rob) in the CC list and I even added private mailing list in CC. I'm sure you will in the CC list with next version. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014-7-28 17:07, Arnd Bergmann wrote: > On Saturday 26 July 2014 19:34:48 Olof Johansson wrote: >> On Thu, Jul 24, 2014 at 6:00 AM, Hanjun Guo <hanjun.guo@linaro.org> wrote: >>> +Relationship with Device Tree >>> +----------------------------- >>> + >>> +ACPI support in drivers and subsystems for ARMv8 should never be mutually >>> +exclusive with DT support at compile time. >>> + >>> +At boot time the kernel will only use one description method depending on >>> +parameters passed from the bootloader. >> >> Possibly overriden by kernel bootargs. And as debated for quite a >> while earlier this year, acpi should still default to off -- if a DT >> and ACPI are both passed in, DT should at this time be given priority. > > I think this would be harder to do with the way that ACPI is passed in > to the kernel. IIRC, you always have a minimal DT information based on > the ARM64 boot protocol, but in the case of ACPI, this contains pointers > to the ACPI tables, which are then used for populating the Linux platform > devices (unless acpi=disabled is set), while the other contents of the > DTB may be present but we skip the of_platform_populate state. > > If this is correct, then replacing the firmware-generated dtb with a > user-provided on would implicitly remove the ACPI tables from visibility, > which is exactly what we want. > > It's possible that I'm misremembering it though, and it should be > documented better. > >>> +Regardless of whether DT or ACPI is used, the kernel must always be capable >>> +of booting with either scheme. >> >> It should always be possible to compile out ACPI. There will be plenty >> of platforms that will not implement it, so disabling CONFIG_ACPI >> needs to be possible. > > Right. Actually, if platforms don't implement ACPI, acpi_disabled will always be set to 1 at early boot stage which before the device tree is unflattened, so device tree will work as expected even if CONFIG_ACPI=y on such platforms. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28/07/2014 17:14, Andre Przywara wrote: > > On 28/07/14 16:23, Arnd Bergmann wrote: >> On Monday 28 July 2014 15:20:06 Andre Przywara wrote: >>> On 28/07/14 11:46, Arnd Bergmann wrote: >>>> On Monday 28 July 2014 10:23:57 Graeme Gregory wrote: >>>>> The PL011 UART is the use-case I keep hitting, that IP block has a >>>>> variable input clock on pretty much everything I have seen in the wild. >>>> Ok, I see. What does ACPI-5.1 say about pl011? >>>> >>>> Interestingly, the subset of pl011 that is specified by SBSA does not >>>> contain the IBRD/FBRD registers, effectively making it a fixed-rated >>>> UART (I guess that would be a ART, without the U then), and you >>>> consequently don't even need to know the clock rate. >>> The idea of this was probably to let the baudrate set by some firmware >>> code to the "right" value and the spec just didn't want to expose the >>> details for the generic UART: >>> "This specification does not cover registers needed to configure the >>> UART as these are considered hardware-specific and will be set up by >>> hardware-specific software." >>> To me that reads like the SBSA UART is just for debugging, and you are >>> expected just to access the data register. >> Right, makes sense. It also avoids the case where Linux for some reason >> ends up using a different line rate than the firmware, which can >> cause a lot of unnecessary pain. I must be suffering snow blindness reading specs, I totally missed that the pl011 subset does not allow baud setting. This means that my current test hardware "Juno" does not actually need any clocks defined in DSDT at this stage (given that this new driver is created). I may then return to my original opinion of not defining clocks in the DSDT at all. Graeme >>>> However, my guess is that most hardware in the real world contains >>>> an actual pl011 and it does make a lot of sense to allow setting >>>> the baud rate on it, which then requires knowing the input clock. >>>> >>>> If there is any hardware that implements just the SBSA-mandated subset >>>> rather than the full pl011, we should probably implement both >>>> in the kernel: a dumb driver that can only send and receive, and the >>>> more complex one that can set the bit rates and flow-control but that >>>> requires a standardized ACPI table with the input clock rate. >>> The fast model I use can be switched to use the SBSA restricted PL011, >>> and as expected the Linux kernel crashes at the device doesn't support >>> DMA (and a lot more stuff) - but the current code requires it. >> It does? We have a lot of platforms that don't have DMA support for >> pl011. > Well, to be honest I just booted the full featured kernel with the > changed fast model config, so the platform and the DT claimed DMA > support, just the emulated hardware doesn't implement it ;-) > And beside that a whole lot of other PL011 registers are not available, > so the crash could be caused by anything. I didn't look to closely why > it broke. > >>> So I am about to implement a new driver for that SBSA subset. So far >>> this will be a separate driver, starting from a copy of amba-pl011.c, >>> but removing most of the code ;-) >> Ok. You might want to consider starting from a different base though. >> IIRC, pl011 uses uart_port as the basic abstraction, while the >> new driver should probably use the raw tty_port instead. >> drivers/tty/goldfish.c is probably a good example to look at for >> that. > Good hint, will look at this. > > Cheers, > Andre. > >> You could also make it a hvc_driver like drivers/tty/hvc/hvc_vio.c, >> but I'm not sure if that model seen favorable by the tty maintainers. >> It would probably be the shortest driver though. >> >> Arnd >> >> -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 24, 2014 at 02:19:14PM -0700, Randy Dunlap wrote: > On 07/24/2014 02:16 PM, Naresh Bhat wrote: > > > > On 24 July 2014 18:30, Hanjun Guo <hanjun.guo@linaro.org <mailto:hanjun.guo@linaro.org>> wrote: > > > > From: Graeme Gregory <graeme.gregory@linaro.org <mailto:graeme.gregory@linaro.org>> > > > > Add documentation for the guidelines of how to use ACPI > > on ARM64. > > > > Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org <mailto:graeme.gregory@linaro.org>> > > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org <mailto:hanjun.guo@linaro.org>> > > --- > > Documentation/arm64/arm-acpi.txt | 240 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 240 insertions(+) > > create mode 100644 Documentation/arm64/arm-acpi.txt > > > > diff --git a/Documentation/arm64/arm-acpi.txt b/Documentation/arm64/arm-acpi.txt > > new file mode 100644 > > index 0000000..12cd550 > > --- /dev/null > > +++ b/Documentation/arm64/arm-acpi.txt > > @@ -0,0 +1,240 @@ > > +ACPI on ARMv8 Servers > > +--------------------- > > + > > +ACPI will be used for ARMv8 general purpose servers designed to follow > > +the SBSA specification (currently available to people with an ARM login at > > +http://silver.arm.com) > > + > > +The implemented ACPI version is 5.1 + errata as released by the UEFI Forum, > > +which is available at <http://www.uefi.org/acpi/specs>. > > + > > +If the machine does not meet these requirements then it is likely that Device > > +Tree (DT) is more suitable for the hardware. > > + > > +Relationship with Device Tree > > +----------------------------- > > + > > +ACPI support in drivers and subsystems for ARMv8 should never be mutually > > +exclusive with DT support at compile time. > > + > > +At boot time the kernel will only use one description method depending on > > +parameters passed from the bootloader. > > + > > +Regardless of whether DT or ACPI is used, the kernel must always be capable > > +of booting with either scheme. > > + > > +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. > > + > > +Booting using ACPI tables > > +------------------------- > > + > > +Currently, the only defined method to pass ACPI tables to the kernel on ARMv8 > > +is via the UEFI system configuration table. > > + > > +The UEFI implementation MUST set the ACPI_20_TABLE_GUID to point to the > > +RSDP table (the table with the ACPI signature "RSD PTR "). > > + > > +The pointer to the RSDP table will be retrieved from EFI by the ACPI core. > > + > > +Processing of ACPI tables may be disabled by passing acpi=off on the kernel > > +command line. > > + > > +DO use an XSDT, RSDTs are deprecated and should not be used on arm64. They > > +only allow for 32bit addresses. > > + > > +DO NOT use the 32-bit address fields in the FADT, they are deprecated, the > > +64-bit alternatives MUST be used. > > + > > +The minimum set of tables MUST include RSDP, XSDT, FACS, FADT, DSDT, MADT > > +and GTDT. If PCI is used the MCFG table MUST also be present. > > + > > +ACPI Detection > > +-------------- > > + > > +Drivers should determine their probe() type by checking for ACPI_HANDLE, > > +or .of_node, or other information in the device structure. This is > > +detailed further in the "Driver Recomendations" section. > > + > > +If the presence of ACPI needs to be detected at runtime, then check the value > > +of acpi_disabled. If CONFIG_ACPI not being set acpi_disabled will always be 1. > > + > > +Device Enumeration > > +------------------ > > + > > +Device descriptions in ACPI should use standard recognised ACPI interfaces. > > > > > > recognized > > Yeah, I saw all of these also, but we accept British or American spelling of these words. > Would be good to check for a consistent spelling in this doc though. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 24, 2014 at 09:00:25PM +0800, Hanjun Guo wrote: > From: Graeme Gregory <graeme.gregory@linaro.org> > > Add documentation for the guidelines of how to use ACPI > on ARM64. > > Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org> > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > --- > Documentation/arm64/arm-acpi.txt | 240 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 240 insertions(+) > create mode 100644 Documentation/arm64/arm-acpi.txt > > diff --git a/Documentation/arm64/arm-acpi.txt b/Documentation/arm64/arm-acpi.txt > new file mode 100644 > index 0000000..12cd550 > --- /dev/null > +++ b/Documentation/arm64/arm-acpi.txt > @@ -0,0 +1,240 @@ > +ACPI on ARMv8 Servers > +--------------------- [...] > + > + > +There exists in the ACPI 5.1 specification no standard binding for these objects > +to enable programmable levels or rates so this should be avoid if possible and > +the resources set to appropriate level by the firmware. If this is not possible > +then any manipulation should be abstracted in ASL. > + I'm not a native English speaker, but this wording sounds strange to me. I would suggest "There exists no specification in the the..." or more simply "The ACPI 5.1 specification does not contain any standard...". -Christoffer -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Additional driver specific > +data should be passed in the appropriate _DSM (ACPI Section 9.14.1) method or > +_DSD (ACPI Section 6.2.5). Re: _DSD Yes. Re: _DSM No, not if it can be handled by _DSD. cheers, Len Brown, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 24 Jul 2014 21:00:25 +0800, Hanjun Guo <hanjun.guo@linaro.org> wrote: > +Clocks > +------ > + > +Like clocks that are part of the power resources there is no standard way > +to represent a clock tree in ACPI 5.1 in a similar manner to how it is > +described in DT. > + > +Devices affected by this include things like UARTs, SoC driven LCD displays, > +etc. > + > +The firmware for example UEFI should initialise these clocks to fixed working > +values before the kernel is executed. If a driver requires to know rates of > +clocks set by firmware then they can be passed to kernel using _DSD. > + > +example :- > + > +Device (CLK0) { > + ... > + > + Name (_DSD, Package() { > + ToUUID("XXXXX"), > + Package() { > + Package(2) {"#clock-cells", 0}, > + Package(2) {"clock-frequency", "10000"} > + } > + }) > + > + ... > +} > + > +Device (USR1) { > + ... > + > + Name (_DSD, Package() { > + ToUUID("XXXXX"), > + Package() { > + Package(2) {"clocks", Package() {1, ^CLK0}}}, > + } > + }) > + > + ... > +} Really? This looks wrong. The above example goes right back to conceptually putting the clock tree into ACPI. I would expect the ACPI way to expose current clock rate to an individual device driver is to expose a clock rate method that internally knows how to return the currently set rate. g. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/arm64/arm-acpi.txt b/Documentation/arm64/arm-acpi.txt new file mode 100644 index 0000000..12cd550 --- /dev/null +++ b/Documentation/arm64/arm-acpi.txt @@ -0,0 +1,240 @@ +ACPI on ARMv8 Servers +--------------------- + +ACPI will be used for ARMv8 general purpose servers designed to follow +the SBSA specification (currently available to people with an ARM login at +http://silver.arm.com) + +The implemented ACPI version is 5.1 + errata as released by the UEFI Forum, +which is available at <http://www.uefi.org/acpi/specs>. + +If the machine does not meet these requirements then it is likely that Device +Tree (DT) is more suitable for the hardware. + +Relationship with Device Tree +----------------------------- + +ACPI support in drivers and subsystems for ARMv8 should never be mutually +exclusive with DT support at compile time. + +At boot time the kernel will only use one description method depending on +parameters passed from the bootloader. + +Regardless of whether DT or ACPI is used, the kernel must always be capable +of booting with either scheme. + +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. + +Booting using ACPI tables +------------------------- + +Currently, the only defined method to pass ACPI tables to the kernel on ARMv8 +is via the UEFI system configuration table. + +The UEFI implementation MUST set the ACPI_20_TABLE_GUID to point to the +RSDP table (the table with the ACPI signature "RSD PTR "). + +The pointer to the RSDP table will be retrieved from EFI by the ACPI core. + +Processing of ACPI tables may be disabled by passing acpi=off on the kernel +command line. + +DO use an XSDT, RSDTs are deprecated and should not be used on arm64. They +only allow for 32bit addresses. + +DO NOT use the 32-bit address fields in the FADT, they are deprecated, the +64-bit alternatives MUST be used. + +The minimum set of tables MUST include RSDP, XSDT, FACS, FADT, DSDT, MADT +and GTDT. If PCI is used the MCFG table MUST also be present. + +ACPI Detection +-------------- + +Drivers should determine their probe() type by checking for ACPI_HANDLE, +or .of_node, or other information in the device structure. This is +detailed further in the "Driver Recomendations" section. + +If the presence of ACPI needs to be detected at runtime, then check the value +of acpi_disabled. If CONFIG_ACPI not being set acpi_disabled will always be 1. + +Device Enumeration +------------------ + +Device descriptions in ACPI should use standard recognised ACPI interfaces. +These are far simpler than the information provided via Device Tree. Drivers +should take into account this simplicity and work with sensible defaults. + +On no account should a Device Tree attempt to be replicated in ASL using such +constructs as Name(KEY0, "Value1") type constructs. Additional driver specific +data should be passed in the appropriate _DSM (ACPI Section 9.14.1) method or +_DSD (ACPI Section 6.2.5). This data should be rare and not OS specific. + +Common _DSD bindings should be submitted to ASWG to be included in the +document :- + +http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm + +TODO: Clarification and examples from Juno implementation. + +Programmable Power Control Resources +------------------------------------ + +Programmable power control resources include such resources as voltage/current +providers (regulators) and clock sources. + +For power control of these resources they should be represented with Power +Resource Objects (ACPI Section 7.1). The ACPI core will then handle correctly +enabling/disabling of resources as they are needed. + +There exists in the ACPI 5.1 specification no standard binding for these objects +to enable programmable levels or rates so this should be avoid if possible and +the resources set to appropriate level by the firmware. If this is not possible +then any manipulation should be abstracted in ASL. + +Each device in ACPI has D-states and these can be controlled through +the optional methods _PS0..._PS3 where _PS0 is full on and _PS3 is full off. + +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 organise that it is allocated/enabled using the _PS0 method. + +Resources allocated/enabled in the _PS0 method should be disabled/de-allocated +in the _PS3 method. + +Such code in _PS? methods will of course be very platform specific but +should allow the driver to operate the device without special non standard +values being read from ASL. Further, abstracting the use of these resources +allows hardware revisions without requiring updates to the kernel. + +TODO: Clarification and examples from Juno implementation. + +Clocks +------ + +Like clocks that are part of the power resources there is no standard way +to represent a clock tree in ACPI 5.1 in a similar manner to how it is +described in DT. + +Devices affected by this include things like UARTs, SoC driven LCD displays, +etc. + +The firmware for example UEFI should initialise these clocks to fixed working +values before the kernel is executed. If a driver requires to know rates of +clocks set by firmware then they can be passed to kernel using _DSD. + +example :- + +Device (CLK0) { + ... + + Name (_DSD, Package() { + ToUUID("XXXXX"), + Package() { + Package(2) {"#clock-cells", 0}, + Package(2) {"clock-frequency", "10000"} + } + }) + + ... +} + +Device (USR1) { + ... + + Name (_DSD, Package() { + ToUUID("XXXXX"), + Package() { + Package(2) {"clocks", Package() {1, ^CLK0}}}, + } + }) + + ... +} + +Driver Recommendations +---------------------- + +DO NOT remove any FDT handling when adding ACPI support for a driver, different +systems may use the same device. + +DO try and keep complex sections of ACPI and DT functionality seperate. This +may mean a patch to break out some complex DT to another function before +the patch to add ACPI. This may happen in other functions but is most likely +in probe function. This gives a clearer flow of data for reviewing driver +source. + +probe() :- + +TODO: replace this with a specific real example from Juno? + +static int device_probe_dt(struct platform_device *pdev) +{ + /* DT specific functionality */ + ... +} + +static int device_probe_acpi(struct platform_device *pdev) +{ + /* ACPI specific functionality */ + ... +} + +static int device_probe(stuct platform_device *pdev) +{ + ... + acpi_handle handle = ACPI_HANDLE(&pdev->dev); + struct device_node node = pdev->dev.of_node; + ... + + if (node) + ret = device_probe_dt(pdev); + else if (handle) + ret = device_probe_acpi(pdev); + else + /* other initialisation */ + ... + /* Continue with any generic probe operations */ + ... +} + +DO keep the MODULE_DEVICE_TABLE entries together in the driver to make it clear +the different names the driver is probed for, both from DT and from ACPI. + +module device tables :- + +static struct of_device_id virtio_mmio_match[] = { + { .compatible = "virtio,mmio", }, + {}, +}; +MODULE_DEVICE_TABLE(of, virtio_mmio_match); + +static const struct acpi_device_id virtio_mmio_acpi_match[] = { + { "LNRO0005", }, + { } +}; +MODULE_DEVICE_TABLE(acpi, virtio_mmio_acpi_match); + +TODO: Add any other helpful rules that develop from Juno ACPI work. + +ASWG +---- + +The following areas are not yet well defined for ARM in the current ACPI +specification and are expected to be worked through in the UEFI ACPI +Specification Working Group (ASWG) <http://www.uefi.org/workinggroups>. +Participation in this group is open to all UEFI members. + + - ACPI based CPU topology + - ACPI based Power management + - CPU idle control based on PSCI + - CPU performance control (CPPC) + +No code shall be accepted into the kernel unless it complies with the released +standards from UEFI ASWG. If there are features missing from ACPI to make it +function on a platform ECRs should be submitted to ASWG and go through the +approval process.