* [PATCH] gpio: crystalcove: Do not write regular gpio registers for virtual GPIOs
@ 2017-05-13 12:39 Hans de Goede
2017-05-15 8:57 ` Andy Shevchenko
2017-05-23 8:03 ` Linus Walleij
0 siblings, 2 replies; 4+ messages in thread
From: Hans de Goede @ 2017-05-13 12:39 UTC (permalink / raw)
To: Mika Westerberg, Heikki Krogerus, Linus Walleij,
Alexandre Courbot
Cc: Hans de Goede, Andy Shevchenko, linux-gpio, Aaron Lu
The Crystal Cove PMIC has 16 real GPIOs but the ACPI code for devices
with this PMIC may address up to 95 GPIOs, these extra GPIOs are
called virtual GPIOs and are used by the ACPI code as a method of
accessing various non GPIO bits of PMIC.
Commit dcdc3018d635 ("gpio: crystalcove: support virtual GPIO") added
dummy support for these to avoid a bunch of ACPI errors, but instead of
ignoring writes / reads to them by doing:
if (gpio >= CRYSTALCOVE_GPIO_NUM)
return 0;
It accidentally introduced the following wrong check:
if (gpio > CRYSTALCOVE_VGPIO_NUM)
return 0;
Which means that attempts by the ACPI code to access these gpios
causes some arbitrary gpio to get touched through for example
GPIO1P0CTLO + gpionr % 8.
Since we do support input/output (but not interrupts) on the 0x5e
virtual GPIO, this commit makes to_reg return -ENOTSUPP for unsupported
virtual GPIOs so as to not have to check for (gpio >= CRYSTALCOVE_GPIO_NUM
&& gpio != 0x5e) everywhere and to make it easier to add support for more
virtual GPIOs in the future.
It then adds a check for to_reg returning an error to all callers where
this may happen fixing the ACPI code accessing virtual GPIOs accidentally
causing changes to real GPIOs.
Cc: Aaron Lu <aaron.lu@intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/gpio/gpio-crystalcove.c | 54 +++++++++++++++++++++++++++--------------
1 file changed, 36 insertions(+), 18 deletions(-)
diff --git a/drivers/gpio/gpio-crystalcove.c b/drivers/gpio/gpio-crystalcove.c
index 2197368cc899..e60156ec0c18 100644
--- a/drivers/gpio/gpio-crystalcove.c
+++ b/drivers/gpio/gpio-crystalcove.c
@@ -90,8 +90,18 @@ static inline int to_reg(int gpio, enum ctrl_register reg_type)
{
int reg;
- if (gpio == 94)
- return GPIOPANELCTL;
+ if (gpio >= CRYSTALCOVE_GPIO_NUM) {
+ /*
+ * Virtual GPIO called from ACPI, for now we only support
+ * the panel ctl.
+ */
+ switch (gpio) {
+ case 0x5e:
+ return GPIOPANELCTL;
+ default:
+ return -EOPNOTSUPP;
+ }
+ }
if (reg_type == CTRL_IN) {
if (gpio < 8)
@@ -130,36 +140,36 @@ static void crystalcove_update_irq_ctrl(struct crystalcove_gpio *cg, int gpio)
static int crystalcove_gpio_dir_in(struct gpio_chip *chip, unsigned gpio)
{
struct crystalcove_gpio *cg = gpiochip_get_data(chip);
+ int reg = to_reg(gpio, CTRL_OUT);
- if (gpio > CRYSTALCOVE_VGPIO_NUM)
+ if (reg < 0)
return 0;
- return regmap_write(cg->regmap, to_reg(gpio, CTRL_OUT),
- CTLO_INPUT_SET);
+ return regmap_write(cg->regmap, reg, CTLO_INPUT_SET);
}
static int crystalcove_gpio_dir_out(struct gpio_chip *chip, unsigned gpio,
int value)
{
struct crystalcove_gpio *cg = gpiochip_get_data(chip);
+ int reg = to_reg(gpio, CTRL_OUT);
- if (gpio > CRYSTALCOVE_VGPIO_NUM)
+ if (reg < 0)
return 0;
- return regmap_write(cg->regmap, to_reg(gpio, CTRL_OUT),
- CTLO_OUTPUT_SET | value);
+ return regmap_write(cg->regmap, reg, CTLO_OUTPUT_SET | value);
}
static int crystalcove_gpio_get(struct gpio_chip *chip, unsigned gpio)
{
struct crystalcove_gpio *cg = gpiochip_get_data(chip);
- int ret;
unsigned int val;
+ int ret, reg = to_reg(gpio, CTRL_IN);
- if (gpio > CRYSTALCOVE_VGPIO_NUM)
+ if (reg < 0)
return 0;
- ret = regmap_read(cg->regmap, to_reg(gpio, CTRL_IN), &val);
+ ret = regmap_read(cg->regmap, reg, &val);
if (ret)
return ret;
@@ -170,14 +180,15 @@ static void crystalcove_gpio_set(struct gpio_chip *chip,
unsigned gpio, int value)
{
struct crystalcove_gpio *cg = gpiochip_get_data(chip);
+ int reg = to_reg(gpio, CTRL_OUT);
- if (gpio > CRYSTALCOVE_VGPIO_NUM)
+ if (reg < 0)
return;
if (value)
- regmap_update_bits(cg->regmap, to_reg(gpio, CTRL_OUT), 1, 1);
+ regmap_update_bits(cg->regmap, reg, 1, 1);
else
- regmap_update_bits(cg->regmap, to_reg(gpio, CTRL_OUT), 1, 0);
+ regmap_update_bits(cg->regmap, reg, 1, 0);
}
static int crystalcove_irq_type(struct irq_data *data, unsigned type)
@@ -185,6 +196,9 @@ static int crystalcove_irq_type(struct irq_data *data, unsigned type)
struct crystalcove_gpio *cg =
gpiochip_get_data(irq_data_get_irq_chip_data(data));
+ if (data->hwirq >= CRYSTALCOVE_GPIO_NUM)
+ return 0;
+
switch (type) {
case IRQ_TYPE_NONE:
cg->intcnt_value = CTLI_INTCNT_DIS;
@@ -235,8 +249,10 @@ static void crystalcove_irq_unmask(struct irq_data *data)
struct crystalcove_gpio *cg =
gpiochip_get_data(irq_data_get_irq_chip_data(data));
- cg->set_irq_mask = false;
- cg->update |= UPDATE_IRQ_MASK;
+ if (data->hwirq < CRYSTALCOVE_GPIO_NUM) {
+ cg->set_irq_mask = false;
+ cg->update |= UPDATE_IRQ_MASK;
+ }
}
static void crystalcove_irq_mask(struct irq_data *data)
@@ -244,8 +260,10 @@ static void crystalcove_irq_mask(struct irq_data *data)
struct crystalcove_gpio *cg =
gpiochip_get_data(irq_data_get_irq_chip_data(data));
- cg->set_irq_mask = true;
- cg->update |= UPDATE_IRQ_MASK;
+ if (data->hwirq < CRYSTALCOVE_GPIO_NUM) {
+ cg->set_irq_mask = true;
+ cg->update |= UPDATE_IRQ_MASK;
+ }
}
static struct irq_chip crystalcove_irqchip = {
--
2.12.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] gpio: crystalcove: Do not write regular gpio registers for virtual GPIOs
2017-05-13 12:39 [PATCH] gpio: crystalcove: Do not write regular gpio registers for virtual GPIOs Hans de Goede
@ 2017-05-15 8:57 ` Andy Shevchenko
2017-05-15 17:10 ` Hans de Goede
2017-05-23 8:03 ` Linus Walleij
1 sibling, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2017-05-15 8:57 UTC (permalink / raw)
To: Hans de Goede, Mika Westerberg, Heikki Krogerus, Linus Walleij,
Alexandre Courbot
Cc: linux-gpio, Aaron Lu
On Sat, 2017-05-13 at 14:39 +0200, Hans de Goede wrote:
> The Crystal Cove PMIC has 16 real GPIOs but the ACPI code for devices
> with this PMIC may address up to 95 GPIOs, these extra GPIOs are
> called virtual GPIOs and are used by the ACPI code as a method of
> accessing various non GPIO bits of PMIC.
>
> Commit dcdc3018d635 ("gpio: crystalcove: support virtual GPIO") added
> dummy support for these to avoid a bunch of ACPI errors, but instead
> of
> ignoring writes / reads to them by doing:
>
> if (gpio >= CRYSTALCOVE_GPIO_NUM)
> return 0;
>
> It accidentally introduced the following wrong check:
>
> if (gpio > CRYSTALCOVE_VGPIO_NUM)
> return 0;
>
> Which means that attempts by the ACPI code to access these gpios
> causes some arbitrary gpio to get touched through for example
> GPIO1P0CTLO + gpionr % 8.
>
> Since we do support input/output (but not interrupts) on the 0x5e
> virtual GPIO, this commit makes to_reg return -ENOTSUPP for
> unsupported
> virtual GPIOs so as to not have to check for (gpio >=
> CRYSTALCOVE_GPIO_NUM
> && gpio != 0x5e) everywhere and to make it easier to add support for
> more
> virtual GPIOs in the future.
>
> It then adds a check for to_reg returning an error to all callers
> where
> this may happen fixing the ACPI code accessing virtual GPIOs
> accidentally
> causing changes to real GPIOs.
Looks fine to me.
FWIW:
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
P.S. Should it have Fixes tag?
>
> Cc: Aaron Lu <aaron.lu@intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/gpio/gpio-crystalcove.c | 54 +++++++++++++++++++++++++++-----
> ---------
> 1 file changed, 36 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpio/gpio-crystalcove.c b/drivers/gpio/gpio-
> crystalcove.c
> index 2197368cc899..e60156ec0c18 100644
> --- a/drivers/gpio/gpio-crystalcove.c
> +++ b/drivers/gpio/gpio-crystalcove.c
> @@ -90,8 +90,18 @@ static inline int to_reg(int gpio, enum
> ctrl_register reg_type)
> {
> int reg;
>
> - if (gpio == 94)
> - return GPIOPANELCTL;
> + if (gpio >= CRYSTALCOVE_GPIO_NUM) {
> + /*
> + * Virtual GPIO called from ACPI, for now we only
> support
> + * the panel ctl.
> + */
> + switch (gpio) {
> + case 0x5e:
> + return GPIOPANELCTL;
> + default:
> + return -EOPNOTSUPP;
> + }
> + }
>
> if (reg_type == CTRL_IN) {
> if (gpio < 8)
> @@ -130,36 +140,36 @@ static void crystalcove_update_irq_ctrl(struct
> crystalcove_gpio *cg, int gpio)
> static int crystalcove_gpio_dir_in(struct gpio_chip *chip, unsigned
> gpio)
> {
> struct crystalcove_gpio *cg = gpiochip_get_data(chip);
> + int reg = to_reg(gpio, CTRL_OUT);
>
> - if (gpio > CRYSTALCOVE_VGPIO_NUM)
> + if (reg < 0)
> return 0;
>
> - return regmap_write(cg->regmap, to_reg(gpio, CTRL_OUT),
> - CTLO_INPUT_SET);
> + return regmap_write(cg->regmap, reg, CTLO_INPUT_SET);
> }
>
> static int crystalcove_gpio_dir_out(struct gpio_chip *chip, unsigned
> gpio,
> int value)
> {
> struct crystalcove_gpio *cg = gpiochip_get_data(chip);
> + int reg = to_reg(gpio, CTRL_OUT);
>
> - if (gpio > CRYSTALCOVE_VGPIO_NUM)
> + if (reg < 0)
> return 0;
>
> - return regmap_write(cg->regmap, to_reg(gpio, CTRL_OUT),
> - CTLO_OUTPUT_SET | value);
> + return regmap_write(cg->regmap, reg, CTLO_OUTPUT_SET |
> value);
> }
>
> static int crystalcove_gpio_get(struct gpio_chip *chip, unsigned
> gpio)
> {
> struct crystalcove_gpio *cg = gpiochip_get_data(chip);
> - int ret;
> unsigned int val;
> + int ret, reg = to_reg(gpio, CTRL_IN);
>
> - if (gpio > CRYSTALCOVE_VGPIO_NUM)
> + if (reg < 0)
> return 0;
>
> - ret = regmap_read(cg->regmap, to_reg(gpio, CTRL_IN), &val);
> + ret = regmap_read(cg->regmap, reg, &val);
> if (ret)
> return ret;
>
> @@ -170,14 +180,15 @@ static void crystalcove_gpio_set(struct
> gpio_chip *chip,
> unsigned gpio, int value)
> {
> struct crystalcove_gpio *cg = gpiochip_get_data(chip);
> + int reg = to_reg(gpio, CTRL_OUT);
>
> - if (gpio > CRYSTALCOVE_VGPIO_NUM)
> + if (reg < 0)
> return;
>
> if (value)
> - regmap_update_bits(cg->regmap, to_reg(gpio,
> CTRL_OUT), 1, 1);
> + regmap_update_bits(cg->regmap, reg, 1, 1);
> else
> - regmap_update_bits(cg->regmap, to_reg(gpio,
> CTRL_OUT), 1, 0);
> + regmap_update_bits(cg->regmap, reg, 1, 0);
> }
>
> static int crystalcove_irq_type(struct irq_data *data, unsigned type)
> @@ -185,6 +196,9 @@ static int crystalcove_irq_type(struct irq_data
> *data, unsigned type)
> struct crystalcove_gpio *cg =
> gpiochip_get_data(irq_data_get_irq_chip_data(data));
>
> + if (data->hwirq >= CRYSTALCOVE_GPIO_NUM)
> + return 0;
> +
> switch (type) {
> case IRQ_TYPE_NONE:
> cg->intcnt_value = CTLI_INTCNT_DIS;
> @@ -235,8 +249,10 @@ static void crystalcove_irq_unmask(struct
> irq_data *data)
> struct crystalcove_gpio *cg =
> gpiochip_get_data(irq_data_get_irq_chip_data(data));
>
> - cg->set_irq_mask = false;
> - cg->update |= UPDATE_IRQ_MASK;
> + if (data->hwirq < CRYSTALCOVE_GPIO_NUM) {
> + cg->set_irq_mask = false;
> + cg->update |= UPDATE_IRQ_MASK;
> + }
> }
>
> static void crystalcove_irq_mask(struct irq_data *data)
> @@ -244,8 +260,10 @@ static void crystalcove_irq_mask(struct irq_data
> *data)
> struct crystalcove_gpio *cg =
> gpiochip_get_data(irq_data_get_irq_chip_data(data));
>
> - cg->set_irq_mask = true;
> - cg->update |= UPDATE_IRQ_MASK;
> + if (data->hwirq < CRYSTALCOVE_GPIO_NUM) {
> + cg->set_irq_mask = true;
> + cg->update |= UPDATE_IRQ_MASK;
> + }
> }
>
> static struct irq_chip crystalcove_irqchip = {
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] gpio: crystalcove: Do not write regular gpio registers for virtual GPIOs
2017-05-15 8:57 ` Andy Shevchenko
@ 2017-05-15 17:10 ` Hans de Goede
0 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2017-05-15 17:10 UTC (permalink / raw)
To: Andy Shevchenko, Mika Westerberg, Heikki Krogerus, Linus Walleij,
Alexandre Courbot
Cc: linux-gpio, Aaron Lu
Hi,
On 15-05-17 10:57, Andy Shevchenko wrote:
> On Sat, 2017-05-13 at 14:39 +0200, Hans de Goede wrote:
>> The Crystal Cove PMIC has 16 real GPIOs but the ACPI code for devices
>> with this PMIC may address up to 95 GPIOs, these extra GPIOs are
>> called virtual GPIOs and are used by the ACPI code as a method of
>> accessing various non GPIO bits of PMIC.
>>
>> Commit dcdc3018d635 ("gpio: crystalcove: support virtual GPIO") added
>> dummy support for these to avoid a bunch of ACPI errors, but instead
>> of
>> ignoring writes / reads to them by doing:
>>
>> if (gpio >= CRYSTALCOVE_GPIO_NUM)
>> return 0;
>>
>> It accidentally introduced the following wrong check:
>>
>> if (gpio > CRYSTALCOVE_VGPIO_NUM)
>> return 0;
>>
>> Which means that attempts by the ACPI code to access these gpios
>> causes some arbitrary gpio to get touched through for example
>> GPIO1P0CTLO + gpionr % 8.
>>
>> Since we do support input/output (but not interrupts) on the 0x5e
>> virtual GPIO, this commit makes to_reg return -ENOTSUPP for
>> unsupported
>> virtual GPIOs so as to not have to check for (gpio >=
>> CRYSTALCOVE_GPIO_NUM
>> && gpio != 0x5e) everywhere and to make it easier to add support for
>> more
>> virtual GPIOs in the future.
>>
>> It then adds a check for to_reg returning an error to all callers
>> where
>> this may happen fixing the ACPI code accessing virtual GPIOs
>> accidentally
>> causing changes to real GPIOs.
>
> Looks fine to me.
> FWIW:
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Thank you.
> P.S. Should it have Fixes tag?
I guess that a:
Fixes: dcdc3018d635 ("gpio: crystalcove: support virtual GPIO")
Might be a good idea to add when merging.
Regards,
Hans
>
>>
>> Cc: Aaron Lu <aaron.lu@intel.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/gpio/gpio-crystalcove.c | 54 +++++++++++++++++++++++++++-----
>> ---------
>> 1 file changed, 36 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-crystalcove.c b/drivers/gpio/gpio-
>> crystalcove.c
>> index 2197368cc899..e60156ec0c18 100644
>> --- a/drivers/gpio/gpio-crystalcove.c
>> +++ b/drivers/gpio/gpio-crystalcove.c
>> @@ -90,8 +90,18 @@ static inline int to_reg(int gpio, enum
>> ctrl_register reg_type)
>> {
>> int reg;
>>
>> - if (gpio == 94)
>> - return GPIOPANELCTL;
>> + if (gpio >= CRYSTALCOVE_GPIO_NUM) {
>> + /*
>> + * Virtual GPIO called from ACPI, for now we only
>> support
>> + * the panel ctl.
>> + */
>> + switch (gpio) {
>> + case 0x5e:
>> + return GPIOPANELCTL;
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> + }
>>
>> if (reg_type == CTRL_IN) {
>> if (gpio < 8)
>> @@ -130,36 +140,36 @@ static void crystalcove_update_irq_ctrl(struct
>> crystalcove_gpio *cg, int gpio)
>> static int crystalcove_gpio_dir_in(struct gpio_chip *chip, unsigned
>> gpio)
>> {
>> struct crystalcove_gpio *cg = gpiochip_get_data(chip);
>> + int reg = to_reg(gpio, CTRL_OUT);
>>
>> - if (gpio > CRYSTALCOVE_VGPIO_NUM)
>> + if (reg < 0)
>> return 0;
>>
>> - return regmap_write(cg->regmap, to_reg(gpio, CTRL_OUT),
>> - CTLO_INPUT_SET);
>> + return regmap_write(cg->regmap, reg, CTLO_INPUT_SET);
>> }
>>
>> static int crystalcove_gpio_dir_out(struct gpio_chip *chip, unsigned
>> gpio,
>> int value)
>> {
>> struct crystalcove_gpio *cg = gpiochip_get_data(chip);
>> + int reg = to_reg(gpio, CTRL_OUT);
>>
>> - if (gpio > CRYSTALCOVE_VGPIO_NUM)
>> + if (reg < 0)
>> return 0;
>>
>> - return regmap_write(cg->regmap, to_reg(gpio, CTRL_OUT),
>> - CTLO_OUTPUT_SET | value);
>> + return regmap_write(cg->regmap, reg, CTLO_OUTPUT_SET |
>> value);
>> }
>>
>> static int crystalcove_gpio_get(struct gpio_chip *chip, unsigned
>> gpio)
>> {
>> struct crystalcove_gpio *cg = gpiochip_get_data(chip);
>> - int ret;
>> unsigned int val;
>> + int ret, reg = to_reg(gpio, CTRL_IN);
>>
>> - if (gpio > CRYSTALCOVE_VGPIO_NUM)
>> + if (reg < 0)
>> return 0;
>>
>> - ret = regmap_read(cg->regmap, to_reg(gpio, CTRL_IN), &val);
>> + ret = regmap_read(cg->regmap, reg, &val);
>> if (ret)
>> return ret;
>>
>> @@ -170,14 +180,15 @@ static void crystalcove_gpio_set(struct
>> gpio_chip *chip,
>> unsigned gpio, int value)
>> {
>> struct crystalcove_gpio *cg = gpiochip_get_data(chip);
>> + int reg = to_reg(gpio, CTRL_OUT);
>>
>> - if (gpio > CRYSTALCOVE_VGPIO_NUM)
>> + if (reg < 0)
>> return;
>>
>> if (value)
>> - regmap_update_bits(cg->regmap, to_reg(gpio,
>> CTRL_OUT), 1, 1);
>> + regmap_update_bits(cg->regmap, reg, 1, 1);
>> else
>> - regmap_update_bits(cg->regmap, to_reg(gpio,
>> CTRL_OUT), 1, 0);
>> + regmap_update_bits(cg->regmap, reg, 1, 0);
>> }
>>
>> static int crystalcove_irq_type(struct irq_data *data, unsigned type)
>> @@ -185,6 +196,9 @@ static int crystalcove_irq_type(struct irq_data
>> *data, unsigned type)
>> struct crystalcove_gpio *cg =
>> gpiochip_get_data(irq_data_get_irq_chip_data(data));
>>
>> + if (data->hwirq >= CRYSTALCOVE_GPIO_NUM)
>> + return 0;
>> +
>> switch (type) {
>> case IRQ_TYPE_NONE:
>> cg->intcnt_value = CTLI_INTCNT_DIS;
>> @@ -235,8 +249,10 @@ static void crystalcove_irq_unmask(struct
>> irq_data *data)
>> struct crystalcove_gpio *cg =
>> gpiochip_get_data(irq_data_get_irq_chip_data(data));
>>
>> - cg->set_irq_mask = false;
>> - cg->update |= UPDATE_IRQ_MASK;
>> + if (data->hwirq < CRYSTALCOVE_GPIO_NUM) {
>> + cg->set_irq_mask = false;
>> + cg->update |= UPDATE_IRQ_MASK;
>> + }
>> }
>>
>> static void crystalcove_irq_mask(struct irq_data *data)
>> @@ -244,8 +260,10 @@ static void crystalcove_irq_mask(struct irq_data
>> *data)
>> struct crystalcove_gpio *cg =
>> gpiochip_get_data(irq_data_get_irq_chip_data(data));
>>
>> - cg->set_irq_mask = true;
>> - cg->update |= UPDATE_IRQ_MASK;
>> + if (data->hwirq < CRYSTALCOVE_GPIO_NUM) {
>> + cg->set_irq_mask = true;
>> + cg->update |= UPDATE_IRQ_MASK;
>> + }
>> }
>>
>> static struct irq_chip crystalcove_irqchip = {
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio: crystalcove: Do not write regular gpio registers for virtual GPIOs
2017-05-13 12:39 [PATCH] gpio: crystalcove: Do not write regular gpio registers for virtual GPIOs Hans de Goede
2017-05-15 8:57 ` Andy Shevchenko
@ 2017-05-23 8:03 ` Linus Walleij
1 sibling, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2017-05-23 8:03 UTC (permalink / raw)
To: Hans de Goede
Cc: Mika Westerberg, Heikki Krogerus, Alexandre Courbot,
Andy Shevchenko, linux-gpio@vger.kernel.org, Aaron Lu
On Sat, May 13, 2017 at 2:39 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> The Crystal Cove PMIC has 16 real GPIOs but the ACPI code for devices
> with this PMIC may address up to 95 GPIOs, these extra GPIOs are
> called virtual GPIOs and are used by the ACPI code as a method of
> accessing various non GPIO bits of PMIC.
>
> Commit dcdc3018d635 ("gpio: crystalcove: support virtual GPIO") added
> dummy support for these to avoid a bunch of ACPI errors, but instead of
> ignoring writes / reads to them by doing:
>
> if (gpio >= CRYSTALCOVE_GPIO_NUM)
> return 0;
>
> It accidentally introduced the following wrong check:
>
> if (gpio > CRYSTALCOVE_VGPIO_NUM)
> return 0;
>
> Which means that attempts by the ACPI code to access these gpios
> causes some arbitrary gpio to get touched through for example
> GPIO1P0CTLO + gpionr % 8.
>
> Since we do support input/output (but not interrupts) on the 0x5e
> virtual GPIO, this commit makes to_reg return -ENOTSUPP for unsupported
> virtual GPIOs so as to not have to check for (gpio >= CRYSTALCOVE_GPIO_NUM
> && gpio != 0x5e) everywhere and to make it easier to add support for more
> virtual GPIOs in the future.
>
> It then adds a check for to_reg returning an error to all callers where
> this may happen fixing the ACPI code accessing virtual GPIOs accidentally
> causing changes to real GPIOs.
>
> Cc: Aaron Lu <aaron.lu@intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Patch applied for fixes with Andy's review tag and
the suggested Fixes: tag.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-05-23 8:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-13 12:39 [PATCH] gpio: crystalcove: Do not write regular gpio registers for virtual GPIOs Hans de Goede
2017-05-15 8:57 ` Andy Shevchenko
2017-05-15 17:10 ` Hans de Goede
2017-05-23 8:03 ` Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.