Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

Revert "gpiolib: Split GPIO flags parsing and GPIO configuration"

This reverts commit 923b93e451db876d1479d3e4458fce14fec31d1c.

Make sure consumers do not overwrite gpio flags for pins that have
already been claimed.

While adding support for gpio drivers to refuse a request using
unsupported flags, the order of when the requested flag was checked and
the new flags were applied was reversed to that consumers could
overwrite flags for already requested gpios.

This not only affects device-tree setups where two drivers could request
the same gpio using conflicting configurations, but also allowed user
space to clear gpio flags for already claimed pins simply by attempting
to export them through the sysfs interface. By for example clearing the
FLAG_ACTIVE_LOW flag this way, user space could effectively change the
polarity of a signal.

Reverting this change obviously prevents gpio drivers from doing sanity
checks on the flags in their request callbacks. Fortunately only one
recently added driver (gpio-tps65218 in v4.6) appears to do this, and a
follow up patch could restore this functionality through a different
interface.

Cc: stable <stable@vger.kernel.org> # 4.4
Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

authored by

Johan Hovold and committed by
Linus Walleij
85b03b30 87041a58

+20 -40
+4 -4
drivers/gpio/gpiolib-legacy.c
··· 28 28 if (!desc && gpio_is_valid(gpio)) 29 29 return -EPROBE_DEFER; 30 30 31 + err = gpiod_request(desc, label); 32 + if (err) 33 + return err; 34 + 31 35 if (flags & GPIOF_OPEN_DRAIN) 32 36 set_bit(FLAG_OPEN_DRAIN, &desc->flags); 33 37 ··· 40 36 41 37 if (flags & GPIOF_ACTIVE_LOW) 42 38 set_bit(FLAG_ACTIVE_LOW, &desc->flags); 43 - 44 - err = gpiod_request(desc, label); 45 - if (err) 46 - return err; 47 39 48 40 if (flags & GPIOF_DIR_IN) 49 41 err = gpiod_direction_input(desc);
+16 -36
drivers/gpio/gpiolib.c
··· 1352 1352 spin_lock_irqsave(&gpio_lock, flags); 1353 1353 } 1354 1354 done: 1355 - if (status < 0) { 1356 - /* Clear flags that might have been set by the caller before 1357 - * requesting the GPIO. 1358 - */ 1359 - clear_bit(FLAG_ACTIVE_LOW, &desc->flags); 1360 - clear_bit(FLAG_OPEN_DRAIN, &desc->flags); 1361 - clear_bit(FLAG_OPEN_SOURCE, &desc->flags); 1362 - } 1363 1355 spin_unlock_irqrestore(&gpio_lock, flags); 1364 1356 return status; 1365 1357 } ··· 2579 2587 } 2580 2588 EXPORT_SYMBOL_GPL(gpiod_get_optional); 2581 2589 2582 - /** 2583 - * gpiod_parse_flags - helper function to parse GPIO lookup flags 2584 - * @desc: gpio to be setup 2585 - * @lflags: gpio_lookup_flags - returned from of_find_gpio() or 2586 - * of_get_gpio_hog() 2587 - * 2588 - * Set the GPIO descriptor flags based on the given GPIO lookup flags. 2589 - */ 2590 - static void gpiod_parse_flags(struct gpio_desc *desc, unsigned long lflags) 2591 - { 2592 - if (lflags & GPIO_ACTIVE_LOW) 2593 - set_bit(FLAG_ACTIVE_LOW, &desc->flags); 2594 - if (lflags & GPIO_OPEN_DRAIN) 2595 - set_bit(FLAG_OPEN_DRAIN, &desc->flags); 2596 - if (lflags & GPIO_OPEN_SOURCE) 2597 - set_bit(FLAG_OPEN_SOURCE, &desc->flags); 2598 - } 2599 2590 2600 2591 /** 2601 2592 * gpiod_configure_flags - helper function to configure a given GPIO 2602 2593 * @desc: gpio whose value will be assigned 2603 2594 * @con_id: function within the GPIO consumer 2595 + * @lflags: gpio_lookup_flags - returned from of_find_gpio() or 2596 + * of_get_gpio_hog() 2604 2597 * @dflags: gpiod_flags - optional GPIO initialization flags 2605 2598 * 2606 2599 * Return 0 on success, -ENOENT if no GPIO has been assigned to the ··· 2593 2616 * occurred while trying to acquire the GPIO. 2594 2617 */ 2595 2618 static int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id, 2596 - enum gpiod_flags dflags) 2619 + unsigned long lflags, enum gpiod_flags dflags) 2597 2620 { 2598 2621 int status; 2622 + 2623 + if (lflags & GPIO_ACTIVE_LOW) 2624 + set_bit(FLAG_ACTIVE_LOW, &desc->flags); 2625 + if (lflags & GPIO_OPEN_DRAIN) 2626 + set_bit(FLAG_OPEN_DRAIN, &desc->flags); 2627 + if (lflags & GPIO_OPEN_SOURCE) 2628 + set_bit(FLAG_OPEN_SOURCE, &desc->flags); 2599 2629 2600 2630 /* No particular flag request, return here... */ 2601 2631 if (!(dflags & GPIOD_FLAGS_BIT_DIR_SET)) { ··· 2670 2686 return desc; 2671 2687 } 2672 2688 2673 - gpiod_parse_flags(desc, lookupflags); 2674 - 2675 2689 status = gpiod_request(desc, con_id); 2676 2690 if (status < 0) 2677 2691 return ERR_PTR(status); 2678 2692 2679 - status = gpiod_configure_flags(desc, con_id, flags); 2693 + status = gpiod_configure_flags(desc, con_id, lookupflags, flags); 2680 2694 if (status < 0) { 2681 2695 dev_dbg(dev, "setup of GPIO %s failed\n", con_id); 2682 2696 gpiod_put(desc); ··· 2730 2748 if (IS_ERR(desc)) 2731 2749 return desc; 2732 2750 2751 + ret = gpiod_request(desc, NULL); 2752 + if (ret) 2753 + return ERR_PTR(ret); 2754 + 2733 2755 if (active_low) 2734 2756 set_bit(FLAG_ACTIVE_LOW, &desc->flags); 2735 2757 ··· 2743 2757 else 2744 2758 set_bit(FLAG_OPEN_SOURCE, &desc->flags); 2745 2759 } 2746 - 2747 - ret = gpiod_request(desc, NULL); 2748 - if (ret) 2749 - return ERR_PTR(ret); 2750 2760 2751 2761 return desc; 2752 2762 } ··· 2796 2814 chip = gpiod_to_chip(desc); 2797 2815 hwnum = gpio_chip_hwgpio(desc); 2798 2816 2799 - gpiod_parse_flags(desc, lflags); 2800 - 2801 2817 local_desc = gpiochip_request_own_desc(chip, hwnum, name); 2802 2818 if (IS_ERR(local_desc)) { 2803 2819 status = PTR_ERR(local_desc); ··· 2804 2824 return status; 2805 2825 } 2806 2826 2807 - status = gpiod_configure_flags(desc, name, dflags); 2827 + status = gpiod_configure_flags(desc, name, lflags, dflags); 2808 2828 if (status < 0) { 2809 2829 pr_err("setup of hog GPIO %s (chip %s, offset %d) failed, %d\n", 2810 2830 name, chip->label, hwnum, status);