[PATCH 2/4] gpiolib: add bitmask for valid GPIO lines
From: Archit Taneja <hidden>
Date: 2017-12-01 11:38:25
Also in:
linux-arm-msm, linux-gpio
Hi, On 11/08/2017 04:37 AM, Timur Tabi wrote:
quoted hunk ↗ jump to hunk
Add support for specifying that some GPIOs within a range are unavailable. Some systems have a sparse list of GPIOs, where a range of GPIOs is specified (usually 0 to n-1), but some subset within that range is absent or unavailable for whatever reason. To support this, allow drivers to specify a bitmask of GPIOs that are present or absent. Gpiolib will then block access to those that are absent. Signed-off-by: Timur Tabi <redacted> --- drivers/gpio/gpiolib.c | 43 +++++++++++++++++++++++++++++++++++-------- include/linux/gpio/driver.h | 2 ++ 2 files changed, 37 insertions(+), 8 deletions(-)diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 60553af4c004..c32387936cdd 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c@@ -1481,22 +1481,36 @@ static struct gpio_chip *find_chip_by_name(const char *name) static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gpiochip) { - if (!gpiochip->irq_need_valid_mask) - return 0; + if (gpiochip->irq_need_valid_mask) { + gpiochip->irq_valid_mask = + kcalloc(BITS_TO_LONGS(gpiochip->ngpio), + sizeof(long), GFP_KERNEL); + if (!gpiochip->irq_valid_mask) + return -ENOMEM; - gpiochip->irq_valid_mask = kcalloc(BITS_TO_LONGS(gpiochip->ngpio), - sizeof(long), GFP_KERNEL); - if (!gpiochip->irq_valid_mask) - return -ENOMEM; + /* Assume by default all GPIOs are valid */ + bitmap_fill(gpiochip->irq_valid_mask, gpiochip->ngpio); + } - /* Assume by default all GPIOs are valid */ - bitmap_fill(gpiochip->irq_valid_mask, gpiochip->ngpio); + if (gpiochip->line_need_valid_mask) { + gpiochip->line_valid_mask = + kcalloc(BITS_TO_LONGS(gpiochip->ngpio), + sizeof(long), GFP_KERNEL); + if (!gpiochip->line_valid_mask) + return -ENOMEM; + + /* Assume by default all GPIOs are valid */ + bitmap_fill(gpiochip->line_valid_mask, gpiochip->ngpio); + } return 0; } static void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gpiochip) { + kfree(gpiochip->line_valid_mask); + gpiochip->line_valid_mask = NULL; + kfree(gpiochip->irq_valid_mask); gpiochip->irq_valid_mask = NULL; }@@ -1510,6 +1524,15 @@ static bool gpiochip_irqchip_irq_valid(const struct gpio_chip *gpiochip, return test_bit(offset, gpiochip->irq_valid_mask); } +static bool gpiochip_irqchip_line_valid(const struct gpio_chip *gpiochip, + unsigned int offset) +{ + /* No mask means all valid */ + if (likely(!gpiochip->line_valid_mask)) + return true; + return test_bit(offset, gpiochip->line_valid_mask); +} + /** * gpiochip_set_cascaded_irqchip() - connects a cascaded irqchip to a gpiochip * @gpiochip: the gpiochip to set the irqchip chain to@@ -3320,6 +3343,10 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev, return desc; } + /* Make sure the GPIO is valid before we request it. */ + if (!gpiochip_irqchip_line_valid(desc->gdev->chip, idx)) + return ERR_PTR(-EACCES);
We're hitting an issue here ^ when a consumer calls the devm_gpiod_get() API. This API
internally sets idx in gpiod_get_index to always 0. For example, in the call sequence
below, the idx argument to gpiochip_irqchip_line_valid is always 0:
devm_gpiod_get(dev, con_id, flags)
devm_gpiod_get_index(dev, con_id, 0, flags)
gpiod_get_index(dev, con_id, 0, flags)
if (!gpiochip_irqchip_line_valid(desc->gdev->chip, 0))
return ERR_PTR(-EACCES);
Therefore, with these patches, if we create a sparse gpio map, and the
0th gpio is set to "not accessible", all gpio requests end up failing with EACCESS
because idx is always passed as 0.
In gpiochip_irqchip_line_valid, shouldn't we test the nth bit (that corresponds to
this gpio_desc) in chip->line_valid_mask instead of idx passed above, which is always
0?
Thanks,
Archit
quoted hunk ↗ jump to hunk
+ status = gpiod_request(desc, con_id); if (status < 0) return ERR_PTR(status);diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 424e5139ff10..853828ccabc8 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h@@ -173,6 +173,8 @@ struct gpio_chip { bool irq_nested; bool irq_need_valid_mask; unsigned long *irq_valid_mask; + bool line_need_valid_mask; + unsigned long *line_valid_mask; struct lock_class_key *lock_key; #endif
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project