As described in Documentation/firmware-guide/acpi/gpio-properties.rst a
GPIO can be referenced from ACPI ASL _DSD with the "gpios"-property of the
form:
Package () { "gpios", Package () { ref, index, pin, active_low }}
The second patch of this series adds support for specifing a PWM
reference in ASL of the form
Package () { "pwms", Package () { ref, index, pwm-period [, pwm flags]}}
The first patch of this series is necessary to resolve the "ref" in ASL
if the table has been loaded by efivar_ssdt_load() or configfs.
The third patch of this series makes leds-pwm use the ACPI-enabled
PWM framework.
v2:
- fixes by Pavel Machek and Dan Murphy
Nikolaus Voss (3):
ACPI: Resolve objects on host-directed table loads
PWM framework: add support referencing PWMs from ACPI
leds-pwm.c: support ACPI via firmware-node framework
drivers/acpi/acpi_configfs.c | 6 +-
drivers/acpi/acpica/tbxfload.c | 11 ++++
drivers/leds/leds-pwm.c | 45 +++++++------
drivers/pwm/core.c | 113 +++++++++++++++++++++++++++++++++
include/linux/pwm.h | 9 +++
5 files changed, 161 insertions(+), 23 deletions(-)
--
2.17.1
In analogy to referencing a GPIO using the "gpios" property from ACPI,
support referencing a PWM using the "pwms" property.
ACPI entries must look like
Package () {"pwms", Package ()
{ <PWM device reference>, <PWM index>, <PWM period> [, <PWM flags>]}}
In contrast to the DT implementation, only _one_ PWM entry in the "pwms"
property is supported. As a consequence "pwm-names"-property and
con_id lookup aren't supported.
Support for ACPI is added via the firmware-node framework which is an
abstraction layer on top of ACPI/DT. To keep this patch clean, DT and
ACPI paths are kept separate. The firmware-node framework could be used
to unify both paths in a future patch.
To support leds-pwm driver, an additional method devm_fwnode_pwm_get()
which supports both ACPI and DT configuration is exported.
Signed-off-by: Nikolaus Voss <[email protected]>
---
drivers/pwm/core.c | 113 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/pwm.h | 9 ++++
2 files changed, 122 insertions(+)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 275b5f399a1a..5fed419d0833 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -6,6 +6,7 @@
* Copyright (C) 2011-2012 Avionic Design GmbH
*/
+#include <linux/acpi.h>
#include <linux/module.h>
#include <linux/pwm.h>
#include <linux/radix-tree.h>
@@ -700,6 +701,76 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id)
}
EXPORT_SYMBOL_GPL(of_pwm_get);
+static struct pwm_chip *device_to_pwmchip(struct device *dev)
+{
+ struct pwm_chip *chip;
+
+ mutex_lock(&pwm_lock);
+
+ list_for_each_entry(chip, &pwm_chips, list) {
+ struct acpi_device *adev = ACPI_COMPANION(chip->dev);
+
+ if ((chip->dev == dev) || (adev && &adev->dev == dev)) {
+ mutex_unlock(&pwm_lock);
+ return chip;
+ }
+ }
+
+ mutex_unlock(&pwm_lock);
+
+ return ERR_PTR(-EPROBE_DEFER);
+}
+
+/**
+ * acpi_pwm_get() - request a PWM via parsing "pwms" property in ACPI
+ * @fwnode: firmware node to get the "pwm" property from
+ *
+ * Returns the PWM device parsed from the fwnode and index specified in the
+ * "pwms" property or a negative error-code on failure.
+ * Values parsed from the device tree are stored in the returned PWM device
+ * object.
+ *
+ * This is analogous to of_pwm_get() except con_id is not yet supported.
+ * ACPI entries must look like
+ * Package () {"pwms", Package ()
+ * { <PWM device reference>, <PWM index>, <PWM period> [, <PWM flags>]}}
+ *
+ * Returns: A pointer to the requested PWM device or an ERR_PTR()-encoded
+ * error code on failure.
+ */
+static struct pwm_device *acpi_pwm_get(struct fwnode_handle *fwnode)
+{
+ struct fwnode_reference_args args;
+ struct pwm_chip *chip;
+ struct pwm_device *pwm = ERR_PTR(-ENODEV);
+ int ret;
+
+ memset(&args, 0, sizeof(args));
+ ret = __acpi_node_get_property_reference(fwnode, "pwms", 0, 3, &args);
+
+ if (!to_acpi_device_node(args.fwnode))
+ return ERR_PTR(-EINVAL);
+
+ if (args.nargs < 2)
+ return ERR_PTR(-EPROTO);
+
+ chip = device_to_pwmchip(&to_acpi_device_node(args.fwnode)->dev);
+ if (IS_ERR(chip))
+ return ERR_CAST(chip);
+
+ pwm = pwm_request_from_chip(chip, args.args[0], NULL);
+ if (IS_ERR(pwm))
+ return pwm;
+
+ pwm->args.period = args.args[1];
+ pwm->args.polarity = PWM_POLARITY_NORMAL;
+
+ if (args.nargs > 2 && args.args[2] & PWM_POLARITY_INVERTED)
+ pwm->args.polarity = PWM_POLARITY_INVERSED;
+
+ return pwm;
+}
+
/**
* pwm_add_table() - register PWM device consumers
* @table: array of consumers to register
@@ -763,6 +834,10 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
return of_pwm_get(dev->of_node, con_id);
+ /* then lookup via ACPI */
+ if (dev && is_acpi_node(dev->fwnode))
+ return acpi_pwm_get(dev->fwnode);
+
/*
* We look up the provider in the static table typically provided by
* board setup code. We first try to lookup the consumer device by
@@ -942,6 +1017,44 @@ struct pwm_device *devm_of_pwm_get(struct device *dev, struct device_node *np,
}
EXPORT_SYMBOL_GPL(devm_of_pwm_get);
+/**
+ * devm_fwnode_pwm_get() - request a resource managed PWM from firmware node
+ * @dev: device for PWM consumer
+ * @fwnode: firmware node to get the PWM from
+ * @con_id: consumer name
+ *
+ * Returns the PWM device parsed from the firmware node. See of_pwm_get() and
+ * acpi_pwm_get() for a detailed description.
+ *
+ * Returns: A pointer to the requested PWM device or an ERR_PTR()-encoded
+ * error code on failure.
+ */
+struct pwm_device *devm_fwnode_pwm_get(struct device *dev,
+ struct fwnode_handle *fwnode,
+ const char *con_id)
+{
+ struct pwm_device **ptr, *pwm = ERR_PTR(-ENODEV);
+
+ ptr = devres_alloc(devm_pwm_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ if (is_of_node(fwnode))
+ pwm = of_pwm_get(to_of_node(fwnode), con_id);
+ else if (is_acpi_node(fwnode))
+ pwm = acpi_pwm_get(fwnode);
+
+ if (!IS_ERR(pwm)) {
+ *ptr = pwm;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return pwm;
+}
+EXPORT_SYMBOL_GPL(devm_fwnode_pwm_get);
+
static int devm_pwm_match(struct device *dev, void *res, void *data)
{
struct pwm_device **p = res;
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index eaa5c6e3fc9f..1a635916cdfb 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -411,6 +411,9 @@ void pwm_put(struct pwm_device *pwm);
struct pwm_device *devm_pwm_get(struct device *dev, const char *con_id);
struct pwm_device *devm_of_pwm_get(struct device *dev, struct device_node *np,
const char *con_id);
+struct pwm_device *devm_fwnode_pwm_get(struct device *dev,
+ struct fwnode_handle *fwnode,
+ const char *con_id);
void devm_pwm_put(struct device *dev, struct pwm_device *pwm);
#else
static inline struct pwm_device *pwm_request(int pwm_id, const char *label)
@@ -516,6 +519,12 @@ static inline struct pwm_device *devm_of_pwm_get(struct device *dev,
return ERR_PTR(-ENODEV);
}
+static inline struct pwm_device *devm_fwnode_pwm_get(
+ struct device *dev, struct fwnode_handle *fwnode, const char *con_id)
+{
+ return ERR_PTR(-ENODEV);
+}
+
static inline void devm_pwm_put(struct device *dev, struct pwm_device *pwm)
{
}
--
2.17.1
DT specific handling is replaced by firmware-node abstration to support
ACPI specification of PWM LEDS.
Example ASL:
Device (PWML)
{
Name (_HID, "PRP0001")
Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () { Package () {"compatible",
Package () {"pwm-leds"}}}})
Device (PWL0)
{
Name (_HID, "PRP0001")
Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
Package () {"label", "alarm-led"},
Package () {"pwms", Package ()
{\_SB_.PCI0.PWM, 0, 600000, 0}},
Package () {"linux,default-state", "off"}}})
}
}
Signed-off-by: Nikolaus Voss <[email protected]>
---
drivers/leds/leds-pwm.c | 45 ++++++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index af08bcdc4fd8..3ce4d53c0cc9 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -75,7 +75,7 @@ static inline size_t sizeof_pwm_leds_priv(int num_leds)
}
static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
- struct led_pwm *led, struct device_node *child)
+ struct led_pwm *led, struct fwnode_handle *fwnode)
{
struct led_pwm_data *led_data = &priv->leds[priv->num_leds];
struct pwm_args pargs;
@@ -88,8 +88,8 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
led_data->cdev.max_brightness = led->max_brightness;
led_data->cdev.flags = LED_CORE_SUSPENDRESUME;
- if (child)
- led_data->pwm = devm_of_pwm_get(dev, child, NULL);
+ if (fwnode)
+ led_data->pwm = devm_fwnode_pwm_get(dev, fwnode, NULL);
else
led_data->pwm = devm_pwm_get(dev, led->name);
if (IS_ERR(led_data->pwm)) {
@@ -114,7 +114,8 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
if (!led_data->period && (led->pwm_period_ns > 0))
led_data->period = led->pwm_period_ns;
- ret = devm_of_led_classdev_register(dev, child, &led_data->cdev);
+ ret = devm_of_led_classdev_register(dev, to_of_node(fwnode),
+ &led_data->cdev);
if (ret == 0) {
priv->num_leds++;
led_pwm_set(&led_data->cdev, led_data->cdev.brightness);
@@ -126,27 +127,35 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
return ret;
}
-static int led_pwm_create_of(struct device *dev, struct led_pwm_priv *priv)
+static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
{
- struct device_node *child;
+ struct fwnode_handle *fwnode;
struct led_pwm led;
int ret = 0;
memset(&led, 0, sizeof(led));
- for_each_child_of_node(dev->of_node, child) {
- led.name = of_get_property(child, "label", NULL) ? :
- child->name;
+ device_for_each_child_node(dev, fwnode) {
+ ret = fwnode_property_read_string(fwnode, "label", &led.name);
+ if (ret && is_of_node(fwnode))
+ led.name = to_of_node(fwnode)->name;
- led.default_trigger = of_get_property(child,
- "linux,default-trigger", NULL);
- led.active_low = of_property_read_bool(child, "active-low");
- of_property_read_u32(child, "max-brightness",
- &led.max_brightness);
+ if (!led.name) {
+ fwnode_handle_put(fwnode);
+ return -EINVAL;
+ }
- ret = led_pwm_add(dev, priv, &led, child);
+ fwnode_property_read_string(fwnode, "linux,default-trigger",
+ &led.default_trigger);
+
+ led.active_low = fwnode_property_read_bool(fwnode,
+ "active-low");
+ fwnode_property_read_u32(fwnode, "max-brightness",
+ &led.max_brightness);
+
+ ret = led_pwm_add(dev, priv, &led, fwnode);
if (ret) {
- of_node_put(child);
+ fwnode_handle_put(fwnode);
break;
}
}
@@ -164,7 +173,7 @@ static int led_pwm_probe(struct platform_device *pdev)
if (pdata)
count = pdata->num_leds;
else
- count = of_get_child_count(pdev->dev.of_node);
+ count = device_get_child_node_count(&pdev->dev);
if (!count)
return -EINVAL;
@@ -182,7 +191,7 @@ static int led_pwm_probe(struct platform_device *pdev)
break;
}
} else {
- ret = led_pwm_create_of(&pdev->dev, priv);
+ ret = led_pwm_create_fwnode(&pdev->dev, priv);
}
if (ret)
--
2.17.1
If an ACPI SSDT overlay is loaded after built-in tables
have been loaded e.g. via configfs or efivar_ssdt_load()
it is necessary to rewalk the namespace to resolve
references. Without this, relative and absolute paths
like ^PCI0.SBUS or \_SB.PCI0.SBUS are not resolved
correctly.
Make configfs load use the same method as efivar_ssdt_load().
Signed-off-by: Nikolaus Voss <[email protected]>
---
drivers/acpi/acpi_configfs.c | 6 +-----
drivers/acpi/acpica/tbxfload.c | 11 +++++++++++
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/acpi_configfs.c b/drivers/acpi/acpi_configfs.c
index f92033661239..663f0d88f912 100644
--- a/drivers/acpi/acpi_configfs.c
+++ b/drivers/acpi/acpi_configfs.c
@@ -56,11 +56,7 @@ static ssize_t acpi_table_aml_write(struct config_item *cfg,
if (!table->header)
return -ENOMEM;
- ACPI_INFO(("Host-directed Dynamic ACPI Table Load:"));
- ret = acpi_tb_install_and_load_table(
- ACPI_PTR_TO_PHYSADDR(table->header),
- ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL, FALSE,
- &table->index);
+ ret = acpi_load_table(table->header);
if (ret) {
kfree(table->header);
table->header = NULL;
diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
index 4f30f06a6f78..ef8f8a9f3c9c 100644
--- a/drivers/acpi/acpica/tbxfload.c
+++ b/drivers/acpi/acpica/tbxfload.c
@@ -297,6 +297,17 @@ acpi_status acpi_load_table(struct acpi_table_header *table)
status = acpi_tb_install_and_load_table(ACPI_PTR_TO_PHYSADDR(table),
ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL,
FALSE, &table_index);
+
+ if (ACPI_SUCCESS(status)) {
+ /* Complete the initialization/resolution of package objects */
+
+ status = acpi_ns_walk_namespace(ACPI_TYPE_PACKAGE,
+ ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, 0,
+ acpi_ns_init_one_package,
+ NULL, NULL, NULL);
+ }
+
return_ACPI_STATUS(status);
}
--
2.17.1
On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
<[email protected]> wrote:
>
> If an ACPI SSDT overlay is loaded after built-in tables
> have been loaded e.g. via configfs or efivar_ssdt_load()
> it is necessary to rewalk the namespace to resolve
> references. Without this, relative and absolute paths
> like ^PCI0.SBUS or \_SB.PCI0.SBUS are not resolved
> correctly.
>
> Make configfs load use the same method as efivar_ssdt_load().
>
> Signed-off-by: Nikolaus Voss <[email protected]>
This is fine by me, so
Acked-by: Rafael J. Wysocki <[email protected]>
Or if you want me to take this patch (without the other two in the
series), please let me know.
As for the other two patches, someone else needs to review them for
you as I'm not particularly familiar with the PWM subsystem.
> ---
> drivers/acpi/acpi_configfs.c | 6 +-----
> drivers/acpi/acpica/tbxfload.c | 11 +++++++++++
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/acpi_configfs.c b/drivers/acpi/acpi_configfs.c
> index f92033661239..663f0d88f912 100644
> --- a/drivers/acpi/acpi_configfs.c
> +++ b/drivers/acpi/acpi_configfs.c
> @@ -56,11 +56,7 @@ static ssize_t acpi_table_aml_write(struct config_item *cfg,
> if (!table->header)
> return -ENOMEM;
>
> - ACPI_INFO(("Host-directed Dynamic ACPI Table Load:"));
> - ret = acpi_tb_install_and_load_table(
> - ACPI_PTR_TO_PHYSADDR(table->header),
> - ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL, FALSE,
> - &table->index);
> + ret = acpi_load_table(table->header);
> if (ret) {
> kfree(table->header);
> table->header = NULL;
> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> index 4f30f06a6f78..ef8f8a9f3c9c 100644
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -297,6 +297,17 @@ acpi_status acpi_load_table(struct acpi_table_header *table)
> status = acpi_tb_install_and_load_table(ACPI_PTR_TO_PHYSADDR(table),
> ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL,
> FALSE, &table_index);
> +
> + if (ACPI_SUCCESS(status)) {
> + /* Complete the initialization/resolution of package objects */
> +
> + status = acpi_ns_walk_namespace(ACPI_TYPE_PACKAGE,
> + ACPI_ROOT_OBJECT,
> + ACPI_UINT32_MAX, 0,
> + acpi_ns_init_one_package,
> + NULL, NULL, NULL);
> + }
> +
> return_ACPI_STATUS(status);
> }
>
> --
> 2.17.1
>
Hi Rafael,
On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
> <[email protected]> wrote:
>>
>> If an ACPI SSDT overlay is loaded after built-in tables
>> have been loaded e.g. via configfs or efivar_ssdt_load()
>> it is necessary to rewalk the namespace to resolve
>> references. Without this, relative and absolute paths
>> like ^PCI0.SBUS or \_SB.PCI0.SBUS are not resolved
>> correctly.
>>
>> Make configfs load use the same method as efivar_ssdt_load().
>>
>> Signed-off-by: Nikolaus Voss <[email protected]>
>
> This is fine by me, so
>
> Acked-by: Rafael J. Wysocki <[email protected]>
>
> Or if you want me to take this patch (without the other two in the
> series), please let me know.
thanks. I think it would be the best if you take up this patch as it is an
independent topic. In retrospect it wasn't a good idea to put it into this
series.
Kind regards,
Niko
[...]
-----Original Message-----
From: Nikolaus Voss [mailto:[email protected]]
Sent: Friday, June 14, 2019 2:26 AM
To: Rafael J. Wysocki <[email protected]>
Cc: Rafael J. Wysocki <[email protected]>; Len Brown <[email protected]>; Moore, Robert <[email protected]>; Schmauss, Erik <[email protected]>; Jacek Anaszewski <[email protected]>; Pavel Machek <[email protected]>; Dan Murphy <[email protected]>; Thierry Reding <[email protected]>; ACPI Devel Maling List <[email protected]>; open list:ACPI COMPONENT ARCHITECTURE (ACPICA) <[email protected]>; [email protected]; Linux PWM List <[email protected]>; Linux Kernel Mailing List <[email protected]>
Subject: Re: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table loads
Hi Rafael,
On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
> <[email protected]> wrote:
>>
>> If an ACPI SSDT overlay is loaded after built-in tables have been
>> loaded e.g. via configfs or efivar_ssdt_load() it is necessary to
>> rewalk the namespace to resolve references. Without this, relative
>> and absolute paths like ^PCI0.SBUS or \_SB.PCI0.SBUS are not resolved
>> correctly.
>>
>> Make configfs load use the same method as efivar_ssdt_load().
>>
>> Signed-off-by: Nikolaus Voss <[email protected]>
>
> This is fine by me, so
>
> Acked-by: Rafael J. Wysocki <[email protected]>
>
> Or if you want me to take this patch (without the other two in the
> series), please let me know.
thanks. I think it would be the best if you take up this patch as it is an independent topic. In retrospect it wasn't a good idea to put it into this series.
Kind regards,
Niko
I would have to ask, why is additional code needed for package initialization/resolution? It already happens elsewhere in acpica.
Bob
[...]
Bob,
On Fri, 14 Jun 2019, Moore, Robert wrote:
>
>
> -----Original Message-----
> From: Nikolaus Voss [mailto:[email protected]]
> Sent: Friday, June 14, 2019 2:26 AM
> To: Rafael J. Wysocki <[email protected]>
> Cc: Rafael J. Wysocki <[email protected]>; Len Brown <[email protected]>; Moore, Robert <[email protected]>; Schmauss, Erik <[email protected]>; Jacek Anaszewski <[email protected]>; Pavel Machek <[email protected]>; Dan Murphy <[email protected]>; Thierry Reding <[email protected]>; ACPI Devel Maling List <[email protected]>; open list:ACPI COMPONENT ARCHITECTURE (ACPICA) <[email protected]>; [email protected]; Linux PWM List <[email protected]>; Linux Kernel Mailing List <[email protected]>
> Subject: Re: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table loads
>
> Hi Rafael,
>
> On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
>> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
>> <[email protected]> wrote:
>>>
>>> If an ACPI SSDT overlay is loaded after built-in tables have been
>>> loaded e.g. via configfs or efivar_ssdt_load() it is necessary to
>>> rewalk the namespace to resolve references. Without this, relative
>>> and absolute paths like ^PCI0.SBUS or \_SB.PCI0.SBUS are not resolved
>>> correctly.
>>>
>>> Make configfs load use the same method as efivar_ssdt_load().
>>>
>>> Signed-off-by: Nikolaus Voss <[email protected]>
>>
>> This is fine by me, so
>>
>> Acked-by: Rafael J. Wysocki <[email protected]>
>>
>> Or if you want me to take this patch (without the other two in the
>> series), please let me know.
>
> thanks. I think it would be the best if you take up this patch as it is
> an independent topic. In retrospect it wasn't a good idea to put it into
> this series.
>
> Kind regards,
> Niko
>
> I would have to ask, why is additional code needed for package
> initialization/resolution? It already happens elsewhere in acpica. Bob
for built-in tables loaded via acpi_ex_load_table_op() everything is fine,
because after acpi_tb_load_table() acpi_ns_walk_namespace() is called to
resolve references.
My fix only affects tables loaded dynamically via either
acpi_configfs driver (for debugging purposes) or efivar_ssdt_load() (to
specify a table on the kernel's command line). They use acpi_load_table()
to load the table from a caller-owned buffer. To resolve the references,
it is again necessary to rewalk the namespace, which was simply missing in
acpi_load_table().
Niko
> -----Original Message-----
> From: Nikolaus Voss [mailto:[email protected]]
> Sent: Sunday, June 16, 2019 11:24 PM
> To: Moore, Robert <[email protected]>
> Cc: Rafael J. Wysocki <[email protected]>; Rafael J. Wysocki
> <[email protected]>; Len Brown <[email protected]>; Schmauss, Erik
> <[email protected]>; Jacek Anaszewski
> <[email protected]>; Pavel Machek <[email protected]>; Dan Murphy
> <[email protected]>; Thierry Reding <[email protected]>; ACPI Devel
> Maling List <[email protected]>; open list:ACPI COMPONENT
> ARCHITECTURE (ACPICA) <[email protected]>; [email protected];
> Linux PWM List <[email protected]>; Linux Kernel Mailing List
> <[email protected]>; [email protected]
> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> loads
>
> Bob,
>
> On Fri, 14 Jun 2019, Moore, Robert wrote:
> >
> >
> > -----Original Message-----
> > From: Nikolaus Voss [mailto:[email protected]]
> > Sent: Friday, June 14, 2019 2:26 AM
> > To: Rafael J. Wysocki <[email protected]>
> > Cc: Rafael J. Wysocki <[email protected]>; Len Brown
> > <[email protected]>; Moore, Robert <[email protected]>; Schmauss,
> > Erik <[email protected]>; Jacek Anaszewski
> > <[email protected]>; Pavel Machek <[email protected]>; Dan Murphy
> > <[email protected]>; Thierry Reding <[email protected]>; ACPI
> > Devel Maling List <[email protected]>; open list:ACPI
> > COMPONENT ARCHITECTURE (ACPICA) <[email protected]>;
> > [email protected]; Linux PWM List
> > <[email protected]>; Linux Kernel Mailing List
> > <[email protected]>
> > Subject: Re: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> > table loads
> >
> > Hi Rafael,
> >
> > On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
> >> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
> >> <[email protected]> wrote:
> >>>
> >>> If an ACPI SSDT overlay is loaded after built-in tables have been
> >>> loaded e.g. via configfs or efivar_ssdt_load() it is necessary to
> >>> rewalk the namespace to resolve references. Without this, relative
> >>> and absolute paths like ^PCI0.SBUS or \_SB.PCI0.SBUS are not
> >>> resolved correctly.
> >>>
> >>> Make configfs load use the same method as efivar_ssdt_load().
> >>>
> >>> Signed-off-by: Nikolaus Voss <[email protected]>
> >>
> >> This is fine by me, so
> >>
> >> Acked-by: Rafael J. Wysocki <[email protected]>
> >>
> >> Or if you want me to take this patch (without the other two in the
> >> series), please let me know.
> >
> > thanks. I think it would be the best if you take up this patch as it
> > is an independent topic. In retrospect it wasn't a good idea to put it
> > into this series.
> >
> > Kind regards,
> > Niko
> >
> > I would have to ask, why is additional code needed for package
> > initialization/resolution? It already happens elsewhere in acpica. Bob
>
> for built-in tables loaded via acpi_ex_load_table_op() everything is
> fine, because after acpi_tb_load_table() acpi_ns_walk_namespace() is
> called to resolve references.
>
> My fix only affects tables loaded dynamically via either acpi_configfs
> driver (for debugging purposes) or efivar_ssdt_load() (to specify a
> table on the kernel's command line). They use acpi_load_table() to load
> the table from a caller-owned buffer. To resolve the references, it is
> again necessary to rewalk the namespace, which was simply missing in
> acpi_load_table().
>
[Moore, Robert]
Perhaps you should call AcpiInitializeObjects after the call to AcpiLoadTable, but I will check.
> Niko
On Mon, 17 Jun 2019, Moore, Robert wrote:
>> -----Original Message-----
>> From: Nikolaus Voss [mailto:[email protected]]
>> Sent: Sunday, June 16, 2019 11:24 PM
>> To: Moore, Robert <[email protected]>
>> Cc: Rafael J. Wysocki <[email protected]>; Rafael J. Wysocki
>> <[email protected]>; Len Brown <[email protected]>; Schmauss, Erik
>> <[email protected]>; Jacek Anaszewski
>> <[email protected]>; Pavel Machek <[email protected]>; Dan Murphy
>> <[email protected]>; Thierry Reding <[email protected]>; ACPI Devel
>> Maling List <[email protected]>; open list:ACPI COMPONENT
>> ARCHITECTURE (ACPICA) <[email protected]>; [email protected];
>> Linux PWM List <[email protected]>; Linux Kernel Mailing List
>> <[email protected]>; [email protected]
>> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
>> loads
>>
>> Bob,
>>
>> On Fri, 14 Jun 2019, Moore, Robert wrote:
>>>
>>>
>>> -----Original Message-----
>>> From: Nikolaus Voss [mailto:[email protected]]
>>> Sent: Friday, June 14, 2019 2:26 AM
>>> To: Rafael J. Wysocki <[email protected]>
>>> Cc: Rafael J. Wysocki <[email protected]>; Len Brown
>>> <[email protected]>; Moore, Robert <[email protected]>; Schmauss,
>>> Erik <[email protected]>; Jacek Anaszewski
>>> <[email protected]>; Pavel Machek <[email protected]>; Dan Murphy
>>> <[email protected]>; Thierry Reding <[email protected]>; ACPI
>>> Devel Maling List <[email protected]>; open list:ACPI
>>> COMPONENT ARCHITECTURE (ACPICA) <[email protected]>;
>>> [email protected]; Linux PWM List
>>> <[email protected]>; Linux Kernel Mailing List
>>> <[email protected]>
>>> Subject: Re: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
>>> table loads
>>>
>>> Hi Rafael,
>>>
>>> On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
>>>> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
>>>> <[email protected]> wrote:
>>>>>
>>>>> If an ACPI SSDT overlay is loaded after built-in tables have been
>>>>> loaded e.g. via configfs or efivar_ssdt_load() it is necessary to
>>>>> rewalk the namespace to resolve references. Without this, relative
>>>>> and absolute paths like ^PCI0.SBUS or \_SB.PCI0.SBUS are not
>>>>> resolved correctly.
>>>>>
>>>>> Make configfs load use the same method as efivar_ssdt_load().
>>>>>
>>>>> Signed-off-by: Nikolaus Voss <[email protected]>
>>>>
>>>> This is fine by me, so
>>>>
>>>> Acked-by: Rafael J. Wysocki <[email protected]>
>>>>
>>>> Or if you want me to take this patch (without the other two in the
>>>> series), please let me know.
>>>
>>> thanks. I think it would be the best if you take up this patch as it
>>> is an independent topic. In retrospect it wasn't a good idea to put it
>>> into this series.
>>>
>>> Kind regards,
>>> Niko
>>>
>>> I would have to ask, why is additional code needed for package
>>> initialization/resolution? It already happens elsewhere in acpica. Bob
>>
>> for built-in tables loaded via acpi_ex_load_table_op() everything is
>> fine, because after acpi_tb_load_table() acpi_ns_walk_namespace() is
>> called to resolve references.
>>
>> My fix only affects tables loaded dynamically via either acpi_configfs
>> driver (for debugging purposes) or efivar_ssdt_load() (to specify a
>> table on the kernel's command line). They use acpi_load_table() to load
>> the table from a caller-owned buffer. To resolve the references, it is
>> again necessary to rewalk the namespace, which was simply missing in
>> acpi_load_table().
>>
> [Moore, Robert]
>
> Perhaps you should call AcpiInitializeObjects after the call to
> AcpiLoadTable, but I will check.
My usage of acpi_load_table() is to load a SSDT which is the intended use
of this method according to its description. And my expectation is that
the package objects of the loaded table are initialized when this function
successfully returns so it aligns with the behavior of
acpi_ex_load_table_op() for built-in SSDTs. Otherwise there would be no
point in having this function at all, because
acpi_tb_install_and_load_table() could be called directly without any
difference.
Niko
I'm looking at this. We've changed the initialization of objects in the namespace recently, so I'm checking this out.
> -----Original Message-----
> From: Nikolaus Voss [mailto:[email protected]]
> Sent: Tuesday, June 18, 2019 2:22 AM
> To: Moore, Robert <[email protected]>
> Cc: Rafael J. Wysocki <[email protected]>; Rafael J. Wysocki
> <[email protected]>; Len Brown <[email protected]>; Schmauss, Erik
> <[email protected]>; Jacek Anaszewski
> <[email protected]>; Pavel Machek <[email protected]>; Dan Murphy
> <[email protected]>; Thierry Reding <[email protected]>; ACPI Devel
> Maling List <[email protected]>; open list:ACPI COMPONENT
> ARCHITECTURE (ACPICA) <[email protected]>; [email protected];
> Linux PWM List <[email protected]>; Linux Kernel Mailing List
> <[email protected]>
> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> loads
>
> On Mon, 17 Jun 2019, Moore, Robert wrote:
> >> -----Original Message-----
> >> From: Nikolaus Voss [mailto:[email protected]]
> >> Sent: Sunday, June 16, 2019 11:24 PM
> >> To: Moore, Robert <[email protected]>
> >> Cc: Rafael J. Wysocki <[email protected]>; Rafael J. Wysocki
> >> <[email protected]>; Len Brown <[email protected]>; Schmauss, Erik
> >> <[email protected]>; Jacek Anaszewski
> >> <[email protected]>; Pavel Machek <[email protected]>; Dan Murphy
> >> <[email protected]>; Thierry Reding <[email protected]>; ACPI
> >> Devel Maling List <[email protected]>; open list:ACPI
> >> COMPONENT ARCHITECTURE (ACPICA) <[email protected]>;
> >> [email protected]; Linux PWM List
> >> <[email protected]>; Linux Kernel Mailing List
> >> <[email protected]>; [email protected]
> >> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> >> table loads
> >>
> >> Bob,
> >>
> >> On Fri, 14 Jun 2019, Moore, Robert wrote:
> >>>
> >>>
> >>> -----Original Message-----
> >>> From: Nikolaus Voss [mailto:[email protected]]
> >>> Sent: Friday, June 14, 2019 2:26 AM
> >>> To: Rafael J. Wysocki <[email protected]>
> >>> Cc: Rafael J. Wysocki <[email protected]>; Len Brown
> >>> <[email protected]>; Moore, Robert <[email protected]>; Schmauss,
> >>> Erik <[email protected]>; Jacek Anaszewski
> >>> <[email protected]>; Pavel Machek <[email protected]>; Dan
> >>> Murphy <[email protected]>; Thierry Reding <[email protected]>;
> >>> ACPI Devel Maling List <[email protected]>; open list:ACPI
> >>> COMPONENT ARCHITECTURE (ACPICA) <[email protected]>;
> >>> [email protected]; Linux PWM List
> >>> <[email protected]>; Linux Kernel Mailing List
> >>> <[email protected]>
> >>> Subject: Re: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> >>> table loads
> >>>
> >>> Hi Rafael,
> >>>
> >>> On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
> >>>> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
> >>>> <[email protected]> wrote:
> >>>>>
> >>>>> If an ACPI SSDT overlay is loaded after built-in tables have been
> >>>>> loaded e.g. via configfs or efivar_ssdt_load() it is necessary to
> >>>>> rewalk the namespace to resolve references. Without this, relative
> >>>>> and absolute paths like ^PCI0.SBUS or \_SB.PCI0.SBUS are not
> >>>>> resolved correctly.
> >>>>>
> >>>>> Make configfs load use the same method as efivar_ssdt_load().
> >>>>>
> >>>>> Signed-off-by: Nikolaus Voss <[email protected]>
> >>>>
> >>>> This is fine by me, so
> >>>>
> >>>> Acked-by: Rafael J. Wysocki <[email protected]>
> >>>>
> >>>> Or if you want me to take this patch (without the other two in the
> >>>> series), please let me know.
> >>>
> >>> thanks. I think it would be the best if you take up this patch as it
> >>> is an independent topic. In retrospect it wasn't a good idea to put
> >>> it into this series.
> >>>
> >>> Kind regards,
> >>> Niko
> >>>
> >>> I would have to ask, why is additional code needed for package
> >>> initialization/resolution? It already happens elsewhere in acpica.
> >>> Bob
> >>
> >> for built-in tables loaded via acpi_ex_load_table_op() everything is
> >> fine, because after acpi_tb_load_table() acpi_ns_walk_namespace() is
> >> called to resolve references.
> >>
> >> My fix only affects tables loaded dynamically via either
> >> acpi_configfs driver (for debugging purposes) or efivar_ssdt_load()
> >> (to specify a table on the kernel's command line). They use
> >> acpi_load_table() to load the table from a caller-owned buffer. To
> >> resolve the references, it is again necessary to rewalk the
> >> namespace, which was simply missing in acpi_load_table().
> >>
> > [Moore, Robert]
> >
> > Perhaps you should call AcpiInitializeObjects after the call to
> > AcpiLoadTable, but I will check.
>
> My usage of acpi_load_table() is to load a SSDT which is the intended
> use of this method according to its description. And my expectation is
> that the package objects of the loaded table are initialized when this
> function successfully returns so it aligns with the behavior of
> acpi_ex_load_table_op() for built-in SSDTs. Otherwise there would be no
> point in having this function at all, because
> acpi_tb_install_and_load_table() could be called directly without any
> difference.
>
> Niko
It looks to me that the package objects are being initialized properly already, unless I'm missing something. Please check the examples below and in the attached files.
Attached is a small test case that dynamically loads an SSDT which contains a package object which in turn contains references to other objects.
Main DSDT:
Method (LD1)
{
Load (BUF1, HNDL) // SSDT is in BUF1
Store (HNDL, Debug)
Return
}
Loaded table:
External (DEV1, DeviceObj)
Name (PKG1, Package() {
1,2, DEV2, DEV1, 4})
Device (DEV2) {}
AcpiExec Output:
- ev ld1
Evaluating \LD1
ACPI: Dynamic OEM Table Load:
ACPI: SSDT 0x00000000006DEEB8 000051 (v02 Intel Load 00000001 INTL 20190509)
ACPI Exec: Table Event INSTALL, [SSDT] 006DEEB8
Table [SSDT: Load ] (id 06) - 5 Objects with 1 Devices, 0 Regions, 1 Methods
ACPI Exec: Table Event LOAD, [SSDT] 006DEEB8
ACPI Debug: Reference [DdbHandle] Table Index 0x3
0x7 Outstanding allocations after evaluation of \LD1
Evaluation of \LD1 returned object 006D2FE8, external buffer length 18
[Integer] = 0000000000000000
- ev pkg1
Evaluating \PKG1
Evaluation of \PKG1 returned object 006D2FE8, external buffer length 90
[Package] Contains 5 Elements:
[Integer] = 0000000000000001
[Integer] = 0000000000000002
[Object Reference] = 006DDF88 <Node> Name DEV2 Device
[Object Reference] = 006DD608 <Node> Name DEV1 Device
[Integer] = 0000000000000004
If it is in fact the AcpiLoadTable interface that is incorrect, that of course is different. I'll check that out next.
> -----Original Message-----
> From: Moore, Robert
> Sent: Tuesday, June 18, 2019 1:23 PM
> To: 'Nikolaus Voss' <[email protected]>
> Cc: 'Rafael J. Wysocki' <[email protected]>; 'Rafael J. Wysocki'
> <[email protected]>; 'Len Brown' <[email protected]>; Schmauss, Erik
> <[email protected]>; 'Jacek Anaszewski'
> <[email protected]>; 'Pavel Machek' <[email protected]>; 'Dan
> Murphy' <[email protected]>; 'Thierry Reding' <[email protected]>;
> 'ACPI Devel Maling List' <[email protected]>; 'open list:ACPI
> COMPONENT ARCHITECTURE (ACPICA)' <[email protected]>; 'linux-
> [email protected]' <[email protected]>; 'Linux PWM List'
> <[email protected]>; 'Linux Kernel Mailing List' <linux-
> [email protected]>
> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> loads
>
> It looks to me that the package objects are being initialized properly
> already, unless I'm missing something. Please check the examples below
> and in the attached files.
>
> Attached is a small test case that dynamically loads an SSDT which
> contains a package object which in turn contains references to other
> objects.
>
>
> Main DSDT:
> Method (LD1)
> {
> Load (BUF1, HNDL) // SSDT is in BUF1
> Store (HNDL, Debug)
> Return
> }
>
> Loaded table:
> External (DEV1, DeviceObj)
> Name (PKG1, Package() {
> 1,2, DEV2, DEV1, 4})
> Device (DEV2) {}
>
>
> AcpiExec Output:
> - ev ld1
> Evaluating \LD1
> ACPI: Dynamic OEM Table Load:
> ACPI: SSDT 0x00000000006DEEB8 000051 (v02 Intel Load 00000001 INTL
> 20190509)
> ACPI Exec: Table Event INSTALL, [SSDT] 006DEEB8
> Table [SSDT: Load ] (id 06) - 5 Objects with 1 Devices, 0
> Regions, 1 Methods
> ACPI Exec: Table Event LOAD, [SSDT] 006DEEB8 ACPI Debug: Reference
> [DdbHandle] Table Index 0x3
> 0x7 Outstanding allocations after evaluation of \LD1 Evaluation of \LD1
> returned object 006D2FE8, external buffer length 18
> [Integer] = 0000000000000000
>
> - ev pkg1
> Evaluating \PKG1
> Evaluation of \PKG1 returned object 006D2FE8, external buffer length 90
> [Package] Contains 5 Elements:
> [Integer] = 0000000000000001
> [Integer] = 0000000000000002
> [Object Reference] = 006DDF88 <Node> Name DEV2 Device
> [Object Reference] = 006DD608 <Node> Name DEV1 Device
> [Integer] = 0000000000000004
> -----Original Message-----
> From: Moore, Robert
> Sent: Tuesday, June 18, 2019 1:25 PM
> To: 'Nikolaus Voss' <[email protected]>
> Cc: 'Rafael J. Wysocki' <[email protected]>; 'Rafael J. Wysocki'
> <[email protected]>; 'Len Brown' <[email protected]>; Schmauss, Erik
> <[email protected]>; 'Jacek Anaszewski'
> <[email protected]>; 'Pavel Machek' <[email protected]>; 'Dan
> Murphy' <[email protected]>; 'Thierry Reding' <[email protected]>;
> 'ACPI Devel Maling List' <[email protected]>; 'open list:ACPI
> COMPONENT ARCHITECTURE (ACPICA)' <[email protected]>; 'linux-
> [email protected]' <[email protected]>; 'Linux PWM List'
> <[email protected]>; 'Linux Kernel Mailing List' <linux-
> [email protected]>
> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> loads
>
> If it is in fact the AcpiLoadTable interface that is incorrect, that of
> course is different. I'll check that out next.
>
[Moore, Robert]
Yes, this is the issue, not specifically the Load() operator, but the AcpiLoadTable interface only.
>
> > -----Original Message-----
> > From: Moore, Robert
> > Sent: Tuesday, June 18, 2019 1:23 PM
> > To: 'Nikolaus Voss' <[email protected]>
> > Cc: 'Rafael J. Wysocki' <[email protected]>; 'Rafael J. Wysocki'
> > <[email protected]>; 'Len Brown' <[email protected]>; Schmauss, Erik
> > <[email protected]>; 'Jacek Anaszewski'
> > <[email protected]>; 'Pavel Machek' <[email protected]>; 'Dan
> > Murphy' <[email protected]>; 'Thierry Reding' <[email protected]>;
> > 'ACPI Devel Maling List' <[email protected]>; 'open list:ACPI
> > COMPONENT ARCHITECTURE (ACPICA)' <[email protected]>; 'linux-
> > [email protected]' <[email protected]>; 'Linux PWM List'
> > <[email protected]>; 'Linux Kernel Mailing List' <linux-
> > [email protected]>
> > Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> > table loads
> >
> > It looks to me that the package objects are being initialized properly
> > already, unless I'm missing something. Please check the examples below
> > and in the attached files.
> >
> > Attached is a small test case that dynamically loads an SSDT which
> > contains a package object which in turn contains references to other
> > objects.
> >
> >
> > Main DSDT:
> > Method (LD1)
> > {
> > Load (BUF1, HNDL) // SSDT is in BUF1
> > Store (HNDL, Debug)
> > Return
> > }
> >
> > Loaded table:
> > External (DEV1, DeviceObj)
> > Name (PKG1, Package() {
> > 1,2, DEV2, DEV1, 4})
> > Device (DEV2) {}
> >
> >
> > AcpiExec Output:
> > - ev ld1
> > Evaluating \LD1
> > ACPI: Dynamic OEM Table Load:
> > ACPI: SSDT 0x00000000006DEEB8 000051 (v02 Intel Load 00000001
> INTL
> > 20190509)
> > ACPI Exec: Table Event INSTALL, [SSDT] 006DEEB8
> > Table [SSDT: Load ] (id 06) - 5 Objects with 1 Devices, 0
> > Regions, 1 Methods
> > ACPI Exec: Table Event LOAD, [SSDT] 006DEEB8 ACPI Debug: Reference
> > [DdbHandle] Table Index 0x3
> > 0x7 Outstanding allocations after evaluation of \LD1 Evaluation of
> > \LD1 returned object 006D2FE8, external buffer length 18
> > [Integer] = 0000000000000000
> >
> > - ev pkg1
> > Evaluating \PKG1
> > Evaluation of \PKG1 returned object 006D2FE8, external buffer length
> 90
> > [Package] Contains 5 Elements:
> > [Integer] = 0000000000000001
> > [Integer] = 0000000000000002
> > [Object Reference] = 006DDF88 <Node> Name DEV2 Device
> > [Object Reference] = 006DD608 <Node> Name DEV1 Device
> > [Integer] = 0000000000000004
Hi Bob,
On Tue, 18 Jun 2019, Moore, Robert wrote:
>
>
>> -----Original Message-----
>> From: Moore, Robert
>> Sent: Tuesday, June 18, 2019 1:25 PM
>> To: 'Nikolaus Voss' <[email protected]>
>> Cc: 'Rafael J. Wysocki' <[email protected]>; 'Rafael J. Wysocki'
>> <[email protected]>; 'Len Brown' <[email protected]>; Schmauss, Erik
>> <[email protected]>; 'Jacek Anaszewski'
>> <[email protected]>; 'Pavel Machek' <[email protected]>; 'Dan
>> Murphy' <[email protected]>; 'Thierry Reding' <[email protected]>;
>> 'ACPI Devel Maling List' <[email protected]>; 'open list:ACPI
>> COMPONENT ARCHITECTURE (ACPICA)' <[email protected]>; 'linux-
>> [email protected]' <[email protected]>; 'Linux PWM List'
>> <[email protected]>; 'Linux Kernel Mailing List' <linux-
>> [email protected]>
>> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
>> loads
>>
>> If it is in fact the AcpiLoadTable interface that is incorrect, that of
>> course is different. I'll check that out next.
>>
> [Moore, Robert]
>
> Yes, this is the issue, not specifically the Load() operator, but the
> AcpiLoadTable interface only.
thanks for checking this out. So what is the conclusion? Is my fix
of AcpiLoadTable() sufficient or do we need a different solution?
Niko
[...]
> -----Original Message-----
> From: Nikolaus Voss [mailto:[email protected]]
> Sent: Wednesday, June 19, 2019 2:31 AM
> To: Moore, Robert <[email protected]>
> Cc: Rafael J. Wysocki <[email protected]>; Rafael J. Wysocki
> <[email protected]>; Len Brown <[email protected]>; Schmauss, Erik
> <[email protected]>; Jacek Anaszewski
> <[email protected]>; Pavel Machek <[email protected]>; Dan Murphy
> <[email protected]>; Thierry Reding <[email protected]>; ACPI Devel
> Maling List <[email protected]>; open list:ACPI COMPONENT
> ARCHITECTURE (ACPICA) <[email protected]>; [email protected];
> Linux PWM List <[email protected]>; Linux Kernel Mailing List
> <[email protected]>; [email protected]
> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> loads
>
> Hi Bob,
>
> On Tue, 18 Jun 2019, Moore, Robert wrote:
> >
> >
> >> -----Original Message-----
> >> From: Moore, Robert
> >> Sent: Tuesday, June 18, 2019 1:25 PM
> >> To: 'Nikolaus Voss' <[email protected]>
> >> Cc: 'Rafael J. Wysocki' <[email protected]>; 'Rafael J. Wysocki'
> >> <[email protected]>; 'Len Brown' <[email protected]>; Schmauss, Erik
> >> <[email protected]>; 'Jacek Anaszewski'
> >> <[email protected]>; 'Pavel Machek' <[email protected]>; 'Dan
> >> Murphy' <[email protected]>; 'Thierry Reding'
> >> <[email protected]>; 'ACPI Devel Maling List'
> >> <[email protected]>; 'open list:ACPI COMPONENT ARCHITECTURE
> >> (ACPICA)' <[email protected]>; 'linux- [email protected]' <linux-
> [email protected]>; 'Linux PWM List'
> >> <[email protected]>; 'Linux Kernel Mailing List' <linux-
> >> [email protected]>
> >> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> >> table loads
> >>
> >> If it is in fact the AcpiLoadTable interface that is incorrect, that
> >> of course is different. I'll check that out next.
> >>
> > [Moore, Robert]
> >
> > Yes, this is the issue, not specifically the Load() operator, but the
> > AcpiLoadTable interface only.
>
> thanks for checking this out. So what is the conclusion? Is my fix of
> AcpiLoadTable() sufficient or do we need a different solution?
>
> Niko
>
Your change is in the correct area. We want to do something like this, however:
diff --git a/source/components/executer/exconfig.c b/source/components/executer/exconfig.c
index 84a058ada..eba1a6d28 100644
--- a/source/components/executer/exconfig.c
+++ b/source/components/executer/exconfig.c
@@ -342,10 +342,9 @@ AcpiExLoadTableOp (
return_ACPI_STATUS (Status);
}
- /* Complete the initialization/resolution of package objects */
+ /* Complete the initialization/resolution of new objects */
- Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
- ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
+ AcpiNsInitializeObjects ();
/* Parameter Data (optional) */
@@ -620,10 +619,11 @@ AcpiExLoadOp (
return_ACPI_STATUS (Status);
}
- /* Complete the initialization/resolution of package objects */
+ /* Complete the initialization/resolution of new objects */
- Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
- ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
+ AcpiExExitInterpreter ();
+ AcpiNsInitializeObjects ();
+ AcpiExEnterInterpreter ();
/* Store the DdbHandle into the Target operand */
diff --git a/source/components/tables/tbxfload.c b/source/components/tables/tbxfload.c
index 217d54bf0..1e17db6c8 100644
--- a/source/components/tables/tbxfload.c
+++ b/source/components/tables/tbxfload.c
@@ -479,6 +479,13 @@ AcpiLoadTable (
ACPI_INFO (("Host-directed Dynamic ACPI Table Load:"));
Status = AcpiTbInstallAndLoadTable (ACPI_PTR_TO_PHYSADDR (Table),
ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL, FALSE, &TableIndex);
+ if (ACPI_SUCCESS (Status))
+ {
+ /* Complete the initialization/resolution of new objects */
+
+ AcpiNsInitializeObjects ();
+ }
+
return_ACPI_STATUS (Status);
}
> [...]
On Wed, 19 Jun 2019, Moore, Robert wrote:
>
>
>> -----Original Message-----
>> From: Nikolaus Voss [mailto:[email protected]]
>> Sent: Wednesday, June 19, 2019 2:31 AM
>> To: Moore, Robert <[email protected]>
>> Cc: Rafael J. Wysocki <[email protected]>; Rafael J. Wysocki
>> <[email protected]>; Len Brown <[email protected]>; Schmauss, Erik
>> <[email protected]>; Jacek Anaszewski
>> <[email protected]>; Pavel Machek <[email protected]>; Dan Murphy
>> <[email protected]>; Thierry Reding <[email protected]>; ACPI Devel
>> Maling List <[email protected]>; open list:ACPI COMPONENT
>> ARCHITECTURE (ACPICA) <[email protected]>; [email protected];
>> Linux PWM List <[email protected]>; Linux Kernel Mailing List
>> <[email protected]>; [email protected]
>> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
>> loads
>>
>> Hi Bob,
>>
>> On Tue, 18 Jun 2019, Moore, Robert wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Moore, Robert
>>>> Sent: Tuesday, June 18, 2019 1:25 PM
>>>> To: 'Nikolaus Voss' <[email protected]>
>>>> Cc: 'Rafael J. Wysocki' <[email protected]>; 'Rafael J. Wysocki'
>>>> <[email protected]>; 'Len Brown' <[email protected]>; Schmauss, Erik
>>>> <[email protected]>; 'Jacek Anaszewski'
>>>> <[email protected]>; 'Pavel Machek' <[email protected]>; 'Dan
>>>> Murphy' <[email protected]>; 'Thierry Reding'
>>>> <[email protected]>; 'ACPI Devel Maling List'
>>>> <[email protected]>; 'open list:ACPI COMPONENT ARCHITECTURE
>>>> (ACPICA)' <[email protected]>; 'linux- [email protected]' <linux-
>> [email protected]>; 'Linux PWM List'
>>>> <[email protected]>; 'Linux Kernel Mailing List' <linux-
>>>> [email protected]>
>>>> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
>>>> table loads
>>>>
>>>> If it is in fact the AcpiLoadTable interface that is incorrect, that
>>>> of course is different. I'll check that out next.
>>>>
>>> [Moore, Robert]
>>>
>>> Yes, this is the issue, not specifically the Load() operator, but the
>>> AcpiLoadTable interface only.
>>
>> thanks for checking this out. So what is the conclusion? Is my fix of
>> AcpiLoadTable() sufficient or do we need a different solution?
>>
>> Niko
>>
>
>
> Your change is in the correct area. We want to do something like this, however:
>
> diff --git a/source/components/executer/exconfig.c b/source/components/executer/exconfig.c
> index 84a058ada..eba1a6d28 100644
> --- a/source/components/executer/exconfig.c
> +++ b/source/components/executer/exconfig.c
> @@ -342,10 +342,9 @@ AcpiExLoadTableOp (
> return_ACPI_STATUS (Status);
> }
>
> - /* Complete the initialization/resolution of package objects */
> + /* Complete the initialization/resolution of new objects */
>
> - Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
> - ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
> + AcpiNsInitializeObjects ();
>
> /* Parameter Data (optional) */
>
> @@ -620,10 +619,11 @@ AcpiExLoadOp (
> return_ACPI_STATUS (Status);
> }
>
> - /* Complete the initialization/resolution of package objects */
> + /* Complete the initialization/resolution of new objects */
>
> - Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
> - ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
> + AcpiExExitInterpreter ();
> + AcpiNsInitializeObjects ();
> + AcpiExEnterInterpreter ();
>
> /* Store the DdbHandle into the Target operand */
>
> diff --git a/source/components/tables/tbxfload.c b/source/components/tables/tbxfload.c
> index 217d54bf0..1e17db6c8 100644
> --- a/source/components/tables/tbxfload.c
> +++ b/source/components/tables/tbxfload.c
> @@ -479,6 +479,13 @@ AcpiLoadTable (
> ACPI_INFO (("Host-directed Dynamic ACPI Table Load:"));
> Status = AcpiTbInstallAndLoadTable (ACPI_PTR_TO_PHYSADDR (Table),
> ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL, FALSE, &TableIndex);
> + if (ACPI_SUCCESS (Status))
> + {
> + /* Complete the initialization/resolution of new objects */
> +
> + AcpiNsInitializeObjects ();
> + }
> +
> return_ACPI_STATUS (Status);
> }
Ok, I see your are taking this up (I was a bit unsure after your previous
post). Thanks,
Niko
On Wed 2019-06-12 10:36:08, Nikolaus Voss wrote:
> DT specific handling is replaced by firmware-node abstration to support
> ACPI specification of PWM LEDS.
>
> Example ASL:
> Device (PWML)
> {
> Name (_HID, "PRP0001")
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () { Package () {"compatible",
> Package () {"pwm-leds"}}}})
>
> Device (PWL0)
> {
> Name (_HID, "PRP0001")
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package () {"label", "alarm-led"},
> Package () {"pwms", Package ()
> {\_SB_.PCI0.PWM, 0, 600000, 0}},
> Package () {"linux,default-state", "off"}}})
> }
> }
>
> Signed-off-by: Nikolaus Voss <[email protected]>
Acked-by: Pavel Machek <[email protected]>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Thu, Jun 20, 2019 at 8:49 AM Nikolaus Voss <[email protected]> wrote:
>
> On Wed, 19 Jun 2019, Moore, Robert wrote:
> >
> >
> >> -----Original Message-----
> >> From: Nikolaus Voss [mailto:[email protected]]
> >> Sent: Wednesday, June 19, 2019 2:31 AM
> >> To: Moore, Robert <[email protected]>
> >> Cc: Rafael J. Wysocki <[email protected]>; Rafael J. Wysocki
> >> <[email protected]>; Len Brown <[email protected]>; Schmauss, Erik
> >> <[email protected]>; Jacek Anaszewski
> >> <[email protected]>; Pavel Machek <[email protected]>; Dan Murphy
> >> <[email protected]>; Thierry Reding <[email protected]>; ACPI Devel
> >> Maling List <[email protected]>; open list:ACPI COMPONENT
> >> ARCHITECTURE (ACPICA) <[email protected]>; [email protected];
> >> Linux PWM List <[email protected]>; Linux Kernel Mailing List
> >> <[email protected]>; [email protected]
> >> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> >> loads
> >>
> >> Hi Bob,
> >>
> >> On Tue, 18 Jun 2019, Moore, Robert wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Moore, Robert
> >>>> Sent: Tuesday, June 18, 2019 1:25 PM
> >>>> To: 'Nikolaus Voss' <[email protected]>
> >>>> Cc: 'Rafael J. Wysocki' <[email protected]>; 'Rafael J. Wysocki'
> >>>> <[email protected]>; 'Len Brown' <[email protected]>; Schmauss, Erik
> >>>> <[email protected]>; 'Jacek Anaszewski'
> >>>> <[email protected]>; 'Pavel Machek' <[email protected]>; 'Dan
> >>>> Murphy' <[email protected]>; 'Thierry Reding'
> >>>> <[email protected]>; 'ACPI Devel Maling List'
> >>>> <[email protected]>; 'open list:ACPI COMPONENT ARCHITECTURE
> >>>> (ACPICA)' <[email protected]>; 'linux- [email protected]' <linux-
> >> [email protected]>; 'Linux PWM List'
> >>>> <[email protected]>; 'Linux Kernel Mailing List' <linux-
> >>>> [email protected]>
> >>>> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> >>>> table loads
> >>>>
> >>>> If it is in fact the AcpiLoadTable interface that is incorrect, that
> >>>> of course is different. I'll check that out next.
> >>>>
> >>> [Moore, Robert]
> >>>
> >>> Yes, this is the issue, not specifically the Load() operator, but the
> >>> AcpiLoadTable interface only.
> >>
> >> thanks for checking this out. So what is the conclusion? Is my fix of
> >> AcpiLoadTable() sufficient or do we need a different solution?
> >>
> >> Niko
> >>
> >
> >
> > Your change is in the correct area. We want to do something like this, however:
> >
> > diff --git a/source/components/executer/exconfig.c b/source/components/executer/exconfig.c
> > index 84a058ada..eba1a6d28 100644
> > --- a/source/components/executer/exconfig.c
> > +++ b/source/components/executer/exconfig.c
> > @@ -342,10 +342,9 @@ AcpiExLoadTableOp (
> > return_ACPI_STATUS (Status);
> > }
> >
> > - /* Complete the initialization/resolution of package objects */
> > + /* Complete the initialization/resolution of new objects */
> >
> > - Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
> > - ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
> > + AcpiNsInitializeObjects ();
> >
> > /* Parameter Data (optional) */
> >
> > @@ -620,10 +619,11 @@ AcpiExLoadOp (
> > return_ACPI_STATUS (Status);
> > }
> >
> > - /* Complete the initialization/resolution of package objects */
> > + /* Complete the initialization/resolution of new objects */
> >
> > - Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
> > - ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
> > + AcpiExExitInterpreter ();
> > + AcpiNsInitializeObjects ();
> > + AcpiExEnterInterpreter ();
> >
> > /* Store the DdbHandle into the Target operand */
> >
> > diff --git a/source/components/tables/tbxfload.c b/source/components/tables/tbxfload.c
> > index 217d54bf0..1e17db6c8 100644
> > --- a/source/components/tables/tbxfload.c
> > +++ b/source/components/tables/tbxfload.c
> > @@ -479,6 +479,13 @@ AcpiLoadTable (
> > ACPI_INFO (("Host-directed Dynamic ACPI Table Load:"));
> > Status = AcpiTbInstallAndLoadTable (ACPI_PTR_TO_PHYSADDR (Table),
> > ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL, FALSE, &TableIndex);
> > + if (ACPI_SUCCESS (Status))
> > + {
> > + /* Complete the initialization/resolution of new objects */
> > +
> > + AcpiNsInitializeObjects ();
> > + }
> > +
> > return_ACPI_STATUS (Status);
> > }
>
> Ok, I see your are taking this up (I was a bit unsure after your previous
> post). Thanks,
The $subject patch has been queued for 5.3. If I should drop it,
please let me know.
Thanks!