[PATCH] gpio: of: Fix hard-assigned valid_mask for OF case

[PATCH] gpio: of: Fix hard-assigned valid_mask for OF case

[Linus Walleij]

From: Stephen Boyd <swboyd@chromium.org>

The recent refactoring to break out OF code to its own file
contained a bug by yours truly letting the need_valid_mask
be overridden by the need of the device tree range check,
and if there were no ranges, but device tree was active
and the reserved GPIO used in another way, things likely
crash.

Cc: Mark Brown <broonie@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Stephen Boyd <swboyd@chromium.org>
Reported-by: Mark Brown <broonie@kernel.org>
Suggested-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib-of.c | 2 +-
 drivers/gpio/gpiolib-of.h | 4 ++--
 drivers/gpio/gpiolib.c    | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index b10d04dd9296..e39b4290b80c 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -87,7 +87,7 @@ static struct gpio_desc *of_xlate_and_get_gpiod_flags(struct gpio_chip *chip,
  * @dev: the device for the GPIO provider
  * @return: true if the valid mask needs to be set
  */
-bool of_gpio_need_valid_mask(struct gpio_chip *gc)
+bool of_gpio_need_valid_mask(const struct gpio_chip *gc)
 {
 	int size;
 	struct device_node *np = gc->of_node;
diff --git a/drivers/gpio/gpiolib-of.h b/drivers/gpio/gpiolib-of.h
index 34954921d96e..454d1658ee2d 100644
--- a/drivers/gpio/gpiolib-of.h
+++ b/drivers/gpio/gpiolib-of.h
@@ -16,7 +16,7 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
 int of_gpiochip_add(struct gpio_chip *gc);
 void of_gpiochip_remove(struct gpio_chip *gc);
 int of_gpio_get_count(struct device *dev, const char *con_id);
-bool of_gpio_need_valid_mask(struct gpio_chip *gc);
+bool of_gpio_need_valid_mask(const struct gpio_chip *gc);
 #else
 static inline struct gpio_desc *of_find_gpio(struct device *dev,
 					     const char *con_id,
@@ -36,7 +36,7 @@ static inline int of_gpio_get_count(struct device *dev, const char *con_id)
 {
 	return 0;
 }
-static inline bool of_gpio_need_valid_mask(struct gpio_chip *gc)
+static inline bool of_gpio_need_valid_mask(const struct gpio_chip *gc)
 {
 	return false;
 }
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f83870a18ae7..20f13129cdaa 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -363,8 +363,8 @@ static unsigned long *gpiochip_allocate_mask(struct gpio_chip *chip)
 
 static int gpiochip_alloc_valid_mask(struct gpio_chip *gc)
 {
-	if (IS_ENABLED(CONFIG_OF_GPIO))
-		gc->need_valid_mask = of_gpio_need_valid_mask(gc);
+	if (of_gpio_need_valid_mask(gc))
+		gc->need_valid_mask = true;
 	if (!gc->need_valid_mask)
 		return 0;
 
-- 
2.21.0

Re: [PATCH] gpio: of: Fix hard-assigned valid_mask for OF case

[Stephen Boyd]

Quoting Linus Walleij (2019-07-31 15:42:08)
> From: Stephen Boyd <swboyd@chromium.org>
> 
> The recent refactoring to break out OF code to its own file
> contained a bug by yours truly letting the need_valid_mask

I'm not yours truly :)

> be overridden by the need of the device tree range check,
> and if there were no ranges, but device tree was active
> and the reserved GPIO used in another way, things likely
> crash.
> 
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Stephen Boyd <swboyd@chromium.org>
> Reported-by: Mark Brown <broonie@kernel.org>
> Suggested-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---

Thanks for taking care of writing up the commit text for me.

Re: [PATCH] gpio: of: Fix hard-assigned valid_mask for OF case

[Linus Walleij]

On Thu, Aug 1, 2019 at 12:51 AM Stephen Boyd <swboyd@chromium.org> wrote:
> Quoting Linus Walleij (2019-07-31 15:42:08)
> > From: Stephen Boyd <swboyd@chromium.org>
> >
> > The recent refactoring to break out OF code to its own file
> > contained a bug by yours truly letting the need_valid_mask
>
> I'm not yours truly :)

Ooops too much first person patching...

I fix it up in the git.

Thanks a lot for helping out Stephen!

Yours,
Linus Walleij