From 5923ea6c2ce626f0aa8a547d5b7e5fce705dd3dc Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Fri, 26 Apr 2019 14:40:18 +0200 Subject: gpio: pass lookup and descriptor flags to request_own When a gpio_chip wants to request a descriptor from itself using gpiochip_request_own_desc() it needs to be able to specify fully how to use the descriptor, notably line inversion semantics. The workaround in the gpiolib.c can be removed and cases (such as SPI CS) where we need at times to request a GPIO with line inversion semantics directly on a chip for workarounds, can be fully supported with this call. Fix up some users of the API that weren't really using the last flag to set up the line as input or output properly but instead just calling direction setting explicitly after requesting the line. Cc: Martin Sperl Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index e013d417a936..4561cb39bdb4 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -2503,7 +2503,11 @@ EXPORT_SYMBOL_GPL(gpiochip_is_requested); * @chip: GPIO chip * @hwnum: hardware number of the GPIO for which to request the descriptor * @label: label for the GPIO - * @flags: flags for this GPIO or 0 if default + * @lflags: lookup flags for this GPIO or 0 if default, this can be used to + * specify things like line inversion semantics with the machine flags + * such as GPIO_OUT_LOW + * @dflags: descriptor request flags for this GPIO or 0 if default, this + * can be used to specify consumer semantics such as open drain * * Function allows GPIO chip drivers to request and use their own GPIO * descriptors via gpiolib API. Difference to gpiod_request() is that this @@ -2517,9 +2521,9 @@ EXPORT_SYMBOL_GPL(gpiochip_is_requested); */ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *chip, u16 hwnum, const char *label, - enum gpiod_flags flags) + enum gpio_lookup_flags lflags, + enum gpiod_flags dflags) { - unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT; struct gpio_desc *desc = gpiochip_get_desc(chip, hwnum); int err; @@ -2532,7 +2536,7 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *chip, u16 hwnum, if (err < 0) return ERR_PTR(err); - err = gpiod_configure_flags(desc, label, lflags, flags); + err = gpiod_configure_flags(desc, label, lflags, dflags); if (err) { chip_err(chip, "setup of own GPIO %s failed\n", label); gpiod_free_commit(desc); @@ -4420,15 +4424,8 @@ int gpiod_hog(struct gpio_desc *desc, const char *name, chip = gpiod_to_chip(desc); hwnum = gpio_chip_hwgpio(desc); - /* - * FIXME: not very elegant that we call gpiod_configure_flags() - * twice here (once inside gpiochip_request_own_desc() and - * again here), but the gpiochip_request_own_desc() is external - * and cannot really pass the lflags so this is the lesser evil - * at the moment. Pass zero as dflags on this first call so we - * don't screw anything up. - */ - local_desc = gpiochip_request_own_desc(chip, hwnum, name, 0); + local_desc = gpiochip_request_own_desc(chip, hwnum, name, + lflags, dflags); if (IS_ERR(local_desc)) { status = PTR_ERR(local_desc); pr_err("requesting hog GPIO %s (chip %s, offset %d) failed, %d\n", @@ -4436,14 +4433,6 @@ int gpiod_hog(struct gpio_desc *desc, const char *name, return status; } - status = gpiod_configure_flags(desc, name, lflags, dflags); - if (status < 0) { - pr_err("setup of hog GPIO %s (chip %s, offset %d) failed, %d\n", - name, chip->label, hwnum, status); - gpiochip_free_own_desc(desc); - return status; - } - /* Mark GPIO as hogged so it can be identified and removed later */ set_bit(FLAG_IS_HOGGED, &desc->flags); -- cgit v1.2.3-70-g09d2 From 4892d3a6a009f7eba2e806b9183e5d8790769f41 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Fri, 14 Jun 2019 10:12:26 +0200 Subject: gpio: Drop the parent_irq from gpio_irq_chip We already have an array named "parents" so instead of letting one point to the other, simply allocate a dynamic array to hold the parents, just one if desired and drop the number of members in gpio_irq_chip by 1. Rename gpiochip to gc in the process. Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 30 +++++++++++++++++++----------- include/linux/gpio/driver.h | 7 ------- 2 files changed, 19 insertions(+), 18 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 4561cb39bdb4..71cd685ed6c4 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1644,39 +1644,47 @@ EXPORT_SYMBOL_GPL(gpiochip_irqchip_irq_valid); /** * gpiochip_set_cascaded_irqchip() - connects a cascaded irqchip to a gpiochip - * @gpiochip: the gpiochip to set the irqchip chain to + * @gc: the gpiochip to set the irqchip chain to * @parent_irq: the irq number corresponding to the parent IRQ for this * chained irqchip * @parent_handler: the parent interrupt handler for the accumulated IRQ * coming out of the gpiochip. If the interrupt is nested rather than * cascaded, pass NULL in this handler argument */ -static void gpiochip_set_cascaded_irqchip(struct gpio_chip *gpiochip, +static void gpiochip_set_cascaded_irqchip(struct gpio_chip *gc, unsigned int parent_irq, irq_flow_handler_t parent_handler) { - if (!gpiochip->irq.domain) { - chip_err(gpiochip, "called %s before setting up irqchip\n", + struct gpio_irq_chip *girq = &gc->irq; + struct device *dev = &gc->gpiodev->dev; + + if (!girq->domain) { + chip_err(gc, "called %s before setting up irqchip\n", __func__); return; } if (parent_handler) { - if (gpiochip->can_sleep) { - chip_err(gpiochip, + if (gc->can_sleep) { + chip_err(gc, "you cannot have chained interrupts on a chip that may sleep\n"); return; } + girq->parents = devm_kcalloc(dev, 1, + sizeof(*girq->parents), + GFP_KERNEL); + if (!girq->parents) { + chip_err(gc, "out of memory allocating parent IRQ\n"); + return; + } + girq->parents[0] = parent_irq; + girq->num_parents = 1; /* * The parent irqchip is already using the chip_data for this * irqchip, so our callbacks simply use the handler_data. */ irq_set_chained_handler_and_data(parent_irq, parent_handler, - gpiochip); - - gpiochip->irq.parent_irq = parent_irq; - gpiochip->irq.parents = &gpiochip->irq.parent_irq; - gpiochip->irq.num_parents = 1; + gc); } } diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 937c40fb61f7..02698c0f34ea 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -102,13 +102,6 @@ struct gpio_irq_chip { */ unsigned int num_parents; - /** - * @parent_irq: - * - * For use by gpiochip_set_cascaded_irqchip() - */ - unsigned int parent_irq; - /** * @parents: * -- cgit v1.2.3-70-g09d2 From acc68b0e70e8c9cb4355f9cafeaf51a441f64cc2 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 18 Jun 2019 17:50:45 +0200 Subject: gpio: No need to cast away return value of debugfs_create_file() It is fine to ignore the return value (and encouraged), so need to cast away the return value, you will not get a build warning at all. Cc: Bartosz Golaszewski Cc: linux-gpio@vger.kernel.org Signed-off-by: Greg Kroah-Hartman Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 71cd685ed6c4..12bd0f094abc 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -4802,8 +4802,8 @@ static const struct file_operations gpiolib_operations = { static int __init gpiolib_debugfs_init(void) { /* /sys/kernel/debug/gpio */ - (void) debugfs_create_file("gpio", S_IFREG | S_IRUGO, - NULL, NULL, &gpiolib_operations); + debugfs_create_file("gpio", S_IFREG | S_IRUGO, NULL, NULL, + &gpiolib_operations); return 0; } subsys_initcall(gpiolib_debugfs_init); -- cgit v1.2.3-70-g09d2 From 3285170f28a850638794cdfe712eb6d93e51e706 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Mon, 1 Jul 2019 16:27:38 +0200 Subject: gpiolib: Fix references to gpiod_[gs]et_*value_cansleep() variants Commit 372e722ea4dd4ca1 ("gpiolib: use descriptors internally") renamed the functions to use a "gpiod" prefix, and commit 79a9becda8940deb ("gpiolib: export descriptor-based GPIO interface") introduced the "raw" variants, but both changes forgot to update the comments. Readd a similar reference to gpiod_set_value(), which was accidentally removed by commit 1e77fc82110ac36f ("gpio: Add missing open drain/source handling to gpiod_set_value_cansleep()"). Signed-off-by: Geert Uytterhoeven Link: https://lore.kernel.org/r/20190701142738.25219-1-geert+renesas@glider.be Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 12bd0f094abc..b9fc77b18183 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -3037,7 +3037,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, int gpiod_get_raw_value(const struct gpio_desc *desc) { VALIDATE_DESC(desc); - /* Should be using gpio_get_value_cansleep() */ + /* Should be using gpiod_get_raw_value_cansleep() */ WARN_ON(desc->gdev->chip->can_sleep); return gpiod_get_raw_value_commit(desc); } @@ -3058,7 +3058,7 @@ int gpiod_get_value(const struct gpio_desc *desc) int value; VALIDATE_DESC(desc); - /* Should be using gpio_get_value_cansleep() */ + /* Should be using gpiod_get_value_cansleep() */ WARN_ON(desc->gdev->chip->can_sleep); value = gpiod_get_raw_value_commit(desc); @@ -3329,7 +3329,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep, void gpiod_set_raw_value(struct gpio_desc *desc, int value) { VALIDATE_DESC_VOID(desc); - /* Should be using gpiod_set_value_cansleep() */ + /* Should be using gpiod_set_raw_value_cansleep() */ WARN_ON(desc->gdev->chip->can_sleep); gpiod_set_raw_value_commit(desc, value); } @@ -3370,6 +3370,7 @@ static void gpiod_set_value_nocheck(struct gpio_desc *desc, int value) void gpiod_set_value(struct gpio_desc *desc, int value) { VALIDATE_DESC_VOID(desc); + /* Should be using gpiod_set_value_cansleep() */ WARN_ON(desc->gdev->chip->can_sleep); gpiod_set_value_nocheck(desc, value); } -- cgit v1.2.3-70-g09d2 From 827a9b8bbfdc976775eb0dc650d8485081271e27 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Mon, 1 Jul 2019 16:28:09 +0200 Subject: gpiolib: Clarify use of non-sleeping functions Obviously functions that are safe to be called from atomic contexts, can be called from non-atomic contexts, too. Signed-off-by: Geert Uytterhoeven Link: https://lore.kernel.org/r/20190701142809.25308-1-geert+renesas@glider.be Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index b9fc77b18183..47a67c4deed4 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -3031,7 +3031,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, * Return the GPIO's raw value, i.e. the value of the physical line disregarding * its ACTIVE_LOW status, or negative errno on failure. * - * This function should be called from contexts where we cannot sleep, and will + * This function can be called from contexts where we cannot sleep, and will * complain if the GPIO chip functions potentially sleep. */ int gpiod_get_raw_value(const struct gpio_desc *desc) @@ -3050,7 +3050,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_value); * Return the GPIO's logical value, i.e. taking the ACTIVE_LOW status into * account, or negative errno on failure. * - * This function should be called from contexts where we cannot sleep, and will + * This function can be called from contexts where we cannot sleep, and will * complain if the GPIO chip functions potentially sleep. */ int gpiod_get_value(const struct gpio_desc *desc) @@ -3083,7 +3083,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_value); * without regard for their ACTIVE_LOW status. Return 0 in case of success, * else an error code. * - * This function should be called from contexts where we cannot sleep, + * This function can be called from contexts where we cannot sleep, * and it will complain if the GPIO chip functions potentially sleep. */ int gpiod_get_raw_array_value(unsigned int array_size, @@ -3109,7 +3109,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value); * Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status * into account. Return 0 in case of success, else an error code. * - * This function should be called from contexts where we cannot sleep, + * This function can be called from contexts where we cannot sleep, * and it will complain if the GPIO chip functions potentially sleep. */ int gpiod_get_array_value(unsigned int array_size, @@ -3323,7 +3323,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep, * Set the raw value of the GPIO, i.e. the value of its physical line without * regard for its ACTIVE_LOW status. * - * This function should be called from contexts where we cannot sleep, and will + * This function can be called from contexts where we cannot sleep, and will * complain if the GPIO chip functions potentially sleep. */ void gpiod_set_raw_value(struct gpio_desc *desc, int value) @@ -3364,7 +3364,7 @@ static void gpiod_set_value_nocheck(struct gpio_desc *desc, int value) * Set the logical value of the GPIO, i.e. taking its ACTIVE_LOW, * OPEN_DRAIN and OPEN_SOURCE flags into account. * - * This function should be called from contexts where we cannot sleep, and will + * This function can be called from contexts where we cannot sleep, and will * complain if the GPIO chip functions potentially sleep. */ void gpiod_set_value(struct gpio_desc *desc, int value) @@ -3386,7 +3386,7 @@ EXPORT_SYMBOL_GPL(gpiod_set_value); * Set the raw values of the GPIOs, i.e. the values of the physical lines * without regard for their ACTIVE_LOW status. * - * This function should be called from contexts where we cannot sleep, and will + * This function can be called from contexts where we cannot sleep, and will * complain if the GPIO chip functions potentially sleep. */ int gpiod_set_raw_array_value(unsigned int array_size, @@ -3411,7 +3411,7 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value); * Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status * into account. * - * This function should be called from contexts where we cannot sleep, and will + * This function can be called from contexts where we cannot sleep, and will * complain if the GPIO chip functions potentially sleep. */ int gpiod_set_array_value(unsigned int array_size, -- cgit v1.2.3-70-g09d2