From: Aaron Lu <aaro...@intel.com>

Make use of device property API in this driver so that both OF based
system and ACPI based system can use this driver.

Signed-off-by: Aaron Lu <aaro...@intel.com>

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
---

drivers/input/keyboard/gpio_keys_polled.c | 125 ++++++++++++++----------------
1 file changed, 59 insertions(+), 66 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index b7a514ced509..9afd9a6c43f4 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -25,9 +25,7 @@
#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
#include <linux/gpio_keys.h>
-#include <linux/of.h>
-#include <linux/of_platform.h>
-#include <linux/of_gpio.h>
+#include <linux/property.h>

#define DRV_NAME "gpio-keys-polled"

@@ -45,6 +43,11 @@ struct gpio_keys_polled_dev {
struct gpio_keys_button_data data[0];
};

+struct gpio_keys_devtree_data {
+ struct device *dev;
+ struct gpio_keys_platform_data *pdata;
+};
+
static void gpio_keys_polled_check_state(struct input_dev *input,
struct gpio_keys_button *button,
struct gpio_keys_button_data *bdata)
@@ -102,21 +105,56 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
pdata->disable(bdev->dev);
}

-#ifdef CONFIG_OF
+static int gpio_keys_polled_get_button(struct fw_dev_node *fdn, void *data)
+{
+ struct gpio_keys_devtree_data *dtdata = data;
+ struct gpio_keys_platform_data *pdata = dtdata->pdata;
+ struct device *dev = dtdata->dev;
+ struct gpio_keys_button *button;
+ struct gpio_desc *desc;
+
+ desc = devm_node_get_named_gpiod(dev, fdn, "gpios", 0);
+ if (IS_ERR(desc)) {
+ int err = PTR_ERR(desc);
+
+ if (err != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get gpio flags, error: %d\n",
+ err);
+ return err;
+ }
+
+ button = &pdata->buttons[pdata->nbuttons++];
+ button->gpiod = desc;
+
+ if (dev_node_property_read_u32(fdn, "linux,code", &button->code)) {
+ dev_err(dev, "Button without keycode: %d\n",
+ pdata->nbuttons - 1);

+ return -EINVAL;
+ }
+

+ dev_node_property_read_string(fdn, "label", &button->desc);
+
+ if (dev_node_property_read_u32(fdn, "linux,input-type", &button->type))
+ button->type = EV_KEY;
+
+ button->wakeup = !dev_node_get_property(fdn, "gpio-key,wakeup", NULL);
+
+ if (dev_node_property_read_u32(fdn, "debounce-interval",
+ &button->debounce_interval))
+ button->debounce_interval = 5;

+
+ return 0;
+}
+

static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct device *dev)
{
- struct device_node *node, *pp;
+ struct gpio_keys_devtree_data dtdata;
struct gpio_keys_platform_data *pdata;
struct gpio_keys_button *button;
int error;
int nbuttons;
- int i;

- node = dev->of_node;
- if (!node)
- return NULL;
-
- nbuttons = of_get_child_count(node);
+ nbuttons = device_get_child_node_count(dev);
if (nbuttons == 0)
return NULL;

@@ -126,54 +164,18 @@ static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct
return ERR_PTR(-ENOMEM);

pdata->buttons = (struct gpio_keys_button *)(pdata + 1);
- pdata->nbuttons = nbuttons;
-
- pdata->rep = !!of_get_property(node, "autorepeat", NULL);
- of_property_read_u32(node, "poll-interval", &pdata->poll_interval);
-
- i = 0;
- for_each_child_of_node(node, pp) {
- int gpio;
- enum of_gpio_flags flags;
-
- if (!of_find_property(pp, "gpios", NULL)) {
- pdata->nbuttons--;
- dev_warn(dev, "Found button without gpios\n");
- continue;
- }
-
- gpio = of_get_gpio_flags(pp, 0, &flags);
- if (gpio < 0) {
- error = gpio;
- if (error != -EPROBE_DEFER)
- dev_err(dev,
- "Failed to get gpio flags, error: %d\n",
- error);
- return ERR_PTR(error);
- }

- button = &pdata->buttons[i++];
+ pdata->rep = !device_get_property(dev, "autorepeat", NULL);
+ device_property_read_u32(dev, "poll-interval", &pdata->poll_interval);

- button->gpio = gpio;
- button->active_low = flags & OF_GPIO_ACTIVE_LOW;
+ memset(&dtdata, 0, sizeof(dtdata));
+ dtdata.pdata = pdata;
+ dtdata.dev = dev;

- if (of_property_read_u32(pp, "linux,code", &button->code)) {
- dev_err(dev, "Button without keycode: 0x%x\n",
- button->gpio);

- return ERR_PTR(-EINVAL);
- }
-

- button->desc = of_get_property(pp, "label", NULL);
-
- if (of_property_read_u32(pp, "linux,input-type", &button->type))
- button->type = EV_KEY;
-
- button->wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL);
-
- if (of_property_read_u32(pp, "debounce-interval",
- &button->debounce_interval))
- button->debounce_interval = 5;
- }
+ error = device_for_each_child_node(dev, gpio_keys_polled_get_button,
+ &dtdata);
+ if (error)
+ return ERR_PTR(error);

if (pdata->nbuttons == 0)
return ERR_PTR(-EINVAL);
@@ -187,15 +189,6 @@ static const struct of_device_id gpio_keys_polled_of_match[] = {
};
MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);

-#else
-
-static inline struct gpio_keys_platform_data *
-gpio_keys_polled_get_devtree_pdata(struct device *dev)
-{
- return NULL;
-}
-#endif
-
static int gpio_keys_polled_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -323,7 +316,7 @@ static struct platform_driver gpio_keys_polled_driver = {
.driver = {
.name = DRV_NAME,
.owner = THIS_MODULE,
- .of_match_table = of_match_ptr(gpio_keys_polled_of_match),
+ .of_match_table = gpio_keys_polled_of_match,
},
};
module_platform_driver(gpio_keys_polled_driver);

Mika Westerberg

unread,

Sep 16, 2014, 7:53:22 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

This is actually a single device with two sets of identical registers,
which just happen to start from a different offset. Instead of having
separate GPIO chips created we consolidate them to be single GPIO chip.

In addition having a single GPIO chip allows us to handle ACPI GPIO
translation in the core in a more generic way, since the two GPIO chips
share the same parent ACPI device.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
Acked-by: Linus Walleij <linus....@linaro.org>
---
drivers/gpio/gpio-sch.c | 293 ++++++++++++++++++------------------------------
1 file changed, 112 insertions(+), 181 deletions(-)

diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 41e91d70301e..99720c8bc8ed 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -29,290 +29,221 @@

#include <linux/gpio.h>

-static DEFINE_SPINLOCK(gpio_lock);
-
-#define CGEN (0x00)
-#define CGIO (0x04)
-#define CGLV (0x08)
-
-#define RGEN (0x20)
-#define RGIO (0x24)
-#define RGLV (0x28)
-
-static unsigned short gpio_ba;
-
-static int sch_gpio_core_direction_in(struct gpio_chip *gc, unsigned gpio_num)
-{
- u8 curr_dirs;
- unsigned short offset, bit;
-
- spin_lock(&gpio_lock);
-
- offset = CGIO + gpio_num / 8;
- bit = gpio_num % 8;
-
- curr_dirs = inb(gpio_ba + offset);
-
- if (!(curr_dirs & (1 << bit)))
- outb(curr_dirs | (1 << bit), gpio_ba + offset);
+#define GEN 0x00
+#define GIO 0x04
+#define GLV 0x08
+
+struct sch_gpio {
+ struct gpio_chip chip;
+ spinlock_t lock;
+ unsigned short iobase;
+ unsigned short core_base;
+ unsigned short resume_base;
+};

- spin_unlock(&gpio_lock);
- return 0;
-}
+#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)

-static int sch_gpio_core_get(struct gpio_chip *gc, unsigned gpio_num)
+static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
- int res;
- unsigned short offset, bit;
+ unsigned base = 0;

- offset = CGLV + gpio_num / 8;
- bit = gpio_num % 8;
+ if (gpio >= sch->resume_base) {
+ gpio -= sch->resume_base;
+ base += 0x20;
+ }

- res = !!(inb(gpio_ba + offset) & (1 << bit));
- return res;
+ return base + reg + gpio / 8;
}

-static void sch_gpio_core_set(struct gpio_chip *gc, unsigned gpio_num, int val)
+static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned gpio)
{
- u8 curr_vals;
- unsigned short offset, bit;
-
- spin_lock(&gpio_lock);
-
- offset = CGLV + gpio_num / 8;
- bit = gpio_num % 8;
-
- curr_vals = inb(gpio_ba + offset);
-
- if (val)
- outb(curr_vals | (1 << bit), gpio_ba + offset);
- else
- outb((curr_vals & ~(1 << bit)), gpio_ba + offset);
- spin_unlock(&gpio_lock);
+ if (gpio >= sch->resume_base)
+ gpio -= sch->resume_base;
+ return gpio % 8;
}

-static int sch_gpio_core_direction_out(struct gpio_chip *gc,
- unsigned gpio_num, int val)
+static void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio)
{
- u8 curr_dirs;
unsigned short offset, bit;
+ u8 enable;

- spin_lock(&gpio_lock);
+ spin_lock(&sch->lock);

- offset = CGIO + gpio_num / 8;
- bit = gpio_num % 8;
-
- curr_dirs = inb(gpio_ba + offset);
- if (curr_dirs & (1 << bit))
- outb(curr_dirs & ~(1 << bit), gpio_ba + offset);
+ offset = sch_gpio_offset(sch, gpio, GEN);
+ bit = sch_gpio_bit(sch, gpio);

- spin_unlock(&gpio_lock);
+ enable = inb(sch->iobase + offset);
+ if (!(enable & (1 << bit)))
+ outb(enable | (1 << bit), sch->iobase + offset);

- /*
- * according to the datasheet, writing to the level register has no
- * effect when GPIO is programmed as input.
- * Actually the the level register is read-only when configured as input.
- * Thus presetting the output level before switching to output is _NOT_ possible.
- * Hence we set the level after configuring the GPIO as output.
- * But we cannot prevent a short low pulse if direction is set to high
- * and an external pull-up is connected.
- */
- sch_gpio_core_set(gc, gpio_num, val);
- return 0;
+ spin_unlock(&sch->lock);
}

-static struct gpio_chip sch_gpio_core = {
- .label = "sch_gpio_core",
- .owner = THIS_MODULE,
- .direction_input = sch_gpio_core_direction_in,
- .get = sch_gpio_core_get,
- .direction_output = sch_gpio_core_direction_out,
- .set = sch_gpio_core_set,
-};
-
-static int sch_gpio_resume_direction_in(struct gpio_chip *gc,
- unsigned gpio_num)
+static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
{
+ struct sch_gpio *sch = to_sch_gpio(gc);
u8 curr_dirs;
unsigned short offset, bit;

- spin_lock(&gpio_lock);
+ spin_lock(&sch->lock);

- offset = RGIO + gpio_num / 8;
- bit = gpio_num % 8;
+ offset = sch_gpio_offset(sch, gpio_num, GIO);
+ bit = sch_gpio_bit(sch, gpio_num);

- curr_dirs = inb(gpio_ba + offset);
+ curr_dirs = inb(sch->iobase + offset);

if (!(curr_dirs & (1 << bit)))
- outb(curr_dirs | (1 << bit), gpio_ba + offset);
+ outb(curr_dirs | (1 << bit), sch->iobase + offset);

- spin_unlock(&gpio_lock);
+ spin_unlock(&sch->lock);
return 0;
}

-static int sch_gpio_resume_get(struct gpio_chip *gc, unsigned gpio_num)
+static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+ int res;
unsigned short offset, bit;

- offset = RGLV + gpio_num / 8;
- bit = gpio_num % 8;
+ offset = sch_gpio_offset(sch, gpio_num, GLV);
+ bit = sch_gpio_bit(sch, gpio_num);
+
+ res = !!(inb(sch->iobase + offset) & (1 << bit));

- return !!(inb(gpio_ba + offset) & (1 << bit));
+ return res;
}

-static void sch_gpio_resume_set(struct gpio_chip *gc,
- unsigned gpio_num, int val)
+static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
+ struct sch_gpio *sch = to_sch_gpio(gc);
u8 curr_vals;
unsigned short offset, bit;

- spin_lock(&gpio_lock);
+ spin_lock(&sch->lock);

- offset = RGLV + gpio_num / 8;
- bit = gpio_num % 8;
+ offset = sch_gpio_offset(sch, gpio_num, GLV);
+ bit = sch_gpio_bit(sch, gpio_num);

- curr_vals = inb(gpio_ba + offset);
+ curr_vals = inb(sch->iobase + offset);

if (val)
- outb(curr_vals | (1 << bit), gpio_ba + offset);
+ outb(curr_vals | (1 << bit), sch->iobase + offset);
else
- outb((curr_vals & ~(1 << bit)), gpio_ba + offset);
+ outb((curr_vals & ~(1 << bit)), sch->iobase + offset);

- spin_unlock(&gpio_lock);
+ spin_unlock(&sch->lock);
}

-static int sch_gpio_resume_direction_out(struct gpio_chip *gc,
- unsigned gpio_num, int val)
+static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
+ int val)
{
+ struct sch_gpio *sch = to_sch_gpio(gc);
u8 curr_dirs;
unsigned short offset, bit;

- offset = RGIO + gpio_num / 8;
- bit = gpio_num % 8;
+ spin_lock(&sch->lock);

- spin_lock(&gpio_lock);
+ offset = sch_gpio_offset(sch, gpio_num, GIO);
+ bit = sch_gpio_bit(sch, gpio_num);

- curr_dirs = inb(gpio_ba + offset);
+ curr_dirs = inb(sch->iobase + offset);
if (curr_dirs & (1 << bit))
- outb(curr_dirs & ~(1 << bit), gpio_ba + offset);
+ outb(curr_dirs & ~(1 << bit), sch->iobase + offset);

- spin_unlock(&gpio_lock);
+ spin_unlock(&sch->lock);

/*
- * according to the datasheet, writing to the level register has no
- * effect when GPIO is programmed as input.
- * Actually the the level register is read-only when configured as input.
- * Thus presetting the output level before switching to output is _NOT_ possible.
- * Hence we set the level after configuring the GPIO as output.
- * But we cannot prevent a short low pulse if direction is set to high
- * and an external pull-up is connected.
- */
- sch_gpio_resume_set(gc, gpio_num, val);
+ * according to the datasheet, writing to the level register has no
+ * effect when GPIO is programmed as input.
+ * Actually the the level register is read-only when configured as input.
+ * Thus presetting the output level before switching to output is _NOT_ possible.
+ * Hence we set the level after configuring the GPIO as output.
+ * But we cannot prevent a short low pulse if direction is set to high
+ * and an external pull-up is connected.
+ */
+ sch_gpio_set(gc, gpio_num, val);
return 0;
}

-static struct gpio_chip sch_gpio_resume = {
- .label = "sch_gpio_resume",
+static struct gpio_chip sch_gpio_chip = {
+ .label = "sch_gpio",
.owner = THIS_MODULE,
- .direction_input = sch_gpio_resume_direction_in,
- .get = sch_gpio_resume_get,
- .direction_output = sch_gpio_resume_direction_out,
- .set = sch_gpio_resume_set,
+ .direction_input = sch_gpio_direction_in,
+ .get = sch_gpio_get,
+ .direction_output = sch_gpio_direction_out,
+ .set = sch_gpio_set,
};

static int sch_gpio_probe(struct platform_device *pdev)
{
+ struct sch_gpio *sch;
struct resource *res;
- int err, id;

- id = pdev->id;
- if (!id)
- return -ENODEV;
+ sch = devm_kzalloc(&pdev->dev, sizeof(*sch), GFP_KERNEL);
+ if (!sch)
+ return -ENOMEM;

res = platform_get_resource(pdev, IORESOURCE_IO, 0);
if (!res)
return -EBUSY;

- if (!request_region(res->start, resource_size(res), pdev->name))
+ if (!devm_request_region(&pdev->dev, res->start, resource_size(res),
+ pdev->name))
return -EBUSY;

- gpio_ba = res->start;
+ spin_lock_init(&sch->lock);
+ sch->iobase = res->start;
+ sch->chip = sch_gpio_chip;
+ sch->chip.label = dev_name(&pdev->dev);
+ sch->chip.dev = &pdev->dev;

- switch (id) {
+ switch (pdev->id) {
case PCI_DEVICE_ID_INTEL_SCH_LPC:
- sch_gpio_core.base = 0;
- sch_gpio_core.ngpio = 10;
- sch_gpio_resume.base = 10;
- sch_gpio_resume.ngpio = 4;
+ sch->core_base = 0;
+ sch->resume_base = 10;
+ sch->chip.ngpio = 14;
+
/*
* GPIO[6:0] enabled by default
* GPIO7 is configured by the CMC as SLPIOVR
* Enable GPIO[9:8] core powered gpios explicitly
*/
- outb(0x3, gpio_ba + CGEN + 1);
+ sch_gpio_enable(sch, 8);
+ sch_gpio_enable(sch, 9);
/*
* SUS_GPIO[2:0] enabled by default
* Enable SUS_GPIO3 resume powered gpio explicitly
*/
- outb(0x8, gpio_ba + RGEN);
+ sch_gpio_enable(sch, 13);
break;

case PCI_DEVICE_ID_INTEL_ITC_LPC:
- sch_gpio_core.base = 0;
- sch_gpio_core.ngpio = 5;
- sch_gpio_resume.base = 5;
- sch_gpio_resume.ngpio = 9;
+ sch->core_base = 0;
+ sch->resume_base = 5;
+ sch->chip.ngpio = 14;
break;

case PCI_DEVICE_ID_INTEL_CENTERTON_ILB:
- sch_gpio_core.base = 0;
- sch_gpio_core.ngpio = 21;
- sch_gpio_resume.base = 21;
- sch_gpio_resume.ngpio = 9;
+ sch->core_base = 0;
+ sch->resume_base = 21;
+ sch->chip.ngpio = 30;
break;

default:
- err = -ENODEV;
- goto err_sch_gpio_core;
+ return -ENODEV;
}

- sch_gpio_core.dev = &pdev->dev;
- sch_gpio_resume.dev = &pdev->dev;
-
- err = gpiochip_add(&sch_gpio_core);
- if (err < 0)
- goto err_sch_gpio_core;
+ platform_set_drvdata(pdev, sch);

- err = gpiochip_add(&sch_gpio_resume);
- if (err < 0)
- goto err_sch_gpio_resume;
-
- return 0;
-
-err_sch_gpio_resume:
- gpiochip_remove(&sch_gpio_core);
-
-err_sch_gpio_core:
- release_region(res->start, resource_size(res));
- gpio_ba = 0;
-
- return err;
+ return gpiochip_add(&sch->chip);
}

static int sch_gpio_remove(struct platform_device *pdev)
{
- struct resource *res;
- if (gpio_ba) {
-
- gpiochip_remove(&sch_gpio_core);
- gpiochip_remove(&sch_gpio_resume);
-
- res = platform_get_resource(pdev, IORESOURCE_IO, 0);
-
- release_region(res->start, resource_size(res));
- gpio_ba = 0;
- }
+ struct sch_gpio *sch = platform_get_drvdata(pdev);

+ gpiochip_remove(&sch->chip);
return 0;

Mika Westerberg

unread,

Sep 16, 2014, 7:53:35 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

From: Max Eliaser <max.e...@intel.com>

Make use of device property API in this driver so that both OF and ACPI
based system can use the same driver.

Signed-off-by: Max Eliaser <max.e...@intel.com>

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
---

drivers/leds/leds-gpio.c | 102 +++++++++++++++++++++--------------------------
1 file changed, 45 insertions(+), 57 deletions(-)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index c84e913527f0..5dbf7485da66 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -15,13 +15,11 @@
#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
#include <linux/leds.h>

-#include <linux/of.h>
-#include <linux/of_platform.h>
-#include <linux/of_gpio.h>

#include <linux/slab.h>
#include <linux/workqueue.h>
#include <linux/module.h>
#include <linux/err.h>
+#include <linux/property.h>

struct gpio_led_data {
struct led_classdev cdev;
@@ -161,6 +159,7 @@ static void delete_gpio_led(struct gpio_led_data *led)
}

struct gpio_leds_priv {
+ struct device *dev;
int num_leds;
struct gpio_led_data leds[];
};
@@ -171,65 +170,61 @@ static inline int sizeof_gpio_leds_priv(int num_leds)
(sizeof(struct gpio_led_data) * num_leds);
}

-/* Code to create from OpenFirmware platform devices */
-#ifdef CONFIG_OF_GPIO
-static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev)
+static int gpio_leds_create_led(struct fw_dev_node *fdn, void *data)
+{
+ struct gpio_leds_priv *priv = data;
+ struct gpio_led led = {};
+ const char *state = NULL;
+
+ led.gpiod = devm_node_get_named_gpiod(priv->dev, fdn, "gpios", 0);
+ if (IS_ERR(led.gpiod))
+ return PTR_ERR(led.gpiod);
+
+ dev_node_property_read_string(fdn, "label", &led.name);
+ dev_node_property_read_string(fdn, "linux,default-trigger",
+ &led.default_trigger);
+
+ dev_node_property_read_string(fdn, "linux,default_state", &state);
+ if (state) {
+ if (!strcmp(state, "keep"))
+ led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+ else if (!strcmp(state, "on"))
+ led.default_state = LEDS_GPIO_DEFSTATE_ON;
+ else
+ led.default_state = LEDS_GPIO_DEFSTATE_OFF;
+ }
+
+ if (!dev_node_get_property(fdn, "retain-state-suspended", NULL))
+ led.retain_state_suspended = 1;
+
+ return create_gpio_led(&led, &priv->leds[priv->num_leds++], priv->dev,
+ NULL);
+}
+
+static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
{
- struct device_node *np = pdev->dev.of_node, *child;
struct gpio_leds_priv *priv;
- int count, ret;
+ int ret, count;

- /* count LEDs in this device, so we know how much to allocate */
- count = of_get_available_child_count(np);
+ count = device_get_child_node_count(&pdev->dev);
if (!count)
return ERR_PTR(-ENODEV);

- for_each_available_child_of_node(np, child)
- if (of_get_gpio(child, 0) == -EPROBE_DEFER)
- return ERR_PTR(-EPROBE_DEFER);
-
priv = devm_kzalloc(&pdev->dev, sizeof_gpio_leds_priv(count),
GFP_KERNEL);
if (!priv)
return ERR_PTR(-ENOMEM);

- for_each_available_child_of_node(np, child) {
- struct gpio_led led = {};
- enum of_gpio_flags flags;
- const char *state;
-
- led.gpio = of_get_gpio_flags(child, 0, &flags);
- led.active_low = flags & OF_GPIO_ACTIVE_LOW;
- led.name = of_get_property(child, "label", NULL) ? : child->name;
- led.default_trigger =
- of_get_property(child, "linux,default-trigger", NULL);
- state = of_get_property(child, "default-state", NULL);
- if (state) {
- if (!strcmp(state, "keep"))
- led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
- else if (!strcmp(state, "on"))
- led.default_state = LEDS_GPIO_DEFSTATE_ON;
- else
- led.default_state = LEDS_GPIO_DEFSTATE_OFF;
- }
-
- if (of_get_property(child, "retain-state-suspended", NULL))
- led.retain_state_suspended = 1;
-
- ret = create_gpio_led(&led, &priv->leds[priv->num_leds++],
- &pdev->dev, NULL);
- if (ret < 0) {
- of_node_put(child);
- goto err;
- }
+ priv->dev = &pdev->dev;
+ ret = device_for_each_child_node(&pdev->dev, gpio_leds_create_led,
+ priv);
+ if (ret) {
+ for (count = priv->num_leds - 2; count >= 0; count--)
+ delete_gpio_led(&priv->leds[count]);
+ return ERR_PTR(ret);
}

return priv;
-
-err:
- for (count = priv->num_leds - 2; count >= 0; count--)
- delete_gpio_led(&priv->leds[count]);
- return ERR_PTR(-ENODEV);
}

static const struct of_device_id of_gpio_leds_match[] = {
@@ -238,13 +233,6 @@ static const struct of_device_id of_gpio_leds_match[] = {
};

MODULE_DEVICE_TABLE(of, of_gpio_leds_match);
-#else /* CONFIG_OF_GPIO */
-static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev)
-{
- return ERR_PTR(-ENODEV);
-}
-#endif /* CONFIG_OF_GPIO */
-

static int gpio_led_probe(struct platform_device *pdev)
{
@@ -273,7 +261,7 @@ static int gpio_led_probe(struct platform_device *pdev)
}
}
} else {
- priv = gpio_leds_create_of(pdev);
+ priv = gpio_leds_create(pdev);
if (IS_ERR(priv))
return PTR_ERR(priv);
}
@@ -300,7 +288,7 @@ static struct platform_driver gpio_led_driver = {
.driver = {
.name = "leds-gpio",
.owner = THIS_MODULE,
- .of_match_table = of_match_ptr(of_gpio_leds_match),
+ .of_match_table = of_gpio_leds_match,
},

Mika Westerberg

unread,

Sep 16, 2014, 7:53:53 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

From: Max Eliaser <max.e...@intel.com>

This allows the driver to probe from ACPI namespace.

Signed-off-by: Max Eliaser <max.e...@intel.com>
Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
---

drivers/leds/leds-gpio.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 5dbf7485da66..24e05adbbaf8 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -234,6 +234,13 @@ static const struct of_device_id of_gpio_leds_match[] = {

MODULE_DEVICE_TABLE(of, of_gpio_leds_match);

+static const struct acpi_device_id acpi_gpio_leds_match[] = {
+ { "PRP0001" }, /* Device Tree shoehorned into ACPI */
+ {},
+};
+
+MODULE_DEVICE_TABLE(acpi, acpi_gpio_leds_match);
+

static int gpio_led_probe(struct platform_device *pdev)
{

struct gpio_led_platform_data *pdata = dev_get_platdata(&pdev->dev);
@@ -289,6 +296,7 @@ static struct platform_driver gpio_led_driver = {

.name = "leds-gpio",
.owner = THIS_MODULE,

.of_match_table = of_gpio_leds_match,
+ .acpi_match_table = acpi_gpio_leds_match,

Mika Westerberg

unread,

Sep 16, 2014, 7:54:14 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

If an MFD device is backed by ACPI namespace, we should allow subdevice
drivers to access their corresponding ACPI companion devices through normal
means (e.g using ACPI_COMPANION()).

This patch adds such support to the MFD core. If the MFD parent device
does not specify any ACPI _HID/_CID for the child device, the child
device will share the parent ACPI companion device. Otherwise the child
device will be assigned with the corresponding ACPI companion, if found
in the namespace below the parent.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
Reviewed-by: Darren Hart <dvh...@linux.intel.com>
---
Lee, I tried to get rid of #ifdefs in the below patch but it wasn't
possible because we are using functions that are not available when
!CONFIG_ACPI.

Documentation/acpi/enumeration.txt | 27 +++++++++++++++++++++++++
drivers/mfd/mfd-core.c | 40 ++++++++++++++++++++++++++++++++++++++
include/linux/mfd/core.h | 3 +++
3 files changed, 70 insertions(+)

diff --git a/Documentation/acpi/enumeration.txt b/Documentation/acpi/enumeration.txt
index e182be5e3c83..b60d2ab69497 100644
--- a/Documentation/acpi/enumeration.txt
+++ b/Documentation/acpi/enumeration.txt
@@ -312,3 +312,30 @@ a code like this:

There are also devm_* versions of these functions which release the
descriptors once the device is released.
+
+MFD devices
+~~~~~~~~~~~
+The MFD devices register their children as platform devices. For the child
+devices there needs to be an ACPI handle that they can use to reference
+parts of the ACPI namespace that relate to them. In the Linux MFD subsystem
+we provide two ways:
+
+ o The children share the parent ACPI handle.
+ o The MFD cell can specify the ACPI id of the device.
+
+For the first case, the MFD drivers do not need to do anything. The
+resulting child platform device will have its ACPI_COMPANION() set to point
+to the parent device.
+
+If the ACPI namespace has a device that we can match using an ACPI id,
+the id should be set like:
+
+ static struct mfd_cell my_subdevice_cell = {
+ .name = "my_subdevice",
+ /* set the resources relative to the parent */
+ .acpi_pnpid = "XYZ0001",
+ };
+
+The ACPI id "XYZ0001" is then used to lookup an ACPI device directly under
+the MFD device and if found, that ACPI companion device is bound to the
+resulting child platform device.
diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 892d343193ad..e25f584be55b 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -78,6 +78,44 @@ static int mfd_platform_add_cell(struct platform_device *pdev,
return 0;
}

+#if IS_ENABLED(CONFIG_ACPI)
+static void mfd_acpi_add_device(const struct mfd_cell *cell,
+ struct platform_device *pdev)
+{
+ struct acpi_device *parent_adev;
+ struct acpi_device *adev;
+
+ parent_adev = ACPI_COMPANION(pdev->dev.parent);
+ if (!parent_adev)
+ return;
+
+ /*
+ * MFD child device gets its ACPI handle either from the ACPI
+ * device directly under the parent that matches the acpi_pnpid or
+ * it will use the parent handle if is no acpi_pnpid is given.
+ */
+ adev = parent_adev;
+ if (cell->acpi_pnpid) {
+ struct acpi_device_id ids[2] = {};
+ struct acpi_device *child_adev;
+
+ strlcpy(ids[0].id, cell->acpi_pnpid, sizeof(ids[0].id));
+ list_for_each_entry(child_adev, &parent_adev->children, node)
+ if (acpi_match_device_ids(child_adev, ids)) {
+ adev = child_adev;

+ break;
+ }
+ }
+

+ ACPI_COMPANION_SET(&pdev->dev, adev);
+}
+#else
+static inline void mfd_acpi_add_device(const struct mfd_cell *cell,
+ struct platform_device *pdev)
+{
+}
+#endif
+
static int mfd_add_device(struct device *parent, int id,
const struct mfd_cell *cell, atomic_t *usage_count,
struct resource *mem_base,
@@ -118,6 +156,8 @@ static int mfd_add_device(struct device *parent, int id,
}
}

+ mfd_acpi_add_device(cell, pdev);
+
if (cell->pdata_size) {
ret = platform_device_add_data(pdev,
cell->platform_data, cell->pdata_size);
diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index f543de91ce19..73e1709d4c09 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -44,6 +44,9 @@ struct mfd_cell {
*/
const char *of_compatible;

+ /* Matches ACPI PNP id, either _HID or _CID */
+ const char *acpi_pnpid;
+
/*
* These resources can be specified relative to the parent device.
* For accessing hardware you should use resources from the platform dev

Mika Westerberg

unread,

Sep 16, 2014, 7:54:34 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

Add support for matching using DT compatible string from ACPI _DSD.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
---

drivers/misc/eeprom/at25.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index 58f6cdd2551c..1a760cd966bd 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -459,11 +459,18 @@ static const struct of_device_id at25_of_match[] = {
};
MODULE_DEVICE_TABLE(of, at25_of_match);

+static const struct acpi_device_id at25_acpi_match[] = {
+ { "PRP0001" },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, at25_acpi_match);
+
static struct spi_driver at25_driver = {
.driver = {
.name = "at25",
.owner = THIS_MODULE,
.of_match_table = at25_of_match,
+ .acpi_match_table = at25_acpi_match,
},
.probe = at25_probe,
.remove = at25_remove,

Mika Westerberg

unread,

Sep 16, 2014, 7:55:41 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

Make use of device property API in this driver so that both DT and ACPI
based systems can use this driver.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
---

drivers/misc/eeprom/at25.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index 634f72929e12..58f6cdd2551c 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -18,7 +18,7 @@

#include <linux/spi/spi.h>
#include <linux/spi/eeprom.h>
-#include <linux/of.h>
+#include <linux/property.h>

/*
* NOTE: this is an *EEPROM* driver. The vagaries of product naming
@@ -301,35 +301,33 @@ static ssize_t at25_mem_write(struct memory_accessor *mem, const char *buf,

/*-------------------------------------------------------------------------*/

-static int at25_np_to_chip(struct device *dev,
- struct device_node *np,
- struct spi_eeprom *chip)
+static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip)
{
u32 val;

memset(chip, 0, sizeof(*chip));
- strncpy(chip->name, np->name, sizeof(chip->name));
+ strncpy(chip->name, "at25", sizeof(chip->name));

- if (of_property_read_u32(np, "size", &val) == 0 ||
- of_property_read_u32(np, "at25,byte-len", &val) == 0) {
+ if (device_property_read_u32(dev, "size", &val) == 0 ||
+ device_property_read_u32(dev, "at25,byte-len", &val) == 0) {
chip->byte_len = val;
} else {
dev_err(dev, "Error: missing \"size\" property\n");
return -ENODEV;
}

- if (of_property_read_u32(np, "pagesize", &val) == 0 ||
- of_property_read_u32(np, "at25,page-size", &val) == 0) {
+ if (device_property_read_u32(dev, "pagesize", &val) == 0 ||
+ device_property_read_u32(dev, "at25,page-size", &val) == 0) {
chip->page_size = (u16)val;
} else {
dev_err(dev, "Error: missing \"pagesize\" property\n");
return -ENODEV;
}

- if (of_property_read_u32(np, "at25,addr-mode", &val) == 0) {
+ if (device_property_read_u32(dev, "at25,addr-mode", &val) == 0) {
chip->flags = (u16)val;
} else {
- if (of_property_read_u32(np, "address-width", &val)) {
+ if (device_property_read_u32(dev, "address-width", &val)) {
dev_err(dev,
"Error: missing \"address-width\" property\n");
return -ENODEV;
@@ -350,7 +348,7 @@ static int at25_np_to_chip(struct device *dev,
val);
return -ENODEV;
}
- if (of_find_property(np, "read-only", NULL))
+ if (!device_get_property(dev, "read-only", NULL))
chip->flags |= EE_READONLY;
}
return 0;
@@ -360,21 +358,15 @@ static int at25_probe(struct spi_device *spi)
{
struct at25_data *at25 = NULL;
struct spi_eeprom chip;
- struct device_node *np = spi->dev.of_node;
int err;
int sr;
int addrlen;

/* Chip description */
if (!spi->dev.platform_data) {
- if (np) {
- err = at25_np_to_chip(&spi->dev, np, &chip);
- if (err)
- return err;
- } else {
- dev_err(&spi->dev, "Error: no chip description\n");
- return -ENODEV;
- }
+ err = at25_fw_to_chip(&spi->dev, &chip);
+ if (err)
+ return err;
} else
chip = *(struct spi_eeprom *)spi->dev.platform_data;

Mika Westerberg

unread,

Sep 16, 2014, 7:56:05 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

Allow the driver to probe from ACPI namespace.

Signed-off-by: Aaron Lu <aaro...@intel.com>

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
---

drivers/input/keyboard/gpio_keys_polled.c | 7 +++++++

1 file changed, 7 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 9afd9a6c43f4..89bf5732d200 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -189,6 +189,12 @@ static const struct of_device_id gpio_keys_polled_of_match[] = {
};
MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);

+static const struct acpi_device_id gpio_keys_polled_acpi_match[] = {

+ { "PRP0001" }, /* Device Tree shoehorned into ACPI */
+ { },
+};

+MODULE_DEVICE_TABLE(acpi, gpio_keys_polled_acpi_match);
+

static int gpio_keys_polled_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;

@@ -317,6 +323,7 @@ static struct platform_driver gpio_keys_polled_driver = {
.name = DRV_NAME,
.owner = THIS_MODULE,
.of_match_table = gpio_keys_polled_of_match,
+ .acpi_match_table = gpio_keys_polled_acpi_match,
},
};
module_platform_driver(gpio_keys_polled_driver);

Mika Westerberg

unread,

Sep 16, 2014, 7:56:37 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

From: Aaron Lu <aaro...@intel.com>

GPIO descriptors are the preferred way over legacy GPIO numbers
nowadays. Convert the driver to use GPIO descriptors internally but
still allow passing legacy GPIO numbers from platform data to support
existing platforms.

Signed-off-by: Aaron Lu <aaro...@intel.com>
Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
---

drivers/input/keyboard/gpio_keys_polled.c | 39 +++++++++++++++++++++----------
include/linux/gpio_keys.h | 3 +++
2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 432d36395f35..b7a514ced509 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -23,6 +23,7 @@
#include <linux/ioport.h>
#include <linux/platform_device.h>
#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/gpio_keys.h>
#include <linux/of.h>
#include <linux/of_platform.h>
@@ -51,15 +52,14 @@ static void gpio_keys_polled_check_state(struct input_dev *input,
int state;

if (bdata->can_sleep)
- state = !!gpio_get_value_cansleep(button->gpio);
+ state = !!gpiod_get_value_cansleep(button->gpiod);
else
- state = !!gpio_get_value(button->gpio);
+ state = !!gpiod_get_value(button->gpiod);

if (state != bdata->last_state) {
unsigned int type = button->type ?: EV_KEY;

- input_event(input, type, button->code,
- !!(state ^ button->active_low));
+ input_event(input, type, button->code, state);
input_sync(input);
bdata->count = 0;
bdata->last_state = state;
@@ -259,7 +259,6 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
for (i = 0; i < pdata->nbuttons; i++) {
struct gpio_keys_button *button = &pdata->buttons[i];
struct gpio_keys_button_data *bdata = &bdev->data[i];
- unsigned int gpio = button->gpio;
unsigned int type = button->type ?: EV_KEY;

if (button->wakeup) {
@@ -267,15 +266,31 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
return -EINVAL;
}

- error = devm_gpio_request_one(&pdev->dev, gpio, GPIOF_IN,
- button->desc ? : DRV_NAME);
- if (error) {
- dev_err(dev, "unable to claim gpio %u, err=%d\n",
- gpio, error);
- return error;
+ /*
+ * Legacy GPIO number so request the GPIO here and
+ * convert it to descriptor.
+ */
+ if (!button->gpiod && gpio_is_valid(button->gpio)) {
+ unsigned flags = 0;
+
+ if (button->active_low)
+ flags |= GPIOF_ACTIVE_LOW;
+
+ error = devm_gpio_request_one(&pdev->dev, button->gpio,
+ flags, button->desc ? : DRV_NAME);
+ if (error) {
+ dev_err(dev, "unable to claim gpio %u, err=%d\n",
+ button->gpio, error);
+ return error;
+ }
+
+ button->gpiod = gpio_to_desc(button->gpio);
}

- bdata->can_sleep = gpio_cansleep(gpio);
+ if (IS_ERR(button->gpiod))
+ return PTR_ERR(button->gpiod);
+
+ bdata->can_sleep = gpiod_cansleep(button->gpiod);
bdata->last_state = -1;
bdata->threshold = DIV_ROUND_UP(button->debounce_interval,
pdata->poll_interval);
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index 8b622468952c..ee2d8c6f9130 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -2,6 +2,7 @@
#define _GPIO_KEYS_H

struct device;
+struct gpio_desc;

/**
* struct gpio_keys_button - configuration parameters
@@ -17,6 +18,7 @@ struct device;
* disable button via sysfs
* @value: axis value for %EV_ABS
* @irq: Irq number in case of interrupt keys
+ * @gpiod: GPIO descriptor
*/
struct gpio_keys_button {
unsigned int code;
@@ -29,6 +31,7 @@ struct gpio_keys_button {
bool can_disable;
int value;
unsigned int irq;
+ struct gpio_desc *gpiod;
};

/**

Mika Westerberg

unread,

Sep 16, 2014, 7:56:49 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

GPIO descriptors are the preferred way over legacy GPIO numbers
nowadays. Convert the driver to use GPIO descriptors internally but
still allow passing legacy GPIO numbers from platform data to support
existing platforms.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
---

drivers/leds/leds-gpio.c | 80 +++++++++++++++++++++++++++---------------------
include/linux/leds.h | 1 +
2 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 57ff20fecf57..c84e913527f0 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -13,6 +13,7 @@
#include <linux/kernel.h>

#include <linux/platform_device.h>
#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>

#include <linux/leds.h>
#include <linux/of.h>
#include <linux/of_platform.h>
@@ -24,11 +25,10 @@

struct gpio_led_data {
struct led_classdev cdev;

- unsigned gpio;
+ struct gpio_desc *gpiod;
struct work_struct work;
u8 new_level;
u8 can_sleep;
- u8 active_low;
u8 blinking;
int (*platform_gpio_blink_set)(unsigned gpio, int state,
unsigned long *delay_on, unsigned long *delay_off);
@@ -40,12 +40,16 @@ static void gpio_led_work(struct work_struct *work)
container_of(work, struct gpio_led_data, work);

if (led_dat->blinking) {
- led_dat->platform_gpio_blink_set(led_dat->gpio,
- led_dat->new_level,
- NULL, NULL);
+ int gpio = desc_to_gpio(led_dat->gpiod);
+ int level = led_dat->new_level;
+
+ if (gpiod_is_active_low(led_dat->gpiod))
+ level = !level;
+
+ led_dat->platform_gpio_blink_set(gpio, level, NULL, NULL);
led_dat->blinking = 0;
} else
- gpio_set_value_cansleep(led_dat->gpio, led_dat->new_level);
+ gpiod_set_value_cansleep(led_dat->gpiod, led_dat->new_level);
}

static void gpio_led_set(struct led_classdev *led_cdev,
@@ -60,9 +64,6 @@ static void gpio_led_set(struct led_classdev *led_cdev,
else
level = 1;

- if (led_dat->active_low)
- level = !level;
-
/* Setting GPIOs with I2C/etc requires a task context, and we don't
* seem to have a reliable way to know if we're already in one; so
* let's just assume the worst.
@@ -72,11 +73,16 @@ static void gpio_led_set(struct led_classdev *led_cdev,
schedule_work(&led_dat->work);
} else {
if (led_dat->blinking) {
- led_dat->platform_gpio_blink_set(led_dat->gpio, level,
- NULL, NULL);
+ int gpio = desc_to_gpio(led_dat->gpiod);
+
+ if (gpiod_is_active_low(led_dat->gpiod))
+ level = !level;
+
+ led_dat->platform_gpio_blink_set(gpio, level, NULL,
+ NULL);
led_dat->blinking = 0;
} else
- gpio_set_value(led_dat->gpio, level);
+ gpiod_set_value(led_dat->gpiod, level);
}
}

@@ -85,9 +91,10 @@ static int gpio_blink_set(struct led_classdev *led_cdev,
{
struct gpio_led_data *led_dat =
container_of(led_cdev, struct gpio_led_data, cdev);
+ int gpio = desc_to_gpio(led_dat->gpiod);

led_dat->blinking = 1;
- return led_dat->platform_gpio_blink_set(led_dat->gpio, GPIO_LED_BLINK,
+ return led_dat->platform_gpio_blink_set(gpio, GPIO_LED_BLINK,
delay_on, delay_off);
}

@@ -97,24 +104,33 @@ static int create_gpio_led(const struct gpio_led *template,
{
int ret, state;

- led_dat->gpio = -1;
+ if (!template->gpiod) {
+ unsigned long flags = 0;

- /* skip leds that aren't available */
- if (!gpio_is_valid(template->gpio)) {
- dev_info(parent, "Skipping unavailable LED gpio %d (%s)\n",
- template->gpio, template->name);
- return 0;
- }
+ /* skip leds that aren't available */
+ if (!gpio_is_valid(template->gpio)) {
+ dev_info(parent, "Skipping unavailable LED gpio %d (%s)\n",
+ template->gpio, template->name);
+ return 0;
+ }

- ret = devm_gpio_request(parent, template->gpio, template->name);
- if (ret < 0)
- return ret;
+ if (template->active_low)

+ flags |= GPIOF_ACTIVE_LOW;
+

+ ret = devm_gpio_request_one(parent, template->gpio, flags,
+ template->name);
+ if (ret < 0)
+ return ret;
+
+ led_dat->gpiod = gpio_to_desc(template->gpio);
+ if (IS_ERR(led_dat->gpiod))
+ return PTR_ERR(led_dat->gpiod);
+ }

led_dat->cdev.name = template->name;
led_dat->cdev.default_trigger = template->default_trigger;
- led_dat->gpio = template->gpio;
- led_dat->can_sleep = gpio_cansleep(template->gpio);
- led_dat->active_low = template->active_low;
+ led_dat->gpiod = template->gpiod;
+ led_dat->can_sleep = gpiod_cansleep(template->gpiod);
led_dat->blinking = 0;
if (blink_set) {
led_dat->platform_gpio_blink_set = blink_set;
@@ -122,30 +138,24 @@ static int create_gpio_led(const struct gpio_led *template,
}
led_dat->cdev.brightness_set = gpio_led_set;
if (template->default_state == LEDS_GPIO_DEFSTATE_KEEP)
- state = !!gpio_get_value_cansleep(led_dat->gpio) ^ led_dat->active_low;
+ state = !!gpiod_get_value_cansleep(led_dat->gpiod);
else
state = (template->default_state == LEDS_GPIO_DEFSTATE_ON);
led_dat->cdev.brightness = state ? LED_FULL : LED_OFF;
if (!template->retain_state_suspended)
led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;

- ret = gpio_direction_output(led_dat->gpio, led_dat->active_low ^ state);
+ ret = gpiod_direction_output(led_dat->gpiod, state);
if (ret < 0)
return ret;

INIT_WORK(&led_dat->work, gpio_led_work);

- ret = led_classdev_register(parent, &led_dat->cdev);
- if (ret < 0)
- return ret;
-
- return 0;
+ return led_classdev_register(parent, &led_dat->cdev);

}

static void delete_gpio_led(struct gpio_led_data *led)

{
- if (!gpio_is_valid(led->gpio))
- return;
led_classdev_unregister(&led->cdev);
cancel_work_sync(&led->work);
}
diff --git a/include/linux/leds.h b/include/linux/leds.h
index e43686472197..879a113b3d57 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -246,6 +246,7 @@ struct led_platform_data {
struct gpio_led {
const char *name;
const char *default_trigger;
+ struct gpio_desc *gpiod;
unsigned gpio;
unsigned active_low : 1;
unsigned retain_state_suspended : 1;

Mika Westerberg

unread,

Sep 16, 2014, 7:57:13 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

Some drivers need to deal with only firmware representation of its
GPIOs. An example would be a GPIO button array driver where each button
is described as a separate firmware node in device tree. Typically these
child nodes do not have physical representation in the Linux device
model.

In order to help device drivers to handle such firmware child nodes we
add dev[m]_node_get_named_gpiod() that takes a firmware node pointer as
parameter, finds the GPIO using whatever is the underlying firmware
method, and requests the GPIO properly.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
---

drivers/gpio/devres.c | 35 +++++++++++++++++++++++++++
drivers/gpio/gpiolib.c | 55 +++++++++++++++++++++++++++++++++++++++++++
include/linux/gpio/consumer.h | 7 ++++++
3 files changed, 97 insertions(+)

diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c
index 954b9f6b0ef8..1556a251fc8e 100644
--- a/drivers/gpio/devres.c
+++ b/drivers/gpio/devres.c
@@ -109,6 +109,41 @@ struct gpio_desc *__must_check __devm_gpiod_get_index(struct device *dev,
EXPORT_SYMBOL(__devm_gpiod_get_index);

/**
+ * devm_node_get_named_gpiod - resource-managed dev_node_get_named_gpiod()
+ * @dev: GPIO consumer
+ * @fdn: firmware device node
+ * @propname: name of the firmware property
+ * @idx: index of the GPIO in the property value in case of many
+ *
+ * Managed dev_node_get_named_gpiod(). GPIO descriptors returned from
+ * this function are automatically disposed on driver detach.
+ */
+struct gpio_desc *devm_node_get_named_gpiod(struct device *dev,
+ struct fw_dev_node *fdn,
+ const char *propname, int index)
+{
+ struct gpio_desc **dr;

+ struct gpio_desc *desc;
+

+ dr = devres_alloc(devm_gpiod_release, sizeof(struct gpio_desc *),
+ GFP_KERNEL);
+ if (!dr)
+ return ERR_PTR(-ENOMEM);
+
+ desc = dev_node_get_named_gpiod(fdn, propname, index);
+ if (IS_ERR(desc)) {
+ devres_free(dr);
+ return desc;
+ }
+
+ *dr = desc;
+ devres_add(dev, dr);
+
+ return desc;
+}
+EXPORT_SYMBOL(devm_node_get_named_gpiod);
+
+/**
* devm_gpiod_get_index_optional - Resource-managed gpiod_get_index_optional()
* @dev: GPIO consumer
* @con_id: function within the GPIO consumer
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 444d43c9fd3e..d364214d2946 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1717,6 +1717,61 @@ struct gpio_desc *__must_check __gpiod_get_index(struct device *dev,
EXPORT_SYMBOL_GPL(__gpiod_get_index);

/**
+ * dev_node_get_named_gpiod - obtain a GPIO from firmware device node
+ * @fdn: firmware device node
+ * @propname: name of the firmware property
+ * @idx: index of the GPIO in the property value in case of many
+ *
+ * This function can be used for drivers that get their configuration
+ * from firmware in such way that there is not always corresponding
+ * physical device pointer available. For example some properties are
+ * described as a child nodes for the parent device in DT or ACPI.
+ *
+ * Function properly finds the corresponding GPIO using whatever is the
+ * underlying firmware interface and then makes sure that the GPIO
+ * descriptor is requested before it is returned to the caller.
+ *
+ * In case of error an ERR_PTR() is returned.
+ */
+struct gpio_desc *dev_node_get_named_gpiod(struct fw_dev_node *fdn,
+ const char *propname, int index)
+{
+ struct gpio_desc *desc = ERR_PTR(-ENODEV);
+ struct acpi_device *adev = fdn->acpi_node;
+ struct device_node *np = fdn->of_node;

+ bool active_low = false;

+ int ret;
+
+ if (IS_ENABLED(CONFIG_OF) && np) {
+ enum of_gpio_flags flags;
+
+ desc = of_get_named_gpiod_flags(np, propname, index, &flags);
+ if (!IS_ERR(desc))
+ active_low = flags & OF_GPIO_ACTIVE_LOW;
+ } else if (IS_ENABLED(CONFIG_ACPI) && adev) {
+ struct acpi_gpio_info info;
+
+ desc = acpi_get_gpiod_by_index(adev, propname, index, &info);
+ if (!IS_ERR(desc))
+ active_low = info.active_low;
+ }
+

+ if (IS_ERR(desc))
+ return desc;
+

+ ret = gpiod_request(desc, NULL);

+ if (ret)
+ return ERR_PTR(ret);
+

+ /* Only value flag can be set from both DT and ACPI is active_low */
+ if (active_low)
+ set_bit(FLAG_ACTIVE_LOW, &desc->flags);
+
+ return desc;
+}
+EXPORT_SYMBOL_GPL(dev_node_get_named_gpiod);
+
+/**
* gpiod_get_index_optional - obtain an optional GPIO from a multi-index GPIO
* function
* @dev: GPIO consumer, can be NULL for system-global GPIOs
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 12f146fa6604..aa1b273f0e38 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -6,6 +6,7 @@
#include <linux/kernel.h>

struct device;
+struct fw_dev_node;

/**
* Opaque descriptor for a GPIO. These are obtained using gpiod_get() and are
@@ -94,6 +95,12 @@ int gpiod_to_irq(const struct gpio_desc *desc);
struct gpio_desc *gpio_to_desc(unsigned gpio);
int desc_to_gpio(const struct gpio_desc *desc);

+/* Firmware node interface */
+struct gpio_desc *dev_node_get_named_gpiod(struct fw_dev_node *fdn,
+ const char *propname, int index);
+struct gpio_desc *devm_node_get_named_gpiod(struct device *dev,
+ struct fw_dev_node *fdn,
+ const char *propname, int index);
#else /* CONFIG_GPIOLIB */

static inline struct gpio_desc *__must_check __gpiod_get(struct device *dev,

Mika Westerberg

unread,

Sep 16, 2014, 7:57:27 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

This document describes the data format and interfaces of ACPI device
specific properties.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
Signed-off-by: Darren Hart <dvh...@linux.intel.com>
---
Documentation/acpi/properties.txt | 410 ++++++++++++++++++++++++++++++++++++++
1 file changed, 410 insertions(+)
create mode 100644 Documentation/acpi/properties.txt

diff --git a/Documentation/acpi/properties.txt b/Documentation/acpi/properties.txt
new file mode 100644
index 000000000000..43404523e282
--- /dev/null
+++ b/Documentation/acpi/properties.txt
@@ -0,0 +1,410 @@
+ACPI device properties
+======================
+This document describes the format and interfaces of ACPI device
+properties as specified in "Device Properties UUID For _DSD" available
+here:
+
+http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
+
+1. Introduction
+---------------
+In systems that use ACPI and want to take advantage of device specific
+properties, there needs to be a standard way to return and extract
+name-value pairs for a given ACPI device.
+
+An ACPI device that wants to export its properties must implement a
+static name called _DSD that takes no arguments and returns a package of
+packages:
+
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"name1", <VALUE1>},
+ Package () {"name2", <VALUE2>}
+ }
+ })
+
+The UUID identifies contents of the following package. In case of ACPI
+device properties it is daffd814-6eba-4d8c-8a91-bc9bbf4aa301.
+
+In each returned package, the first item is the name and must be a string.
+The corresponding value can be a string, integer, reference, or package. If
+a package it may only contain strings, integers, and references.
+
+An example device where we might need properties is a device that uses
+GPIOs. In addition to the GpioIo/GpioInt resources the driver needs to
+know which GPIO is used for which purpose.
+
+To solve this we add the following ACPI device properties to the device:
+
+ Device (DEV0)
+ {
+ Name (_CRS, ResourceTemplate () {
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ "\\_SB.PCI0.LPC", 0, ResourceConsumer) {0}
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ "\\_SB.PCI0.LPC", 0, ResourceConsumer) {1}
+ ...
+ })
+
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"reset-gpio", {^DEV0, 0, 0, 0}},
+ Package () {"shutdown-gpio", {^DEV0, 1, 0, 0}},
+ }
+ })
+ }
+
+Now the device driver can reference the GPIOs using names instead of
+using indexes.
+
+If there is an existing Device Tree binding for a device, it is expected
+that the same bindings are used with ACPI properties, so that the driver
+dealing with the device needs only minor modifications if any.
+
+2. Formal definition of properties
+----------------------------------
+The following chapters define the currently supported properties. For
+these there exists a helper function that can be used to extract the
+property value.
+
+2.1 Integer types
+-----------------
+ACPI integers are always 64-bit. However, for drivers the full range is
+typically not needed so we provide a set of functions which convert the
+64-bit integer to a smaller Linux integer type.
+
+An integer property looks like this:
+
+ Package () {"i2c-sda-hold-time-ns", 300},
+ Package () {"clock-frequency", 400000},
+
+To read a property value, use a unified property accessor as shown
+below:
+
+ u32 val;
+ int ret;
+
+ ret = device_property_read_u32(dev, "clock-frequency", &val);
+ if (ret)
+ /* Handle error */
+
+The function returns 0 if the property is copied to 'val' or negative
+errno if something went wrong (or the property does not exist).
+
+2.2 Integer arrays
+------------------
+An integer array is a package holding only integers. Arrays can be used to
+represent different things like Linux input key codes to GPIO mappings, pin
+control settings, dma request lines, etc.
+
+An integer array looks like this:
+
+ Package () {
+ "max8952,dvs-mode-microvolt",
+ Package () {
+ 1250000,
+ 1200000,
+ 1050000,
+ 950000,
+ }
+ }
+
+The above array property can be accessed like:
+
+ u32 voltages[4];
+ int ret;
+
+ ret = device_property_read_u32_array(dev, "max8952,dvs-mode-microvolt",
+ voltages, ARRAY_SIZE(voltages));
+ if (ret)
+ /* Handle error */
+
+
+All functions copy the resulting values cast to a requested type to the
+caller supplied array. If you pass NULL in the value pointer ('voltages' in
+this case), the function returns number of items in the array. This can be
+useful if caller does not know size of the array beforehand.
+
+2.3 Strings
+-----------
+String properties can be used to describe many things like labels for GPIO
+buttons, compability ids, etc.
+
+A string property looks like this:
+
+ Package () {"pwm-names", "backlight"},
+ Package () {"label", "Status-LED"},
+
+You can use device_property_read_string() to extract strings:
+
+ const char *val;
+ int ret;
+
+ ret = device_property_read_string(dev, "label", &val);
+ if (ret)
+ /* Handle error */
+
+Note that the function does not copy the returned string but instead the
+value is modified to point to the string property itself.
+
+The memory is owned by the associated ACPI device object and released
+when it is removed. The user need not free the associated memory.
+
+2.4 String arrays
+-----------------
+String arrays can be useful in describing a list of labels, names for
+DMA channels, etc.
+
+A string array property looks like this:
+
+ Package () {"dma-names", Package () {"tx", "rx", "rx-tx"}},
+ Package () {"clock-output-names", Package () {"pll", "pll-switched"}},
+
+And these can be read in similar way that the integer arrrays:
+
+ const char *dma_names[3];
+ int ret;
+
+ ret = device_property_read_string_array(dev, "dma-names", dma_names,
+ ARRAY_SIZE(dma_names));
+ if (ret)
+ /* Handle error */
+
+The memory management rules follow what is specified for single strings.
+Specifically the returned pointers should be treated as constant and not to
+be freed. That is done automatically when the correspondig ACPI device
+object is released.
+
+2.5 Object references
+---------------------
+An ACPI object reference is used to refer to some object in the
+namespace. For example, if a device has dependencies with some other
+object, an object reference can be used.
+
+An object reference looks like this:
+
+ Package () {"dev0", \_SB.DEV0},
+
+At the time of writing this, there is no unified device_property_* accessor
+for references so one needs to use the following ACPI helper function:
+
+ int acpi_dev_get_property_reference(struct acpi_device *adev,
+ const char *name,
+ const char *size_prop, int index,
+ struct acpi_reference_args *args);
+
+The referenced ACPI device is returned in args->adev if found.
+
+In addition to simple object references it is also possible to have object
+references with arguments. These are represented in ASL as follows:
+
+ Device (\_SB.PCI0.PWM)
+ {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"#pwm-cells", 2}
+ }
+ })
+ }
+
+ Device (\_SB.PCI0.BL)
+ {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {
+ "pwms",
+ Package () {
+ \_SB.PCI0.PWM, 0, 5000000,
+ \_SB.PCI0.PWM, 1, 4500000,
+ }
+ }
+ }
+ })
+ }
+
+In the above example, the referenced device declares a property that
+returns the number of expected arguments (here it is "#pwm-cells"). If
+no such property is given we assume that all the integers following the
+reference are arguments.
+
+In the above example PWM device expects 2 additional arguments. This
+will be validated by the ACPI property core.
+
+The additional arguments must be integers. Nothing else is supported.
+
+It is possible, as in the above example, to have multiple references
+with varying number of integer arguments. It is up to the referenced
+device to declare how many arguments it expects. The 'index' parameter
+selects which reference is returned.
+
+One can use acpi_dev_get_property_reference() as well to extract the
+information in additional parameters:
+
+ struct acpi_reference_args args;
+ struct acpi_device *adev = /* this will point to the BL device */
+ int ret;
+
+ /* extract the first reference */
+ acpi_dev_get_property_reference(adev, "pwms", "#pwm-cells", 0, &args);
+
+ BUG_ON(args.nargs != 2);
+ BUG_ON(args.args[0] != 0);
+ BUG_ON(args.args[1] != 5000000);
+
+ /* extract the second reference */
+ acpi_dev_get_property_reference(adev, "pwms", "#pwm-cells", 1, &args);
+
+ BUG_ON(args.nargs != 2);
+ BUG_ON(args.args[0] != 1);
+ BUG_ON(args.args[1] != 4500000);
+
+In addition to arguments, args.adev now points to the ACPI device that
+corresponds to \_SB.PCI0.PWM.
+
+It is intended that this function is not used directly but instead
+subsystems like pwm implement their ACPI support on top of this function
+in such way that it is hidden from the client drivers, such as via
+pwm_get().
+
+3. Device property hierarchies
+------------------------------
+Devices are organized in a tree within the Linux kernel. It follows that
+the configuration data would also be hierarchical. In order to reach
+equivalence with Device Tree, the ACPI mechanism must also provide some
+sort of tree-like representation. Fortunately, the ACPI namespace is
+already such a structure.
+
+For example, we could have the following device in ACPI namespace. The
+KEYS device is much like gpio_keys_polled.c in that it includes "pseudo"
+devices for each GPIO:
+
+ Device (KEYS)
+ {
+ Name (_CRS, ResourceTemplate () {
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ "\\_SB.PCI0.LPC", 0, ResourceConsumer) {0}
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ "\\_SB.PCI0.LPC", 0, ResourceConsumer) {1}
+ ...
+ })
+
+ // "pseudo" devices declared under the parent device
+ Device (BTN0) {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"label", "minnow_btn0"}
+ Package () {"gpios", Package () {^KEYS, 0, 0, 1}}
+ }
+ })
+ }
+
+ Device (BTN1) {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"label", "minnow_btn1"}
+ Package () {"gpios", Package () {^KEYS, 1, 0, 1}}
+ }
+ })
+ }
+ }
+
+We can extract the above in gpio_keys_polled.c like:
+
+ static int gpio_keys_polled_create_button(struct fw_dev_node *fdn,
+ void *data)
+ {
+ struct button_data *bdata = data;
+ const char *label = NULL;
+
+ /*
+ * We need to use dev_node_ variant here to access the
+ * firmware properties.
+ */
+ dev_node_property_read_string(fdn, "label", &label);
+ /* and so on */
+ }
+
+ static void gpio_keys_polled_probe(struct device *dev)
+ {
+ /* Properties for the KEYS device itself */
+ device_property_read(dev, ...);
+
+ /*
+ * Iterate over button devices and extract their
+ * firmware configuration.
+ */
+ ret = device_for_each_child_node(dev, gpio_keys_polled_create_button,
+ &bdata);
+ if (ret)
+ /* Handle error */
+ }
+
+Note that you still need proper error handling which is omitted in the
+above example.
+
+4. Existing Device Tree enabled drivers
+---------------------------------------
+At the time of writing this, there are ~250 existing DT enabled drivers.
+Allocating _HID/_CID for each would not be feasible. To make sure that
+those drivers can still be used on ACPI systems, we provide an
+alternative way to get these matched.
+
+There is a special _HID "PRP0001" which means that use the DT bindings
+for matching this device to a driver. The driver needs to have
+.of_match_table filled in even when !CONFIG_OF.
+
+An example device would be leds that can be controlled via GPIOs. This
+is represented as "leds-gpio" device and looks like this in the ACPI
+namespace:
+
+ Device (LEDS)
+ {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"compatible", Package () {"gpio-leds"}},
+ }
+ })
+ ...
+ }
+
+In order to get the existing drivers/leds/leds-gpio.c bound to this
+device, we take advantage of "PRP0001":
+
+ /* Following already exists in the driver */
+ static const struct of_device_id of_gpio_leds_match[] = {
+ { .compatible = "gpio-leds", },
+ {},
+ };
+ MODULE_DEVICE_TABLE(of, of_gpio_leds_match);
+
+ /* This we add to the driver to get it probed */
+ static const struct acpi_device_id acpi_gpio_leds_match[] = {

+ { "PRP0001" }, /* Device Tree shoehorned into ACPI */
+ {},
+ };

+ MODULE_DEVICE_TABLE(acpi, acpi_gpio_leds_match);
+
+ static struct platform_driver gpio_led_driver = {
+ .driver = {
+ /*
+ * No of_match_ptr() here because we want this
+ * table to be visible even when !CONFIG_OF to
+ * match against "compatible" in _DSD.
+ */
+ .of_match_table = of_gpio_leds_match,
+ .acpi_match_table = acpi_gpio_leds_match,
+ },
+ };
+
+Once ACPI core sees "PRP0001" and that the device has "compatible"
+property it will do the match using .of_match_table instead.
+
+It is preferred that new devices get a proper _HID allocated for them
+instead of inventing new DT "compatible" devices.

Mika Westerberg

unread,

Sep 16, 2014, 8:28:47 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

This is a second revision of the patches first submitted here [1].

The recent publication of the ACPI 5.1 specification [2] adds a reserved name
for Device Specific Data (_DSD, Section 6.2.5). This mechanism allows for
passing arbitrary hardware description data to the OS. The exact format of the
_DSD data is specific to the UUID paired with it [3].

An ACPI Device Properties UUID has been defined [4] to provide a format
compatible with existing device tree schemas. The purpose for this was to
allow for the reuse of the existing schemas and encourage the development
of firmware agnostic device drivers.

This series accomplishes the following (as well as some other dependencies):

* Add _DSD support to the ACPI core
This simply reads the UUID and the accompanying Package

* Add ACPI Device Properties _DSD format support
This understands the hierarchical key:value pair structure
defined by the Device Properties UUID

* Add a unified device properties API with ACPI and OF backends
This provides for the firmware agnostic device properties
Interface to be used by drivers

* Provides 3 example drivers that were previously Device Tree aware that
can now be used with either Device Tree or ACPI Device Properties. The
drivers use "PRP0001" as their _HID which means that the match should be
done using driver's .of_match_table instead.

The patch series has been tested on Minnoboard and Minnowboard MAX and the
relevant part of DSDTs are at the end of this cover letter.

This series does not provide for a means to append to a system DSDT. That
will ultimately be required to make the most effective use of the _DSD
mechanism. Work is underway on that as a separate effort.

Most important changes to the previous RFC version:

* Added wrapper functions for most used property types
* Return -EOVERFLOW in case integer would not fit to a type
* Dropped dev_prop_ops
* We now have dev_node_xxx() functions to access firmware node
properties without dev pointer
* The accessor function names try to be close to their corresponding of_*
counterpart
* Tried to have a bit better examples in the documentation patch
* gpiolib got support for _DSD and also it now understand firmware node
properties with dev_node_get_named_gpiod() that requests the GPIO
properly.
* Support for "PRP0001" _HID/_CID. This means that the match should be
done using driver .of_match_table instead.
* Add unified property support for at25 SPI eeprom driver as well.

[1] https://lkml.org/lkml/2014/8/17/10
[2] http://www.uefi.org/sites/default/files/resources/ACPI_5_1release.pdf
[3] http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm
[4] http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf

Aaron Lu (2):
input: gpio_keys_polled - Add support for GPIO descriptors
input: gpio_keys_polled - Make use of device property API

Max Eliaser (2):
leds: leds-gpio: Make use of device property API
leds: leds-gpio: Add ACPI probing support

Mika Westerberg (11):
ACPI: Add support for device specific properties
ACPI: Allow drivers to match using Device Tree compatible property
ACPI: Document ACPI device specific properties
mfd: Add ACPI support
gpio / ACPI: Add support for _DSD device properties
gpio: Add support for unified device properties interface
gpio: sch: Consolidate core and resume banks
leds: leds-gpio: Add support for GPIO descriptors
input: gpio_keys_polled - Add ACPI probing support
misc: at25: Make use of device property API
misc: at25: Add ACPI probing support

Rafael J. Wysocki (1):
Driver core: Unified device properties interface for platform firmware

Documentation/acpi/enumeration.txt | 27 ++
Documentation/acpi/properties.txt | 410 +++++++++++++++++++++
drivers/acpi/Makefile | 1 +
drivers/acpi/internal.h | 6 +
drivers/acpi/property.c | 584 ++++++++++++++++++++++++++++++
drivers/acpi/scan.c | 93 ++++-

drivers/base/Makefile | 2 +-
drivers/base/property.c | 196 ++++++++++

create mode 100644 drivers/base/property.c
create mode 100644 include/linux/property.h

DSDT modifications for Minnowboard (for leds-gpio.c and gpio_keys_polled.c)
---------------------------------------------------------------------------

Scope (\_SB.PCI0.LPC)
{
Device (LEDS)
{
Name (_HID, "PRP0001")

Name (_CRS, ResourceTemplate () {
GpioIo (Exclusive, PullDown, 0, 0, IoRestrictionOutputOnly,
"\\_SB.PCI0.LPC", 0, ResourceConsumer) {10}
GpioIo (Exclusive, PullDown, 0, 0, IoRestrictionOutputOnly,
"\\_SB.PCI0.LPC", 0, ResourceConsumer) {11}

})

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {

Package () {"compatible", Package () {"gpio-leds"}},
}

})

Device (LEDH)
{

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {

Package () {"label", "Heartbeat"},
Package () {"gpios", Package () {^^LEDS, 0, 0, 0}},
Package () {"linux,default-trigger", "heartbeat"},
Package () {"linux,default-state", "off"},
Package () {"linux,retain-state-suspended", 1},
}
})
}

Device (LEDM)
{

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {

Package () {"label", "MMC0 Activity"},
Package () {"gpios", Package () {^^LEDS, 1, 0, 0}},
Package () {"linux,default-trigger", "mmc0"},
Package () {"linux,default-state", "off"},
Package () {"linux,retain-state-suspended", 1},
}
})
}
}

Device (BTNS)
{
Name (_HID, "PRP0001")

Name (_CRS, ResourceTemplate () {

GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,

"\\_SB.PCI0.LPC", 0, ResourceConsumer) {0, 1, 2, 3}

})

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {

Package () {"compatible", Package () {"gpio-keys-polled"}},
Package () {"poll-interval", 100},
Package () {"autorepeat", 1}
}
})

Device (BTN0)
{

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {

Package () {"linux,code", 105},
Package () {"linux,input-type", 1},
Package () {"gpios", Package () {^^BTNS, 0, 0, 1}},
}
})
}

Device (BTN1)
{

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {

Package () {"linux,code", 108},
Package () {"linux,input-type", 1},
Package () {"gpios", Package (4) {^^BTNS, 0, 1, 1}},
}
})
}

Device (BTN2)
{

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {

Package () {"linux,code", 103},
Package () {"linux,input-type", 1},
Package () {"gpios", Package () {^^BTNS, 0, 2, 1}},
}
})
}

Device (BTN3)
{

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package ()
{

Package () {"linux,code", 106},
Package () {"linux,input-type", 1},
Package () {"gpios", Package (4) {^^BTNS, 0, 3, 1}},
}
})
}
}
}

DSDT modifications for Minnowboard MAX (for at25.c)
---------------------------------------------------

Scope (\_SB.SPI1)
{
Device (AT25)
{
Name (_HID, "PRP0001")
Method (_CRS, 0, Serialized) {
Name (UBUF, ResourceTemplate () {
SpiSerialBus (0x0000, PolarityLow, FourWireMode, 0x08,
ControllerInitiated, 0x007A1200, ClockPolarityLow,
ClockPhaseSecond, "\\_SB.SPI1",
0x00, ResourceConsumer)
})
Return (UBUF)

}

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {

Package () {"compatible", Package () {"atmel,at25"}},
Package () {"size", 1024},
Package () {"pagesize", 32},
Package () {"address-width", 16},
}
})

Method (_STA, 0, NotSerialized)
{
Return (0xF)

Mika Westerberg

unread,

Sep 16, 2014, 8:28:58 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

We have lots of existing Device Tree enabled drivers and allocating
separate _HID for each is not feasible. Instead we allocate special _HID
"PRP0001" that means that the match should be done using Device Tree
compatible property using driver's .of_match_table instead.

If there is a need to distinguish from where the device is enumerated
(DT/ACPI) driver can check dev->of_node or ACPI_COMPATION(dev).

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
---

drivers/acpi/property.c | 34 ++++++++++++++++++
drivers/acpi/scan.c | 91 +++++++++++++++++++++++++++++++++++++++++++------
include/acpi/acpi_bus.h | 1 +
include/linux/acpi.h | 8 ++---
4 files changed, 118 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index e50964012da8..c6c3ab6f68c3 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -76,6 +76,37 @@ static bool acpi_properties_format_valid(const union acpi_object *properties)
return true;
}

+static void acpi_init_of_compatible(struct acpi_device *adev)
+{
+ const union acpi_object *of_compatible;
+ struct acpi_hardware_id *hwid;
+ bool acpi_of = false;
+
+ /*
+ * Check if the special PRP0001 ACPI ID is present and in that
+ * case we fill in Device Tree compatible properties for this
+ * device.
+ */
+ list_for_each_entry(hwid, &adev->pnp.ids, list) {
+ if (!strcmp(hwid->id, "PRP0001")) {
+ acpi_of = true;

+ break;
+ }
+ }
+

+ if (!acpi_of)
+ return;
+
+ if (acpi_dev_get_property_array(adev, "compatible", ACPI_TYPE_STRING,
+ &of_compatible)) {
+ acpi_handle_warn(adev->handle,
+ "PRP0001 requires compatible property\n");
+ return;
+ }
+
+ adev->data.of_compatible = of_compatible;
+}
+
void acpi_init_properties(struct acpi_device *adev)
{
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
@@ -119,6 +150,8 @@ void acpi_init_properties(struct acpi_device *adev)

adev->data.pointer = buf.pointer;
adev->data.properties = properties;
+
+ acpi_init_of_compatible(adev);
return;
}

@@ -130,6 +163,7 @@ void acpi_init_properties(struct acpi_device *adev)
void acpi_free_properties(struct acpi_device *adev)
{
ACPI_FREE((void *)adev->data.pointer);
+ adev->data.of_compatible = NULL;
adev->data.pointer = NULL;
adev->data.properties = NULL;
}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index f4aabf9c0aad..0c8e751cbe6b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -124,17 +124,43 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias,
if (list_empty(&acpi_dev->pnp.ids))
return 0;

- len = snprintf(modalias, size, "acpi:");
- size -= len;
+ /*
+ * If the device has PRP0001 we expose DT compatible modalias
+ * instead.
+ */
+ if (acpi_dev->data.of_compatible) {
+ const union acpi_object *of_compatible, *obj;
+ int i;
+
+ len = snprintf(modalias, size, "of:Nprp0001Tacpi");
+
+ of_compatible = acpi_dev->data.of_compatible;
+ for (i = 0; i < of_compatible->package.count; i++) {
+ obj = &of_compatible->package.elements[i];

- list_for_each_entry(id, &acpi_dev->pnp.ids, list) {
- count = snprintf(&modalias[len], size, "%s:", id->id);
- if (count < 0)
- return -EINVAL;
- if (count >= size)
- return -ENOMEM;
- len += count;
- size -= count;
+ count = snprintf(&modalias[len], size, "C%s",
+ obj->string.pointer);
+ if (count < 0)
+ return -EINVAL;
+ if (count >= size)
+ return -ENOMEM;
+
+ len += count;
+ size -= count;
+ }
+ } else {
+ len = snprintf(modalias, size, "acpi:");
+ size -= len;
+
+ list_for_each_entry(id, &acpi_dev->pnp.ids, list) {
+ count = snprintf(&modalias[len], size, "%s:", id->id);
+ if (count < 0)
+ return -EINVAL;
+ if (count >= size)
+ return -ENOMEM;
+ len += count;
+ size -= count;
+ }
}

modalias[len] = '\0';
@@ -864,6 +890,51 @@ int acpi_match_device_ids(struct acpi_device *device,
}
EXPORT_SYMBOL(acpi_match_device_ids);

+/* Performs match for special "PRP0001" shoehorn ACPI ID */
+static bool acpi_of_driver_match_device(struct device *dev,
+ const struct device_driver *drv)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+ const union acpi_object *of_compatible;
+ int i;
+
+ /*
+ * If the ACPI device does not have corresponding compatible
+ * property or the driver in question does not have DT matching
+ * table we consider the match succesful (matches the ACPI ID).
+ */
+ of_compatible = adev->data.of_compatible;
+ if (!drv->of_match_table || !of_compatible)
+ return true;
+
+ /* Now we can look for the driver DT compatible strings */
+ for (i = 0; i < of_compatible->package.count; i++) {
+ const struct of_device_id *id;

+ const union acpi_object *obj;
+

+ obj = &of_compatible->package.elements[i];
+
+ for (id = drv->of_match_table; id->compatible[0]; id++)
+ if (!strcasecmp(obj->string.pointer, id->compatible))
+ return true;
+ }
+
+ return false;
+}
+
+bool acpi_driver_match_device(struct device *dev,
+ const struct device_driver *drv)
+{
+ const struct acpi_device_id *id;
+
+ id = acpi_match_device(drv->acpi_match_table, dev);
+ if (!id)
+ return false;
+
+ return acpi_of_driver_match_device(dev, drv);
+}
+EXPORT_SYMBOL_GPL(acpi_driver_match_device);
+
static void acpi_free_power_resources_lists(struct acpi_device *device)
{
int i;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 620b4f89c87b..65866e23e3e9 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -341,6 +341,7 @@ struct acpi_device_physical_node {
struct acpi_device_data {
const union acpi_object *pointer;
const union acpi_object *properties;
+ const union acpi_object *of_compatible;
};

/* Device */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 288abbdf2fb4..37a5280a64a5 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -424,12 +424,8 @@ extern int acpi_nvs_for_each_region(int (*func)(__u64, __u64, void *),
const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
const struct device *dev);

-static inline bool acpi_driver_match_device(struct device *dev,
- const struct device_driver *drv)
-{
- return !!acpi_match_device(drv->acpi_match_table, dev);
-}
-
+extern bool acpi_driver_match_device(struct device *dev,
+ const struct device_driver *drv);
int acpi_device_uevent_modalias(struct device *, struct kobj_uevent_env *);
int acpi_device_modalias(struct device *, char *, int);

Mika Westerberg

unread,

Sep 16, 2014, 8:29:02 AM9/16/14

to Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart, Mika Westerberg

Device Tree is used in many embedded systems to describe the system
configuration to the OS. It supports attaching properties or name-value
pairs to the devices it describe. With these properties one can pass
additional information to the drivers that would not be available
otherwise.

ACPI is another configuration mechanism (among other things) typically
seen, but not limited to, x86 machines. ACPI allows passing arbitrary
data from methods but there has not been mechanism equivalent to Device
Tree until the introduction of _DSD in the recent publication of the
ACPI 5.1 specification.

In order to facilitate ACPI usage in systems where Device Tree is
typically used, it would be beneficial to standardize a way to retrieve
Device Tree style properties from ACPI devices, which is what we do in
this patch.

If a given device described in ACPI namespace wants to export properties it
must implement _DSD method (Device Specific Data, introduced with ACPI 5.1)
that returns the properties in a package of packages. For example:

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {

Package () {"name1", <VALUE1>},
Package () {"name2", <VALUE2>},
...
}
})

The UUID reserved for properties is daffd814-6eba-4d8c-8a91-bc9bbf4aa301
and is documented in the ACPI 5.1 companion document called "_DSD
Implementation Guide" [1], [2].

We add several helper functions that can be used to extract these
properties and convert them to different Linux data types.

The ultimate goal is that we only have one device property API that
retrieves the requested properties from Device Tree or from ACPI
transparent to the caller.

[1] http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm
[2] http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf

Reviewed-by: Hanjun Guo <hanju...@linaro.org>
Reviewed-by: Josh Triplett <jo...@joshtriplett.org>
Signed-off-by: Darren Hart <dvh...@linux.intel.com>

Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
---

drivers/acpi/Makefile | 1 +
drivers/acpi/internal.h | 6 +

drivers/acpi/property.c | 364 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/acpi/scan.c | 2 +
include/acpi/acpi_bus.h | 7 +
include/linux/acpi.h | 40 ++++++
6 files changed, 420 insertions(+)
create mode 100644 drivers/acpi/property.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 505d4d79fe3e..ba2cafe18fe4 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -46,6 +46,7 @@ acpi-y += acpi_pnp.o
acpi-y += power.o
acpi-y += event.o
acpi-y += sysfs.o
+acpi-y += property.o
acpi-$(CONFIG_X86) += acpi_cmos_rtc.o
acpi-$(CONFIG_DEBUG_FS) += debugfs.o
acpi-$(CONFIG_ACPI_NUMA) += numa.o
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 4c5cf77e7576..e34290c7af9f 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -181,4 +181,10 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev);
bool acpi_osi_is_win8(void);
#endif

+/*--------------------------------------------------------------------------
+ Device properties
+ -------------------------------------------------------------------------- */
+void acpi_init_properties(struct acpi_device *adev);
+void acpi_free_properties(struct acpi_device *adev);
+
#endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
new file mode 100644
index 000000000000..c4a3e800e82c
--- /dev/null
+++ b/drivers/acpi/property.c
@@ -0,0 +1,364 @@
+/*
+ * ACPI device specific properties support.

+ *
+ * Copyright (C) 2014, Intel Corporation

+ * All rights reserved.
+ *
+ * Authors: Mika Westerberg <mika.we...@linux.intel.com>
+ * Darren Hart <dvh...@linux.intel.com>
+ * Rafael J. Wysocki <rafael.j...@intel.com>
+ *

+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+

+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/export.h>
+
+#include "internal.h"
+
+/* ACPI _DSD device properties UUID: daffd814-6eba-4d8c-8a91-bc9bbf4aa301 */
+static const u8 prp_uuid[16] = {
+ 0x14, 0xd8, 0xff, 0xda, 0xba, 0x6e, 0x8c, 0x4d,
+ 0x8a, 0x91, 0xbc, 0x9b, 0xbf, 0x4a, 0xa3, 0x01
+};
+
+static bool acpi_property_value_ok(const union acpi_object *value)
+{
+ int j;
+
+ /*
+ * The value must be an integer, a string, a reference, or a package
+ * whose every element must be an integer, a string, or a reference.
+ */
+ switch (value->type) {
+ case ACPI_TYPE_INTEGER:
+ case ACPI_TYPE_STRING:
+ case ACPI_TYPE_LOCAL_REFERENCE:
+ return true;
+
+ case ACPI_TYPE_PACKAGE:
+ for (j = 0; j < value->package.count; j++)
+ switch (value->package.elements[j].type) {
+ case ACPI_TYPE_INTEGER:
+ case ACPI_TYPE_STRING:
+ case ACPI_TYPE_LOCAL_REFERENCE:
+ continue;
+
+ default:

+ return false;
+ }
+

+ return true;
+ }
+ return false;
+}
+
+static bool acpi_properties_format_valid(const union acpi_object *properties)
+{
+ int i;
+
+ for (i = 0; i < properties->package.count; i++) {
+ const union acpi_object *property;
+
+ property = &properties->package.elements[i];
+ /*
+ * Only two elements allowed, the first one must be a string and
+ * the second one has to satisfy certain conditions.
+ */
+ if (property->package.count != 2
+ || property->package.elements[0].type != ACPI_TYPE_STRING
+ || !acpi_property_value_ok(&property->package.elements[1]))
+ return false;
+ }
+ return true;
+}
+
+void acpi_init_properties(struct acpi_device *adev)
+{
+ struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
+ const union acpi_object *desc;
+ acpi_status status;
+ int i;
+
+ status = acpi_evaluate_object_typed(adev->handle, "_DSD", NULL, &buf,
+ ACPI_TYPE_PACKAGE);
+ if (ACPI_FAILURE(status))
+ return;
+
+ desc = buf.pointer;
+ if (desc->package.count % 2)
+ goto fail;
+
+ /* Look for the device properties UUID. */
+ for (i = 0; i < desc->package.count; i += 2) {
+ const union acpi_object *uuid, *properties;
+
+ uuid = &desc->package.elements[i];
+ properties = &desc->package.elements[i + 1];
+
+ /*
+ * The first element must be a UUID and the second one must be
+ * a package.
+ */
+ if (uuid->type != ACPI_TYPE_BUFFER || uuid->buffer.length != 16
+ || properties->type != ACPI_TYPE_PACKAGE)
+ break;
+
+ if (memcmp(uuid->buffer.pointer, prp_uuid, sizeof(prp_uuid)))
+ continue;
+
+ /*
+ * We found the matching UUID. Now validate the format of the
+ * package immediately following it.
+ */
+ if (!acpi_properties_format_valid(properties))
+ break;
+
+ adev->data.pointer = buf.pointer;
+ adev->data.properties = properties;
+ return;
+ }
+
+ fail:
+ dev_warn(&adev->dev, "Returned _DSD data is not valid, skipping\n");
+ ACPI_FREE(buf.pointer);
+}
+
+void acpi_free_properties(struct acpi_device *adev)
+{
+ ACPI_FREE((void *)adev->data.pointer);
+ adev->data.pointer = NULL;
+ adev->data.properties = NULL;
+}
+
+/**
+ * acpi_dev_get_property - return an ACPI property with given name
+ * @adev: ACPI device to get property
+ * @name: Name of the property
+ * @type: Expected property type
+ * @obj: Location to store the property value (if not %NULL)
+ *
+ * Look up a property with @name and store a pointer to the resulting ACPI
+ * object at the location pointed to by @obj if found.
+ *
+ * Callers must not attempt to free the returned objects. These objects will be
+ * freed by the ACPI core automatically during the removal of @adev.
+ *
+ * Return: %0 if property with @name has been found (success),
+ * %-EINVAL if the arguments are invalid,
+ * %-ENODATA if the property doesn't exist,
+ * %-EPROTO if the property value type doesn't match @type.
+ */
+int acpi_dev_get_property(struct acpi_device *adev, const char *name,
+ acpi_object_type type, const union acpi_object **obj)
+{
+ const union acpi_object *properties;
+ int i;
+
+ if (!adev || !name)
+ return -EINVAL;
+
+ if (!adev->data.pointer || !adev->data.properties)
+ return -ENODATA;
+
+ properties = adev->data.properties;
+ for (i = 0; i < properties->package.count; i++) {
+ const union acpi_object *propname, *propvalue;
+ const union acpi_object *property;
+
+ property = &properties->package.elements[i];
+
+ propname = &property->package.elements[0];
+ propvalue = &property->package.elements[1];
+
+ if (!strcmp(name, propname->string.pointer)) {
+ if (type != ACPI_TYPE_ANY && propvalue->type != type)
+ return -EPROTO;
+ else if (obj)
+ *obj = propvalue;

+
+ return 0;
+ }
+ }

+ return -ENODATA;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_get_property);
+
+/**
+ * acpi_dev_get_property_array - return an ACPI array property with given name
+ * @adev: ACPI device to get property
+ * @name: Name of the property
+ * @type: Expected type of array elements
+ * @obj: Location to store a pointer to the property value (if not NULL)
+ *
+ * Look up an array property with @name and store a pointer to the resulting
+ * ACPI object at the location pointed to by @obj if found.
+ *
+ * Callers must not attempt to free the returned objects. Those objects will be
+ * freed by the ACPI core automatically during the removal of @adev.
+ *
+ * Return: %0 if array property (package) with @name has been found (success),
+ * %-EINVAL if the arguments are invalid,
+ * %-ENODATA if the property doesn't exist,
+ * %-EPROTO if the property is not a package or the type of its elements
+ * doesn't match @type.
+ */
+int acpi_dev_get_property_array(struct acpi_device *adev, const char *name,
+ acpi_object_type type,
+ const union acpi_object **obj)
+{
+ const union acpi_object *prop;
+ int ret, i;
+
+ ret = acpi_dev_get_property(adev, name, ACPI_TYPE_PACKAGE, &prop);
+ if (ret)
+ return ret;
+
+ if (type != ACPI_TYPE_ANY) {
+ /* Check that all elements are of correct type. */
+ for (i = 0; i < prop->package.count; i++)
+ if (prop->package.elements[i].type != type)
+ return -EPROTO;
+ }
+ if (obj)
+ *obj = prop;

+
+ return 0;
+}

+EXPORT_SYMBOL_GPL(acpi_dev_get_property_array);
+
+/**
+ * acpi_dev_get_property_reference - returns handle to the referenced object
+ * @adev: ACPI device to get property
+ * @name: Name of the property
+ * @size_prop: Name of the "size" property in referenced object
+ * @index: Index of the reference to return
+ * @args: Location to store the returned reference with optional arguments
+ *
+ * Find property with @name, verifify that it is a package containing at least
+ * one object reference and if so, store the ACPI device object pointer to the
+ * target object in @args->adev.
+ *
+ * If the reference includes arguments (@size_prop is not %NULL) follow the
+ * reference and check whether or not there is an integer property @size_prop
+ * under the target object and if so, whether or not its value matches the
+ * number of arguments that follow the reference. If there's more than one
+ * reference in the property value package, @index is used to select the one to
+ * return.
+ *
+ * Return: %0 on success, negative error code on failure.
+ */
+int acpi_dev_get_property_reference(struct acpi_device *adev, const char *name,
+ const char *size_prop, size_t index,
+ struct acpi_reference_args *args)
+{
+ const union acpi_object *element, *end;

+ const union acpi_object *obj;

+ struct acpi_device *device;
+ int ret, idx = 0;
+
+ ret = acpi_dev_get_property(adev, name, ACPI_TYPE_ANY, &obj);
+ if (ret)
+ return ret;
+
+ /*
+ * The simplest case is when the value is a single reference. Just
+ * return that reference then.
+ */
+ if (obj->type == ACPI_TYPE_LOCAL_REFERENCE) {
+ if (size_prop || index)
+ return -EINVAL;
+
+ ret = acpi_bus_get_device(obj->reference.handle, &device);
+ if (ret)
+ return ret;
+
+ args->adev = device;
+ args->nargs = 0;
+ return 0;
+ }
+
+ /*
+ * If it is not a single reference, then it is a package of
+ * references followed by number of ints as follows:
+ *
+ * Package () { REF, INT, REF, INT, INT }
+ *
+ * The index argument is then used to determine which reference
+ * the caller wants (along with the arguments).
+ */
+ if (obj->type != ACPI_TYPE_PACKAGE || index >= obj->package.count)
+ return -EPROTO;
+
+ element = obj->package.elements;
+ end = element + obj->package.count;
+
+ while (element < end) {
+ u32 nargs, i;
+
+ if (element->type != ACPI_TYPE_LOCAL_REFERENCE)
+ return -EPROTO;
+
+ ret = acpi_bus_get_device(element->reference.handle, &device);
+ if (ret)
+ return -ENODEV;
+
+ element++;
+ nargs = 0;
+
+ if (size_prop) {
+ const union acpi_object *prop;
+
+ /*
+ * Find out how many arguments the refenced object
+ * expects by reading its size_prop property.
+ */
+ ret = acpi_dev_get_property(device, size_prop,
+ ACPI_TYPE_INTEGER, &prop);
+ if (ret)
+ return ret;
+
+ nargs = prop->integer.value;
+ if (nargs > MAX_ACPI_REFERENCE_ARGS
+ || element + nargs > end)
+ return -EPROTO;
+
+ /*
+ * Skip to the start of the arguments and verify
+ * that they all are in fact integers.
+ */
+ for (i = 0; i < nargs; i++)
+ if (element[i].type != ACPI_TYPE_INTEGER)
+ return -EPROTO;
+ } else {
+ /* assume following integer elements are all args */
+ for (i = 0; element + i < end; i++) {
+ int type = element[i].type;
+
+ if (type == ACPI_TYPE_INTEGER)
+ nargs++;
+ else if (type == ACPI_TYPE_LOCAL_REFERENCE)
+ break;
+ else
+ return -EPROTO;
+ }
+ }
+
+ if (idx++ == index) {
+ args->adev = device;
+ args->nargs = nargs;
+ for (i = 0; i < nargs; i++)
+ args->args[i] = element[i].integer.value;

+
+ return 0;
+ }
+

+ element += nargs;
+ }
+
+ return -EPROTO;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_get_property_reference);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d5e6ac5042d8..f4aabf9c0aad 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -884,6 +884,7 @@ static void acpi_device_release(struct device *dev)
{
struct acpi_device *acpi_dev = to_acpi_device(dev);

+ acpi_free_properties(acpi_dev);
acpi_free_pnp_ids(&acpi_dev->pnp);
acpi_free_power_resources_lists(acpi_dev);
kfree(acpi_dev);
@@ -1888,6 +1889,7 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
acpi_set_device_status(device, sta);
acpi_device_get_busid(device);
acpi_set_pnp_ids(handle, &device->pnp, type);
+ acpi_init_properties(device);
acpi_bus_get_flags(device);
device->flags.match_driver = false;
device->flags.initialized = true;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index c1c9de19edbe..620b4f89c87b 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -337,6 +337,12 @@ struct acpi_device_physical_node {
bool put_online:1;
};

+/* ACPI Device Specific Data (_DSD) */
+struct acpi_device_data {
+ const union acpi_object *pointer;
+ const union acpi_object *properties;
+};
+
/* Device */
struct acpi_device {
int device_type;
@@ -353,6 +359,7 @@ struct acpi_device {
struct acpi_device_wakeup wakeup;
struct acpi_device_perf performance;
struct acpi_device_dir dir;
+ struct acpi_device_data data;
struct acpi_scan_handler *handler;
struct acpi_hotplug_context *hp;
struct acpi_driver *driver;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 807cbc46d73e..104321d994f5 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -660,4 +660,44 @@ do { \
#endif
#endif

+/* Device properties */
+
+#define MAX_ACPI_REFERENCE_ARGS 8
+struct acpi_reference_args {
+ struct acpi_device *adev;
+ size_t nargs;
+ u64 args[MAX_ACPI_REFERENCE_ARGS];
+};
+
+#ifdef CONFIG_ACPI
+int acpi_dev_get_property(struct acpi_device *adev, const char *name,
+ acpi_object_type type, const union acpi_object **obj);
+int acpi_dev_get_property_array(struct acpi_device *adev, const char *name,
+ acpi_object_type type,
+ const union acpi_object **obj);
+int acpi_dev_get_property_reference(struct acpi_device *adev, const char *name,
+ const char *cells_name, size_t index,
+ struct acpi_reference_args *args);
+#else
+static inline int acpi_dev_get_property(struct acpi_device *adev,
+ const char *name, acpi_object_type type,
+ const union acpi_object **obj)

+{
+ return -ENXIO;
+}

+static inline int acpi_dev_get_property_array(struct acpi_device *adev,
+ const char *name,
+ acpi_object_type type,
+ const union acpi_object **obj)

+{
+ return -ENXIO;
+}

+static inline int acpi_dev_get_property_reference(struct acpi_device *adev,
+ const char *name, const char *cells_name,
+ size_t index, struct acpi_reference_args *args)
+{
+ return -ENXIO;
+}
+#endif
+
#endif /*_LINUX_ACPI_H*/

Lee Jones

unread,

Sep 16, 2014, 5:54:57 PM9/16/14

to Mika Westerberg, Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

On Tue, 16 Sep 2014, Mika Westerberg wrote:

> If an MFD device is backed by ACPI namespace, we should allow subdevice
> drivers to access their corresponding ACPI companion devices through normal
> means (e.g using ACPI_COMPANION()).
>
> This patch adds such support to the MFD core. If the MFD parent device
> does not specify any ACPI _HID/_CID for the child device, the child
> device will share the parent ACPI companion device. Otherwise the child
> device will be assigned with the corresponding ACPI companion, if found
> in the namespace below the parent.
>
> Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
> Reviewed-by: Darren Hart <dvh...@linux.intel.com>
> ---
> Lee, I tried to get rid of #ifdefs in the below patch but it wasn't
> possible because we are using functions that are not available when
> !CONFIG_ACPI.
>
> Documentation/acpi/enumeration.txt | 27 +++++++++++++++++++++++++
> drivers/mfd/mfd-core.c | 40 ++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/core.h | 3 +++
> 3 files changed, 70 insertions(+)

Acked-by: Lee Jones <lee....@linaro.org>

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Greg Kroah-Hartman

unread,

Sep 17, 2014, 2:29:34 PM9/17/14

to Mika Westerberg, Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

Looks good to me, feel free to take this through your tree with my:

Acked-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

Alexandre Courbot

unread,

Sep 19, 2014, 4:19:19 AM9/19/14

to Mika Westerberg, Rafael J. Wysocki, ACPI Devel Maling List, devic...@vger.kernel.org, Linux Kernel Mailing List, Greg Kroah-Hartman, Linus Walleij, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

On Tue, Sep 16, 2014 at 8:52 PM, Mika Westerberg
<mika.we...@linux.intel.com> wrote:
> GPIO descriptors are the preferred way over legacy GPIO numbers
> nowadays. Convert the driver to use GPIO descriptors internally but
> still allow passing legacy GPIO numbers from platform data to support
> existing platforms.
>
> Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>

Obviously a good thing to do.

Acked-by: Alexandre Courbot <acou...@nvidia.com>

Alexandre Courbot

unread,

Sep 19, 2014, 4:22:45 AM9/19/14

to Mika Westerberg, Rafael J. Wysocki, ACPI Devel Maling List, devic...@vger.kernel.org, Linux Kernel Mailing List, Greg Kroah-Hartman, Linus Walleij, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

On Tue, Sep 16, 2014 at 8:52 PM, Mika Westerberg
<mika.we...@linux.intel.com> wrote:

> From: Aaron Lu <aaro...@intel.com>
>
> GPIO descriptors are the preferred way over legacy GPIO numbers
> nowadays. Convert the driver to use GPIO descriptors internally but
> still allow passing legacy GPIO numbers from platform data to support
> existing platforms.
>
> Signed-off-by: Aaron Lu <aaro...@intel.com>
> Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>

Acked-by: Alexandre Courbot <acou...@nvidia.com>

Rafael J. Wysocki

unread,

Sep 20, 2014, 8:06:29 PM9/20/14

to Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

Given the ACKs that we've got already (the Greg's one in particular) and
the apparent lack of objections (or indeed any comments at all), I'm about
to queue this up for 3.18 next week.

Rafael

Bryan Wu

unread,

Sep 22, 2014, 7:30:15 PM9/22/14

to Mika Westerberg, Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, lkml, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

I'm good with the LEDs change, please go ahead with my Ack
Acked-by: Bryan Wu <cool...@gmail.com>

Thanks,
-Bryan

Linus Walleij

unread,

Sep 23, 2014, 11:25:57 AM9/23/14

to Mika Westerberg, Grant Likely, Arnd Bergmann, Darren Hart, Mark Rutland, Rafael J. Wysocki, ACPI Devel Maling List, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Aaron Lu

On Tue, Sep 16, 2014 at 1:52 PM, Mika Westerberg
<mika.we...@linux.intel.com> wrote:

> Some drivers need to deal with only firmware representation of its
> GPIOs. An example would be a GPIO button array driver where each button
> is described as a separate firmware node in device tree. Typically these
> child nodes do not have physical representation in the Linux device
> model.
>
> In order to help device drivers to handle such firmware child nodes we
> add dev[m]_node_get_named_gpiod() that takes a firmware node pointer as
> parameter, finds the GPIO using whatever is the underlying firmware
> method, and requests the GPIO properly.
>
> Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>

I have a hard time figuring out if this is what we want for common
accessors between DT and ACPI.

Can I get some input from Grant, Arnd, Mark, Darren...?

Yours,
Linus Walleij

Linus Walleij

unread,

Sep 23, 2014, 11:27:27 AM9/23/14

to Mika Westerberg, Rafael J. Wysocki, ACPI Devel Maling List, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

On Tue, Sep 16, 2014 at 1:52 PM, Mika Westerberg
<mika.we...@linux.intel.com> wrote:

Acked-by: Linus Walleij <linus....@linaro.org>

If Rafael is happy with this approach, and you decide to take it
through the ACPI tree.

Yours,
Linus Walleij

Arnd Bergmann

unread,

Sep 23, 2014, 11:46:23 AM9/23/14

to Linus Walleij, Mika Westerberg, Grant Likely, Darren Hart, Mark Rutland, Rafael J. Wysocki, ACPI Devel Maling List, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Aaron Lu

On Tuesday 23 September 2014 17:25:50 Linus Walleij wrote:
> On Tue, Sep 16, 2014 at 1:52 PM, Mika Westerberg
> <mika.we...@linux.intel.com> wrote:
>
> > Some drivers need to deal with only firmware representation of its
> > GPIOs. An example would be a GPIO button array driver where each button
> > is described as a separate firmware node in device tree. Typically these
> > child nodes do not have physical representation in the Linux device
> > model.
> >
> > In order to help device drivers to handle such firmware child nodes we
> > add dev[m]_node_get_named_gpiod() that takes a firmware node pointer as
> > parameter, finds the GPIO using whatever is the underlying firmware
> > method, and requests the GPIO properly.
> >
> > Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
>
> I have a hard time figuring out if this is what we want for common
> accessors between DT and ACPI.
>
> Can I get some input from Grant, Arnd, Mark, Darren...?

I just took a brief look at this. My first impression is that the
fw_dev_node structure is weird when all callers just do (in patch 2)

+ struct fw_dev_node fdn = {
+ .of_node = dev->of_node,
+ .acpi_node = ACPI_COMPANION(dev),
+ };

I'd much rather see an interface that passes the 'struct device'
pointer down to dev_get_named_gpiod() and all other exported
functions, and then internally does the conversion at the point
where the access is done.

Arnd

Mika Westerberg

unread,

Sep 23, 2014, 11:52:52 AM9/23/14

to Arnd Bergmann, Linus Walleij, Grant Likely, Darren Hart, Mark Rutland, Rafael J. Wysocki, ACPI Devel Maling List, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Aaron Lu

Problem is that if you don't have the dev pointer in the first place.
Please look how leds-gpio.c or gpio_keys_polled.c are using this.

Of course you have the first level device but when you need to iterate
"leds" or "buttons" below where there is no Linux device available we
need something like this.

Rafael J. Wysocki

Sep 24, 2014, 3:55:57 AM9/24/14

to Mika Westerberg, Rafael J. Wysocki, ACPI Devel Maling List, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

On Tue, Sep 16, 2014 at 1:52 PM, Mika Westerberg
<mika.we...@linux.intel.com> wrote:

> GPIO descriptors are the preferred way over legacy GPIO numbers
> nowadays. Convert the driver to use GPIO descriptors internally but
> still allow passing legacy GPIO numbers from platform data to support
> existing platforms.
>
> Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>

Great!

> #include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>

You should not need to include <linux/gpio.h> at all after this change.

> @@ -85,9 +91,10 @@ static int gpio_blink_set(struct led_classdev *led_cdev,
> {
> struct gpio_led_data *led_dat =
> container_of(led_cdev, struct gpio_led_data, cdev);
> + int gpio = desc_to_gpio(led_dat->gpiod);
>
> led_dat->blinking = 1;
> - return led_dat->platform_gpio_blink_set(led_dat->gpio, GPIO_LED_BLINK,
> + return led_dat->platform_gpio_blink_set(gpio, GPIO_LED_BLINK,
> delay_on, delay_off);
> }

While this is a nice first step, it must be possible to patch all in-kernel
users of this callback to take a gpiod too...

It's actually just:

$ git grep gpio_blink_set
arch/arm/mach-orion5x/dns323-setup.c: .gpio_blink_set =
orion_gpio_led_blink_set,
arch/arm/mach-orion5x/dns323-setup.c: .gpio_blink_set =
orion_gpio_led_blink_set,
arch/arm/mach-s3c24xx/mach-h1940.c: .gpio_blink_set = h1940_led_blink_set,
arch/arm/mach-s3c24xx/mach-rx1950.c: .gpio_blink_set = rx1950_led_blink_set,

However we can do that as a follow-up patch. (Add to TODO...)

Yours,
Linus Walleij

Arnd Bergmann

unread,

Sep 24, 2014, 3:56:42 AM9/24/14

to Rafael J. Wysocki, Linus Walleij, Mika Westerberg, Grant Likely, Darren Hart, Mark Rutland, ACPI Devel Maling List, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Aaron Lu

On Tuesday 23 September 2014 22:47:36 Rafael J. Wysocki wrote:
> Quite frankly, I'm not sure what you're asking for.
>
> It seems to mean "I kind of don't like the current implementation", but
> then the last part is quite unclear to me. Are you suggesting to add more
> "if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) etc" type of checks to
> device drivers? That I'd like to avoid to be honest.

No, that is not what I want. Device drivers should ideally call interfaces
that just take a 'struct device' or 'struct platform_device' pointer,
and those should be implemented in an appropriate way.

> Instead of the current proposal we can introduce something like
>
> int device_get_child_property(struct device *dev, void *child_node,
> const char *propname, void **valptr);
>
> (and analogously for device_read_property*) and use that in the drivers that
> need to iterate over child nodes of a device. Quite along the lines of what
> Dmitry is suggesting.
>
> Then, fn() in acpi_for_each_child_node() (and the of_ counterpart of it)
> would become
>
> int (*fn)(struct device *dev, void *child_node, void *data)
>
> and so on.
>
> Would you prefer that?

I must still be missing part of what you are trying to achieve above.
We definitely need an interface to get properties from the device itself,
like

int device_get_property(struct device *dev, const char *propname, void **valptr);

(whatever valptr ends up being, that would be a separate discussion).

As soon as it comes to devices that have child nodes, I don't see a
necessity to have a generic abstraction for them, as this is typically
only done for some of the more obscure bindings, or for child nodes that
are defined in a subsystem-wide binding rather than a device private
binding.

For the former case, I think they are indeed better left in drivers that
actively know the difference between DT and ACPI, and that don't necessarily
use the same binding for both. In the latter case, I'd leave the
implementation up to subsystem code, which again would know what
interface it is using.

Arnd

Linus Walleij

unread,

Sep 24, 2014, 4:02:59 AM9/24/14

to Mika Westerberg, Rafael J. Wysocki, ACPI Devel Maling List, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

On Tue, Sep 16, 2014 at 1:52 PM, Mika Westerberg
<mika.we...@linux.intel.com> wrote:

OK, I'll add that to my TODO list :)

Mika Westerberg

unread,

Sep 24, 2014, 5:45:19 AM9/24/14

to Lee Jones, Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

On Wed, Sep 24, 2014 at 09:34:18AM +0100, Lee Jones wrote:
> I'd prefer to take the MFD patch through the MFD tree if that's
> possible. Are there any technical reasons why this would prove
> difficult?

I don't see any reasons why it cannot be taken via MFD tree.

Lee Jones

unread,

Sep 24, 2014, 8:01:10 AM9/24/14

to Mika Westerberg, Rafael J. Wysocki, linux...@vger.kernel.org, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

On Tue, 16 Sep 2014, Mika Westerberg wrote:
> If an MFD device is backed by ACPI namespace, we should allow subdevice
> drivers to access their corresponding ACPI companion devices through normal
> means (e.g using ACPI_COMPANION()).
>
> This patch adds such support to the MFD core. If the MFD parent device
> does not specify any ACPI _HID/_CID for the child device, the child
> device will share the parent ACPI companion device. Otherwise the child
> device will be assigned with the corresponding ACPI companion, if found
> in the namespace below the parent.
>
> Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
> Reviewed-by: Darren Hart <dvh...@linux.intel.com>
> ---
> Lee, I tried to get rid of #ifdefs in the below patch but it wasn't
> possible because we are using functions that are not available when
> !CONFIG_ACPI.
>
> Documentation/acpi/enumeration.txt | 27 +++++++++++++++++++++++++
> drivers/mfd/mfd-core.c | 40 ++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/core.h | 3 +++
> 3 files changed, 70 insertions(+)

Applied, thanks.

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Rafael J. Wysocki

unread,

Sep 24, 2014, 9:48:46 AM9/24/14

to Arnd Bergmann, Linus Walleij, Mika Westerberg, Grant Likely, Darren Hart, Mark Rutland, ACPI Devel Maling List, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Aaron Lu

On Wednesday, September 24, 2014 09:55:12 AM Arnd Bergmann wrote:
> On Tuesday 23 September 2014 22:47:36 Rafael J. Wysocki wrote:
> > Quite frankly, I'm not sure what you're asking for.
> >
> > It seems to mean "I kind of don't like the current implementation", but
> > then the last part is quite unclear to me. Are you suggesting to add more
> > "if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) etc" type of checks to
> > device drivers? That I'd like to avoid to be honest.
>
> No, that is not what I want. Device drivers should ideally call interfaces
> that just take a 'struct device' or 'struct platform_device' pointer,
> and those should be implemented in an appropriate way.

But then you have drivers that access properties of their child nodes which
have no corresponding struct device(s).

> > Instead of the current proposal we can introduce something like
> >
> > int device_get_child_property(struct device *dev, void *child_node,
> > const char *propname, void **valptr);
> >
> > (and analogously for device_read_property*) and use that in the drivers that
> > need to iterate over child nodes of a device. Quite along the lines of what
> > Dmitry is suggesting.
> >
> > Then, fn() in acpi_for_each_child_node() (and the of_ counterpart of it)
> > would become
> >
> > int (*fn)(struct device *dev, void *child_node, void *data)
> >
> > and so on.
> >
> > Would you prefer that?
>
> I must still be missing part of what you are trying to achieve above.
> We definitely need an interface to get properties from the device itself,
> like
>
> int device_get_property(struct device *dev, const char *propname, void **valptr);
>
> (whatever valptr ends up being, that would be a separate discussion).
>
> As soon as it comes to devices that have child nodes, I don't see a
> necessity to have a generic abstraction for them, as this is typically
> only done for some of the more obscure bindings, or for child nodes that
> are defined in a subsystem-wide binding rather than a device private
> binding.

Well, I don't really agree here.

We can demonstrably reduce code duplication (and therefore complexity too) in
at least two drivers by doing that at a reasonably low cost, which is
simply exposing the API for child nodes and adding helpers for iterating
over them.

We don't have to use struct fw_dev_node (or similar) for that if that's what
bothers you, but in my opinion something like "get me a property of this thing
which may be either a device tree node or an ACPI object" (ie. what the current
dev_node_get_property() does) is more straightforwad than "get me a property
of that child of this device which may be either a device tree node or an ACPI
object" (ie. what device_get_child_property() as described above would do).

> For the former case, I think they are indeed better left in drivers that
> actively know the difference between DT and ACPI, and that don't necessarily
> use the same binding for both. In the latter case, I'd leave the
> implementation up to subsystem code, which again would know what
> interface it is using.

The case in question is drivers that need not know the difference between DT and
ACPI and do use the same binding for both. It seems quite clear what needs to
be done in the other cases.

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Rafael J. Wysocki

unread,

Sep 24, 2014, 9:51:52 AM9/24/14

to Mika Westerberg, Arnd Bergmann, Darren Hart, David Woodhouse, Linus Walleij, Grant Likely, Mark Rutland, ACPI Devel Maling List, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Aaron Lu

On Wednesday, September 24, 2014 12:38:23 PM Mika Westerberg wrote:
> On Wed, Sep 24, 2014 at 11:12:36AM +0200, Arnd Bergmann wrote:
> > As a start, we could probably take the proposed device_for_each_child_node
> > and move that into the leds-core, changing the fw_dev_node argument
> > for an led_classdev with the addition of the of_node and acpi_object
> > members. It would still leave it up to the gpio-leds driver to do
> >
> > if (led_cdev->of_node)
> > gpiod = devm_of_get_gpiod(led_cdev->of_node, ...);
> > else
> > gpiod = devm_acpi_get_gpiod(led_cdev->acpi_object, ...);
> >
> > but there seems little benefit in abstracting this because there is
> > only one driver that needs it.
>
> The same interface is used also in gpio_keys_polled.c driver so if we
> want to avoid duplicating code this needs to be abstracted away from the
> drivers.

Well, precisely.

Moving it to the leds-core doesn't buy us anything.

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Darren Hart

unread,

Sep 25, 2014, 11:21:43 PM9/25/14

to Arnd Bergmann, Rafael J. Wysocki, David Woodhouse, Linus Walleij, Mika Westerberg, Grant Likely, Mark Rutland, ACPI Devel Maling List, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Aaron Lu

On Wed, Sep 24, 2014 at 11:12:36AM +0200, Arnd Bergmann wrote:

> On Tuesday 23 September 2014 14:15:47 Darren Hart wrote:
> > Arnd, I think you meant:
> >
> > Return gpio_leds_create_acpi(pdev) ?
>
> Yes, sorry for the confusion on my part.

Hi Arnd,

No problem, just wanted to make sure I knew what you meant.

So as Mika has pointed out, LEDs aren't the only ones affected. Several drivers
will need to walk through non-device child nodes, and it seems to me that having
a firmware-independent mechanism to do so benefits the drivers by both making
them smaller and by increasing the reusability of new drivers and drivers
updated to use the new API across platforms.

I fear we might be entering bike shed territory as we seem to be repeating
points now. Can you restate your concern with the interface and why this level
of abstraction is worse for the kernel? I'm not seeing this point, so I'm not
sure what to address in my response.

Grant, Linus W? Thoughts?

>
> but there seems little benefit in abstracting this because there is
> only one driver that needs it.
>
> Arnd
>

--

Darren Hart
Intel Open Source Technology Center

Arnd Bergmann

unread,

Sep 26, 2014, 4:36:54 AM9/26/14

to Darren Hart, Rafael J. Wysocki, David Woodhouse, Linus Walleij, Mika Westerberg, Grant Likely, Mark Rutland, ACPI Devel Maling List, devic...@vger.kernel.org, linux-...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Aaron Lu

I think we should have abstractions for all common interfaces but make
them as simple as possible. In the discussions at the kernel summit,
everyone agreed that we should have common accessors for simple properties
(bool, int, string, ...) based on device pointers, as well as subsystem
specific accessors to handle the high-level abstractions (registers,
interrupts, gpio, regulator, pinctrl, dma, reset, pwm, ...).

Having generalized accessors for the same properties in child nodes of
the device goes beyond that, and I think this is the wrong trade-off
between interface simplicity and generality since only few drivers will
be able to use those. I think we will always have to live with a leaky
abstraction because some drivers need to do things beyond what we can
do with a common API.

> Grant, Linus W? Thoughts?

I definitely want to hear other voices on this too. This is really not
a fundamental debate I think, but more a question of how far the abstraction
should go.

Arnd

Rafael J. Wysocki

unread,

unread,

Sep 30, 2014, 10:03:38 PM9/30/14

to linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: Mika Westerberg <mika.we...@linux.intel.com>

Device Tree is used in many embedded systems to describe the system
configuration to the OS. It supports attaching properties or name-value
pairs to the devices it describe. With these properties one can pass
additional information to the drivers that would not be available
otherwise.

ACPI is another configuration mechanism (among other things) typically
seen, but not limited to, x86 machines. ACPI allows passing arbitrary
data from methods but there has not been mechanism equivalent to Device
Tree until the introduction of _DSD in the recent publication of the
ACPI 5.1 specification.

In order to facilitate ACPI usage in systems where Device Tree is
typically used, it would be beneficial to standardize a way to retrieve
Device Tree style properties from ACPI devices, which is what we do in
this patch.

If a given device described in ACPI namespace wants to export properties it
must implement _DSD method (Device Specific Data, introduced with ACPI 5.1)
that returns the properties in a package of packages. For example:

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
Package () {"name1", <VALUE1>},
Package () {"name2", <VALUE2>},
...
}
})

The UUID reserved for properties is daffd814-6eba-4d8c-8a91-bc9bbf4aa301
and is documented in the ACPI 5.1 companion document called "_DSD
Implementation Guide" [1], [2].

We add several helper functions that can be used to extract these
properties and convert them to different Linux data types.

The ultimate goal is that we only have one device property API that
retrieves the requested properties from Device Tree or from ACPI
transparent to the caller.

[1] http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm
[2] http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf

Reviewed-by: Hanjun Guo <hanju...@linaro.org>
Reviewed-by: Josh Triplett <jo...@joshtriplett.org>
Signed-off-by: Darren Hart <dvh...@linux.intel.com>

Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>

Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

drivers/acpi/Makefile | 1
drivers/acpi/internal.h | 6
drivers/acpi/property.c | 364 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/acpi/scan.c | 2
include/acpi/acpi_bus.h | 7
include/linux/acpi.h | 40 +++++
6 files changed, 420 insertions(+)
create mode 100644 drivers/acpi/property.c

Index: linux-pm/drivers/acpi/Makefile
===================================================================
--- linux-pm.orig/drivers/acpi/Makefile
+++ linux-pm/drivers/acpi/Makefile
@@ -46,6 +46,7 @@ acpi-y += acpi_pnp.o
acpi-y += power.o
acpi-y += event.o
acpi-y += sysfs.o
+acpi-y += property.o
acpi-$(CONFIG_X86) += acpi_cmos_rtc.o
acpi-$(CONFIG_DEBUG_FS) += debugfs.o
acpi-$(CONFIG_ACPI_NUMA) += numa.o
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -181,4 +181,10 @@ struct platform_device *acpi_create_plat
bool acpi_osi_is_win8(void);
#endif

+/*--------------------------------------------------------------------------
+ Device properties
+ -------------------------------------------------------------------------- */
+void acpi_init_properties(struct acpi_device *adev);
+void acpi_free_properties(struct acpi_device *adev);
+
#endif /* _ACPI_INTERNAL_H_ */
Index: linux-pm/drivers/acpi/property.c
===================================================================
--- /dev/null
+++ linux-pm/drivers/acpi/property.c
@@ -0,0 +1,364 @@
+/*
+ * ACPI device specific properties support.
+ *
+ * Copyright (C) 2014, Intel Corporation
+ * All rights reserved.
+ *
+ * Authors: Mika Westerberg <mika.we...@linux.intel.com>
+ * Darren Hart <dvh...@linux.intel.com>
+ * Rafael J. Wysocki <rafael.j...@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/export.h>
+
+#include "internal.h"
+
+/* ACPI _DSD device properties UUID: daffd814-6eba-4d8c-8a91-bc9bbf4aa301 */
+static const u8 prp_uuid[16] = {
+ 0x14, 0xd8, 0xff, 0xda, 0xba, 0x6e, 0x8c, 0x4d,
+ 0x8a, 0x91, 0xbc, 0x9b, 0xbf, 0x4a, 0xa3, 0x01
+};
+
+static bool acpi_property_value_ok(const union acpi_object *value)
+{
+ int j;
+
+ /*
+ * The value must be an integer, a string, a reference, or a package
+ * whose every element must be an integer, a string, or a reference.
+ */
+ switch (value->type) {
+ case ACPI_TYPE_INTEGER:
+ case ACPI_TYPE_STRING:
+ case ACPI_TYPE_LOCAL_REFERENCE:
+ return true;
+
+ case ACPI_TYPE_PACKAGE:
+ for (j = 0; j < value->package.count; j++)
+ switch (value->package.elements[j].type) {
+ case ACPI_TYPE_INTEGER:
+ case ACPI_TYPE_STRING:
+ case ACPI_TYPE_LOCAL_REFERENCE:
+ continue;
+
+ default:
+ return false;
+ }
+
+ return true;
+ }
+ return false;
+}
+
+static bool acpi_properties_format_valid(const union acpi_object *properties)
+{
+ int i;
+
+ for (i = 0; i < properties->package.count; i++) {
+ const union acpi_object *property;
+
+ property = &properties->package.elements[i];
+ /*
+ * Only two elements allowed, the first one must be a string and
+ * the second one has to satisfy certain conditions.
+ */
+ if (property->package.count != 2
+ || property->package.elements[0].type != ACPI_TYPE_STRING
+ || !acpi_property_value_ok(&property->package.elements[1]))
+ return false;
+ }
+ return true;
+}
+
+void acpi_init_properties(struct acpi_device *adev)
+{
+ struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
+ const union acpi_object *desc;
+ acpi_status status;
+ int i;
+
+ status = acpi_evaluate_object_typed(adev->handle, "_DSD", NULL, &buf,
+ ACPI_TYPE_PACKAGE);
+ if (ACPI_FAILURE(status))
+ return;
+
+ desc = buf.pointer;
+ if (desc->package.count % 2)
+ goto fail;
+
+ /* Look for the device properties UUID. */
+ for (i = 0; i < desc->package.count; i += 2) {
+ const union acpi_object *uuid, *properties;
+
+ uuid = &desc->package.elements[i];
+ properties = &desc->package.elements[i + 1];
+
+ /*
+ * The first element must be a UUID and the second one must be
+ * a package.
+ */
+ if (uuid->type != ACPI_TYPE_BUFFER || uuid->buffer.length != 16
+ || properties->type != ACPI_TYPE_PACKAGE)
+ break;
+
+ if (memcmp(uuid->buffer.pointer, prp_uuid, sizeof(prp_uuid)))
+ continue;
+
+ /*
+ * We found the matching UUID. Now validate the format of the
+ * package immediately following it.
+ */
+ if (!acpi_properties_format_valid(properties))
+ break;
+
+ adev->data.pointer = buf.pointer;
+ adev->data.properties = properties;
+ return;
+ }
+
+ fail:
+ dev_warn(&adev->dev, "Returned _DSD data is not valid, skipping\n");
+ ACPI_FREE(buf.pointer);
+}
+
+void acpi_free_properties(struct acpi_device *adev)
+{
+ ACPI_FREE((void *)adev->data.pointer);
+ adev->data.pointer = NULL;
+ adev->data.properties = NULL;
+}
+
+/**
+ * acpi_dev_get_property - return an ACPI property with given name
+ * @adev: ACPI device to get property
+ * @name: Name of the property
+ * @type: Expected property type
+ * @obj: Location to store the property value (if not %NULL)
+ *
+ * Look up a property with @name and store a pointer to the resulting ACPI
+ * object at the location pointed to by @obj if found.
+ *
+ * Callers must not attempt to free the returned objects. These objects will be
+ * freed by the ACPI core automatically during the removal of @adev.
+ *
+ * Return: %0 if property with @name has been found (success),
+ * %-EINVAL if the arguments are invalid,
+ * %-ENODATA if the property doesn't exist,
+ * %-EPROTO if the property value type doesn't match @type.
+ */
+int acpi_dev_get_property(struct acpi_device *adev, const char *name,
+ acpi_object_type type, const union acpi_object **obj)
+{
+ const union acpi_object *properties;
+ int i;
+
+ if (!adev || !name)
+ return -EINVAL;
+
+ if (!adev->data.pointer || !adev->data.properties)
+ return -ENODATA;
+
+ properties = adev->data.properties;
+ for (i = 0; i < properties->package.count; i++) {
+ const union acpi_object *propname, *propvalue;
+ const union acpi_object *property;
+
+ property = &properties->package.elements[i];
+
+ propname = &property->package.elements[0];
+ propvalue = &property->package.elements[1];
+
+ if (!strcmp(name, propname->string.pointer)) {
+ if (type != ACPI_TYPE_ANY && propvalue->type != type)
+ return -EPROTO;
+ else if (obj)
+ *obj = propvalue;
+
+ return 0;
+ }
+ }
+ return -ENODATA;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_get_property);
+
+/**
+ * acpi_dev_get_property_array - return an ACPI array property with given name
+ * @adev: ACPI device to get property
+ * @name: Name of the property
+ * @type: Expected type of array elements
+ * @obj: Location to store a pointer to the property value (if not NULL)
+ *
+ * Look up an array property with @name and store a pointer to the resulting
+ * ACPI object at the location pointed to by @obj if found.
+ *
+ * Callers must not attempt to free the returned objects. Those objects will be
+ * freed by the ACPI core automatically during the removal of @adev.
+ *
+ * Return: %0 if array property (package) with @name has been found (success),
+ * %-EINVAL if the arguments are invalid,
+ * %-ENODATA if the property doesn't exist,
+ * %-EPROTO if the property is not a package or the type of its elements
+ * doesn't match @type.
+ */
+int acpi_dev_get_property_array(struct acpi_device *adev, const char *name,
+ acpi_object_type type,
+ const union acpi_object **obj)
+{
+ const union acpi_object *prop;
+ int ret, i;
+
+ ret = acpi_dev_get_property(adev, name, ACPI_TYPE_PACKAGE, &prop);
+ if (ret)
+ return ret;
+
+ if (type != ACPI_TYPE_ANY) {
+ /* Check that all elements are of correct type. */
+ for (i = 0; i < prop->package.count; i++)
+ if (prop->package.elements[i].type != type)
+ return -EPROTO;
+ }
+ if (obj)
+ *obj = prop;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_get_property_array);
+
+/**
+ * acpi_dev_get_property_reference - returns handle to the referenced object
+ * @adev: ACPI device to get property
+ * @name: Name of the property
+ * @size_prop: Name of the "size" property in referenced object
+ * @index: Index of the reference to return
+ * @args: Location to store the returned reference with optional arguments
+ *
+ * Find property with @name, verifify that it is a package containing at least
+ * one object reference and if so, store the ACPI device object pointer to the
+ * target object in @args->adev.
+ *
+ * If the reference includes arguments (@size_prop is not %NULL) follow the
+ * reference and check whether or not there is an integer property @size_prop
+ * under the target object and if so, whether or not its value matches the
+ * number of arguments that follow the reference. If there's more than one
+ * reference in the property value package, @index is used to select the one to
+ * return.
+ *
+ * Return: %0 on success, negative error code on failure.
+ */
+int acpi_dev_get_property_reference(struct acpi_device *adev, const char *name,
+ const char *size_prop, size_t index,
+ struct acpi_reference_args *args)
+{
+ const union acpi_object *element, *end;
+ const union acpi_object *obj;
+ struct acpi_device *device;
+ int ret, idx = 0;
+
+ ret = acpi_dev_get_property(adev, name, ACPI_TYPE_ANY, &obj);
+ if (ret)
+ return ret;
+
+ /*
+ * The simplest case is when the value is a single reference. Just
+ * return that reference then.
+ */
+ if (obj->type == ACPI_TYPE_LOCAL_REFERENCE) {
+ if (size_prop || index)
+ return -EINVAL;
+
+ ret = acpi_bus_get_device(obj->reference.handle, &device);
+ if (ret)
+ return ret;
+
+ args->adev = device;
+ args->nargs = 0;
+ return 0;
+ }
+
+ /*
+ * If it is not a single reference, then it is a package of
+ * references followed by number of ints as follows:
+ *
+ * Package () { REF, INT, REF, INT, INT }
+ *
+ * The index argument is then used to determine which reference
+ * the caller wants (along with the arguments).
+ */
+ if (obj->type != ACPI_TYPE_PACKAGE || index >= obj->package.count)
+ return -EPROTO;
+
+ element = obj->package.elements;
+ end = element + obj->package.count;
+
+ while (element < end) {
+ u32 nargs, i;
+
+ if (element->type != ACPI_TYPE_LOCAL_REFERENCE)
+ return -EPROTO;
+
+ ret = acpi_bus_get_device(element->reference.handle, &device);
+ if (ret)
+ return -ENODEV;
+
+ element++;
+ nargs = 0;
+
+ if (size_prop) {
+ const union acpi_object *prop;
+
+ /*
+ * Find out how many arguments the refenced object
+ * expects by reading its size_prop property.
+ */
+ ret = acpi_dev_get_property(device, size_prop,
+ ACPI_TYPE_INTEGER, &prop);
+ if (ret)
+ return ret;
+
+ nargs = prop->integer.value;
+ if (nargs > MAX_ACPI_REFERENCE_ARGS
+ || element + nargs > end)
+ return -EPROTO;
+
+ /*
+ * Skip to the start of the arguments and verify
+ * that they all are in fact integers.
+ */
+ for (i = 0; i < nargs; i++)
+ if (element[i].type != ACPI_TYPE_INTEGER)
+ return -EPROTO;
+ } else {
+ /* assume following integer elements are all args */
+ for (i = 0; element + i < end; i++) {
+ int type = element[i].type;
+
+ if (type == ACPI_TYPE_INTEGER)
+ nargs++;
+ else if (type == ACPI_TYPE_LOCAL_REFERENCE)
+ break;
+ else
+ return -EPROTO;
+ }
+ }
+
+ if (idx++ == index) {
+ args->adev = device;
+ args->nargs = nargs;
+ for (i = 0; i < nargs; i++)
+ args->args[i] = element[i].integer.value;
+
+ return 0;
+ }
+
+ element += nargs;
+ }
+
+ return -EPROTO;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_get_property_reference);
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -884,6 +884,7 @@ static void acpi_device_release(struct d
{
struct acpi_device *acpi_dev = to_acpi_device(dev);

+ acpi_free_properties(acpi_dev);
acpi_free_pnp_ids(&acpi_dev->pnp);
acpi_free_power_resources_lists(acpi_dev);
kfree(acpi_dev);
@@ -1888,6 +1889,7 @@ void acpi_init_device_object(struct acpi
acpi_set_device_status(device, sta);
acpi_device_get_busid(device);
acpi_set_pnp_ids(handle, &device->pnp, type);
+ acpi_init_properties(device);
acpi_bus_get_flags(device);
device->flags.match_driver = false;
device->flags.initialized = true;
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -337,6 +337,12 @@ struct acpi_device_physical_node {
bool put_online:1;
};

+/* ACPI Device Specific Data (_DSD) */
+struct acpi_device_data {
+ const union acpi_object *pointer;
+ const union acpi_object *properties;
+};
+
/* Device */
struct acpi_device {
int device_type;
@@ -353,6 +359,7 @@ struct acpi_device {
struct acpi_device_wakeup wakeup;
struct acpi_device_perf performance;
struct acpi_device_dir dir;
+ struct acpi_device_data data;
struct acpi_scan_handler *handler;
struct acpi_hotplug_context *hp;
struct acpi_driver *driver;
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -658,4 +658,44 @@ do { \
#endif
#endif

+/* Device properties */
+
+#define MAX_ACPI_REFERENCE_ARGS 8
+struct acpi_reference_args {
+ struct acpi_device *adev;
+ size_t nargs;
+ u64 args[MAX_ACPI_REFERENCE_ARGS];
+};
+
+#ifdef CONFIG_ACPI
+int acpi_dev_get_property(struct acpi_device *adev, const char *name,
+ acpi_object_type type, const union acpi_object **obj);
+int acpi_dev_get_property_array(struct acpi_device *adev, const char *name,
+ acpi_object_type type,
+ const union acpi_object **obj);
+int acpi_dev_get_property_reference(struct acpi_device *adev, const char *name,
+ const char *cells_name, size_t index,
+ struct acpi_reference_args *args);
+#else
+static inline int acpi_dev_get_property(struct acpi_device *adev,
+ const char *name, acpi_object_type type,
+ const union acpi_object **obj)
+{
+ return -ENXIO;
+}
+static inline int acpi_dev_get_property_array(struct acpi_device *adev,
+ const char *name,
+ acpi_object_type type,
+ const union acpi_object **obj)
+{
+ return -ENXIO;
+}
+static inline int acpi_dev_get_property_reference(struct acpi_device *adev,
+ const char *name, const char *cells_name,
+ size_t index, struct acpi_reference_args *args)
+{
+ return -ENXIO;
+}
+#endif
+
#endif /*_LINUX_ACPI_H*/

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:03:47 PM9/30/14

to linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

Hi Everyone,

Since Dmitry was suggesting that instead of using struct fw_dev_node pointers,
as we did in the previous version, we could pass the parent device along with
the child pointer to iterator functions while walking the children of a given
device node, the following patches do just that. Some things look better this
way, but some look worse. Please tell me what you think.

Mika has tested them on Minnowboard and Minnowboard MAX using the MFD patch
(number 5 previously) that is not included as it has been already applied.

The cover letter below is still applicable mostly, so I'm leaving it as is.

The only patches that changed are [2, 4, 6, 9, 12/15], so I didn't add any
ACKs to them to be prudent.

> Rafael J. Wysocki (1):
> Driver core: Unified device properties interface for platform firmware
>
> Documentation/acpi/enumeration.txt | 27 ++
> Documentation/acpi/properties.txt | 410 +++++++++++++++++++++
> drivers/acpi/Makefile | 1 +
> drivers/acpi/internal.h | 6 +
> drivers/acpi/property.c | 584 ++++++++++++++++++++++++++++++
> drivers/acpi/scan.c | 93 ++++-
> drivers/base/Makefile | 2 +-
> drivers/base/property.c | 196 ++++++++++
> drivers/gpio/devres.c | 35 ++
> drivers/gpio/gpio-sch.c | 293 ++++++---------
> drivers/gpio/gpiolib-acpi.c | 78 +++-
> drivers/gpio/gpiolib.c | 85 ++++-
> drivers/gpio/gpiolib.h | 7 +-
> drivers/input/keyboard/gpio_keys_polled.c | 169 +++++----
> drivers/leds/leds-gpio.c | 188 +++++-----
> drivers/mfd/mfd-core.c | 40 ++
> drivers/misc/eeprom/at25.c | 41 +--
> drivers/of/base.c | 188 ++++++++++
> include/acpi/acpi_bus.h | 8 +
> include/linux/acpi.h | 90 ++++-
> include/linux/gpio/consumer.h | 7 +
> include/linux/gpio_keys.h | 3 +
> include/linux/leds.h | 1 +
> include/linux/mfd/core.h | 3 +
> include/linux/of.h | 37 ++
> include/linux/property.h | 193 ++++++++++
> 26 files changed, 2377 insertions(+), 408 deletions(-)
> create mode 100644 Documentation/acpi/properties.txt
> create mode 100644 drivers/acpi/property.c
> create mode 100644 drivers/base/property.c
> create mode 100644 include/linux/property.h
>
> DSDT modifications for Minnowboard (for leds-gpio.c and gpio_keys_polled.c)
> ---------------------------------------------------------------------------
>
> Scope (\_SB.PCI0.LPC)
> {
> Device (LEDS)
> {
> Name (_HID, "PRP0001")
>
> Name (_CRS, ResourceTemplate () {
> GpioIo (Exclusive, PullDown, 0, 0, IoRestrictionOutputOnly,
> "\\_SB.PCI0.LPC", 0, ResourceConsumer) {10}
> GpioIo (Exclusive, PullDown, 0, 0, IoRestrictionOutputOnly,
> "\\_SB.PCI0.LPC", 0, ResourceConsumer) {11}
> })

>
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {

> Package () {"compatible", Package () {"gpio-leds"}},
> }
> })
>
> Device (LEDH)
> {

> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {

> Package () {"label", "Heartbeat"},
> Package () {"gpios", Package () {^^LEDS, 0, 0, 0}},
> Package () {"linux,default-trigger", "heartbeat"},
> Package () {"linux,default-state", "off"},
> Package () {"linux,retain-state-suspended", 1},
> }
> })
> }
>
> Device (LEDM)
> {

> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {

> Package () {"label", "MMC0 Activity"},
> Package () {"gpios", Package () {^^LEDS, 1, 0, 0}},
> Package () {"linux,default-trigger", "mmc0"},
> Package () {"linux,default-state", "off"},
> Package () {"linux,retain-state-suspended", 1},
> }
> })
> }
> }
>
> Device (BTNS)
> {
> Name (_HID, "PRP0001")
>
> Name (_CRS, ResourceTemplate () {
> GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
> "\\_SB.PCI0.LPC", 0, ResourceConsumer) {0, 1, 2, 3}
> })

>
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {

> Package () {"compatible", Package () {"gpio-keys-polled"}},
> Package () {"poll-interval", 100},
> Package () {"autorepeat", 1}
> }
> })
>
> Device (BTN0)
> {

> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {

> Package () {"linux,code", 105},
> Package () {"linux,input-type", 1},
> Package () {"gpios", Package () {^^BTNS, 0, 0, 1}},
> }
> })
> }
>
> Device (BTN1)
> {

> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {

> Package () {"linux,code", 108},
> Package () {"linux,input-type", 1},
> Package () {"gpios", Package (4) {^^BTNS, 0, 1, 1}},
> }
> })
> }
>
> Device (BTN2)
> {

> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {

> Package () {"linux,code", 103},
> Package () {"linux,input-type", 1},
> Package () {"gpios", Package () {^^BTNS, 0, 2, 1}},
> }
> })
> }
>
> Device (BTN3)
> {

> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package ()
> {

> Package () {"linux,code", 106},
> Package () {"linux,input-type", 1},
> Package () {"gpios", Package (4) {^^BTNS, 0, 3, 1}},
> }
> })
> }
> }
> }
>
> DSDT modifications for Minnowboard MAX (for at25.c)
> ---------------------------------------------------
>
> Scope (\_SB.SPI1)
> {
> Device (AT25)
> {
> Name (_HID, "PRP0001")
> Method (_CRS, 0, Serialized) {
> Name (UBUF, ResourceTemplate () {
> SpiSerialBus (0x0000, PolarityLow, FourWireMode, 0x08,
> ControllerInitiated, 0x007A1200, ClockPolarityLow,
> ClockPhaseSecond, "\\_SB.SPI1",
> 0x00, ResourceConsumer)
> })
> Return (UBUF)

> }
>
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {

> Package () {"compatible", Package () {"atmel,at25"}},
> Package () {"size", 1024},
> Package () {"pagesize", 32},
> Package () {"address-width", 16},
> }
> })
>
> Method (_STA, 0, NotSerialized)
> {
> Return (0xF)

> }
> }
> }
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:04:11 PM9/30/14

to linux-...@vger.kernel.org, Greg Kroah-Hartman, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: "Rafael J. Wysocki" <rafael.j...@intel.com>

Add a uniform interface by which device drivers can request device
properties from the platform firmware by providing a property name
and the corresponding data type. The purpose of it is to help to
write portable code that won't depend on any particular platform
firmware interface.

Three general helper functions, device_get_property(),
device_read_property() and device_read_property_array() are provided.
The first one allows the raw value of a given device property to be
accessed. The remaining two allow the value of a numeric or string
property and multiple numeric or string values of one array
property to be acquired, respectively. Static inline wrappers are also
provided for the various property data types that can be passed to
device_read_property() or device_read_property_array() for extra type
checking.

In addition to that, new generic routines are provided for retrieving
properties from device description objects in the platform firmware
in case a device driver needs/wants to access properties of a child
object of a given device object. There are cases in which there is
no struct device representation of such child objects and this
additional API is useful then. Again, three functions are provided,
device_get_child_property(), device_read_child_property(),
device_read_child_property_array(), in analogy with device_get_property(),
device_read_property() and device_read_property_array() described above,
respectively, along with static inline wrappers for all of the propery
data types that can be used. For all of them, the first argument is
a struct device pointer to the parent device object and the second
argument is a (void *) pointer to the child description provided by
the platform firmware (either ACPI or FDT).

Finally, device_for_each_child_node() is added for iterating over
the children of the device description object associated with a
given device.

The interface covers both ACPI and Device Trees.

This change set includes material from Mika Westerberg and Aaron Lu.

Signed-off-by: Aaron Lu <aaro...@intel.com>

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

Greg, please let me know if you're fine with this one.

---
drivers/acpi/property.c | 188 +++++++++++++++++++++++++++++++++++++
drivers/base/Makefile | 2
drivers/base/property.c | 235 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/of/base.c | 186 +++++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 42 ++++++++
include/linux/of.h | 37 +++++++
include/linux/property.h | 207 +++++++++++++++++++++++++++++++++++++++++
7 files changed, 896 insertions(+), 1 deletion(-)

create mode 100644 drivers/base/property.c
create mode 100644 include/linux/property.h

Index: linux-pm/drivers/acpi/property.c
===================================================================
--- linux-pm.orig/drivers/acpi/property.c
+++ linux-pm/drivers/acpi/property.c
@@ -362,3 +362,191 @@ int acpi_dev_get_property_reference(stru
return -EPROTO;
}
EXPORT_SYMBOL_GPL(acpi_dev_get_property_reference);
+
+int acpi_dev_prop_get(struct acpi_device *adev, const char *propname,
+ void **valptr)
+{
+ return acpi_dev_get_property(adev, propname, ACPI_TYPE_ANY,
+ (const union acpi_object **)valptr);
+}
+
+int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
+ enum dev_prop_type proptype, void *val)
+{
+ const union acpi_object *obj;
+ int ret = -EINVAL;
+
+ if (!val)
+ return -EINVAL;
+
+ if (proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64) {
+ ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_INTEGER, &obj);

+ if (ret)
+ return ret;
+

+ switch (proptype) {
+ case DEV_PROP_U8:
+ if (obj->integer.value > U8_MAX)
+ return -EOVERFLOW;
+ *(u8 *)val = obj->integer.value;
+ break;
+ case DEV_PROP_U16:
+ if (obj->integer.value > U16_MAX)
+ return -EOVERFLOW;
+ *(u16 *)val = obj->integer.value;
+ break;
+ case DEV_PROP_U32:
+ if (obj->integer.value > U32_MAX)
+ return -EOVERFLOW;
+ *(u32 *)val = obj->integer.value;
+ break;
+ default:
+ *(u64 *)val = obj->integer.value;
+ break;
+ }
+ } else if (proptype == DEV_PROP_STRING) {
+ ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_STRING, &obj);

+ if (ret)
+ return ret;
+

+ *(char **)val = obj->string.pointer;
+ }

+ return ret;
+}
+

+static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
+ size_t nval)

+{
+ int i;
+

+ for (i = 0; i < nval; i++) {
+ if (items[i].type != ACPI_TYPE_INTEGER)
+ return -EPROTO;
+ if (items[i].integer.value > U8_MAX)
+ return -EOVERFLOW;
+
+ val[i] = items[i].integer.value;

+ }
+ return 0;
+}
+

+static int acpi_copy_property_array_u16(const union acpi_object *items,
+ u16 *val, size_t nval)

+{
+ int i;
+

+ for (i = 0; i < nval; i++) {
+ if (items[i].type != ACPI_TYPE_INTEGER)
+ return -EPROTO;
+ if (items[i].integer.value > U16_MAX)
+ return -EOVERFLOW;
+
+ val[i] = items[i].integer.value;

+ }
+ return 0;
+}
+

+static int acpi_copy_property_array_u32(const union acpi_object *items,
+ u32 *val, size_t nval)

+{
+ int i;
+

+ for (i = 0; i < nval; i++) {
+ if (items[i].type != ACPI_TYPE_INTEGER)
+ return -EPROTO;
+ if (items[i].integer.value > U32_MAX)
+ return -EOVERFLOW;
+
+ val[i] = items[i].integer.value;

+ }
+ return 0;
+}
+

+static int acpi_copy_property_array_u64(const union acpi_object *items,
+ u64 *val, size_t nval)

+{
+ int i;
+

+ for (i = 0; i < nval; i++) {
+ if (items[i].type != ACPI_TYPE_INTEGER)
+ return -EPROTO;
+
+ val[i] = items[i].integer.value;

+ }
+ return 0;
+}
+

+static int acpi_copy_property_array_string(const union acpi_object *items,
+ char **val, size_t nval)

+{
+ int i;
+

+ for (i = 0; i < nval; i++) {
+ if (items[i].type != ACPI_TYPE_STRING)
+ return -EPROTO;
+
+ val[i] = items[i].string.pointer;

+ }
+ return 0;
+}
+

+int acpi_dev_prop_read_array(struct acpi_device *adev, const char *propname,
+ enum dev_prop_type proptype, void *val,
+ size_t nval)
+{
+ const union acpi_object *obj;
+ const union acpi_object *items;
+ int ret;
+
+ ret = acpi_dev_get_property_array(adev, propname, ACPI_TYPE_ANY, &obj);

+ if (ret)
+ return ret;
+

+ if (!val)
+ return obj->package.count;
+
+ if (nval > obj->package.count)
+ nval = obj->package.count;
+
+ items = obj->package.elements;
+ switch (proptype) {
+ case DEV_PROP_U8:
+ ret = acpi_copy_property_array_u8(items, (u8 *)val, nval);
+ break;
+ case DEV_PROP_U16:
+ ret = acpi_copy_property_array_u16(items, (u16 *)val, nval);
+ break;
+ case DEV_PROP_U32:
+ ret = acpi_copy_property_array_u32(items, (u32 *)val, nval);
+ break;
+ case DEV_PROP_U64:
+ ret = acpi_copy_property_array_u64(items, (u64 *)val, nval);
+ break;
+ case DEV_PROP_STRING:
+ ret = acpi_copy_property_array_string(items, (char **)val, nval);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }

+ return ret;
+}
+

+int acpi_for_each_child_node(struct device *dev,
+ int (*fn)(struct device *dev, void *child, void *data),
+ void *data)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+ struct acpi_device *child;
+ int ret = 0;
+
+ if (!adev)
+ return -EINVAL;
+

+ list_for_each_entry(child, &adev->children, node) {

+ ret = fn(dev, child, data);

+ if (ret)
+ break;
+ }
+ return ret;
+}

Index: linux-pm/drivers/base/Makefile
===================================================================
--- linux-pm.orig/drivers/base/Makefile
+++ linux-pm/drivers/base/Makefile
@@ -4,7 +4,7 @@ obj-y := component.o core.o bus.o dd.o
driver.o class.o platform.o \
cpu.o firmware.o init.o map.o devres.o \
attribute_container.o transport_class.o \
- topology.o container.o
+ topology.o container.o property.o
obj-$(CONFIG_DEVTMPFS) += devtmpfs.o
obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
obj-y += power/
Index: linux-pm/drivers/base/property.c
===================================================================
--- /dev/null
+++ linux-pm/drivers/base/property.c
@@ -0,0 +1,235 @@
+/*
+ * property.c - Unified device property interface.

+ *
+ * Copyright (C) 2014, Intel Corporation

+ * Authors: Rafael J. Wysocki <rafael.j...@intel.com>
+ * Mika Westerberg <mika.we...@linux.intel.com>
+ *

+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+

+#include <linux/property.h>
+#include <linux/export.h>
+#include <linux/acpi.h>
+#include <linux/of.h>
+
+/**
+ * device_get_property - return a raw property of a device
+ * @dev: Device get the property of
+ * @propname: Name of the property
+ * @valptr: The raw property value is stored here
+ *
+ * Function reads property @propname from the device firmware description and
+ * stores the raw value into @valptr if found. Otherwise returns a negative
+ * errno as specified below.
+ *
+ * Return: %0 if the property was found (success),
+ * %-EINVAL if given arguments are not valid,
+ * %-ENODATA if the property does not exist.
+ */
+int device_get_property(struct device *dev, const char *propname, void **valptr)
+{
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ return of_dev_prop_get(dev->of_node, propname, valptr);
+
+ return acpi_dev_prop_get(ACPI_COMPANION(dev), propname, valptr);
+}
+EXPORT_SYMBOL_GPL(device_get_property);
+
+/**
+ * device_get_child_property - return a raw property of a device's child
+ * @dev: Parent device
+ * @child: Child to get a property of
+ * @propname: Name of the property
+ * @valptr: The raw property value is stored here
+ *
+ * Function reads property @propname from the firmware description of @child and
+ * stores the raw value into @valptr if found. Otherwise returns a negative
+ * errno as specified below.
+ *
+ * Return: %0 if the property was found (success),
+ * %-EINVAL if given arguments are not valid,
+ * %-ENODATA if the property does not exist.
+ */
+int device_get_child_property(struct device *dev, void *child,
+ const char *propname, void **valptr)
+{
+ if (!child)
+ return -EINVAL;
+
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ return of_dev_prop_get(child, propname, valptr);
+ else if (ACPI_COMPANION(dev))
+ return acpi_dev_prop_get(child, propname, valptr);

+
+ return -ENODATA;
+}

+EXPORT_SYMBOL_GPL(device_get_child_property);
+
+/**
+ * device_read_property - read a typed property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @proptype: Type of the property
+ * @val: The value is stored here
+ *
+ * Function reads property @propname from the device firmware description and
+ * stores the value into @val if found. The value is checked to be of type
+ * @proptype.
+ *
+ * Return: %0 if the property was found (success),
+ * %-EINVAL if given arguments are not valid,
+ * %-ENODATA if the property does not exist,
+ * %-EPROTO if the property type does not match @proptype,
+ * %-EOVERFLOW if the property value is out of bounds of @proptype.
+ */
+int device_read_property(struct device *dev, const char *propname,
+ enum dev_prop_type proptype, void *val)
+{
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ return of_dev_prop_read(dev->of_node, propname, proptype, val);
+
+ return acpi_dev_prop_read(ACPI_COMPANION(dev), propname, proptype, val);
+}
+EXPORT_SYMBOL_GPL(device_read_property);
+
+/**
+ * device_read_child_property - read a typed property of a device's child
+ * @dev: Parent device
+ * @child: Child to read a property of
+ * @propname: Name of the property
+ * @proptype: Type of the property
+ * @val: The value is stored here
+ *
+ * Function reads property @propname from the firmware description of @child and
+ * stores the value into @val if found. The value is checked to be of type
+ * @proptype.
+ *
+ * Return: %0 if the property was found (success),
+ * %-EINVAL if given arguments are not valid,
+ * %-ENODATA if the property does not exist,
+ * %-EPROTO if the property type does not match @proptype,
+ * %-EOVERFLOW if the property value is out of bounds of @proptype.
+ */
+int device_read_child_property(struct device *dev, void *child,
+ const char *propname, enum dev_prop_type proptype,
+ void *val)
+{
+ if (!child)
+ return -EINVAL;
+
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ return of_dev_prop_read(child, propname, proptype, val);
+ else if (ACPI_COMPANION(dev))
+ return acpi_dev_prop_read(child, propname, proptype, val);

+
+ return -ENODATA;
+}

+EXPORT_SYMBOL_GPL(device_read_child_property);
+
+/**
+ * device_read_property_array - read an array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @proptype: Type of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of properties with @propname from the device
+ * firmware description and stores them to @val if found. All the values
+ * in the array must be of type @proptype.
+ *
+ * Return: %0 if the property was found (success),
+ * %-EINVAL if given arguments are not valid,
+ * %-ENODATA if the property does not exist,
+ * %-EPROTO if the property type does not match @proptype,
+ * %-EOVERFLOW if the property value is out of bounds of @proptype.
+ */
+int device_read_property_array(struct device *dev, const char *propname,
+ enum dev_prop_type proptype, void *val,
+ size_t nval)
+{
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ return of_dev_prop_read_array(dev->of_node, propname, proptype,
+ val, nval);
+
+ return acpi_dev_prop_read_array(ACPI_COMPANION(dev), propname, proptype,
+ val, nval);
+}
+EXPORT_SYMBOL_GPL(device_read_property_array);
+
+/**
+ * device_read_child_property_array - read an array property of a device's child
+ * @dev: Parent device
+ * @child: Child to get the property of
+ * @propname: Name of the property
+ * @proptype: Type of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of properties with @propname from the firmware
+ * description of @child and stores them to @val if found. All the values
+ * in the array must be of type @proptype.
+ *
+ * Return: %0 if the property was found (success),
+ * %-EINVAL if given arguments are not valid,
+ * %-ENODATA if the property does not exist,
+ * %-EPROTO if the property type does not match @proptype,
+ * %-EOVERFLOW if the property value is out of bounds of @proptype.
+ */
+int device_read_child_property_array(struct device *dev, void *child,
+ const char *propname,
+ enum dev_prop_type proptype, void *val,
+ size_t nval)
+{
+ if (!child)
+ return -EINVAL;
+
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ return of_dev_prop_read_array(child, propname, proptype,
+ val, nval);
+ else if (ACPI_COMPANION(dev))
+ return acpi_dev_prop_read_array(child, propname, proptype,
+ val, nval);

+
+ return -ENODATA;
+}

+EXPORT_SYMBOL_GPL(device_read_child_property_array);
+
+/**
+ * device_for_each_child_node - execute function for each child node of device
+ * @dev: Device to run the function for
+ * @fn: Function to run
+ * @data: Additional data to pass to the function
+ */
+int device_for_each_child_node(struct device *dev,
+ int (*fn)(struct device *dev, void *child, void *data),
+ void *data)
+{
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ return of_for_each_child_node(dev, fn, data);
+
+ return acpi_for_each_child_node(dev, fn, data);
+}
+EXPORT_SYMBOL_GPL(device_for_each_child_node);
+
+static int increment_count(struct device *dev, void *child, void *data)
+{
+ *((unsigned int *)data) += 1;

+ return 0;
+}
+

+/**
+ * device_get_child_node_count - return the number of child nodes for device
+ * @dev: Device to cound the child nodes for
+ */
+unsigned int device_get_child_node_count(struct device *dev)
+{
+ unsigned int count = 0;
+
+ device_for_each_child_node(dev, increment_count, &count);
+ return count;
+}
+EXPORT_SYMBOL_GPL(device_get_child_node_count);
Index: linux-pm/drivers/of/base.c
===================================================================
--- linux-pm.orig/drivers/of/base.c
+++ linux-pm/drivers/of/base.c
@@ -1247,6 +1247,39 @@ int of_property_read_u64(const struct de
EXPORT_SYMBOL_GPL(of_property_read_u64);

/**
+ * of_property_read_u64_array - Find and read an array of 64 bit integers
+ * from a property.
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ * @out_values: pointer to return value, modified only if return value is 0.
+ * @sz: number of array elements to read
+ *
+ * Search for a property in a device node and read 64-bit value(s) from
+ * it. Returns 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * The out_values is modified only if a valid u64 value can be decoded.
+ */
+int of_property_read_u64_array(const struct device_node *np,
+ const char *propname, u64 *out_values,
+ size_t sz)
+{
+ const __be32 *val = of_find_property_value_of_size(np, propname,
+ (sz * sizeof(*out_values)));
+
+ if (IS_ERR(val))
+ return PTR_ERR(val);
+
+ while (sz--) {
+ *out_values++ = of_read_number(val, 2);
+ val += 2;

+ }
+ return 0;
+}
+

+/**
* of_property_read_string - Find and read a string from a property
* @np: device node from which the property value is to be read.
* @propname: name of the property to be searched.
@@ -1394,6 +1427,49 @@ int of_property_count_strings(struct dev
}
EXPORT_SYMBOL_GPL(of_property_count_strings);

+/**
+ * of_property_read_string_array - Find and read an array of strings
+ * from a multiple strings property.
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ * @out_string: pointer to null terminated return string, modified only if
+ * return value is 0.
+ * @sz: number of array elements to read
+ *
+ * Search for a property in a device tree node and retrieve a list of
+ * terminated string value (pointer to data, not a copy) in that property.
+ * Returns 0 on success, -EINVAL if the property does not exist, -ENODATA if
+ * property does not have a value, and -EOVERFLOW if the string is not
+ * null-terminated within the length of the property data.
+ *
+ * The out_string pointer is modified only if a valid string can be decoded.
+ */
+int of_property_read_string_array(struct device_node *np, const char *propname,
+ char **output, size_t sz)
+{
+ struct property *prop = of_find_property(np, propname, NULL);
+ int i = 0;
+ size_t l = 0, total = 0;
+ char *p;
+
+ if (!prop)
+ return -EINVAL;
+
+ if (!prop->value)
+ return -ENODATA;
+
+ if (strnlen(prop->value, prop->length) >= prop->length)
+ return -EOVERFLOW;
+
+ p = prop->value;
+
+ for (i = 0; total < prop->length; total += l, p += l) {
+ output[i++] = p;
+ l = strlen(p) + 1;

+ }
+ return 0;
+}
+

void of_print_phandle_args(const char *msg, const struct of_phandle_args *args)
{
int i;
@@ -2183,3 +2259,113 @@ struct device_node *of_graph_get_remote_
return of_get_next_parent(np);
}
EXPORT_SYMBOL(of_graph_get_remote_port);
+
+int of_dev_prop_get(struct device_node *dn, const char *propname, void **valptr)
+{
+ struct property *pp = of_find_property(dn, propname, NULL);
+
+ if (!pp)
+ return -ENODATA;
+
+ if (valptr)
+ *valptr = pp->value;

+ return 0;
+}
+

+int of_dev_prop_read(struct device_node *dn, const char *propname,
+ enum dev_prop_type proptype, void *val)
+{
+ void *value;
+ int ret = of_dev_prop_get(dn, propname, &value);
+

+ if (ret)
+ return ret;
+

+ if (proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64) {
+ switch (proptype) {
+ case DEV_PROP_U8: {
+ *(u8 *)val = *(u8 *)value;
+ break;
+ }
+ case DEV_PROP_U16:
+ *(u16 *)val = *(u16 *)value;
+ break;
+ case DEV_PROP_U32:
+ *(u32 *)val = *(u32 *)value;
+ break;
+ default:
+ *(u64 *)val = *(u64 *)value;
+ break;
+ }
+ } else if (proptype == DEV_PROP_STRING) {
+ *(char **)val = value;
+ }

+ return ret;
+
+}

+
+int of_dev_prop_read_array(struct device_node *dn, const char *propname,
+ enum dev_prop_type proptype, void *val, size_t nval)
+{
+ int ret, elem_size;
+
+ if (!val) {
+ switch (proptype) {
+ case DEV_PROP_U8:
+ elem_size = sizeof(u8);
+ break;
+ case DEV_PROP_U16:
+ elem_size = sizeof(u16);
+ break;
+ case DEV_PROP_U32:
+ elem_size = sizeof(u32);
+ break;
+ case DEV_PROP_U64:
+ elem_size = sizeof(u64);
+ break;
+ case DEV_PROP_STRING:
+ return of_property_count_strings(dn, propname);
+ default:
+ return -EINVAL;
+ }
+ return of_property_count_elems_of_size(dn, propname, elem_size);
+ }
+
+ switch (proptype) {
+ case DEV_PROP_U8:
+ ret = of_property_read_u8_array(dn, propname, (u8 *)val, nval);
+ break;
+ case DEV_PROP_U16:
+ ret = of_property_read_u16_array(dn, propname, (u16 *)val, nval);
+ break;
+ case DEV_PROP_U32:
+ ret = of_property_read_u32_array(dn, propname, (u32 *)val, nval);
+ break;
+ case DEV_PROP_U64:
+ ret = of_property_read_u64_array(dn, propname, (u64 *)val, nval);
+ break;
+ case DEV_PROP_STRING:
+ ret = of_property_read_string_array(dn, propname,
+ (char **)val, nval);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }

+ return ret;
+}
+

+int of_for_each_child_node(struct device *dev,
+ int (*fn)(struct device *dev, void *child, void *data),
+ void *data)
+{
+ struct device_node *child;

+ int ret = 0;
+

+ for_each_child_of_node(dev->of_node, child) {
+ ret = fn(dev, child, data);

+ if (ret)
+ break;
+ }
+ return ret;
+}

Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h

@@ -28,6 +28,7 @@
#include <linux/errno.h>
#include <linux/ioport.h> /* for struct resource */
#include <linux/device.h>
+#include <linux/property.h>

#ifndef _LINUX
#define _LINUX
@@ -676,6 +677,17 @@ int acpi_dev_get_property_array(struct a

int acpi_dev_get_property_reference(struct acpi_device *adev, const char *name,

const char *cells_name, size_t index,

struct acpi_reference_args *args);
+
+int acpi_dev_prop_get(struct acpi_device *adev, const char *propname,
+ void **valptr);
+int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
+ enum dev_prop_type proptype, void *val);
+int acpi_dev_prop_read_array(struct acpi_device *adev, const char *propname,
+ enum dev_prop_type proptype, void *val,
+ size_t nval);
+int acpi_for_each_child_node(struct device *dev,
+ int (*fn)(struct device *dev, void *child, void *data),
+ void *data);
#else

static inline int acpi_dev_get_property(struct acpi_device *adev,

const char *name, acpi_object_type type,

@@ -696,6 +708,36 @@ static inline int acpi_dev_get_property_
{
return -ENXIO;
}
+
+static inline int acpi_dev_prop_get(struct acpi_device *adev,
+ const char *propname,
+ void **valptr)

+{
+ return -ENXIO;
+}
+

+static inline int acpi_dev_prop_read(struct acpi_device *adev,
+ const char *propname,
+ enum dev_prop_type proptype, void *val)

+{
+ return -ENXIO;
+}
+

+static inline int acpi_dev_prop_read_array(struct acpi_device *adev,
+ const char *propname,
+ enum dev_prop_type proptype,
+ void *val, size_t nval)

+{
+ return -ENXIO;
+}
+

+static inline int acpi_for_each_child_node(struct device *dev,
+ int (*fn)(struct device *dev, void *child, void *data),
+ void *data)

+{
+ return -ENXIO;
+}
+

#endif

#endif /*_LINUX_ACPI_H*/
Index: linux-pm/include/linux/of.h
===================================================================
--- linux-pm.orig/include/linux/of.h
+++ linux-pm/include/linux/of.h
@@ -23,6 +23,7 @@
#include <linux/spinlock.h>
#include <linux/topology.h>
#include <linux/notifier.h>
+#include <linux/property.h>

#include <asm/byteorder.h>
#include <asm/errno.h>
@@ -355,6 +356,15 @@ const char *of_prop_next_string(struct p

bool of_console_check(struct device_node *dn, char *name, int index);

+int of_dev_prop_get(struct device_node *dn, const char *propname, void **valptr);
+int of_dev_prop_read(struct device_node *dn, const char *propname,
+ enum dev_prop_type proptype, void *val);
+int of_dev_prop_read_array(struct device_node *dn, const char *propname,
+ enum dev_prop_type proptype, void *val, size_t nval);
+int of_for_each_child_node(struct device *dev,
+ int (*fn)(struct device *dev, void *child, void *data),
+ void *data);
+
#else /* CONFIG_OF */

static inline const char* of_node_full_name(const struct device_node *np)
@@ -582,6 +592,33 @@ static inline const char *of_prop_next_s
return NULL;
}

+static inline int of_dev_prop_get(struct device_node *dn, const char *propname,
+ void **valptr)

+{
+ return -ENXIO;
+}
+

+static inline int of_dev_prop_read(struct device_node *dn, const char *propname,
+ enum dev_prop_type proptype, void *val)

+{
+ return -ENXIO;
+}
+

+static inline int of_dev_prop_read_array(struct device_node *dn,
+ const char *propname,
+ enum dev_prop_type proptype,
+ void *val, size_t nval)

+{
+ return -ENXIO;
+}
+

+static inline int of_for_each_child_node(struct device *dev,
+ int (*fn)(struct device *dev, void *child, void *data),
+ void *data)

+{
+ return -ENXIO;
+}
+

#define of_match_ptr(_ptr) NULL
#define of_match_node(_matches, _node) NULL
#endif /* CONFIG_OF */
Index: linux-pm/include/linux/property.h
===================================================================
--- /dev/null
+++ linux-pm/include/linux/property.h
@@ -0,0 +1,207 @@
+/*
+ * property.h - Unified device property interface.

+ *
+ * Copyright (C) 2014, Intel Corporation

+ * Authors: Rafael J. Wysocki <rafael.j...@intel.com>
+ * Mika Westerberg <mika.we...@linux.intel.com>
+ *

+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+

+#ifndef _LINUX_PROPERTY_H_
+#define _LINUX_PROPERTY_H_
+
+#include <linux/device.h>
+
+enum dev_prop_type {
+ DEV_PROP_U8,
+ DEV_PROP_U16,
+ DEV_PROP_U32,
+ DEV_PROP_U64,
+ DEV_PROP_STRING,
+ DEV_PROP_MAX,
+};
+
+int device_get_property(struct device *dev, const char *propname,
+ void **valptr);
+int device_read_property(struct device *dev, const char *propname,
+ enum dev_prop_type proptype, void *val);
+int device_read_property_array(struct device *dev, const char *propname,
+ enum dev_prop_type proptype, void *val,
+ size_t nval);
+int device_get_child_property(struct device *dev, void *child,
+ const char *propname, void **valptr);
+int device_read_child_property(struct device *dev, void *child,
+ const char *propname,
+ enum dev_prop_type proptype, void *val);
+int device_read_child_property_array(struct device *dev, void *child,
+ const char *propname,
+ enum dev_prop_type proptype, void *val,
+ size_t nval);
+int device_for_each_child_node(struct device *dev,
+ int (*fn)(struct device *dev, void *child, void *data),
+ void *data);
+unsigned int device_get_child_node_count(struct device *dev);
+
+static inline int device_property_read_u8(struct device *dev,
+ const char *propname, u8 *out_value)
+{
+ return device_read_property(dev, propname, DEV_PROP_U8, out_value);
+}
+
+static inline int device_property_read_u16(struct device *dev,
+ const char *propname, u16 *out_value)
+{
+ return device_read_property(dev, propname, DEV_PROP_U16, out_value);
+}
+
+static inline int device_property_read_u32(struct device *dev,
+ const char *propname, u32 *out_value)
+{
+ return device_read_property(dev, propname, DEV_PROP_U32, out_value);
+}
+
+static inline int device_property_read_u64(struct device *dev,
+ const char *propname, u64 *out_value)
+{
+ return device_read_property(dev, propname, DEV_PROP_U64, out_value);
+}
+
+static inline int device_property_read_u8_array(struct device *dev,
+ const char *propname,
+ u8 *val, size_t nval)
+{
+ return device_read_property_array(dev, propname, DEV_PROP_U8, val,
+ nval);
+}
+
+static inline int device_property_read_u16_array(struct device *dev,
+ const char *propname,
+ u16 *val, size_t nval)
+{
+ return device_read_property_array(dev, propname, DEV_PROP_U16, val,
+ nval);
+}
+
+static inline int device_property_read_u32_array(struct device *dev,
+ const char *propname,
+ u32 *val, size_t nval)
+{
+ return device_read_property_array(dev, propname, DEV_PROP_U32, val,
+ nval);
+}
+
+static inline int device_property_read_u64_array(struct device *dev,
+ const char *propname,
+ u64 *val, size_t nval)
+{
+ return device_read_property_array(dev, propname, DEV_PROP_U64, val,
+ nval);
+}
+
+static inline int device_property_read_string(struct device *dev,
+ const char *propname,
+ const char **out_string)
+{
+ return device_read_property(dev, propname, DEV_PROP_STRING, out_string);
+}
+
+static inline int device_property_read_string_array(struct device *dev,
+ const char *propname,
+ const char **out_strings,
+ size_t nstrings)
+{
+ return device_read_property_array(dev, propname, DEV_PROP_STRING,
+ out_strings, nstrings);
+}
+
+static inline int device_child_property_read_u8(struct device *dev, void *child,
+ const char *propname,
+ u8 *out_value)
+{
+ return device_read_child_property(dev, child, propname, DEV_PROP_U8,
+ out_value);
+}
+
+static inline int device_child_property_read_u16(struct device *dev, void *child,
+ const char *propname,
+ u16 *out_value)
+{
+ return device_read_child_property(dev, child, propname, DEV_PROP_U16,
+ out_value);
+}
+
+static inline int device_child_property_read_u32(struct device *dev, void *child,
+ const char *propname,
+ u32 *out_value)
+{
+ return device_read_child_property(dev, child, propname, DEV_PROP_U32,
+ out_value);
+}
+
+static inline int device_child_property_read_u64(struct device *dev, void *child,
+ const char *propname,
+ u64 *out_value)
+{
+ return device_read_child_property(dev, child, propname, DEV_PROP_U64,
+ out_value);
+}
+
+static inline int device_child_property_read_u8_array(struct device *dev,
+ void *child,
+ const char *propname,
+ u8 *val, size_t nval)
+{
+ return device_read_child_property_array(dev, child, propname,
+ DEV_PROP_U8, val, nval);
+}
+
+static inline int device_child_property_read_u16_array(struct device *dev,
+ void *child,
+ const char *propname,
+ u16 *val, size_t nval)
+{
+ return device_read_child_property_array(dev, child, propname,
+ DEV_PROP_U16, val, nval);
+}
+
+static inline int device_child_property_read_u32_array(struct device *dev,
+ void *child,
+ const char *propname,
+ u32 *val, size_t nval)
+{
+ return device_read_child_property_array(dev, child, propname,
+ DEV_PROP_U32, val, nval);
+}
+
+static inline int device_child_property_read_u64_array(struct device *dev,
+ void *child,
+ const char *propname,
+ u64 *val, size_t nval)
+{
+ return device_read_child_property_array(dev, child, propname,
+ DEV_PROP_U64, val, nval);
+}
+
+static inline int device_child_property_read_string(struct device *dev,
+ void *child,
+ const char *propname,
+ const char **out_string)
+{
+ return device_read_child_property(dev, child, propname, DEV_PROP_STRING,
+ out_string);
+}
+
+static inline int device_child_property_read_string_array(struct device *dev,
+ void *child,
+ const char *propname,
+ const char **out_strings,
+ size_t nstrings)
+{
+ return device_read_child_property_array(dev, child, propname,
+ DEV_PROP_STRING,
+ out_strings, nstrings);
+}
+#endif /* _LINUX_PROPERTY_H_ */

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:04:33 PM9/30/14

to linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: Mika Westerberg <mika.we...@linux.intel.com>

This is actually a single device with two sets of identical registers,
which just happen to start from a different offset. Instead of having
separate GPIO chips created we consolidate them to be single GPIO chip.

In addition having a single GPIO chip allows us to handle ACPI GPIO
translation in the core in a more generic way, since the two GPIO chips
share the same parent ACPI device.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
Acked-by: Linus Walleij <linus....@linaro.org>

Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

drivers/gpio/gpio-sch.c | 293 ++++++++++++++++++------------------------------
1 file changed, 112 insertions(+), 181 deletions(-)

diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 41e91d70301e..99720c8bc8ed 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -29,290 +29,221 @@

#include <linux/gpio.h>

-static DEFINE_SPINLOCK(gpio_lock);
-
-#define CGEN (0x00)
-#define CGIO (0x04)
-#define CGLV (0x08)
-
-#define RGEN (0x20)
-#define RGIO (0x24)
-#define RGLV (0x28)
-
-static unsigned short gpio_ba;
-
-static int sch_gpio_core_direction_in(struct gpio_chip *gc, unsigned gpio_num)
-{
- u8 curr_dirs;
- unsigned short offset, bit;
-
- spin_lock(&gpio_lock);
-
- offset = CGIO + gpio_num / 8;
- bit = gpio_num % 8;
-
- curr_dirs = inb(gpio_ba + offset);
-
- if (!(curr_dirs & (1 << bit)))
- outb(curr_dirs | (1 << bit), gpio_ba + offset);
+#define GEN 0x00
+#define GIO 0x04
+#define GLV 0x08
+
+struct sch_gpio {
+ struct gpio_chip chip;
+ spinlock_t lock;
+ unsigned short iobase;
+ unsigned short core_base;
+ unsigned short resume_base;
+};

- spin_unlock(&gpio_lock);
- return 0;
-}
+#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)

-static int sch_gpio_core_get(struct gpio_chip *gc, unsigned gpio_num)
+static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
- int res;
- unsigned short offset, bit;
+ unsigned base = 0;

- offset = CGLV + gpio_num / 8;
- bit = gpio_num % 8;
+ if (gpio >= sch->resume_base) {
+ gpio -= sch->resume_base;
+ base += 0x20;
+ }

- res = !!(inb(gpio_ba + offset) & (1 << bit));
- return res;
+ return base + reg + gpio / 8;
}

-static void sch_gpio_core_set(struct gpio_chip *gc, unsigned gpio_num, int val)
+static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned gpio)
{
- u8 curr_vals;
- unsigned short offset, bit;
-
- spin_lock(&gpio_lock);
-
- offset = CGLV + gpio_num / 8;
- bit = gpio_num % 8;
-
- curr_vals = inb(gpio_ba + offset);
-
- if (val)
- outb(curr_vals | (1 << bit), gpio_ba + offset);
- else
- outb((curr_vals & ~(1 << bit)), gpio_ba + offset);
- spin_unlock(&gpio_lock);
+ if (gpio >= sch->resume_base)
+ gpio -= sch->resume_base;
+ return gpio % 8;
}

-static int sch_gpio_core_direction_out(struct gpio_chip *gc,
- unsigned gpio_num, int val)
+static void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio)
{
- u8 curr_dirs;
unsigned short offset, bit;
+ u8 enable;

- spin_lock(&gpio_lock);
+ spin_lock(&sch->lock);

- offset = CGIO + gpio_num / 8;
- bit = gpio_num % 8;
-
- curr_dirs = inb(gpio_ba + offset);
- if (curr_dirs & (1 << bit))
- outb(curr_dirs & ~(1 << bit), gpio_ba + offset);
+ offset = sch_gpio_offset(sch, gpio, GEN);
+ bit = sch_gpio_bit(sch, gpio);

- spin_unlock(&gpio_lock);
+ enable = inb(sch->iobase + offset);
+ if (!(enable & (1 << bit)))
+ outb(enable | (1 << bit), sch->iobase + offset);

- /*
- * according to the datasheet, writing to the level register has no
- * effect when GPIO is programmed as input.
- * Actually the the level register is read-only when configured as input.
- * Thus presetting the output level before switching to output is _NOT_ possible.
- * Hence we set the level after configuring the GPIO as output.
- * But we cannot prevent a short low pulse if direction is set to high
- * and an external pull-up is connected.
- */
- sch_gpio_core_set(gc, gpio_num, val);
- return 0;
+ spin_unlock(&sch->lock);
}

-static struct gpio_chip sch_gpio_core = {
- .label = "sch_gpio_core",
- .owner = THIS_MODULE,
- .direction_input = sch_gpio_core_direction_in,
- .get = sch_gpio_core_get,
- .direction_output = sch_gpio_core_direction_out,
- .set = sch_gpio_core_set,
-};
-
-static int sch_gpio_resume_direction_in(struct gpio_chip *gc,
- unsigned gpio_num)
+static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
{
+ struct sch_gpio *sch = to_sch_gpio(gc);
u8 curr_dirs;
unsigned short offset, bit;

- spin_lock(&gpio_lock);
+ spin_lock(&sch->lock);

- offset = RGIO + gpio_num / 8;
- bit = gpio_num % 8;
+ offset = sch_gpio_offset(sch, gpio_num, GIO);
+ bit = sch_gpio_bit(sch, gpio_num);

- curr_dirs = inb(gpio_ba + offset);
+ curr_dirs = inb(sch->iobase + offset);

if (!(curr_dirs & (1 << bit)))
- outb(curr_dirs | (1 << bit), gpio_ba + offset);
+ outb(curr_dirs | (1 << bit), sch->iobase + offset);

- spin_unlock(&gpio_lock);
+ spin_unlock(&sch->lock);
return 0;
}

-static int sch_gpio_resume_get(struct gpio_chip *gc, unsigned gpio_num)
+static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+ int res;
unsigned short offset, bit;

- offset = RGLV + gpio_num / 8;
- bit = gpio_num % 8;
+ offset = sch_gpio_offset(sch, gpio_num, GLV);
+ bit = sch_gpio_bit(sch, gpio_num);
+
+ res = !!(inb(sch->iobase + offset) & (1 << bit));

- return !!(inb(gpio_ba + offset) & (1 << bit));
+ return res;
}

-static void sch_gpio_resume_set(struct gpio_chip *gc,
- unsigned gpio_num, int val)
+static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
+ struct sch_gpio *sch = to_sch_gpio(gc);
u8 curr_vals;
unsigned short offset, bit;

- spin_lock(&gpio_lock);
+ spin_lock(&sch->lock);

- offset = RGLV + gpio_num / 8;
- bit = gpio_num % 8;
+ offset = sch_gpio_offset(sch, gpio_num, GLV);
+ bit = sch_gpio_bit(sch, gpio_num);

- curr_vals = inb(gpio_ba + offset);
+ curr_vals = inb(sch->iobase + offset);

if (val)
- outb(curr_vals | (1 << bit), gpio_ba + offset);
+ outb(curr_vals | (1 << bit), sch->iobase + offset);
else
- outb((curr_vals & ~(1 << bit)), gpio_ba + offset);
+ outb((curr_vals & ~(1 << bit)), sch->iobase + offset);

- spin_unlock(&gpio_lock);
+ spin_unlock(&sch->lock);
}

-static int sch_gpio_resume_direction_out(struct gpio_chip *gc,
- unsigned gpio_num, int val)
+static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
+ int val)
{
+ struct sch_gpio *sch = to_sch_gpio(gc);
u8 curr_dirs;
unsigned short offset, bit;

- offset = RGIO + gpio_num / 8;
- bit = gpio_num % 8;
+ spin_lock(&sch->lock);

- spin_lock(&gpio_lock);
+ offset = sch_gpio_offset(sch, gpio_num, GIO);
+ bit = sch_gpio_bit(sch, gpio_num);

- curr_dirs = inb(gpio_ba + offset);
+ curr_dirs = inb(sch->iobase + offset);
if (curr_dirs & (1 << bit))
- outb(curr_dirs & ~(1 << bit), gpio_ba + offset);
+ outb(curr_dirs & ~(1 << bit), sch->iobase + offset);

- spin_unlock(&gpio_lock);
+ spin_unlock(&sch->lock);

/*
- * according to the datasheet, writing to the level register has no
- * effect when GPIO is programmed as input.
- * Actually the the level register is read-only when configured as input.
- * Thus presetting the output level before switching to output is _NOT_ possible.
- * Hence we set the level after configuring the GPIO as output.
- * But we cannot prevent a short low pulse if direction is set to high
- * and an external pull-up is connected.
- */
- sch_gpio_resume_set(gc, gpio_num, val);
+ * according to the datasheet, writing to the level register has no
+ * effect when GPIO is programmed as input.
+ * Actually the the level register is read-only when configured as input.
+ * Thus presetting the output level before switching to output is _NOT_ possible.
+ * Hence we set the level after configuring the GPIO as output.
+ * But we cannot prevent a short low pulse if direction is set to high
+ * and an external pull-up is connected.
+ */
+ sch_gpio_set(gc, gpio_num, val);
return 0;
}

-static struct gpio_chip sch_gpio_resume = {
- .label = "sch_gpio_resume",
+static struct gpio_chip sch_gpio_chip = {
+ .label = "sch_gpio",
.owner = THIS_MODULE,
- .direction_input = sch_gpio_resume_direction_in,
- .get = sch_gpio_resume_get,
- .direction_output = sch_gpio_resume_direction_out,
- .set = sch_gpio_resume_set,
+ .direction_input = sch_gpio_direction_in,
+ .get = sch_gpio_get,
+ .direction_output = sch_gpio_direction_out,
+ .set = sch_gpio_set,
};

static int sch_gpio_probe(struct platform_device *pdev)
{
+ struct sch_gpio *sch;
struct resource *res;
- int err, id;

- id = pdev->id;
- if (!id)
- return -ENODEV;
+ sch = devm_kzalloc(&pdev->dev, sizeof(*sch), GFP_KERNEL);
+ if (!sch)
+ return -ENOMEM;

res = platform_get_resource(pdev, IORESOURCE_IO, 0);
if (!res)
return -EBUSY;

- if (!request_region(res->start, resource_size(res), pdev->name))
+ if (!devm_request_region(&pdev->dev, res->start, resource_size(res),
+ pdev->name))
return -EBUSY;

- gpio_ba = res->start;
+ spin_lock_init(&sch->lock);
+ sch->iobase = res->start;
+ sch->chip = sch_gpio_chip;
+ sch->chip.label = dev_name(&pdev->dev);
+ sch->chip.dev = &pdev->dev;

- switch (id) {
+ switch (pdev->id) {
case PCI_DEVICE_ID_INTEL_SCH_LPC:
- sch_gpio_core.base = 0;
- sch_gpio_core.ngpio = 10;
- sch_gpio_resume.base = 10;
- sch_gpio_resume.ngpio = 4;
+ sch->core_base = 0;
+ sch->resume_base = 10;
+ sch->chip.ngpio = 14;
+
/*
* GPIO[6:0] enabled by default
* GPIO7 is configured by the CMC as SLPIOVR
* Enable GPIO[9:8] core powered gpios explicitly
*/
- outb(0x3, gpio_ba + CGEN + 1);
+ sch_gpio_enable(sch, 8);
+ sch_gpio_enable(sch, 9);
/*
* SUS_GPIO[2:0] enabled by default
* Enable SUS_GPIO3 resume powered gpio explicitly
*/
- outb(0x8, gpio_ba + RGEN);
+ sch_gpio_enable(sch, 13);
break;

case PCI_DEVICE_ID_INTEL_ITC_LPC:
- sch_gpio_core.base = 0;
- sch_gpio_core.ngpio = 5;
- sch_gpio_resume.base = 5;
- sch_gpio_resume.ngpio = 9;
+ sch->core_base = 0;
+ sch->resume_base = 5;
+ sch->chip.ngpio = 14;
break;

case PCI_DEVICE_ID_INTEL_CENTERTON_ILB:
- sch_gpio_core.base = 0;
- sch_gpio_core.ngpio = 21;
- sch_gpio_resume.base = 21;
- sch_gpio_resume.ngpio = 9;
+ sch->core_base = 0;
+ sch->resume_base = 21;
+ sch->chip.ngpio = 30;
break;

default:
- err = -ENODEV;
- goto err_sch_gpio_core;
+ return -ENODEV;
}

- sch_gpio_core.dev = &pdev->dev;
- sch_gpio_resume.dev = &pdev->dev;
-
- err = gpiochip_add(&sch_gpio_core);
- if (err < 0)
- goto err_sch_gpio_core;
+ platform_set_drvdata(pdev, sch);

- err = gpiochip_add(&sch_gpio_resume);
- if (err < 0)
- goto err_sch_gpio_resume;
-
- return 0;
-
-err_sch_gpio_resume:
- gpiochip_remove(&sch_gpio_core);
-
-err_sch_gpio_core:
- release_region(res->start, resource_size(res));
- gpio_ba = 0;
-
- return err;
+ return gpiochip_add(&sch->chip);
}

static int sch_gpio_remove(struct platform_device *pdev)
{
- struct resource *res;
- if (gpio_ba) {
-
- gpiochip_remove(&sch_gpio_core);
- gpiochip_remove(&sch_gpio_resume);
-
- res = platform_get_resource(pdev, IORESOURCE_IO, 0);
-
- release_region(res->start, resource_size(res));
- gpio_ba = 0;
- }
+ struct sch_gpio *sch = platform_get_drvdata(pdev);

+ gpiochip_remove(&sch->chip);
return 0;
}

--
1.8.4.5

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:04:36 PM9/30/14

to linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: Mika Westerberg <mika.we...@linux.intel.com>

We have lots of existing Device Tree enabled drivers and allocating
separate _HID for each is not feasible. Instead we allocate special _HID
"PRP0001" that means that the match should be done using Device Tree
compatible property using driver's .of_match_table instead.

If there is a need to distinguish from where the device is enumerated
(DT/ACPI) driver can check dev->of_node or ACPI_COMPATION(dev).

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>

Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

drivers/acpi/property.c | 34 +++++++++++++++++
drivers/acpi/scan.c | 91 ++++++++++++++++++++++++++++++++++++++++++------
include/acpi/acpi_bus.h | 1
include/linux/acpi.h | 8 +---
4 files changed, 118 insertions(+), 16 deletions(-)

Index: linux-pm/drivers/acpi/property.c
===================================================================
--- linux-pm.orig/drivers/acpi/property.c
+++ linux-pm/drivers/acpi/property.c

@@ -76,6 +76,37 @@ static bool acpi_properties_format_valid
return true;
}

+static void acpi_init_of_compatible(struct acpi_device *adev)
+{
+ const union acpi_object *of_compatible;
+ struct acpi_hardware_id *hwid;
+ bool acpi_of = false;
+
+ /*
+ * Check if the special PRP0001 ACPI ID is present and in that
+ * case we fill in Device Tree compatible properties for this
+ * device.
+ */
+ list_for_each_entry(hwid, &adev->pnp.ids, list) {
+ if (!strcmp(hwid->id, "PRP0001")) {
+ acpi_of = true;
+ break;
+ }
+ }
+
+ if (!acpi_of)
+ return;
+
+ if (acpi_dev_get_property_array(adev, "compatible", ACPI_TYPE_STRING,
+ &of_compatible)) {
+ acpi_handle_warn(adev->handle,
+ "PRP0001 requires compatible property\n");
+ return;
+ }
+
+ adev->data.of_compatible = of_compatible;
+}
+
void acpi_init_properties(struct acpi_device *adev)
{

struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };

@@ -119,6 +150,8 @@ void acpi_init_properties(struct acpi_de

adev->data.pointer = buf.pointer;

adev->data.properties = properties;
+

+ acpi_init_of_compatible(adev);
return;
}

@@ -130,6 +163,7 @@ void acpi_init_properties(struct acpi_de
void acpi_free_properties(struct acpi_device *adev)
{
ACPI_FREE((void *)adev->data.pointer);
+ adev->data.of_compatible = NULL;
adev->data.pointer = NULL;
adev->data.properties = NULL;

}
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c

@@ -124,17 +124,43 @@ static int create_modalias(struct acpi_d
if (list_empty(&acpi_dev->pnp.ids))
return 0;

- len = snprintf(modalias, size, "acpi:");
- size -= len;
+ /*
+ * If the device has PRP0001 we expose DT compatible modalias
+ * instead.
+ */
+ if (acpi_dev->data.of_compatible) {
+ const union acpi_object *of_compatible, *obj;
+ int i;
+
+ len = snprintf(modalias, size, "of:Nprp0001Tacpi");
+
+ of_compatible = acpi_dev->data.of_compatible;
+ for (i = 0; i < of_compatible->package.count; i++) {
+ obj = &of_compatible->package.elements[i];
+
+ count = snprintf(&modalias[len], size, "C%s",
+ obj->string.pointer);
+ if (count < 0)
+ return -EINVAL;
+ if (count >= size)
+ return -ENOMEM;
+
+ len += count;
+ size -= count;
+ }
+ } else {
+ len = snprintf(modalias, size, "acpi:");
+ size -= len;

- list_for_each_entry(id, &acpi_dev->pnp.ids, list) {
- count = snprintf(&modalias[len], size, "%s:", id->id);
- if (count < 0)
- return -EINVAL;
- if (count >= size)
- return -ENOMEM;
- len += count;
- size -= count;
+ list_for_each_entry(id, &acpi_dev->pnp.ids, list) {
+ count = snprintf(&modalias[len], size, "%s:", id->id);
+ if (count < 0)
+ return -EINVAL;
+ if (count >= size)
+ return -ENOMEM;
+ len += count;
+ size -= count;
+ }
}

modalias[len] = '\0';
@@ -864,6 +890,51 @@ int acpi_match_device_ids(struct acpi_de
}
EXPORT_SYMBOL(acpi_match_device_ids);

+/* Performs match for special "PRP0001" shoehorn ACPI ID */
+static bool acpi_of_driver_match_device(struct device *dev,
+ const struct device_driver *drv)

+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);

+ const union acpi_object *of_compatible;
+ int i;
+
+ /*
+ * If the ACPI device does not have corresponding compatible
+ * property or the driver in question does not have DT matching
+ * table we consider the match succesful (matches the ACPI ID).
+ */
+ of_compatible = adev->data.of_compatible;
+ if (!drv->of_match_table || !of_compatible)
+ return true;
+
+ /* Now we can look for the driver DT compatible strings */
+ for (i = 0; i < of_compatible->package.count; i++) {
+ const struct of_device_id *id;

+ const union acpi_object *obj;
+

+ obj = &of_compatible->package.elements[i];
+
+ for (id = drv->of_match_table; id->compatible[0]; id++)
+ if (!strcasecmp(obj->string.pointer, id->compatible))

+ return true;
+ }
+

+ return false;
+}
+

+bool acpi_driver_match_device(struct device *dev,
+ const struct device_driver *drv)
+{
+ const struct acpi_device_id *id;
+
+ id = acpi_match_device(drv->acpi_match_table, dev);
+ if (!id)
+ return false;
+
+ return acpi_of_driver_match_device(dev, drv);
+}
+EXPORT_SYMBOL_GPL(acpi_driver_match_device);
+
static void acpi_free_power_resources_lists(struct acpi_device *device)
{
int i;

Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h

@@ -341,6 +341,7 @@ struct acpi_device_physical_node {
struct acpi_device_data {
const union acpi_object *pointer;
const union acpi_object *properties;
+ const union acpi_object *of_compatible;
};

/* Device */

Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h

@@ -424,12 +424,8 @@ extern int acpi_nvs_for_each_region(int
const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
const struct device *dev);

-static inline bool acpi_driver_match_device(struct device *dev,
- const struct device_driver *drv)
-{
- return !!acpi_match_device(drv->acpi_match_table, dev);
-}
-
+extern bool acpi_driver_match_device(struct device *dev,
+ const struct device_driver *drv);
int acpi_device_uevent_modalias(struct device *, struct kobj_uevent_env *);
int acpi_device_modalias(struct device *, char *, int);

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:04:57 PM9/30/14

to linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: Mika Westerberg <mika.we...@linux.intel.com>

This document describes the data format and interfaces of ACPI device
specific properties.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
Signed-off-by: Darren Hart <dvh...@linux.intel.com>

Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

Documentation/acpi/properties.txt | 410 ++++++++++++++++++++++++++++++++++++++
1 file changed, 410 insertions(+)
create mode 100644 Documentation/acpi/properties.txt

diff --git a/Documentation/acpi/properties.txt b/Documentation/acpi/properties.txt
new file mode 100644
index 0000000..13a93c5
--- /dev/null
+++ b/Documentation/acpi/properties.txt
@@ -0,0 +1,410 @@
+ACPI device properties
+======================
+This document describes the format and interfaces of ACPI device
+properties as specified in "Device Properties UUID For _DSD" available
+here:
+
+http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
+
+1. Introduction
+---------------
+In systems that use ACPI and want to take advantage of device specific
+properties, there needs to be a standard way to return and extract
+name-value pairs for a given ACPI device.
+
+An ACPI device that wants to export its properties must implement a
+static name called _DSD that takes no arguments and returns a package of
+packages:
+
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"name1", <VALUE1>},
+ Package () {"name2", <VALUE2>}
+ }
+ })
+
+The UUID identifies contents of the following package. In case of ACPI
+device properties it is daffd814-6eba-4d8c-8a91-bc9bbf4aa301.
+
+In each returned package, the first item is the name and must be a string.
+The corresponding value can be a string, integer, reference, or package. If
+a package it may only contain strings, integers, and references.
+
+An example device where we might need properties is a device that uses
+GPIOs. In addition to the GpioIo/GpioInt resources the driver needs to
+know which GPIO is used for which purpose.
+
+To solve this we add the following ACPI device properties to the device:
+
+ Device (DEV0)
+ {
+ Name (_CRS, ResourceTemplate () {
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ "\\_SB.PCI0.LPC", 0, ResourceConsumer) {0}
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ "\\_SB.PCI0.LPC", 0, ResourceConsumer) {1}
+ ...
+ })
+
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"reset-gpio", {^DEV0, 0, 0, 0}},
+ Package () {"shutdown-gpio", {^DEV0, 1, 0, 0}},
+ }
+ })
+ }
+
+Now the device driver can reference the GPIOs using names instead of
+using indexes.
+
+If there is an existing Device Tree binding for a device, it is expected
+that the same bindings are used with ACPI properties, so that the driver
+dealing with the device needs only minor modifications if any.
+
+2. Formal definition of properties
+----------------------------------
+The following chapters define the currently supported properties. For
+these there exists a helper function that can be used to extract the
+property value.
+
+2.1 Integer types
+-----------------
+ACPI integers are always 64-bit. However, for drivers the full range is
+typically not needed so we provide a set of functions which convert the
+64-bit integer to a smaller Linux integer type.
+
+An integer property looks like this:
+
+ Package () {"i2c-sda-hold-time-ns", 300},
+ Package () {"clock-frequency", 400000},
+
+To read a property value, use a unified property accessor as shown
+below:
+
+ u32 val;
+ int ret;
+
+ ret = device_property_read_u32(dev, "clock-frequency", &val);
+ if (ret)
+ /* Handle error */
+
+The function returns 0 if the property is copied to 'val' or negative
+errno if something went wrong (or the property does not exist).
+
+2.2 Integer arrays
+------------------
+An integer array is a package holding only integers. Arrays can be used to
+represent different things like Linux input key codes to GPIO mappings, pin
+control settings, dma request lines, etc.
+
+An integer array looks like this:
+
+ Package () {
+ "max8952,dvs-mode-microvolt",
+ Package () {
+ 1250000,
+ 1200000,
+ 1050000,
+ 950000,
+ }
+ }
+
+The above array property can be accessed like:
+
+ u32 voltages[4];
+ int ret;
+
+ ret = device_property_read_u32_array(dev, "max8952,dvs-mode-microvolt",
+ voltages, ARRAY_SIZE(voltages));
+ if (ret)
+ /* Handle error */
+
+
+All functions copy the resulting values cast to a requested type to the
+caller supplied array. If you pass NULL in the value pointer ('voltages' in
+this case), the function returns number of items in the array. This can be
+useful if caller does not know size of the array beforehand.
+
+2.3 Strings
+-----------
+String properties can be used to describe many things like labels for GPIO
+buttons, compability ids, etc.
+
+A string property looks like this:
+
+ Package () {"pwm-names", "backlight"},
+ Package () {"label", "Status-LED"},
+
+You can use device_property_read_string() to extract strings:
+
+ const char *val;
+ int ret;
+
+ ret = device_property_read_string(dev, "label", &val);
+ if (ret)
+ /* Handle error */
+
+Note that the function does not copy the returned string but instead the
+value is modified to point to the string property itself.
+
+The memory is owned by the associated ACPI device object and released
+when it is removed. The user need not free the associated memory.
+
+2.4 String arrays
+-----------------
+String arrays can be useful in describing a list of labels, names for
+DMA channels, etc.
+
+A string array property looks like this:
+
+ Package () {"dma-names", Package () {"tx", "rx", "rx-tx"}},
+ Package () {"clock-output-names", Package () {"pll", "pll-switched"}},
+
+And these can be read in similar way that the integer arrrays:
+
+ const char *dma_names[3];
+ int ret;
+
+ ret = device_property_read_string_array(dev, "dma-names", dma_names,
+ ARRAY_SIZE(dma_names));
+ if (ret)
+ /* Handle error */
+
+The memory management rules follow what is specified for single strings.
+Specifically the returned pointers should be treated as constant and not to
+be freed. That is done automatically when the correspondig ACPI device
+object is released.
+
+2.5 Object references
+---------------------
+An ACPI object reference is used to refer to some object in the
+namespace. For example, if a device has dependencies with some other
+object, an object reference can be used.
+
+An object reference looks like this:
+
+ Package () {"dev0", \_SB.DEV0},
+
+At the time of writing this, there is no unified device_property_* accessor
+for references so one needs to use the following ACPI helper function:
+
+ int acpi_dev_get_property_reference(struct acpi_device *adev,
+ const char *name,
+ const char *size_prop, int index,

+ struct acpi_reference_args *args);
+

+The referenced ACPI device is returned in args->adev if found.
+
+In addition to simple object references it is also possible to have object
+references with arguments. These are represented in ASL as follows:
+
+ Device (\_SB.PCI0.PWM)
+ {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"#pwm-cells", 2}
+ }
+ })
+ }
+
+ Device (\_SB.PCI0.BL)
+ {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {
+ "pwms",
+ Package () {
+ \_SB.PCI0.PWM, 0, 5000000,
+ \_SB.PCI0.PWM, 1, 4500000,
+ }
+ }
+ }
+ })
+ }
+
+In the above example, the referenced device declares a property that
+returns the number of expected arguments (here it is "#pwm-cells"). If
+no such property is given we assume that all the integers following the
+reference are arguments.
+
+In the above example PWM device expects 2 additional arguments. This
+will be validated by the ACPI property core.
+
+The additional arguments must be integers. Nothing else is supported.
+
+It is possible, as in the above example, to have multiple references
+with varying number of integer arguments. It is up to the referenced
+device to declare how many arguments it expects. The 'index' parameter
+selects which reference is returned.
+
+One can use acpi_dev_get_property_reference() as well to extract the
+information in additional parameters:
+
+ struct acpi_reference_args args;
+ struct acpi_device *adev = /* this will point to the BL device */
+ int ret;
+
+ /* extract the first reference */
+ acpi_dev_get_property_reference(adev, "pwms", "#pwm-cells", 0, &args);
+
+ BUG_ON(args.nargs != 2);
+ BUG_ON(args.args[0] != 0);
+ BUG_ON(args.args[1] != 5000000);
+
+ /* extract the second reference */
+ acpi_dev_get_property_reference(adev, "pwms", "#pwm-cells", 1, &args);
+
+ BUG_ON(args.nargs != 2);
+ BUG_ON(args.args[0] != 1);
+ BUG_ON(args.args[1] != 4500000);
+
+In addition to arguments, args.adev now points to the ACPI device that
+corresponds to \_SB.PCI0.PWM.
+
+It is intended that this function is not used directly but instead
+subsystems like pwm implement their ACPI support on top of this function
+in such way that it is hidden from the client drivers, such as via
+pwm_get().
+
+3. Device property hierarchies
+------------------------------
+Devices are organized in a tree within the Linux kernel. It follows that
+the configuration data would also be hierarchical. In order to reach
+equivalence with Device Tree, the ACPI mechanism must also provide some
+sort of tree-like representation. Fortunately, the ACPI namespace is
+already such a structure.
+
+For example, we could have the following device in ACPI namespace. The
+KEYS device is much like gpio_keys_polled.c in that it includes "pseudo"
+devices for each GPIO:
+
+ Device (KEYS)
+ {
+ Name (_CRS, ResourceTemplate () {
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ "\\_SB.PCI0.LPC", 0, ResourceConsumer) {0}
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ "\\_SB.PCI0.LPC", 0, ResourceConsumer) {1}
+ ...
+ })
+
+ // "pseudo" devices declared under the parent device
+ Device (BTN0) {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"label", "minnow_btn0"}
+ Package () {"gpios", Package () {^KEYS, 0, 0, 1}}
+ }
+ })
+ }
+
+ Device (BTN1) {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"label", "minnow_btn1"}
+ Package () {"gpios", Package () {^KEYS, 1, 0, 1}}
+ }
+ })
+ }
+ }
+
+We can extract the above in gpio_keys_polled.c like:
+
+ static int gpio_keys_polled_create_button(struct device *dev, void *child,
+ void *data)
+ {
+ struct button_data *bdata = data;
+ const char *label = NULL;
+
+ /*
+ * We need to use device_child_ variant here to access
+ * properties of the child.
+ */
+ device_child_property_read_string(dev, child, "label", &label);
+ /* and so on */
+ }
+
+ static void gpio_keys_polled_probe(struct device *dev)
+ {
+ /* Properties for the KEYS device itself */
+ device_property_read(dev, ...);
+
+ /*
+ * Iterate over button devices and extract their
+ * firmware configuration.
+ */
+ ret = device_for_each_child_node(dev, gpio_keys_polled_create_button,
+ &bdata);
+ if (ret)
+ /* Handle error */
+ }
+
+Note that you still need proper error handling which is omitted in the
+above example.
+
+4. Existing Device Tree enabled drivers
+---------------------------------------
+At the time of writing this, there are ~250 existing DT enabled drivers.
+Allocating _HID/_CID for each would not be feasible. To make sure that
+those drivers can still be used on ACPI systems, we provide an
+alternative way to get these matched.
+
+There is a special _HID "PRP0001" which means that use the DT bindings
+for matching this device to a driver. The driver needs to have
+.of_match_table filled in even when !CONFIG_OF.
+
+An example device would be leds that can be controlled via GPIOs. This
+is represented as "leds-gpio" device and looks like this in the ACPI
+namespace:
+
+ Device (LEDS)
+ {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"compatible", Package () {"gpio-leds"}},
+ }
+ })
+ ...
+ }
+
+In order to get the existing drivers/leds/leds-gpio.c bound to this
+device, we take advantage of "PRP0001":
+
+ /* Following already exists in the driver */
+ static const struct of_device_id of_gpio_leds_match[] = {
+ { .compatible = "gpio-leds", },
+ {},
+ };
+ MODULE_DEVICE_TABLE(of, of_gpio_leds_match);
+
+ /* This we add to the driver to get it probed */
+ static const struct acpi_device_id acpi_gpio_leds_match[] = {

+ { "PRP0001" }, /* Device Tree shoehorned into ACPI */
+ {},
+ };

+ MODULE_DEVICE_TABLE(acpi, acpi_gpio_leds_match);
+
+ static struct platform_driver gpio_led_driver = {
+ .driver = {
+ /*
+ * No of_match_ptr() here because we want this
+ * table to be visible even when !CONFIG_OF to
+ * match against "compatible" in _DSD.
+ */
+ .of_match_table = of_gpio_leds_match,
+ .acpi_match_table = acpi_gpio_leds_match,
+ },
+ };
+
+Once ACPI core sees "PRP0001" and that the device has "compatible"
+property it will do the match using .of_match_table instead.
+
+It is preferred that new devices get a proper _HID allocated for them
+instead of inventing new DT "compatible" devices.
--
1.9.3

--G44BJl3Aq1QbV/QL--

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:05:27 PM9/30/14

to linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: Mika Westerberg <mika.we...@linux.intel.com>

With release of ACPI 5.1 and _DSD method we can finally name GPIOs (and
other things as well) returned by _CRS. Previously we were only able to
use integer index to find the corresponding GPIO, which is pretty error
prone if the order changes.

With _DSD we can now query GPIOs using name instead of an integer index,
like the below example shows:

// Bluetooth device with reset and shutdown GPIOs
Device (BTH)
{
Name (_HID, ...)

Name (_CRS, ResourceTemplate ()
{
GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,

"\\_SB.GPO0", 0, ResourceConsumer) {15}

GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,

"\\_SB.GPO0", 0, ResourceConsumer) {27, 31}

})

Name (_DSD, Package ()
{
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package ()
{

Package () {"reset-gpio", Package() {^BTH, 1, 1, 0 }},
Package () {"shutdown-gpio", Package() {^BTH, 0, 0, 0 }},
}
})
}

The format of the supported GPIO property is:

Package () { "name", Package () { ref, index, pin, active_low }}

ref - The device that has _CRS containing GpioIo()/GpioInt() resources,
typically this is the device itself (BTH in our case).
index - Index of the GpioIo()/GpioInt() resource in _CRS starting from zero.
pin - Pin in the GpioIo()/GpioInt() resource. Typically this is zero.
active_low - If 1 the GPIO is marked as active_low.

Since ACPI GpioIo() resource does not have field saying whether it is
active low or high, the "active_low" argument can be used here. Setting
it to 1 marks the GPIO as active low.

In our Bluetooth example the "reset-gpio" refers to the second GpioIo()
resource, second pin in that resource with the GPIO number of 31.

This patch implements necessary support to gpiolib for extracting GPIOs
using _DSD device properties.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
Acked-by: Linus Walleij <linus....@linaro.org>

Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

drivers/gpio/gpiolib-acpi.c | 78 ++++++++++++++++++++++++++++++++++++--------
drivers/gpio/gpiolib.c | 30 ++++++++++++++--
drivers/gpio/gpiolib.h | 7 ++-
3 files changed, 94 insertions(+), 21 deletions(-)

Index: linux-pm/drivers/gpio/gpiolib-acpi.c
===================================================================
--- linux-pm.orig/drivers/gpio/gpiolib-acpi.c
+++ linux-pm/drivers/gpio/gpiolib-acpi.c
@@ -293,6 +293,7 @@ void acpi_gpiochip_free_interrupts(struc
struct acpi_gpio_lookup {
struct acpi_gpio_info info;
int index;
+ int pin_index;
struct gpio_desc *desc;
int n;
};
@@ -306,13 +307,24 @@ static int acpi_find_gpio(struct acpi_re

if (lookup->n++ == lookup->index && !lookup->desc) {
const struct acpi_resource_gpio *agpio = &ares->data.gpio;
+ int pin_index = lookup->pin_index;
+
+ if (pin_index >= agpio->pin_table_length)
+ return 1;

lookup->desc = acpi_get_gpiod(agpio->resource_source.string_ptr,
- agpio->pin_table[0]);
+ agpio->pin_table[pin_index]);
lookup->info.gpioint =
agpio->connection_type == ACPI_RESOURCE_GPIO_TYPE_INT;
- lookup->info.active_low =
- agpio->polarity == ACPI_ACTIVE_LOW;
+
+ /*
+ * ActiveLow is only specified for GpioInt resource. If
+ * GpioIo is used then the only way to set the flag is
+ * to use _DSD "gpios" property.
+ */
+ if (lookup->info.gpioint)
+ lookup->info.active_low =
+ agpio->polarity == ACPI_ACTIVE_LOW;
}

return 1;
@@ -320,40 +332,75 @@ static int acpi_find_gpio(struct acpi_re

/**
* acpi_get_gpiod_by_index() - get a GPIO descriptor from device resources
- * @dev: pointer to a device to get GPIO from
+ * @adev: pointer to a ACPI device to get GPIO from
+ * @propname: Property name of the GPIO (optional)
* @index: index of GpioIo/GpioInt resource (starting from %0)
* @info: info pointer to fill in (optional)
*
- * Function goes through ACPI resources for @dev and based on @index looks
+ * Function goes through ACPI resources for @adev and based on @index looks
* up a GpioIo/GpioInt resource, translates it to the Linux GPIO descriptor,
* and returns it. @index matches GpioIo/GpioInt resources only so if there
* are total %3 GPIO resources, the index goes from %0 to %2.
*
+ * If @propname is specified the GPIO is looked using device property. In
+ * that case @index is used to select the GPIO entry in the property value
+ * (in case of multiple).
+ *
* If the GPIO cannot be translated or there is an error an ERR_PTR is
* returned.
*
* Note: if the GPIO resource has multiple entries in the pin list, this
* function only returns the first.
*/
-struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
+struct gpio_desc *acpi_get_gpiod_by_index(struct acpi_device *adev,
+ const char *propname, int index,
struct acpi_gpio_info *info)
{
struct acpi_gpio_lookup lookup;
struct list_head resource_list;
- struct acpi_device *adev;
- acpi_handle handle;
+ bool active_low = false;
int ret;

- if (!dev)
- return ERR_PTR(-EINVAL);
-
- handle = ACPI_HANDLE(dev);
- if (!handle || acpi_bus_get_device(handle, &adev))
+ if (!adev)
return ERR_PTR(-ENODEV);

memset(&lookup, 0, sizeof(lookup));
lookup.index = index;

+ if (propname) {

+ struct acpi_reference_args args;
+

+ dev_dbg(&adev->dev, "GPIO: looking up %s\n", propname);
+
+ memset(&args, 0, sizeof(args));
+ ret = acpi_dev_get_property_reference(adev, propname, NULL,
+ index, &args);
+ if (ret)
+ return ERR_PTR(ret);
+
+ /*
+ * The property was found and resolved so need to
+ * lookup the GPIO based on returned args instead.
+ */
+ adev = args.adev;
+ if (args.nargs >= 2) {
+ lookup.index = args.args[0];
+ lookup.pin_index = args.args[1];
+ /*
+ * 3rd argument, if present is used to
+ * specify active_low.
+ */
+ if (args.nargs >= 3)
+ active_low = !!args.args[2];
+ }
+
+ dev_dbg(&adev->dev, "GPIO: _DSD returned %s %zd %llu %llu %llu\n",
+ dev_name(&adev->dev), args.nargs,
+ args.args[0], args.args[1], args.args[2]);
+ } else {
+ dev_dbg(&adev->dev, "GPIO: looking up %d in _CRS\n", index);
+ }
+
INIT_LIST_HEAD(&resource_list);
ret = acpi_dev_get_resources(adev, &resource_list, acpi_find_gpio,
&lookup);
@@ -362,8 +409,11 @@ struct gpio_desc *acpi_get_gpiod_by_inde

acpi_dev_free_resource_list(&resource_list);

- if (lookup.desc && info)
+ if (lookup.desc && info) {
*info = lookup.info;
+ if (active_low)
+ info->active_low = active_low;
+ }

return lookup.desc ? lookup.desc : ERR_PTR(-ENOENT);
}
Index: linux-pm/drivers/gpio/gpiolib.c
===================================================================
--- linux-pm.orig/drivers/gpio/gpiolib.c
+++ linux-pm/drivers/gpio/gpiolib.c
@@ -1487,14 +1487,36 @@ static struct gpio_desc *acpi_find_gpio(
unsigned int idx,
enum gpio_lookup_flags *flags)
{
+ static const char * const suffixes[] = { "gpios", "gpio" };
+ struct acpi_device *adev = ACPI_COMPANION(dev);
struct acpi_gpio_info info;
struct gpio_desc *desc;
+ char propname[32];
+ int i;

- desc = acpi_get_gpiod_by_index(dev, idx, &info);
- if (IS_ERR(desc))
- return desc;
+ /* Try first from _DSD */
+ for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
+ if (con_id && strcmp(con_id, "gpios")) {
+ snprintf(propname, sizeof(propname), "%s-%s",
+ con_id, suffixes[i]);
+ } else {
+ snprintf(propname, sizeof(propname), "%s",
+ suffixes[i]);
+ }

- if (info.gpioint && info.active_low)
+ desc = acpi_get_gpiod_by_index(adev, propname, 0, &info);
+ if (!IS_ERR(desc) || (PTR_ERR(desc) == -EPROBE_DEFER))
+ break;
+ }
+
+ /* Then from plain _CRS GPIOs */
+ if (IS_ERR(desc)) {
+ desc = acpi_get_gpiod_by_index(adev, NULL, idx, &info);
+ if (IS_ERR(desc))
+ return desc;
+ }
+
+ if (info.active_low)
*flags |= GPIO_ACTIVE_LOW;

return desc;
Index: linux-pm/drivers/gpio/gpiolib.h
===================================================================
--- linux-pm.orig/drivers/gpio/gpiolib.h
+++ linux-pm/drivers/gpio/gpiolib.h
@@ -34,7 +34,8 @@ void acpi_gpiochip_remove(struct gpio_ch
void acpi_gpiochip_request_interrupts(struct gpio_chip *chip);
void acpi_gpiochip_free_interrupts(struct gpio_chip *chip);

-struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
+struct gpio_desc *acpi_get_gpiod_by_index(struct acpi_device *adev,
+ const char *propname, int index,
struct acpi_gpio_info *info);
#else
static inline void acpi_gpiochip_add(struct gpio_chip *chip) { }
@@ -47,8 +48,8 @@ static inline void
acpi_gpiochip_free_interrupts(struct gpio_chip *chip) { }

static inline struct gpio_desc *
-acpi_get_gpiod_by_index(struct device *dev, int index,
- struct acpi_gpio_info *info)
+acpi_get_gpiod_by_index(struct acpi_device *adev, const char *propname,
+ int index, struct acpi_gpio_info *info)
{
return ERR_PTR(-ENOSYS);

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:05:50 PM9/30/14

to linux-...@vger.kernel.org, Linus Walleij, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: Mika Westerberg <mika.we...@linux.intel.com>

Some drivers need to deal with only firmware representation of its
GPIOs. An example would be a GPIO button array driver where each button
is described as a separate firmware node in device tree. Typically these
child nodes do not have physical representation in the Linux device
model.

In order to help device drivers to handle such firmware child nodes we
add dev[m]_get_named_gpiod_from_child() that takes a child firmware
node pointer as its second argument (the first one is the parent device
itself), finds the GPIO using whatever is the underlying firmware
method, and requests the GPIO properly.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>

Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

Linus, does this look better than the previous one to you?

---
drivers/gpio/devres.c | 34 +++++++++++++++++++++++++
drivers/gpio/gpiolib.c | 56 ++++++++++++++++++++++++++++++++++++++++++
include/linux/gpio/consumer.h | 5 +++
3 files changed, 95 insertions(+)

Index: linux-pm/drivers/gpio/devres.c
===================================================================
--- linux-pm.orig/drivers/gpio/devres.c
+++ linux-pm/drivers/gpio/devres.c
@@ -109,6 +109,40 @@ struct gpio_desc *__must_check __devm_gp
EXPORT_SYMBOL(__devm_gpiod_get_index);

/**
+ * devm_get_named_gpiod_from_child - managed dev_get_named_gpiod_from_child()
+ * @dev: GPIO consumer
+ * @child: firmware node (child of @dev)
+ * @propname: name of the firmware property
+ * @index: index of the GPIO in the property value in case of many
+ *
+ * GPIO descriptors returned from this function are automatically disposed on
+ * driver detach.
+ */
+struct gpio_desc *devm_get_named_gpiod_from_child(struct device *dev, void *child,
+ const char *propname, int index)
+{
+ struct gpio_desc **dr;
+ struct gpio_desc *desc;
+
+ dr = devres_alloc(devm_gpiod_release, sizeof(struct gpio_desc *),
+ GFP_KERNEL);
+ if (!dr)
+ return ERR_PTR(-ENOMEM);
+
+ desc = dev_get_named_gpiod_from_child(dev, child, propname, index);
+ if (IS_ERR(desc)) {
+ devres_free(dr);

+ return desc;
+ }
+

+ *dr = desc;
+ devres_add(dev, dr);
+
+ return desc;
+}
+EXPORT_SYMBOL(devm_get_named_gpiod_from_child);
+
+/**
* devm_gpiod_get_index_optional - Resource-managed gpiod_get_index_optional()
* @dev: GPIO consumer
* @con_id: function within the GPIO consumer

Index: linux-pm/drivers/gpio/gpiolib.c
===================================================================
--- linux-pm.orig/drivers/gpio/gpiolib.c
+++ linux-pm/drivers/gpio/gpiolib.c

@@ -1717,6 +1717,62 @@ struct gpio_desc *__must_check __gpiod_g
EXPORT_SYMBOL_GPL(__gpiod_get_index);

/**
+ * dev_get_named_gpiod_from_child - obtain a GPIO from firmware node
+ * @dev: parent device
+ * @child: firmware node (child of @dev)
+ * @propname: name of the firmware property
+ * @idx: index of the GPIO in the property value in case of many
+ *
+ * This function can be used for drivers that get their configuration
+ * from firmware in such a way that some properties are described as child
+ * nodes for the parent device in DT or ACPI.
+ *
+ * Function properly finds the corresponding GPIO using whatever is the
+ * underlying firmware interface and then makes sure that the GPIO
+ * descriptor is requested before it is returned to the caller.
+ *
+ * In case of error an ERR_PTR() is returned.
+ */
+struct gpio_desc *dev_get_named_gpiod_from_child(struct device *dev, void *child,
+ const char *propname, int index)
+{
+ struct gpio_desc *desc = ERR_PTR(-ENODEV);

+ bool active_low = false;

+ int ret;
+
+ if (!child)
+ return ERR_PTR(-EINVAL);

+
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node) {

+ enum of_gpio_flags flags;
+
+ desc = of_get_named_gpiod_flags(child, propname, index, &flags);
+ if (!IS_ERR(desc))
+ active_low = flags & OF_GPIO_ACTIVE_LOW;
+ } else if (ACPI_COMPANION(dev)) {
+ struct acpi_gpio_info info;
+
+ desc = acpi_get_gpiod_by_index(child, propname, index, &info);
+ if (!IS_ERR(desc))
+ active_low = info.active_low;
+ }
+

+ if (IS_ERR(desc))
+ return desc;
+

+ ret = gpiod_request(desc, NULL);

+ if (ret)
+ return ERR_PTR(ret);
+

+ /* Only value flag can be set from both DT and ACPI is active_low */
+ if (active_low)
+ set_bit(FLAG_ACTIVE_LOW, &desc->flags);
+
+ return desc;
+}
+EXPORT_SYMBOL_GPL(dev_get_named_gpiod_from_child);
+
+/**
* gpiod_get_index_optional - obtain an optional GPIO from a multi-index GPIO
* function
* @dev: GPIO consumer, can be NULL for system-global GPIOs
Index: linux-pm/include/linux/gpio/consumer.h
===================================================================
--- linux-pm.orig/include/linux/gpio/consumer.h
+++ linux-pm/include/linux/gpio/consumer.h
@@ -94,6 +94,11 @@ int gpiod_to_irq(const struct gpio_desc
struct gpio_desc *gpio_to_desc(unsigned gpio);
int desc_to_gpio(const struct gpio_desc *desc);

+/* Child properties interface */
+struct gpio_desc *dev_get_named_gpiod_from_child(struct device *dev, void *child,
+ const char *propname, int index);
+struct gpio_desc *devm_get_named_gpiod_from_child(struct device *dev, void *child,
+ const char *propname, int index);
#else /* CONFIG_GPIOLIB */

static inline struct gpio_desc *__must_check __gpiod_get(struct device *dev,

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:06:11 PM9/30/14

to linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: Max Eliaser <m...@meliaserlow.dyndns.tv>

Make use of device property API in this driver so that both OF and ACPI
based system can use the same driver.

Signed-off-by: Max Eliaser <max.e...@intel.com>

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

drivers/leds/leds-gpio.c | 99 +++++++++++++++++++----------------------------
1 file changed, 42 insertions(+), 57 deletions(-)

Index: linux-pm/drivers/leds/leds-gpio.c
===================================================================
--- linux-pm.orig/drivers/leds/leds-gpio.c
+++ linux-pm/drivers/leds/leds-gpio.c

@@ -15,13 +15,11 @@
#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
#include <linux/leds.h>
-#include <linux/of.h>
-#include <linux/of_platform.h>
-#include <linux/of_gpio.h>
#include <linux/slab.h>
#include <linux/workqueue.h>
#include <linux/module.h>
#include <linux/err.h>
+#include <linux/property.h>

struct gpio_led_data {
struct led_classdev cdev;

@@ -171,65 +169,59 @@ static inline int sizeof_gpio_leds_priv(
(sizeof(struct gpio_led_data) * num_leds);
}

-/* Code to create from OpenFirmware platform devices */
-#ifdef CONFIG_OF_GPIO
-static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev)
+static int gpio_leds_create_led(struct device *dev, void *child, void *data)
+{
+ struct gpio_leds_priv *priv = data;
+ struct gpio_led led = {};
+ const char *state = NULL;
+
+ led.gpiod = devm_get_named_gpiod_from_child(dev, child, "gpios", 0);
+ if (IS_ERR(led.gpiod))
+ return PTR_ERR(led.gpiod);
+
+ device_child_property_read_string(dev, child, "label", &led.name);
+ device_child_property_read_string(dev, child, "linux,default-trigger",
+ &led.default_trigger);
+
+ device_child_property_read_string(dev, child, "linux,default_state",
+ &state);
+ if (state) {
+ if (!strcmp(state, "keep"))
+ led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+ else if (!strcmp(state, "on"))
+ led.default_state = LEDS_GPIO_DEFSTATE_ON;
+ else
+ led.default_state = LEDS_GPIO_DEFSTATE_OFF;
+ }
+
+ if (!device_get_child_property(dev, child, "retain-state-suspended", NULL))
+ led.retain_state_suspended = 1;
+
+ return create_gpio_led(&led, &priv->leds[priv->num_leds++], dev, NULL);
+}
+
+static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
{
- struct device_node *np = pdev->dev.of_node, *child;
struct gpio_leds_priv *priv;
- int count, ret;
+ int ret, count;

- /* count LEDs in this device, so we know how much to allocate */
- count = of_get_available_child_count(np);
+ count = device_get_child_node_count(&pdev->dev);
if (!count)
return ERR_PTR(-ENODEV);

- for_each_available_child_of_node(np, child)
- if (of_get_gpio(child, 0) == -EPROBE_DEFER)
- return ERR_PTR(-EPROBE_DEFER);
-
priv = devm_kzalloc(&pdev->dev, sizeof_gpio_leds_priv(count),
GFP_KERNEL);
if (!priv)
return ERR_PTR(-ENOMEM);

- for_each_available_child_of_node(np, child) {
- struct gpio_led led = {};
- enum of_gpio_flags flags;
- const char *state;
-
- led.gpio = of_get_gpio_flags(child, 0, &flags);
- led.active_low = flags & OF_GPIO_ACTIVE_LOW;
- led.name = of_get_property(child, "label", NULL) ? : child->name;
- led.default_trigger =
- of_get_property(child, "linux,default-trigger", NULL);
- state = of_get_property(child, "default-state", NULL);
- if (state) {
- if (!strcmp(state, "keep"))
- led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
- else if (!strcmp(state, "on"))
- led.default_state = LEDS_GPIO_DEFSTATE_ON;
- else
- led.default_state = LEDS_GPIO_DEFSTATE_OFF;
- }
-
- if (of_get_property(child, "retain-state-suspended", NULL))
- led.retain_state_suspended = 1;
-
- ret = create_gpio_led(&led, &priv->leds[priv->num_leds++],
- &pdev->dev, NULL);
- if (ret < 0) {
- of_node_put(child);
- goto err;
- }
+ ret = device_for_each_child_node(&pdev->dev, gpio_leds_create_led, priv);
+ if (ret) {
+ for (count = priv->num_leds - 2; count >= 0; count--)
+ delete_gpio_led(&priv->leds[count]);
+ return ERR_PTR(ret);
}

return priv;
-
-err:
- for (count = priv->num_leds - 2; count >= 0; count--)
- delete_gpio_led(&priv->leds[count]);
- return ERR_PTR(-ENODEV);

}

static const struct of_device_id of_gpio_leds_match[] = {

@@ -238,13 +230,6 @@ static const struct of_device_id of_gpio
};

MODULE_DEVICE_TABLE(of, of_gpio_leds_match);
-#else /* CONFIG_OF_GPIO */
-static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev)
-{
- return ERR_PTR(-ENODEV);
-}
-#endif /* CONFIG_OF_GPIO */
-

static int gpio_led_probe(struct platform_device *pdev)
{

@@ -273,7 +258,7 @@ static int gpio_led_probe(struct platfor
}
}
} else {
- priv = gpio_leds_create_of(pdev);
+ priv = gpio_leds_create(pdev);
if (IS_ERR(priv))
return PTR_ERR(priv);
}
@@ -300,7 +285,7 @@ static struct platform_driver gpio_led_d
.driver = {
.name = "leds-gpio",
.owner = THIS_MODULE,
- .of_match_table = of_match_ptr(of_gpio_leds_match),
+ .of_match_table = of_gpio_leds_match,
},
};

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:06:38 PM9/30/14

to linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: Aaron Lu <aaro...@intel.com>

GPIO descriptors are the preferred way over legacy GPIO numbers
nowadays. Convert the driver to use GPIO descriptors internally but
still allow passing legacy GPIO numbers from platform data to support
existing platforms.

Signed-off-by: Aaron Lu <aaro...@intel.com>
Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
Acked-by: Alexandre Courbot <acou...@nvidia.com>
Reviewed-by: Linus Walleij <linus....@linaro.org>

Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

drivers/input/keyboard/gpio_keys_polled.c | 39 +++++++++++++++++++++----------
include/linux/gpio_keys.h | 3 +++
2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 432d36395f35..b7a514ced509 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -23,6 +23,7 @@
#include <linux/ioport.h>

#include <linux/platform_device.h>
#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>

#include <linux/gpio_keys.h>
#include <linux/of.h>
#include <linux/of_platform.h>
@@ -51,15 +52,14 @@ static void gpio_keys_polled_check_state(struct input_dev *input,
int state;

if (bdata->can_sleep)
- state = !!gpio_get_value_cansleep(button->gpio);
+ state = !!gpiod_get_value_cansleep(button->gpiod);
else
- state = !!gpio_get_value(button->gpio);
+ state = !!gpiod_get_value(button->gpiod);

if (state != bdata->last_state) {
unsigned int type = button->type ?: EV_KEY;

- input_event(input, type, button->code,
- !!(state ^ button->active_low));
+ input_event(input, type, button->code, state);
input_sync(input);
bdata->count = 0;
bdata->last_state = state;
@@ -259,7 +259,6 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
for (i = 0; i < pdata->nbuttons; i++) {
struct gpio_keys_button *button = &pdata->buttons[i];
struct gpio_keys_button_data *bdata = &bdev->data[i];
- unsigned int gpio = button->gpio;
unsigned int type = button->type ?: EV_KEY;

if (button->wakeup) {
@@ -267,15 +266,31 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
return -EINVAL;
}

- error = devm_gpio_request_one(&pdev->dev, gpio, GPIOF_IN,
- button->desc ? : DRV_NAME);
- if (error) {
- dev_err(dev, "unable to claim gpio %u, err=%d\n",
- gpio, error);
- return error;

+ /*
+ * Legacy GPIO number so request the GPIO here and
+ * convert it to descriptor.
+ */
+ if (!button->gpiod && gpio_is_valid(button->gpio)) {

+ unsigned flags = 0;
+
+ if (button->active_low)

+ flags |= GPIOF_ACTIVE_LOW;
+

+ error = devm_gpio_request_one(&pdev->dev, button->gpio,
+ flags, button->desc ? : DRV_NAME);
+ if (error) {
+ dev_err(dev, "unable to claim gpio %u, err=%d\n",
+ button->gpio, error);
+ return error;
+ }
+
+ button->gpiod = gpio_to_desc(button->gpio);
}

- bdata->can_sleep = gpio_cansleep(gpio);
+ if (IS_ERR(button->gpiod))
+ return PTR_ERR(button->gpiod);
+
+ bdata->can_sleep = gpiod_cansleep(button->gpiod);
bdata->last_state = -1;
bdata->threshold = DIV_ROUND_UP(button->debounce_interval,
pdata->poll_interval);
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index 8b622468952c..ee2d8c6f9130 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -2,6 +2,7 @@
#define _GPIO_KEYS_H

struct device;
+struct gpio_desc;

/**
* struct gpio_keys_button - configuration parameters
@@ -17,6 +18,7 @@ struct device;
* disable button via sysfs
* @value: axis value for %EV_ABS
* @irq: Irq number in case of interrupt keys
+ * @gpiod: GPIO descriptor
*/
struct gpio_keys_button {
unsigned int code;
@@ -29,6 +31,7 @@ struct gpio_keys_button {
bool can_disable;
int value;
unsigned int irq;
+ struct gpio_desc *gpiod;
};

/**
--
1.8.4.5

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:06:59 PM9/30/14

to linux-...@vger.kernel.org, Dmitry Torokhov, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: Aaron Lu <aaro...@intel.com>

Make use of device property API in this driver so that both OF based
system and ACPI based system can use this driver.

Signed-off-by: Aaron Lu <aaro...@intel.com>
Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>

Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

Dmitry, does this look better than the previous one to you?

---
drivers/input/keyboard/gpio_keys_polled.c | 119 +++++++++++++-----------------
1 file changed, 52 insertions(+), 67 deletions(-)

Index: linux-pm/drivers/input/keyboard/gpio_keys_polled.c
===================================================================
--- linux-pm.orig/drivers/input/keyboard/gpio_keys_polled.c
+++ linux-pm/drivers/input/keyboard/gpio_keys_polled.c
@@ -25,9 +25,7 @@
#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
#include <linux/gpio_keys.h>

-#include <linux/of.h>
-#include <linux/of_platform.h>
-#include <linux/of_gpio.h>

+#include <linux/property.h>

#define DRV_NAME "gpio-keys-polled"

@@ -102,21 +100,57 @@ static void gpio_keys_polled_close(struc
pdata->disable(bdev->dev);
}

-#ifdef CONFIG_OF
+static int gpio_keys_polled_get_button(struct device *dev, void *child,
+ void *data)
+{
+ struct gpio_keys_platform_data *pdata = data;
+ struct gpio_keys_button *button;

+ struct gpio_desc *desc;
+

+ desc = devm_get_named_gpiod_from_child(dev, child, "gpios", 0);
+ if (IS_ERR(desc)) {
+ int err = PTR_ERR(desc);
+
+ if (err != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get gpio flags, error: %d\n",
+ err);
+ return err;
+ }
+
+ button = &pdata->buttons[pdata->nbuttons++];
+ button->gpiod = desc;
+
+ if (device_child_property_read_u32(dev, child, "linux,code",
+ &button->code)) {
+ dev_err(dev, "Button without keycode: %d\n",
+ pdata->nbuttons - 1);
+ return -EINVAL;
+ }
+
+ device_child_property_read_string(dev, child, "label", &button->desc);
+
+ if (device_child_property_read_u32(dev, child, "linux,input-type",
+ &button->type))
+ button->type = EV_KEY;
+
+ button->wakeup = !device_get_child_property(dev, child,
+ "gpio-key,wakeup", NULL);
+
+ if (device_child_property_read_u32(dev, child, "debounce-interval",
+ &button->debounce_interval))
+ button->debounce_interval = 5;

+
+ return 0;
+}
+

static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct device *dev)
{
- struct device_node *node, *pp;
struct gpio_keys_platform_data *pdata;
struct gpio_keys_button *button;
int error;
int nbuttons;
- int i;
-
- node = dev->of_node;
- if (!node)
- return NULL;

- nbuttons = of_get_child_count(node);
+ nbuttons = device_get_child_node_count(dev);
if (nbuttons == 0)
return NULL;

@@ -126,54 +160,14 @@ static struct gpio_keys_platform_data *g
return ERR_PTR(-ENOMEM);

pdata->buttons = (struct gpio_keys_button *)(pdata + 1);
- pdata->nbuttons = nbuttons;

- pdata->rep = !!of_get_property(node, "autorepeat", NULL);
- of_property_read_u32(node, "poll-interval", &pdata->poll_interval);
+ pdata->rep = !device_get_property(dev, "autorepeat", NULL);
+ device_property_read_u32(dev, "poll-interval", &pdata->poll_interval);

- i = 0;
- for_each_child_of_node(node, pp) {
- int gpio;

- enum of_gpio_flags flags;
-

- if (!of_find_property(pp, "gpios", NULL)) {
- pdata->nbuttons--;
- dev_warn(dev, "Found button without gpios\n");
- continue;
- }
-
- gpio = of_get_gpio_flags(pp, 0, &flags);
- if (gpio < 0) {
- error = gpio;
- if (error != -EPROBE_DEFER)
- dev_err(dev,
- "Failed to get gpio flags, error: %d\n",
- error);
- return ERR_PTR(error);
- }
-
- button = &pdata->buttons[i++];
-
- button->gpio = gpio;
- button->active_low = flags & OF_GPIO_ACTIVE_LOW;
-
- if (of_property_read_u32(pp, "linux,code", &button->code)) {
- dev_err(dev, "Button without keycode: 0x%x\n",
- button->gpio);

- return ERR_PTR(-EINVAL);
- }
-

- button->desc = of_get_property(pp, "label", NULL);
-
- if (of_property_read_u32(pp, "linux,input-type", &button->type))
- button->type = EV_KEY;
-
- button->wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL);
-
- if (of_property_read_u32(pp, "debounce-interval",
- &button->debounce_interval))
- button->debounce_interval = 5;
- }
+ error = device_for_each_child_node(dev, gpio_keys_polled_get_button,
+ pdata);
+ if (error)
+ return ERR_PTR(error);

if (pdata->nbuttons == 0)
return ERR_PTR(-EINVAL);
@@ -187,15 +181,6 @@ static const struct of_device_id gpio_ke
};
MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);

-#else
-
-static inline struct gpio_keys_platform_data *
-gpio_keys_polled_get_devtree_pdata(struct device *dev)
-{
- return NULL;
-}
-#endif
-

static int gpio_keys_polled_probe(struct platform_device *pdev)

{
struct device *dev = &pdev->dev;
@@ -323,7 +308,7 @@ static struct platform_driver gpio_keys_
.driver = {
.name = DRV_NAME,
.owner = THIS_MODULE,
- .of_match_table = of_match_ptr(gpio_keys_polled_of_match),
+ .of_match_table = gpio_keys_polled_of_match,
},
};
module_platform_driver(gpio_keys_polled_driver);

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:07:18 PM9/30/14

to linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: Mika Westerberg <mika.we...@linux.intel.com>

Add support for matching using DT compatible string from ACPI _DSD.

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

drivers/misc/eeprom/at25.c | 7 +++++++
drivers/misc/eeprom/at25.c | 7 +++++++
1 file changed, 7 insertions(+)

Index: linux-pm/drivers/misc/eeprom/at25.c
===================================================================
--- linux-pm.orig/drivers/misc/eeprom/at25.c
+++ linux-pm/drivers/misc/eeprom/at25.c
@@ -459,11 +459,18 @@ static const struct of_device_id at25_of
};
MODULE_DEVICE_TABLE(of, at25_of_match);

+static const struct acpi_device_id at25_acpi_match[] = {
+ { "PRP0001" },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, at25_acpi_match);
+
static struct spi_driver at25_driver = {
.driver = {
.name = "at25",
.owner = THIS_MODULE,
.of_match_table = at25_of_match,
+ .acpi_match_table = at25_acpi_match,
},
.probe = at25_probe,
.remove = at25_remove,

Rafael J. Wysocki

unread,

Sep 30, 2014, 10:07:44 PM9/30/14

to linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

From: Mika Westerberg <mika.we...@linux.intel.com>

Allow the driver to probe from ACPI namespace.

Signed-off-by: Aaron Lu <aaro...@intel.com>

Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
---

drivers/input/keyboard/gpio_keys_polled.c | 7 +++++++

1 file changed, 7 insertions(+)

Index: linux-pm/drivers/input/keyboard/gpio_keys_polled.c
===================================================================
--- linux-pm.orig/drivers/input/keyboard/gpio_keys_polled.c
+++ linux-pm/drivers/input/keyboard/gpio_keys_polled.c

@@ -181,6 +181,12 @@ static const struct of_device_id gpio_ke
};
MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);

+static const struct acpi_device_id gpio_keys_polled_acpi_match[] = {

+ { "PRP0001" }, /* Device Tree shoehorned into ACPI */
+ { },
+};

+MODULE_DEVICE_TABLE(acpi, gpio_keys_polled_acpi_match);
+

static int gpio_keys_polled_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;

@@ -309,6 +315,7 @@ static struct platform_driver gpio_keys_
.name = DRV_NAME,
.owner = THIS_MODULE,
.of_match_table = gpio_keys_polled_of_match,
+ .acpi_match_table = gpio_keys_polled_acpi_match,
},
};
module_platform_driver(gpio_keys_polled_driver);

Arnd Bergmann

unread,

Oct 1, 2014, 3:38:28 AM10/1/14

to Rafael J. Wysocki, linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

Looks good to me.

Acked-by: Arnd Bergmann <ar...@arndb.de>

Arnd Bergmann

unread,

Oct 1, 2014, 3:48:07 AM10/1/14

to Rafael J. Wysocki, linux-...@vger.kernel.org, Greg Kroah-Hartman, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

On Wednesday 01 October 2014 04:10:03 Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j...@intel.com>
>
> Add a uniform interface by which device drivers can request device
> properties from the platform firmware by providing a property name
> and the corresponding data type. The purpose of it is to help to
> write portable code that won't depend on any particular platform
> firmware interface.
>
> Three general helper functions, device_get_property(),
> device_read_property() and device_read_property_array() are provided.
> The first one allows the raw value of a given device property to be
> accessed. The remaining two allow the value of a numeric or string
> property and multiple numeric or string values of one array
> property to be acquired, respectively. Static inline wrappers are also
> provided for the various property data types that can be passed to
> device_read_property() or device_read_property_array() for extra type
> checking.

These look great!

> In addition to that, new generic routines are provided for retrieving
> properties from device description objects in the platform firmware
> in case a device driver needs/wants to access properties of a child
> object of a given device object. There are cases in which there is
> no struct device representation of such child objects and this
> additional API is useful then. Again, three functions are provided,
> device_get_child_property(), device_read_child_property(),
> device_read_child_property_array(), in analogy with device_get_property(),
> device_read_property() and device_read_property_array() described above,
> respectively, along with static inline wrappers for all of the propery
> data types that can be used. For all of them, the first argument is
> a struct device pointer to the parent device object and the second
> argument is a (void *) pointer to the child description provided by
> the platform firmware (either ACPI or FDT).

I still have my reservations against the child accessors, and would
like to hear what other people think. Passing a void pointer rather
than struct fw_dev_node has both advantages and disadvantages, and
I won't complain about either one if enough other people on the DT
side would like to see the addition of the child functions.

> Finally, device_for_each_child_node() is added for iterating over
> the children of the device description object associated with a
> given device.
>
> The interface covers both ACPI and Device Trees.
>
> This change set includes material from Mika Westerberg and Aaron Lu.
>

Regarding device_for_each_child_node(), the syntax is inconsistent
with what we normally use, which can probably be changed. All of the
DT for_each_* helpers are macros that are used like

struct device *dev = ...;
void *child; /* iterator */

device_for_each_child_node(dev, child) {
u32 something;
device_child_property_read_u32(dev, child, "propname", &something);

do_something(dev, something);
}

If we get a consensus on having the child interfaces, I'd rather see
them done this way than with a callback pointer, for consistency
reasons.

Arnd

Arnd Bergmann

unread,

Oct 1, 2014, 3:48:16 AM10/1/14

to Rafael J. Wysocki, linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

On Wednesday 01 October 2014 04:10:40 Rafael J. Wysocki wrote:
> From: Mika Westerberg <mika.we...@linux.intel.com>
>
> We have lots of existing Device Tree enabled drivers and allocating
> separate _HID for each is not feasible. Instead we allocate special _HID
> "PRP0001" that means that the match should be done using Device Tree
> compatible property using driver's .of_match_table instead.
>
> If there is a need to distinguish from where the device is enumerated
> (DT/ACPI) driver can check dev->of_node or ACPI_COMPATION(dev).
>
> Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
>

Acked-by: Arnd Bergmann <ar...@arndb.de>

Dmitry Torokhov

unread,

Oct 1, 2014, 3:48:58 AM10/1/14

to Rafael J. Wysocki, linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

o
Hmm, why do we need the generic "PRP0001" in every driver? The ACPI device
should have PRP0001 and ACPI bus should know to look into OF matching table
for such devices.

Thanks.

--
Dmitry

Arnd Bergmann

unread,

Oct 1, 2014, 3:59:31 AM10/1/14

to Rafael J. Wysocki, linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

On Wednesday 01 October 2014 04:11:20 Rafael J. Wysocki wrote:
> From: Mika Westerberg <mika.we...@linux.intel.com>
>
> This document describes the data format and interfaces of ACPI device
> specific properties.
>
> Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
> Signed-off-by: Darren Hart <dvh...@linux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>

unread,

Oct 1, 2014, 10:05:45 AM10/1/14

to Arnd Bergmann, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

Right, got it now.

> With the entry you create in create_modalias(), you will only ever
> match against the MODULE_DEVICE_TABLE(of, of_gpio_leds_match)
> line, not against the MODULE_DEVICE_TABLE(acpi, acpi_gpio_leds_match),
> so I think you can just drop the latter.

Indeed.

> On the question what to put into the name and type fields, that is
> unrelated. The type is supposed to be for the 'device_type' property
> in DT, which we should never rely on in a driver that supports both
> APCI and DT. In Linux we only use that for "pci", "cpu" and "memory",
> all of which have their own way of getting probed in ACPI.
> The "name" is normally ignored in DT as well, except for backwards
> compatibility with old bindings, but I would argue that you should not
> just put "prp0001" in there. Either leave it empty like type, or use
> the name of the device as it appears in the ACPI tables, such as "DEV0"
> or "PWM".

OK, I think it makes sense to leave them empty. I remember I tried that
at some point but it didn't work without N and T fields. Is there some
example what to put there in case of empty?

Something like "of:N*T*Cgpio-leds" perhaps?

Arnd Bergmann

unread,

Oct 1, 2014, 10:14:35 AM10/1/14

to Mika Westerberg, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

On Wednesday 01 October 2014 17:04:41 Mika Westerberg wrote:
>
> > On the question what to put into the name and type fields, that is
> > unrelated. The type is supposed to be for the 'device_type' property
> > in DT, which we should never rely on in a driver that supports both
> > APCI and DT. In Linux we only use that for "pci", "cpu" and "memory",
> > all of which have their own way of getting probed in ACPI.
> > The "name" is normally ignored in DT as well, except for backwards
> > compatibility with old bindings, but I would argue that you should not
> > just put "prp0001" in there. Either leave it empty like type, or use
> > the name of the device as it appears in the ACPI tables, such as "DEV0"
> > or "PWM".
>
> OK, I think it makes sense to leave them empty. I remember I tried that
> at some point but it didn't work without N and T fields. Is there some
> example what to put there in case of empty?
>
> Something like "of:N*T*Cgpio-leds" perhaps?

Sorry, don't know. If I read the code right, the type field in DT ends
up being "<NULL>" for any device that doesn't set the device_type
property, but that seems a bit silly and probably isn't worth copying.

Arnd

That certainly is doable, although the resulting macro would generate a rather
large chunk of code each time it is used.

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Rafael J. Wysocki

unread,

Oct 1, 2014, 6:41:32 PM10/1/14

to Arnd Bergmann, linux-...@vger.kernel.org, Greg Kroah-Hartman, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

On a second thought I'm not so sure, because we need to iterate either this
way or that way depending on a condition evaluated at run time. I have no
idea how to do that in a macro at the moment.

Greg Kroah-Hartman

unread,

Oct 1, 2014, 8:04:35 PM10/1/14

to Rafael J. Wysocki, linux-...@vger.kernel.org, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

On Wed, Oct 01, 2014 at 04:10:03AM +0200, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j...@intel.com>
>
> Add a uniform interface by which device drivers can request device
> properties from the platform firmware by providing a property name
> and the corresponding data type. The purpose of it is to help to
> write portable code that won't depend on any particular platform
> firmware interface.
>
> Three general helper functions, device_get_property(),
> device_read_property() and device_read_property_array() are provided.
> The first one allows the raw value of a given device property to be
> accessed. The remaining two allow the value of a numeric or string
> property and multiple numeric or string values of one array
> property to be acquired, respectively. Static inline wrappers are also
> provided for the various property data types that can be passed to
> device_read_property() or device_read_property_array() for extra type
> checking.
>

> In addition to that, new generic routines are provided for retrieving
> properties from device description objects in the platform firmware
> in case a device driver needs/wants to access properties of a child
> object of a given device object. There are cases in which there is
> no struct device representation of such child objects and this
> additional API is useful then. Again, three functions are provided,
> device_get_child_property(), device_read_child_property(),
> device_read_child_property_array(), in analogy with device_get_property(),
> device_read_property() and device_read_property_array() described above,
> respectively, along with static inline wrappers for all of the propery
> data types that can be used. For all of them, the first argument is
> a struct device pointer to the parent device object and the second
> argument is a (void *) pointer to the child description provided by
> the platform firmware (either ACPI or FDT).
>

> Finally, device_for_each_child_node() is added for iterating over
> the children of the device description object associated with a
> given device.
>
> The interface covers both ACPI and Device Trees.
>
> This change set includes material from Mika Westerberg and Aaron Lu.
>

> Signed-off-by: Aaron Lu <aaro...@intel.com>
> Signed-off-by: Mika Westerberg <mika.we...@linux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j...@intel.com>
> ---
>

> Greg, please let me know if you're fine with this one.

Looks good to me:

Acked-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

Arnd Bergmann

unread,

Oct 2, 2014, 3:46:45 AM10/2/14

to Rafael J. Wysocki, linux-...@vger.kernel.org, Greg Kroah-Hartman, Mika Westerberg, linux...@vger.kernel.org, devic...@vger.kernel.org, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

On Thursday 02 October 2014 00:09:44 Rafael J. Wysocki wrote:
> On Wednesday, October 01, 2014 09:47:40 AM Arnd Bergmann wrote:
> > On Wednesday 01 October 2014 04:10:03 Rafael J. Wysocki wrote:
> > I still have my reservations against the child accessors, and would
> > like to hear what other people think. Passing a void pointer rather
> > than struct fw_dev_node has both advantages and disadvantages, and
> > I won't complain about either one if enough other people on the DT
> > side would like to see the addition of the child functions.
>
> I actually would rather like to know if the people on the DT side have any
> problems with the child functions.

Sure, any kind of feedback would be helpful really.

> Because, suppose that they wouldn't like those functions at all. What are we
> supposed to do, then, honestly? Add the whole DT vs ACPI logic to the leds-gpio
> and gpio_keys_polled drivers? But these drivers have no reason whatsoever
> to include that. Zero.
>
> So suggestions welcome.
>
> [BTW, In principle we also could use something like
>
> typedef union dev_node {
> struct acpi_device *acpi_node;
> struct device_node *of_node;
> } dev_node_t;
>
> instead of the (void *) for more type safety. It still is useful to pass the
> parent pointer along with that, though.]

Yes, I'm not worried about the implementation details.

> > > Finally, device_for_each_child_node() is added for iterating over
> > > the children of the device description object associated with a
> > > given device.
> > >
> > > The interface covers both ACPI and Device Trees.
> > >
> > > This change set includes material from Mika Westerberg and Aaron Lu.
> > >
> >
> > Regarding device_for_each_child_node(), the syntax is inconsistent
> > with what we normally use, which can probably be changed. All of the
> > DT for_each_* helpers are macros that are used like
> >
> > struct device *dev = ...;
> > void *child; /* iterator */
> >
> > device_for_each_child_node(dev, child) {
> > u32 something;
> > device_child_property_read_u32(dev, child, "propname", &something);
> >
> > do_something(dev, something);
> > }
> >
> > If we get a consensus on having the child interfaces, I'd rather see
> > them done this way than with a callback pointer, for consistency
> > reasons.
>
> That certainly is doable, although the resulting macro would generate a rather
> large chunk of code each time it is used.

#define device_for_each_child_node(dev, child) \
for (child = device_get_next_child_node(dev, NULL), child, \
child = device_get_next_child_node(dev, child))

void *device_get_next_child_node(struct device *dev, void *child)
{
if (IS_ENABLED(CONFIG_OF) && dev->of_node)
return of_get_next_child(dev->of_node, child);
else if (IS_ENABLED(CONFIG_ACPI) && ...)
return acpi_get_next_child(dev, child);
return NULL;
}

Not any more code than what we have today for the DT-only case, and it's
really just a function call in a loop.

Arnd

Mika Westerberg

unread,

Oct 2, 2014, 5:53:21 AM10/2/14

to Dmitry Torokhov, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Bryan Wu, Lee Jones, Grant Likely, Arnd Bergmann, Aaron Lu, Darren Hart

You are right - we can get rid of that completely.

In case a device has compatible property, .of_match_table and does not
have .acpi_match_table we match against .of_match_table in ACPI core.

Mika Westerberg

unread,

Oct 2, 2014, 5:56:20 AM10/2/14

to Arnd Bergmann, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

On Wed, Oct 01, 2014 at 04:14:20PM +0200, Arnd Bergmann wrote:
> On Wednesday 01 October 2014 17:04:41 Mika Westerberg wrote:
> >
> > > On the question what to put into the name and type fields, that is
> > > unrelated. The type is supposed to be for the 'device_type' property
> > > in DT, which we should never rely on in a driver that supports both
> > > APCI and DT. In Linux we only use that for "pci", "cpu" and "memory",
> > > all of which have their own way of getting probed in ACPI.
> > > The "name" is normally ignored in DT as well, except for backwards
> > > compatibility with old bindings, but I would argue that you should not
> > > just put "prp0001" in there. Either leave it empty like type, or use
> > > the name of the device as it appears in the ACPI tables, such as "DEV0"
> > > or "PWM".
> >
> > OK, I think it makes sense to leave them empty. I remember I tried that
> > at some point but it didn't work without N and T fields. Is there some
> > example what to put there in case of empty?
> >
> > Something like "of:N*T*Cgpio-leds" perhaps?
>
> Sorry, don't know. If I read the code right, the type field in DT ends
> up being "<NULL>" for any device that doesn't set the device_type
> property, but that seems a bit silly and probably isn't worth copying.

OK, I checked and udev wants to have both N and T but they can be left
empty. If there are no objections the modalias will look like this:

[root@mnw01 ~]# cat /sys/bus/spi/devices/spi-PRP0001\:00/modalias
of:Nat25TCatmel,at25

In other words name is the ACPI device name (AT25) in lower case, T is
left empty and C is the compatible property.

Mika Westerberg

unread,

Oct 2, 2014, 6:41:39 AM10/2/14

to Arnd Bergmann, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

ACPI packages can hold anything (even other packages) but one goal with
the _DSD implementation was to reuse existing DT bindings wherever it
makes sense even if they would not always be optimal for ACPI. So the
current acpi_dev_get_property_reference() is modelled after
corresponding DT function and it allows only integer arguments to
accompany the reference.

Doing that allows taking the existing DT description, package it inside
ACPI _DSD and be done with it.

> > +The referenced ACPI device is returned in args->adev if found.
> > +
> > +In addition to simple object references it is also possible to have object
> > +references with arguments. These are represented in ASL as follows:
> > +
> > + Device (\_SB.PCI0.PWM)
> > + {
> > + Name (_DSD, Package () {
> > + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > + Package () {
> > + Package () {"#pwm-cells", 2}
> > + }
> > + })
> > + }
> > +
>
> Similarly, the "#foo-cells" syntax is an artifact of the limitations of the
> DT syntax, and I'd assume there would be a better way to encode this
> in ACPI. Also, a "cell" in Open Firmware is defined as a big-endian
> 32-bit value, which doesn't directly correspond to something in ACPI,
> and the '#' character is an artifact of the use of the Forth language
> in Open Firmware, which you also don't have here.

Same here, we tried to make it follow closely the DT description. It is
probably not the best/optimal encoding for ACPI but it is documented
well in Documentation/devicetree/bindings so why not use it.

The summary email from Darren at KS also mentions that for the existing
drivers, the existing schemas should be common for both implementations [1].

For new bindings we probably should look out if they can be better
represented using ACPI types.

[1] http://lwn.net/Articles/609373/

Arnd Bergmann

unread,

Oct 2, 2014, 6:44:40 AM10/2/14

to Mika Westerberg, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

Yes, seems ok to me.

Arnd

Arnd Bergmann

unread,

Oct 2, 2014, 7:51:50 AM10/2/14

to Mika Westerberg, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

On Thursday 02 October 2014 13:41:23 Mika Westerberg wrote:
> On Wed, Oct 01, 2014 at 09:59:14AM +0200, Arnd Bergmann wrote:
> > On Wednesday 01 October 2014 04:11:20 Rafael J. Wysocki wrote:
> > > From: Mika Westerberg <mika.we...@linux.intel.com>
>

I thought when we had discussed the subsystem specific bindings, the
consensus there was to have subsystem specific accessors and
properties/tables.

I would argue that for everything that ACPI already has (interrupts,
registers, gpio, dmaengine, ...) the native method should be used,
possibly using _DSD to provide naming for otherwise anonymous references.

Arnd

Mika Westerberg

unread,

Oct 2, 2014, 8:15:45 AM10/2/14

to Arnd Bergmann, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

Absolutely. That's precisely what we do in the GPIO patch of this
series. E.g we use ACPI GpioIo/GpioInt _CRS resources but give name to
the GPIOs with the help of _DSD.

For things that don't have correspondence in ACPI but have well defined
existing DT schema, like PWMs, we should follow that.

Arnd Bergmann

unread,

Oct 2, 2014, 8:46:46 AM10/2/14

to Mika Westerberg, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

On Thursday 02 October 2014 15:15:08 Mika Westerberg wrote:
> > I thought when we had discussed the subsystem specific bindings, the
> > consensus there was to have subsystem specific accessors and
> > properties/tables.
> >
> > I would argue that for everything that ACPI already has (interrupts,
> > registers, gpio, dmaengine, ...) the native method should be used,
> > possibly using _DSD to provide naming for otherwise anonymous references.
>
> Absolutely. That's precisely what we do in the GPIO patch of this
> series. E.g we use ACPI GpioIo/GpioInt _CRS resources but give name to
> the GPIOs with the help of _DSD.

Ok, I've looked at the example again:

+ Device (DEV0)
+ {
+ Name (_CRS, ResourceTemplate () {
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ "\\_SB.PCI0.LPC", 0, ResourceConsumer) {0}
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ "\\_SB.PCI0.LPC", 0, ResourceConsumer) {1}
+ ...
+ })

+
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {

+ Package () {"reset-gpio", {^DEV0, 0, 0, 0}},
+ Package () {"shutdown-gpio", {^DEV0, 1, 0, 0}},
+ }
+ })
+ }

tell me if I'm interpreting this right: you use _CRS to define a reference
to the actual GPIO controller here, but then in your _DSD you have something
that is a bit like the DT gpio binding, except that it doesn't refer to
a GPIO controller but to the GPIO using device itself.

When a driver calls dev_get_named_gpiod_from_child, you then lookup the
"reset-gpio" (or other) properties to find the reference to the _CRS.

Why can't you instead have a "gpio-names" property in _DSD that just
links from a string to an index as we do in all the other bindings?
It sounds to me like that would be simpler and more consistent with
the way things are done in ACPI, and have no effect that would be visible
to the driver.

> For things that don't have correspondence in ACPI but have well defined
> existing DT schema, like PWMs, we should follow that.

Only for stuff that is visible to drivers. I think it's important
that a driver calling pwm_get() today should keep working with
ACPI when the pwm subsystem is extended to support that.

However, that does not necessarily mean using #pwm-cells and pwm-names
properties when there is a better way to express the same in ACPI
syntax. AFAICT, the #foo-names is completely redundant for ACPI
as the length of a property in a list of similar elements is part
of the binary data (if not, please correct me), and I already commented
that I think the foo-names stuff could be encoded in the package
directly in a way we can't do in DT.

Even if you want to do automatic translation between DT and ACPI,
I think it would be possible to treat these two the same:

(forgive any syntax errors)

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
Package () { "pwms" { "led-red", ^PWM0, 0, 10 },
{ "led-green", ^PWM0, 1, 20 }},
}

vs.

pwm-slave {
pwms = <&pwm0 0 10>, <&pwm1 1 20>;
pwm-names = "led-red", "led-green";
};

Arnd

Mika Westerberg

unread,

Oct 2, 2014, 9:37:13 AM10/2/14

to Arnd Bergmann, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

That's right. However, it is not limited to the device itself, it can be
parent device for example. Typically not the GPIO controller, though.

> When a driver calls dev_get_named_gpiod_from_child, you then lookup the
> "reset-gpio" (or other) properties to find the reference to the _CRS.

That's right.

> Why can't you instead have a "gpio-names" property in _DSD that just
> links from a string to an index as we do in all the other bindings?
> It sounds to me like that would be simpler and more consistent with
> the way things are done in ACPI, and have no effect that would be visible
> to the driver.

The ACPI GpioIo/GpioInt entry can also contain pinlist, for example:

Name (_CRS, ResourceTemplate () {

GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,

"\\_SB.PCI0.LPC", 0, ResourceConsumer) {10, 21, 50, 13} <-- this is the pinlist
...
})

Device (BTN0)
{

Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {

...
Package () {"gpios", Package () {^^BTNS, 0, 2, 1}},
}
})
}

Format of the package is:

{ reference, crs_index, pin_index, active_low },

So {^^BTNS, 0, 2, 1} will select pin 50 and mark it as active_low.

In addition to name we also use it to select correct pin in pinlist and
provide additional active_low information which would not be available
for plain GpioIo() resource.

> > For things that don't have correspondence in ACPI but have well defined
> > existing DT schema, like PWMs, we should follow that.
>
> Only for stuff that is visible to drivers. I think it's important
> that a driver calling pwm_get() today should keep working with
> ACPI when the pwm subsystem is extended to support that.

Of course.

> However, that does not necessarily mean using #pwm-cells and pwm-names
> properties when there is a better way to express the same in ACPI
> syntax. AFAICT, the #foo-names is completely redundant for ACPI
> as the length of a property in a list of similar elements is part
> of the binary data (if not, please correct me), and I already commented
> that I think the foo-names stuff could be encoded in the package
> directly in a way we can't do in DT.

Yes, it can be included in the package, however see below.

> Even if you want to do automatic translation between DT and ACPI,
> I think it would be possible to treat these two the same:
>
> (forgive any syntax errors)
>
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package () { "pwms" { "led-red", ^PWM0, 0, 10 },
> { "led-green", ^PWM0, 1, 20 }},

Even though the above would fit better in ACPI, it is not allowed to
have multiple values for a single property. One reason for that is that
we validate each property and check that they match what is expected and
having strict set of possible values makes it easier.

Putting everything to a single package results this:

Package () { "pwms", Package () {"led-red", ^PWM0, 0, 10, "led-green", ^PWM0, 1, 10 }}

But I think the below looks better:

Package () { "pwms", Package () {^PWM0, 0, 10, ^PWM0, 1, 10 }}
Package () { "pwm-names", Package () {"led-red", "led-green"}}

and it is trivial to match with the corresponding DT fragment.

> }
>
> vs.
>
> pwm-slave {
> pwms = <&pwm0 0 10>, <&pwm1 1 20>;
> pwm-names = "led-red", "led-green";
> };
>

I don't have strong feelings which way it should be. The current
implementation limits references so that you can have only integer
arguments, like {ref0, int, int, ref1, int} but if people think it is
better to allow strings there as well, it can be changed.

I would like to get comments from Darren and Rafael about this, though.

Arnd Bergmann

unread,

Oct 2, 2014, 10:29:22 AM10/2/14

to Mika Westerberg, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

On Thursday 02 October 2014 16:36:54 Mika Westerberg wrote:
>
> > Even if you want to do automatic translation between DT and ACPI,
> > I think it would be possible to treat these two the same:
> >
> > (forgive any syntax errors)
> >
> > Name (_DSD, Package () {
> > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > Package () {
> > Package () { "pwms" { "led-red", ^PWM0, 0, 10 },
> > { "led-green", ^PWM0, 1, 20 }},
>
> Even though the above would fit better in ACPI, it is not allowed to
> have multiple values for a single property. One reason for that is that
> we validate each property and check that they match what is expected and
> having strict set of possible values makes it easier.

Ok, so you basically have the same limitation that we have in DT.

We have syntactical sugar in dtc that allows us to write

pwms = <&pwm0 0 10>, <&pwm1 1 20>;

or

pwms = <&pwm0 0 10 &pwm1 1 20>;

but the binary representation of the two is the same. This is the
only reason why we need the #size-cells property and I guess you
do too.

> Putting everything to a single package results this:
>
> Package () { "pwms", Package () {"led-red", ^PWM0, 0, 10, "led-green", ^PWM0, 1, 10 }}
>
> But I think the below looks better:
>
> Package () { "pwms", Package () {^PWM0, 0, 10, ^PWM0, 1, 10 }}
> Package () { "pwm-names", Package () {"led-red", "led-green"}}

Ok, if you have no way to separate the individual entries in a list of
references, I don't care much, and I can see the "pwm-names" as being more
consistent with the DT syntax.

Is this a limitation in the way that the AML syntax and compiler works,
or is this a decision you made specifically for the _DSD syntax and that
could still be changed if there is an overwhelming interest?

Arnd

Mika Westerberg

unread,

Oct 2, 2014, 10:38:26 AM10/2/14

to Arnd Bergmann, Rafael J. Wysocki, linux-...@vger.kernel.org, linux...@vger.kernel.org, devic...@vger.kernel.org, Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Lee Jones, Grant Likely, Aaron Lu, Darren Hart

On Thu, Oct 02, 2014 at 04:29:03PM +0200, Arnd Bergmann wrote:
> Is this a limitation in the way that the AML syntax and compiler works,
> or is this a decision you made specifically for the _DSD syntax and that
> could still be changed if there is an overwhelming interest?

It is only limitation of the _DSD device property UUID specification and
our implementation. It can be changed if needed.