Hi Simon,
On Sun, Jun 14, 2020 at 10:55 AM Simon Glass sjg@chromium.org wrote:
When generating ACPI tables we need to convert GPIOs in U-Boot to the ACPI structures required by ACPI. This is a SoC-specific conversion and cannot be handled by generic code, so add a new GPIO method to do the conversion.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
(no changes since v1)
Changes in v1:
- Update sandbox driver slightly for testing
drivers/gpio/gpio-uclass.c | 21 +++++++++ drivers/gpio/sandbox.c | 86 ++++++++++++++++++++++++++++++++++++ include/acpi/acpi_device.h | 90 ++++++++++++++++++++++++++++++++++++++ include/asm-generic/gpio.h | 27 ++++++++++++ test/dm/gpio.c | 62 ++++++++++++++++++++++++++ test/dm/irq.c | 1 + 6 files changed, 287 insertions(+)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 9eeab22eef..e2f4d7e02c 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -13,6 +13,7 @@ #include <errno.h> #include <fdtdec.h> #include <malloc.h> +#include <acpi/acpi_device.h> #include <asm/gpio.h> #include <dm/device_compat.h> #include <linux/bug.h> @@ -809,6 +810,26 @@ int gpio_get_status(struct udevice *dev, int offset, char *buf, int buffsize) return 0; }
+#if CONFIG_IS_ENABLED(ACPIGEN) +int gpio_get_acpi(const struct gpio_desc *desc, struct acpi_gpio *gpio) +{
struct dm_gpio_ops *ops;if (!dm_gpio_is_valid(desc)) {/* Indicate that the GPIO is not valid */gpio->pin_count = 0;gpio->pins[0] = 0;return -EINVAL;}
Let's do memset() to gpio so that GPIO driver doesn't need to clear by themselves, to avoid potential issues.
ops = gpio_get_ops(desc->dev);if (!ops->get_acpi)return -ENOSYS;return ops->get_acpi(desc, gpio);+} +#endif
int gpio_claim_vector(const int *gpio_num_array, const char *fmt) { int i, ret; diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c index 98b7fa4bb3..6cd2a1455a 100644 --- a/drivers/gpio/sandbox.c +++ b/drivers/gpio/sandbox.c @@ -8,7 +8,9 @@ #include <fdtdec.h> #include <log.h> #include <malloc.h> +#include <acpi/acpi_device.h> #include <asm/gpio.h> +#include <dm/acpi.h> #include <dm/device_compat.h> #include <dm/lists.h> #include <dm/of.h> @@ -197,6 +199,72 @@ static int sb_gpio_get_dir_flags(struct udevice *dev, unsigned int offset, return 0; }
+#if CONFIG_IS_ENABLED(ACPIGEN) +static int sb_gpio_get_acpi(const struct gpio_desc *desc,
struct acpi_gpio *gpio)+{
int ret;/* All of these values are just used for testing */if (desc->flags & GPIOD_ACTIVE_LOW) {gpio->pin_count = 1;gpio->pins[0] = desc->offset;gpio->pin0_addr = 0x80012 + desc->offset;gpio->type = ACPI_GPIO_TYPE_INTERRUPT;gpio->pull = ACPI_GPIO_PULL_DOWN;ret = acpi_device_scope(desc->dev, gpio->resource,sizeof(gpio->resource));if (ret)return log_ret(ret);gpio->interrupt_debounce_timeout = 4321;memset(&gpio->irq, '\0', sizeof(gpio->irq));/* We use the GpioInt part */gpio->irq.pin = desc->offset;gpio->irq.polarity = ACPI_IRQ_ACTIVE_BOTH;gpio->irq.shared = ACPI_IRQ_SHARED;gpio->irq.wake = ACPI_IRQ_WAKE;/* The GpioIo part is not used */gpio->output_drive_strength = 0;gpio->io_shared = false;gpio->io_restrict = 0;gpio->polarity = ACPI_GPIO_ACTIVE_LOW;} else {gpio->pin_count = 1;gpio->pins[0] = desc->offset;gpio->pin0_addr = 0xc00dc + desc->offset;gpio->type = ACPI_GPIO_TYPE_IO;gpio->pull = ACPI_GPIO_PULL_UP;ret = acpi_device_scope(desc->dev, gpio->resource,sizeof(gpio->resource));if (ret)return log_ret(ret);gpio->interrupt_debounce_timeout = 0;/* The GpioInt part is not used */memset(&gpio->irq, '\0', sizeof(gpio->irq));/* We use the GpioIo part */gpio->output_drive_strength = 1234;gpio->io_shared = true;gpio->io_restrict = ACPI_GPIO_IO_RESTRICT_INPUT;gpio->polarity = 0;}
Could we refactor the logic a little bit, such that the common part is put outside the if () else () logic, and only different part is put into the if () or else () part.
return 0;+}
+static int sb_gpio_get_name(const struct udevice *dev, char *out_name) +{
return acpi_copy_name(out_name, "GPIO");+}
+struct acpi_ops gpio_sandbox_acpi_ops = {
.get_name = sb_gpio_get_name,+}; +#endif /* ACPIGEN */
static const struct dm_gpio_ops gpio_sandbox_ops = { .direction_input = sb_gpio_direction_input, .direction_output = sb_gpio_direction_output, @@ -206,6 +274,9 @@ static const struct dm_gpio_ops gpio_sandbox_ops = { .xlate = sb_gpio_xlate, .set_dir_flags = sb_gpio_set_dir_flags, .get_dir_flags = sb_gpio_get_dir_flags, +#if CONFIG_IS_ENABLED(ACPIGEN)
.get_acpi = sb_gpio_get_acpi,+#endif };
static int sandbox_gpio_ofdata_to_platdata(struct udevice *dev) @@ -252,6 +323,7 @@ U_BOOT_DRIVER(gpio_sandbox) = { .probe = gpio_sandbox_probe, .remove = gpio_sandbox_remove, .ops = &gpio_sandbox_ops,
ACPI_OPS_PTR(&gpio_sandbox_acpi_ops)};
/* pincontrol: used only to check GPIO pin configuration (pinmux command) */ @@ -419,6 +491,13 @@ static int sb_pinctrl_get_pin_muxing(struct udevice *dev, return 0; }
+#if CONFIG_IS_ENABLED(ACPIGEN) +static int sb_pinctrl_get_name(const struct udevice *dev, char *out_name) +{
return acpi_copy_name(out_name, "PINC");+} +#endif
static int sandbox_pinctrl_probe(struct udevice *dev) { struct sb_pinctrl_priv *priv = dev_get_priv(dev); @@ -434,6 +513,12 @@ static struct pinctrl_ops sandbox_pinctrl_gpio_ops = { .get_pin_muxing = sb_pinctrl_get_pin_muxing, };
+#if CONFIG_IS_ENABLED(ACPIGEN) +struct acpi_ops pinctrl_sandbox_acpi_ops = {
.get_name = sb_pinctrl_get_name,+}; +#endif
static const struct udevice_id sandbox_pinctrl_gpio_match[] = { { .compatible = "sandbox,pinctrl-gpio" }, { /* sentinel */ } @@ -447,4 +532,5 @@ U_BOOT_DRIVER(sandbox_pinctrl_gpio) = { .bind = dm_scan_fdt_dev, .probe = sandbox_pinctrl_probe, .priv_auto_alloc_size = sizeof(struct sb_pinctrl_priv),
ACPI_OPS_PTR(&pinctrl_sandbox_acpi_ops)}; diff --git a/include/acpi/acpi_device.h b/include/acpi/acpi_device.h index 4f87cd003a..cb9166aeae 100644 --- a/include/acpi/acpi_device.h +++ b/include/acpi/acpi_device.h @@ -92,6 +92,96 @@ struct acpi_irq { enum acpi_irq_wake wake; };
+/**
- enum acpi_gpio_type - type of the descriptor
- @ACPI_GPIO_TYPE_INTERRUPT: GpioInterrupt
- @ACPI_GPIO_TYPE_IO: GpioIo
- */
+enum acpi_gpio_type {
ACPI_GPIO_TYPE_INTERRUPT,ACPI_GPIO_TYPE_IO,+};
+/**
- enum acpi_gpio_pull - pull direction
- @ACPI_GPIO_PULL_DEFAULT: Use default value for pin
- @ACPI_GPIO_PULL_UP: Pull up
- @ACPI_GPIO_PULL_DOWN: Pull down
- @ACPI_GPIO_PULL_NONE: No pullup/pulldown
- */
+enum acpi_gpio_pull {
ACPI_GPIO_PULL_DEFAULT,ACPI_GPIO_PULL_UP,ACPI_GPIO_PULL_DOWN,ACPI_GPIO_PULL_NONE,+};
+/**
- enum acpi_gpio_io_restrict - controls input/output of pin
- @ACPI_GPIO_IO_RESTRICT_NONE: no restrictions
- @ACPI_GPIO_IO_RESTRICT_INPUT: input only (no output)
- @ACPI_GPIO_IO_RESTRICT_OUTPUT: output only (no input)
- @ACPI_GPIO_IO_RESTRICT_PRESERVE: preserve settings when driver not active
- */
+enum acpi_gpio_io_restrict {
ACPI_GPIO_IO_RESTRICT_NONE,ACPI_GPIO_IO_RESTRICT_INPUT,ACPI_GPIO_IO_RESTRICT_OUTPUT,ACPI_GPIO_IO_RESTRICT_PRESERVE,+};
+/** enum acpi_gpio_polarity - controls the GPIO polarity */ +enum acpi_gpio_polarity {
ACPI_GPIO_ACTIVE_HIGH = 0,ACPI_GPIO_ACTIVE_LOW = 1,+};
+#define ACPI_GPIO_REVISION_ID 1 +#define ACPI_GPIO_MAX_PINS 2
+/**
- struct acpi_gpio - representation of an ACPI GPIO
- @pin_count: Number of pins represented
- @pins: List of pins
- @pin0_addr: Address in memory of the control registers for pin 0. This is
- used when generating ACPI tables
- @type: GPIO type
- @pull: Pullup/pulldown setting
- @resource: Resource name for this GPIO controller
- For GpioInt:
- @interrupt_debounce_timeout: Debounce timeout in units of 10us
- @irq: Interrupt
- For GpioIo:
- @output_drive_strength: Drive strength in units of 10uA
- @io_shared; true if GPIO is shared
- @io_restrict: I/O restriction setting
- @polarity: GPIO polarity
- */
+struct acpi_gpio {
int pin_count;u16 pins[ACPI_GPIO_MAX_PINS];ulong pin0_addr;enum acpi_gpio_type type;enum acpi_gpio_pull pull;char resource[ACPI_PATH_MAX];/* GpioInt */u16 interrupt_debounce_timeout;struct acpi_irq irq;/* GpioIo */u16 output_drive_strength;bool io_shared;enum acpi_gpio_io_restrict io_restrict;enum acpi_gpio_polarity polarity;+};
/**
- acpi_device_path() - Get the full path to an ACPI device
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index e16c2f31d9..a57dd2665c 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -10,6 +10,7 @@ #include <dm/ofnode.h> #include <linux/bitops.h>
+struct acpi_gpio; struct ofnode_phandle_args;
/* @@ -329,6 +330,20 @@ struct dm_gpio_ops { */ int (*get_dir_flags)(struct udevice *dev, unsigned int offset, ulong *flags);
+#if CONFIG_IS_ENABLED(ACPIGEN)
/*** get_acpi() - Get the ACPI info for a GPIO** This converts a GPIO to an ACPI structure for adding to the ACPI* tables.** @desc: GPIO description to convert* @gpio: Output ACPI GPIO information* @return ACPI pin number or -ve on error*/int (*get_acpi)(const struct gpio_desc *desc, struct acpi_gpio *gpio);+#endif };
/** @@ -674,4 +689,16 @@ int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flags); */ int gpio_get_number(const struct gpio_desc *desc);
+/**
- gpio_get_acpi() - Get the ACPI pin for a GPIO
- This converts a GPIO to an ACPI pin number for adding to the ACPI
- tables. If the GPIO is invalid, the pin_count and pins[0] are set to 0
- @desc: GPIO description to convert
- @gpio: Output ACPI GPIO information
- @return ACPI pin number or -ve on error
- */
+int gpio_get_acpi(const struct gpio_desc *desc, struct acpi_gpio *gpio);
#endif /* _ASM_GENERIC_GPIO_H_ */ diff --git a/test/dm/gpio.c b/test/dm/gpio.c index b5ee4e4f87..843ba2c894 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -8,6 +8,7 @@ #include <dm.h> #include <log.h> #include <malloc.h> +#include <acpi/acpi_device.h> #include <dm/root.h> #include <dm/test.h> #include <dm/util.h> @@ -385,3 +386,64 @@ static int dm_test_gpio_get_dir_flags(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_gpio_get_dir_flags, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+/* Test of gpio_get_acpi() */ +static int dm_test_gpio_get_acpi(struct unit_test_state *uts) +{
struct acpi_gpio agpio;struct udevice *dev;struct gpio_desc desc;ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));ut_asserteq_str("a-test", dev->name);ut_assertok(gpio_request_by_name(dev, "test-gpios", 1, &desc, 0));/* See sb_gpio_get_acpi() */ut_assertok(gpio_get_acpi(&desc, &agpio));ut_asserteq(1, agpio.pin_count);ut_asserteq(4, agpio.pins[0]);ut_asserteq(ACPI_GPIO_TYPE_IO, agpio.type);ut_asserteq(ACPI_GPIO_PULL_UP, agpio.pull);ut_asserteq_str("\\_SB.PINC", agpio.resource);ut_asserteq(0, agpio.interrupt_debounce_timeout);ut_asserteq(0, agpio.irq.pin);ut_asserteq(1234, agpio.output_drive_strength);ut_asserteq(true, agpio.io_shared);ut_asserteq(ACPI_GPIO_IO_RESTRICT_INPUT, agpio.io_restrict);ut_asserteq(ACPI_GPIO_ACTIVE_HIGH, agpio.polarity);return 0;+} +DM_TEST(dm_test_gpio_get_acpi, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+/* Test of gpio_get_acpi() with an interrupt GPIO */ +static int dm_test_gpio_get_acpi_irq(struct unit_test_state *uts) +{
struct acpi_gpio agpio;struct udevice *dev;struct gpio_desc desc;ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));ut_asserteq_str("a-test", dev->name);ut_assertok(gpio_request_by_name(dev, "test2-gpios", 2, &desc, 0));/* See sb_gpio_get_acpi() */ut_assertok(gpio_get_acpi(&desc, &agpio));ut_asserteq(1, agpio.pin_count);ut_asserteq(6, agpio.pins[0]);ut_asserteq(ACPI_GPIO_TYPE_INTERRUPT, agpio.type);ut_asserteq(ACPI_GPIO_PULL_DOWN, agpio.pull);ut_asserteq_str("\\_SB.PINC", agpio.resource);ut_asserteq(4321, agpio.interrupt_debounce_timeout);ut_asserteq(6, agpio.irq.pin);ut_asserteq(ACPI_IRQ_ACTIVE_BOTH, agpio.irq.polarity);ut_asserteq(ACPI_IRQ_SHARED, agpio.irq.shared);ut_asserteq(true, agpio.irq.wake);ut_asserteq(0, agpio.output_drive_strength);ut_asserteq(false, agpio.io_shared);ut_asserteq(0, agpio.io_restrict);ut_asserteq(ACPI_GPIO_ACTIVE_LOW, agpio.polarity);return 0;+} +DM_TEST(dm_test_gpio_get_acpi_irq, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); diff --git a/test/dm/irq.c b/test/dm/irq.c index 50e505e657..51bae31b0f 100644 --- a/test/dm/irq.c +++ b/test/dm/irq.c @@ -87,6 +87,7 @@ static int dm_test_irq_get_acpi(struct unit_test_state *uts) ut_assertok(uclass_first_device_err(UCLASS_TEST_FDT, &dev)); ut_assertok(irq_get_by_index(dev, 0, &irq));
/* see sandbox_get_acpi() */ ut_assertok(irq_get_acpi(&irq, &airq)); ut_asserteq(3, airq.pin); ut_asserteq(ACPI_IRQ_LEVEL_TRIGGERED, airq.mode);--
Regards, Bin