[v3,2/3] gpiolib: Support 'gpio-reserved-ranges' property
diff mbox series

Message ID 20180321165848.89751-3-swboyd@chromium.org
State New
Headers show
Series
  • Support qcom pinctrl protected pins
Related show

Commit Message

Stephen Boyd March 21, 2018, 4:58 p.m. UTC
From: Stephen Boyd <sboyd@codeaurora.org>

Some qcom platforms make some GPIOs or pins unavailable for use by
non-secure operating systems, and thus reading or writing the registers
for those pins will cause access control issues. Add support for a DT
property to describe the set of GPIOs that are available for use so that
higher level OSes are able to know what pins to avoid reading/writing.
Non-DT platforms can add support by directly updating the
chip->valid_mask.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpio/gpiolib-of.c   | 24 ++++++++++++++++++
 drivers/gpio/gpiolib.c      | 50 +++++++++++++++++++++++++++++++++++++
 include/linux/gpio/driver.h | 16 ++++++++++++
 3 files changed, 90 insertions(+)

Comments

Andy Shevchenko March 21, 2018, 5:59 p.m. UTC | #1
On Wed, 2018-03-21 at 09:58 -0700, Stephen Boyd wrote:
> From: Stephen Boyd <sboyd@codeaurora.org>
> 
> Some qcom platforms make some GPIOs or pins unavailable for use by
> non-secure operating systems, and thus reading or writing the
> registers
> for those pins will cause access control issues. Add support for a DT
> property to describe the set of GPIOs that are available for use so
> that
> higher level OSes are able to know what pins to avoid reading/writing.
> Non-DT platforms can add support by directly updating the
> chip->valid_mask.

> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>

Hmm...

> +	gpiochip->valid_mask = kcalloc(BITS_TO_LONGS(gpiochip-
> >ngpio),
> +				       sizeof(long), GFP_KERNEL);

Just noticed that kcalloc is superfluous here.
kmalloc_array() would be enough.

> +	if (!gpiochip->valid_mask)
> +		return -ENOMEM;
> +
> +	/* Assume by default all GPIOs are valid */
> +	bitmap_fill(gpiochip->valid_mask, gpiochip->ngpio);
Andy Shevchenko March 21, 2018, 6:02 p.m. UTC | #2
On Wed, 2018-03-21 at 19:59 +0200, Andy Shevchenko wrote:
> On Wed, 2018-03-21 at 09:58 -0700, Stephen Boyd wrote:

After addressing comments,

Reviwed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Stephen Boyd March 21, 2018, 7:59 p.m. UTC | #3
Quoting Andy Shevchenko (2018-03-21 10:59:10)
> On Wed, 2018-03-21 at 09:58 -0700, Stephen Boyd wrote:
> > From: Stephen Boyd <sboyd@codeaurora.org>
> > 
> > Some qcom platforms make some GPIOs or pins unavailable for use by
> > non-secure operating systems, and thus reading or writing the
> > registers
> > for those pins will cause access control issues. Add support for a DT
> > property to describe the set of GPIOs that are available for use so
> > that
> > higher level OSes are able to know what pins to avoid reading/writing.
> > Non-DT platforms can add support by directly updating the
> > chip->valid_mask.
> 
> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> 
> Hmm...

Don't look closely! :P

> 
> > +     gpiochip->valid_mask = kcalloc(BITS_TO_LONGS(gpiochip-
> > >ngpio),
> > +                                    sizeof(long), GFP_KERNEL);
> 
> Just noticed that kcalloc is superfluous here.
> kmalloc_array() would be enough.
> 

Ok. I was copying the irqchip style. Should I fold them together into a
helper function and also update to kmalloc_array?
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Shevchenko March 21, 2018, 8:31 p.m. UTC | #4
On Wed, 2018-03-21 at 12:59 -0700, Stephen Boyd wrote:
> Quoting Andy Shevchenko (2018-03-21 10:59:10)
> > On Wed, 2018-03-21 at 09:58 -0700, Stephen Boyd wrote:
> > > 

> > 
> > > +     gpiochip->valid_mask = kcalloc(BITS_TO_LONGS(gpiochip-
> > > > ngpio),
> > > 
> > > +                                    sizeof(long), GFP_KERNEL);
> > 
> > Just noticed that kcalloc is superfluous here.
> > kmalloc_array() would be enough.
> > 
> 
> Ok. I was copying the irqchip style. Should I fold them together into
> a
> helper function and also update to kmalloc_array?

Not sure about first part, but definitely makes sense the second one (as
a separate patch perhaps).
Linus Walleij March 27, 2018, 1:24 p.m. UTC | #5
On Wed, Mar 21, 2018 at 8:59 PM, Stephen Boyd <swboyd@chromium.org> wrote:
> Quoting Andy Shevchenko (2018-03-21 10:59:10)
>> On Wed, 2018-03-21 at 09:58 -0700, Stephen Boyd wrote:
>> > From: Stephen Boyd <sboyd@codeaurora.org>
>> >
>> > Some qcom platforms make some GPIOs or pins unavailable for use by
>> > non-secure operating systems, and thus reading or writing the
>> > registers
>> > for those pins will cause access control issues. Add support for a DT
>> > property to describe the set of GPIOs that are available for use so
>> > that
>> > higher level OSes are able to know what pins to avoid reading/writing.
>> > Non-DT platforms can add support by directly updating the
>> > chip->valid_mask.
>>
>> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>>
>> Hmm...
>
> Don't look closely! :P

Same physical person acting on behalf of two different legal
entities right? Seems OK to me.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox series

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 84e5a9df2344..ed81d9a6316f 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -511,6 +511,28 @@  void of_mm_gpiochip_remove(struct of_mm_gpio_chip *mm_gc)
 }
 EXPORT_SYMBOL(of_mm_gpiochip_remove);
 
+static void of_gpiochip_init_valid_mask(struct gpio_chip *chip)
+{
+	int len, i;
+	u32 start, count;
+	struct device_node *np = chip->of_node;
+
+	len = of_property_count_u32_elems(np,  "gpio-reserved-ranges");
+	if (len < 0 || len % 2 != 0)
+		return;
+
+	for (i = 0; i < len; i += 2) {
+		of_property_read_u32_index(np, "gpio-reserved-ranges",
+					   i, &start);
+		of_property_read_u32_index(np, "gpio-reserved-ranges",
+					   i + 1, &count);
+		if (start >= chip->ngpio || start + count >= chip->ngpio)
+			continue;
+
+		bitmap_clear(chip->valid_mask, start, count);
+	}
+};
+
 #ifdef CONFIG_PINCTRL
 static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
 {
@@ -615,6 +637,8 @@  int of_gpiochip_add(struct gpio_chip *chip)
 	if (chip->of_gpio_n_cells > MAX_PHANDLE_ARGS)
 		return -EINVAL;
 
+	of_gpiochip_init_valid_mask(chip);
+
 	status = of_gpiochip_add_pin_range(chip);
 	if (status)
 		return status;
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d66de67ef307..5861984774ac 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -337,6 +337,47 @@  static int gpiochip_set_desc_names(struct gpio_chip *gc)
 	return 0;
 }
 
+static int gpiochip_init_valid_mask(struct gpio_chip *gpiochip)
+{
+#ifdef CONFIG_OF_GPIO
+	int size;
+	struct device_node *np = gpiochip->of_node;
+
+	size = of_property_count_u32_elems(np,  "gpio-reserved-ranges");
+	if (size > 0 && size % 2 == 0)
+		gpiochip->need_valid_mask = true;
+#endif
+
+	if (!gpiochip->need_valid_mask)
+		return 0;
+
+	gpiochip->valid_mask = kcalloc(BITS_TO_LONGS(gpiochip->ngpio),
+				       sizeof(long), GFP_KERNEL);
+	if (!gpiochip->valid_mask)
+		return -ENOMEM;
+
+	/* Assume by default all GPIOs are valid */
+	bitmap_fill(gpiochip->valid_mask, gpiochip->ngpio);
+
+	return 0;
+}
+
+static void gpiochip_free_valid_mask(struct gpio_chip *gpiochip)
+{
+	kfree(gpiochip->valid_mask);
+	gpiochip->valid_mask = NULL;
+}
+
+bool gpiochip_line_is_valid(const struct gpio_chip *gpiochip,
+				unsigned int offset)
+{
+	/* No mask means all valid */
+	if (likely(!gpiochip->valid_mask))
+		return true;
+	return test_bit(offset, gpiochip->valid_mask);
+}
+EXPORT_SYMBOL_GPL(gpiochip_line_is_valid);
+
 /*
  * GPIO line handle management
  */
@@ -1261,6 +1302,10 @@  int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
 	if (status)
 		goto err_remove_from_list;
 
+	status = gpiochip_init_valid_mask(chip);
+	if (status)
+		goto err_remove_irqchip_mask;
+
 	status = gpiochip_add_irqchip(chip, lock_key, request_key);
 	if (status)
 		goto err_remove_chip;
@@ -1290,6 +1335,8 @@  int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
 	acpi_gpiochip_remove(chip);
 	gpiochip_free_hogs(chip);
 	of_gpiochip_remove(chip);
+	gpiochip_free_valid_mask(chip);
+err_remove_irqchip_mask:
 	gpiochip_irqchip_free_valid_mask(chip);
 err_remove_from_list:
 	spin_lock_irqsave(&gpio_lock, flags);
@@ -1346,6 +1393,7 @@  void gpiochip_remove(struct gpio_chip *chip)
 	acpi_gpiochip_remove(chip);
 	gpiochip_remove_pin_ranges(chip);
 	of_gpiochip_remove(chip);
+	gpiochip_free_valid_mask(chip);
 	/*
 	 * We accept no more calls into the driver from this point, so
 	 * NULL the driver data pointer
@@ -1526,6 +1574,8 @@  static void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gpiochip)
 bool gpiochip_irqchip_irq_valid(const struct gpio_chip *gpiochip,
 				unsigned int offset)
 {
+	if (!gpiochip_line_is_valid(gpiochip, offset))
+		return false;
 	/* No mask means all valid */
 	if (likely(!gpiochip->irq.valid_mask))
 		return true;
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 1ba9a331ec51..5382b5183b7e 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -288,6 +288,21 @@  struct gpio_chip {
 	struct gpio_irq_chip irq;
 #endif
 
+	/**
+	 * @need_valid_mask:
+	 *
+	 * If set core allocates @valid_mask with all bits set to one.
+	 */
+	bool need_valid_mask;
+
+	/**
+	 * @valid_mask:
+	 *
+	 * If not %NULL holds bitmask of GPIOs which are valid to be used
+	 * from the chip.
+	 */
+	unsigned long *valid_mask;
+
 #if defined(CONFIG_OF_GPIO)
 	/*
 	 * If CONFIG_OF is enabled, then all GPIO controllers described in the
@@ -384,6 +399,7 @@  bool gpiochip_line_is_open_source(struct gpio_chip *chip, unsigned int offset);
 
 /* Sleep persistence inquiry for drivers */
 bool gpiochip_line_is_persistent(struct gpio_chip *chip, unsigned int offset);
+bool gpiochip_line_is_valid(const struct gpio_chip *chip, unsigned int offset);
 
 /* get driver data */
 void *gpiochip_get_data(struct gpio_chip *chip);