Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42968 )
Change subject: acpi_device: Replace polarity with active_low in acpi_gpio for GpioIo ......................................................................
acpi_device: Replace polarity with active_low in acpi_gpio for GpioIo
As per ACPI spec, GpioIo does not have any polarity associated with it. Linux kernel uses `active_low` argument within GPIO _DSD property to allow BIOS to indicate if the corresponding GPIO should be treated as active low. Thus, if GPIO has active high polarity or if it does not have any polarity associated with it, then the `active_low` argument is supposed to be set to 0.
Having a `polarity` field in acpi_gpio seems confusing because GPIOs might not always have polarity associated with them. Example, in case of DMIC-select GPIO where 0 means select DMIC0 and 1 means select DMIC1, there is no polarity associated with the GPIO. Thus, it would be clearer for mainboard to use macros without having to specify a particular polarity. In order to enable mainboards to provide GPIO information without polarity for GpioIo usage, this change also adds `ACPI_GPIO_OUTPUT` and `ACPI_GPIO_INPUT` macros.
BUG=b:157603026
Change-Id: I39d2a6ac8f149a74afeb915812fece86c9b9ad93 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/acpi/acpigen.c M src/drivers/generic/gpio_keys/gpio_keys.c M src/drivers/generic/max98357a/max98357a.c M src/drivers/i2c/generic/generic.c M src/drivers/i2c/rt5663/rt5663.c M src/drivers/spi/acpi/acpi.c M src/drivers/uart/acpi/acpi.c M src/drivers/usb/acpi/usb_acpi.c M src/include/acpi/acpi_device.h 9 files changed, 40 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/42968/1
diff --git a/src/acpi/acpigen.c b/src/acpi/acpigen.c index efc5a16..fc6da1b 100644 --- a/src/acpi/acpigen.c +++ b/src/acpi/acpigen.c @@ -1800,15 +1800,15 @@ */ int acpigen_enable_tx_gpio(struct acpi_gpio *gpio) { - if (gpio->polarity == ACPI_GPIO_ACTIVE_HIGH) - return acpigen_soc_set_tx_gpio(gpio->pins[0]); - else + if (gpio->active_low) return acpigen_soc_clear_tx_gpio(gpio->pins[0]); + else + return acpigen_soc_set_tx_gpio(gpio->pins[0]); }
int acpigen_disable_tx_gpio(struct acpi_gpio *gpio) { - if (gpio->polarity == ACPI_GPIO_ACTIVE_LOW) + if (gpio->active_low) return acpigen_soc_set_tx_gpio(gpio->pins[0]); else return acpigen_soc_clear_tx_gpio(gpio->pins[0]); @@ -1818,7 +1818,7 @@ { acpigen_soc_read_rx_gpio(gpio->pins[0]);
- if (gpio->polarity == ACPI_GPIO_ACTIVE_LOW) + if (gpio->active_low) acpigen_write_xor(LOCAL0_OP, 1, LOCAL0_OP); }
@@ -1826,7 +1826,7 @@ { acpigen_soc_get_tx_gpio(gpio->pins[0]);
- if (gpio->polarity == ACPI_GPIO_ACTIVE_LOW) + if (gpio->active_low) acpigen_write_xor(LOCAL0_OP, 1, LOCAL0_OP); }
diff --git a/src/drivers/generic/gpio_keys/gpio_keys.c b/src/drivers/generic/gpio_keys/gpio_keys.c index f9d7187..80ac407 100644 --- a/src/drivers/generic/gpio_keys/gpio_keys.c +++ b/src/drivers/generic/gpio_keys/gpio_keys.c @@ -43,7 +43,7 @@ acpi_dp_add_integer(dsd, "debounce-interval", key->debounce_interval); acpi_dp_add_gpio(dsd, "gpios", parent_path, 0, 0, - config->gpio.polarity); + config->gpio.active_low);
return dsd; } diff --git a/src/drivers/generic/max98357a/max98357a.c b/src/drivers/generic/max98357a/max98357a.c index 575548b..44f8802 100644 --- a/src/drivers/generic/max98357a/max98357a.c +++ b/src/drivers/generic/max98357a/max98357a.c @@ -51,7 +51,7 @@ path = acpi_device_path(dev); dp = acpi_dp_new_table("_DSD"); acpi_dp_add_gpio(dp, "sdmode-gpio", path, 0, 0, - config->sdmode_gpio.polarity); + config->sdmode_gpio.active_low); acpi_dp_add_integer(dp, "sdmode-delay", config->sdmode_delay); acpi_dp_write(dp);
diff --git a/src/drivers/i2c/generic/generic.c b/src/drivers/i2c/generic/generic.c index 5893a6f..18fd55c 100644 --- a/src/drivers/i2c/generic/generic.c +++ b/src/drivers/i2c/generic/generic.c @@ -114,16 +114,15 @@ if (irq_gpio_index != -1) acpi_dp_add_gpio(dsd, "irq-gpios", path, irq_gpio_index, 0, - config->irq_gpio.polarity == - ACPI_GPIO_ACTIVE_LOW); + config->irq_gpio.active_low); if (reset_gpio_index != -1) acpi_dp_add_gpio(dsd, "reset-gpios", path, reset_gpio_index, 0, - config->reset_gpio.polarity); + config->reset_gpio.active_low); if (enable_gpio_index != -1) acpi_dp_add_gpio(dsd, "enable-gpios", path, enable_gpio_index, 0, - config->enable_gpio.polarity); + config->enable_gpio.active_low); /* Add generic property list */ acpi_dp_add_property_list(dsd, config->property_list, config->property_count); diff --git a/src/drivers/i2c/rt5663/rt5663.c b/src/drivers/i2c/rt5663/rt5663.c index da12a4d..272cf78 100644 --- a/src/drivers/i2c/rt5663/rt5663.c +++ b/src/drivers/i2c/rt5663/rt5663.c @@ -53,7 +53,7 @@ dp = acpi_dp_new_table("_DSD"); if (config->irq_gpio.pin_count) acpi_dp_add_gpio(dp, "irq-gpios", acpi_device_path(dev), 0, 0, - config->irq_gpio.polarity == ACPI_GPIO_ACTIVE_LOW); + config->irq_gpio.active_low); RT5663_DP_INT("dc_offset_l_manual", config->dc_offset_l_manual); RT5663_DP_INT("dc_offset_r_manual", config->dc_offset_r_manual); RT5663_DP_INT("dc_offset_l_manual_mic", config->dc_offset_l_manual_mic); diff --git a/src/drivers/spi/acpi/acpi.c b/src/drivers/spi/acpi/acpi.c index 4127f9a..c0e776e 100644 --- a/src/drivers/spi/acpi/acpi.c +++ b/src/drivers/spi/acpi/acpi.c @@ -139,15 +139,15 @@ if (irq_gpio_index >= 0) acpi_dp_add_gpio(dsd, "irq-gpios", path, irq_gpio_index, 0, - config->irq_gpio.polarity); + config->irq_gpio.active_low); if (reset_gpio_index >= 0) acpi_dp_add_gpio(dsd, "reset-gpios", path, reset_gpio_index, 0, - config->reset_gpio.polarity); + config->reset_gpio.active_low); if (enable_gpio_index >= 0) acpi_dp_add_gpio(dsd, "enable-gpios", path, enable_gpio_index, 0, - config->enable_gpio.polarity); + config->enable_gpio.active_low); acpi_dp_write(dsd); }
diff --git a/src/drivers/uart/acpi/acpi.c b/src/drivers/uart/acpi/acpi.c index 9b4d1fa..f9d9d8f 100644 --- a/src/drivers/uart/acpi/acpi.c +++ b/src/drivers/uart/acpi/acpi.c @@ -103,15 +103,15 @@ if (irq_gpio_index >= 0) acpi_dp_add_gpio(dsd, "irq-gpios", path, irq_gpio_index, 0, - config->irq_gpio.polarity); + config->irq_gpio.active_low); if (reset_gpio_index >= 0) acpi_dp_add_gpio(dsd, "reset-gpios", path, reset_gpio_index, 0, - config->reset_gpio.polarity); + config->reset_gpio.active_low); if (enable_gpio_index >= 0) acpi_dp_add_gpio(dsd, "enable-gpios", path, enable_gpio_index, 0, - config->enable_gpio.polarity); + config->enable_gpio.active_low); acpi_dp_write(dsd); }
diff --git a/src/drivers/usb/acpi/usb_acpi.c b/src/drivers/usb/acpi/usb_acpi.c index e136df0..d33b7de 100644 --- a/src/drivers/usb/acpi/usb_acpi.c +++ b/src/drivers/usb/acpi/usb_acpi.c @@ -57,7 +57,7 @@
dsd = acpi_dp_new_table("_DSD"); acpi_dp_add_gpio(dsd, "reset-gpio", path, 0, 0, - config->reset_gpio.polarity); + config->reset_gpio.active_low); acpi_dp_write(dsd); }
diff --git a/src/include/acpi/acpi_device.h b/src/include/acpi/acpi_device.h index e01b6fd..6287ba1 100644 --- a/src/include/acpi/acpi_device.h +++ b/src/include/acpi/acpi_device.h @@ -158,11 +158,6 @@ ACPI_GPIO_IO_RESTRICT_PRESERVE };
-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 8
@@ -182,37 +177,43 @@ uint16_t output_drive_strength; /* 1/100 mA */ int io_shared; enum acpi_gpio_io_restrict io_restrict; - enum acpi_gpio_polarity polarity; + /* + * As per ACPI spec, GpioIo does not have any polarity associated with it. Linux kernel + * uses `active_low` argument within GPIO _DSD property to allow BIOS to indicate if the + * corresponding GPIO should be treated as active low. Thus, if the GPIO has active high + * polarity or if it does not have any polarity, then the `active_low` argument is + * supposed to be set to 0. + * + * Reference: + * https://www.kernel.org/doc/html/latest/firmware-guide/acpi/gpio-properties.h... + */ + bool active_low; };
/* GpioIo-related macros */ -#define ACPI_GPIO_CFG(_gpio, _io_restrict, _polarity) { \ +#define ACPI_GPIO_CFG(_gpio, _io_restrict, _active_low) { \ .type = ACPI_GPIO_TYPE_IO, \ .pull = ACPI_GPIO_PULL_DEFAULT, \ .io_restrict = _io_restrict, \ - .polarity = _polarity, \ + .active_low = _active_low, \ .pin_count = 1, \ .pins = { (_gpio) } }
/* Basic output GPIO with default pull settings */ -#define ACPI_GPIO_OUTPUT_CFG(gpio, polarity) \ - ACPI_GPIO_CFG(gpio, ACPI_GPIO_IO_RESTRICT_OUTPUT, polarity) +#define ACPI_GPIO_OUTPUT_CFG(gpio, active_low) \ + ACPI_GPIO_CFG(gpio, ACPI_GPIO_IO_RESTRICT_OUTPUT, active_low)
-#define ACPI_GPIO_OUTPUT_ACTIVE_HIGH(gpio) \ - ACPI_GPIO_OUTPUT_CFG(gpio, ACPI_GPIO_ACTIVE_HIGH) - -#define ACPI_GPIO_OUTPUT_ACTIVE_LOW(gpio) \ - ACPI_GPIO_OUTPUT_CFG(gpio, ACPI_GPIO_ACTIVE_LOW) +#define ACPI_GPIO_OUTPUT(gpio) ACPI_GPIO_OUTPUT_CFG(gpio, 0) +#define ACPI_GPIO_OUTPUT_ACTIVE_HIGH(gpio) ACPI_GPIO_OUTPUT_CFG(gpio, 0) +#define ACPI_GPIO_OUTPUT_ACTIVE_LOW(gpio) ACPI_GPIO_OUTPUT_CFG(gpio, 1)
/* Basic input GPIO with default pull settings */ #define ACPI_GPIO_INPUT_CFG(gpio, polarity) \ ACPI_GPIO_CFG(gpio, ACPI_GPIO_IO_RESTRICT_INPUT, polarity)
-#define ACPI_GPIO_INPUT_ACTIVE_HIGH(gpio) \ - ACPI_GPIO_INPUT_CFG(gpio, ACPI_GPIO_ACTIVE_HIGH) - -#define ACPI_GPIO_INPUT_ACTIVE_LOW(gpio) \ - ACPI_GPIO_INPUT_CFG(gpio, ACPI_GPIO_ACTIVE_LOW) +#define ACPI_GPIO_INPUT(gpio) ACPI_GPIO_INPUT_CFG(gpio, 0) +#define ACPI_GPIO_INPUT_ACTIVE_HIGH(gpio) ACPI_GPIO_INPUT_CFG(gpio, 0) +#define ACPI_GPIO_INPUT_ACTIVE_LOW(gpio) ACPI_GPIO_INPUT_CFG(gpio, 1)
/* GpioInt-related macros */ #define ACPI_GPIO_IRQ_CFG(_gpio, _mode, _polarity, _wake) { \