linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/5] Support qcom pinctrl protected pins
@ 2018-03-23 16:34 Stephen Boyd
  2018-03-23 16:34 ` [PATCH v4 1/5] dt-bindings: gpio: Add a gpio-reserved-ranges property Stephen Boyd
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Stephen Boyd @ 2018-03-23 16:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-kernel, linux-arm-kernel, devicetree, linux-arm-msm,
	Timur Tabi, Bjorn Andersson, Grant Likely, linux-gpio,
	Andy Shevchenko

This patchset proposes a solution to describing the valid                                                                           
pins for a pin controller in a generic way so that qcom                                                                        
platforms can expose the pins that are really available.                                                                            
                                                                                                                                    
Typically, this has been done by having drivers and firmware                                                                        
descriptions only use pins they know they have access to, and that                                                                  
still works now because we no longer read the pin direction at                                                                      
boot. But there are still some userspace drivers and debugfs facilities                                                             
that don't know what pins are available and attempt to read everything                                                              
they can. On qcom platforms, this may lead to a system hang, which isn't                                                            
very nice behavior, even if root is the only user that can trigger it.                                                              
                                                                                                                                    
The proposal is to describe the valid pins and then not allow things to                                                             
cause problems by using the invalid pins. Obviously, the firmware may                                                               
mess this up, so this is mostly a nice to have feature or a safety net                                                              
so that things don't blow up easily.                                                                                                

Changes from v3:
 * Split out allocation of mask into subroutine
 * Moved that allocation to kmalloc_array()
 * Updated qcom driver to simplifiy ACPI logic and fix mem leak

Changes from v2:
 * Renamed binding to 'gpio-reserved-ranges'
 * Reworked gpiolib patch to not use irqdomains
 * Update qcom driver patch to use new valid_mask field
                                                                                                                                    
Changes from v1:                                                                                                                    
 * Pushed into gpiolib-of core under irq valid line logic                                                                           
 * Fixed up qcom driver patch to free stuff properly and remove custom code                                                         
 * Dropped export patch as it got picked up                                                                                         
 * Renamed binding to 'reserved-gpio-ranges'   


Stephen Boyd (5):
  dt-bindings: gpio: Add a gpio-reserved-ranges property
  gpiolib: Extract mask allocation into subroutine
  gpiolib: Change bitmap allocation to kmalloc_array
  gpiolib: Support 'gpio-reserved-ranges' property
  pinctrl: qcom: Don't allow protected pins to be requested

 .../devicetree/bindings/gpio/gpio.txt         |  7 +-
 drivers/gpio/gpiolib-of.c                     | 24 +++++++
 drivers/gpio/gpiolib.c                        | 66 +++++++++++++++++--
 drivers/pinctrl/qcom/pinctrl-msm.c            | 65 +++++++++++++++++-
 include/linux/gpio/driver.h                   | 16 +++++
 5 files changed, 167 insertions(+), 11 deletions(-)

-- 
Sent by a computer through tubes

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/5] dt-bindings: gpio: Add a gpio-reserved-ranges property
  2018-03-23 16:34 [PATCH v4 0/5] Support qcom pinctrl protected pins Stephen Boyd
@ 2018-03-23 16:34 ` Stephen Boyd
  2018-03-26 22:25   ` Rob Herring
  2018-03-23 16:34 ` [PATCH v4 2/5] gpiolib: Extract mask allocation into subroutine Stephen Boyd
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 10+ messages in thread
From: Stephen Boyd @ 2018-03-23 16:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Stephen Boyd, linux-kernel, linux-arm-kernel, devicetree,
	linux-arm-msm, Timur Tabi, Bjorn Andersson, Grant Likely,
	linux-gpio, Andy Shevchenko

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.
Introduce 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.

Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 Documentation/devicetree/bindings/gpio/gpio.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
index b5de08e3b1a2..a7c31de29362 100644
--- a/Documentation/devicetree/bindings/gpio/gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio.txt
@@ -151,9 +151,9 @@ in a lot of designs, some using all 32 bits, some using 18 and some using
 first 18 GPIOs, at local offset 0 .. 17, are in use.
 
 If these GPIOs do not happen to be the first N GPIOs at offset 0...N-1, an
-additional bitmask is needed to specify which GPIOs are actually in use,
-and which are dummies. The bindings for this case has not yet been
-specified, but should be specified if/when such hardware appears.
+additional set of tuples is needed to specify which GPIOs are unusable, with
+the gpio-reserved-ranges binding. This property indicates the start and size
+of the GPIOs that can't be used.
 
 Optionally, a GPIO controller may have a "gpio-line-names" property. This is
 an array of strings defining the names of the GPIO lines going out of the
@@ -178,6 +178,7 @@ gpio-controller@00000000 {
 	gpio-controller;
 	#gpio-cells = <2>;
 	ngpios = <18>;
+	gpio-reserved-ranges = <0 4>, <12 2>;
 	gpio-line-names = "MMC-CD", "MMC-WP", "VDD eth", "RST eth", "LED R",
 		"LED G", "LED B", "Col A", "Col B", "Col C", "Col D",
 		"Row A", "Row B", "Row C", "Row D", "NMI button",
-- 
Sent by a computer through tubes

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/5] gpiolib: Extract mask allocation into subroutine
  2018-03-23 16:34 [PATCH v4 0/5] Support qcom pinctrl protected pins Stephen Boyd
  2018-03-23 16:34 ` [PATCH v4 1/5] dt-bindings: gpio: Add a gpio-reserved-ranges property Stephen Boyd
@ 2018-03-23 16:34 ` Stephen Boyd
  2018-03-23 16:34 ` [PATCH v4 3/5] gpiolib: Change bitmap allocation to kmalloc_array Stephen Boyd
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2018-03-23 16:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-kernel, linux-arm-kernel, devicetree, linux-arm-msm,
	Timur Tabi, Bjorn Andersson, Grant Likely, linux-gpio,
	Andy Shevchenko

We're going to use similar code to allocate and set all the bits in a
mask for valid gpios to use. Extract the code from the irqchip version
so it can be reused.

Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpio/gpiolib.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d66de67ef307..cc0e1519da45 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -337,6 +337,20 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc)
 	return 0;
 }
 
+static unsigned long *gpiochip_allocate_mask(struct gpio_chip *chip)
+{
+	unsigned long *p;
+
+	p = kcalloc(BITS_TO_LONGS(chip->ngpio), sizeof(long), GFP_KERNEL);
+	if (!p)
+		return NULL;
+
+	/* Assume by default all GPIOs are valid */
+	bitmap_fill(p, chip->ngpio);
+
+	return p;
+}
+
 /*
  * GPIO line handle management
  */
@@ -1506,14 +1520,10 @@ static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gpiochip)
 	if (!gpiochip->irq.need_valid_mask)
 		return 0;
 
-	gpiochip->irq.valid_mask = kcalloc(BITS_TO_LONGS(gpiochip->ngpio),
-					   sizeof(long), GFP_KERNEL);
+	gpiochip->irq.valid_mask = gpiochip_allocate_mask(gpiochip);
 	if (!gpiochip->irq.valid_mask)
 		return -ENOMEM;
 
-	/* Assume by default all GPIOs are valid */
-	bitmap_fill(gpiochip->irq.valid_mask, gpiochip->ngpio);
-
 	return 0;
 }
 
-- 
Sent by a computer through tubes

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/5] gpiolib: Change bitmap allocation to kmalloc_array
  2018-03-23 16:34 [PATCH v4 0/5] Support qcom pinctrl protected pins Stephen Boyd
  2018-03-23 16:34 ` [PATCH v4 1/5] dt-bindings: gpio: Add a gpio-reserved-ranges property Stephen Boyd
  2018-03-23 16:34 ` [PATCH v4 2/5] gpiolib: Extract mask allocation into subroutine Stephen Boyd
@ 2018-03-23 16:34 ` Stephen Boyd
  2018-03-23 16:34 ` [PATCH v4 4/5] gpiolib: Support 'gpio-reserved-ranges' property Stephen Boyd
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2018-03-23 16:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-kernel, linux-arm-kernel, devicetree, linux-arm-msm,
	Timur Tabi, Bjorn Andersson, Grant Likely, linux-gpio,
	Andy Shevchenko

We don't need to clear out these bits when we set them immediately
after. Use kmalloc_array() to skip clearing the bits.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpio/gpiolib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index cc0e1519da45..db3788d17ba0 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -341,7 +341,7 @@ static unsigned long *gpiochip_allocate_mask(struct gpio_chip *chip)
 {
 	unsigned long *p;
 
-	p = kcalloc(BITS_TO_LONGS(chip->ngpio), sizeof(long), GFP_KERNEL);
+	p = kmalloc_array(BITS_TO_LONGS(chip->ngpio), sizeof(*p), GFP_KERNEL);
 	if (!p)
 		return NULL;
 
-- 
Sent by a computer through tubes

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 4/5] gpiolib: Support 'gpio-reserved-ranges' property
  2018-03-23 16:34 [PATCH v4 0/5] Support qcom pinctrl protected pins Stephen Boyd
                   ` (2 preceding siblings ...)
  2018-03-23 16:34 ` [PATCH v4 3/5] gpiolib: Change bitmap allocation to kmalloc_array Stephen Boyd
@ 2018-03-23 16:34 ` Stephen Boyd
  2018-03-23 16:34 ` [PATCH v4 5/5] pinctrl: qcom: Don't allow protected pins to be requested Stephen Boyd
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2018-03-23 16:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Stephen Boyd, linux-kernel, linux-arm-kernel, devicetree,
	linux-arm-msm, Timur Tabi, Bjorn Andersson, Grant Likely,
	linux-gpio, Andy Shevchenko

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      | 46 +++++++++++++++++++++++++++++++++++++
 include/linux/gpio/driver.h | 16 +++++++++++++
 3 files changed, 86 insertions(+)

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 db3788d17ba0..fecbb553e8a4 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -351,6 +351,43 @@ static unsigned long *gpiochip_allocate_mask(struct gpio_chip *chip)
 	return p;
 }
 
+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 = gpiochip_allocate_mask(gpiochip);
+	if (!gpiochip->valid_mask)
+		return -ENOMEM;
+
+	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
  */
@@ -1275,6 +1312,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;
@@ -1304,6 +1345,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);
@@ -1360,6 +1403,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
@@ -1536,6 +1580,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);
-- 
Sent by a computer through tubes

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 5/5] pinctrl: qcom: Don't allow protected pins to be requested
  2018-03-23 16:34 [PATCH v4 0/5] Support qcom pinctrl protected pins Stephen Boyd
                   ` (3 preceding siblings ...)
  2018-03-23 16:34 ` [PATCH v4 4/5] gpiolib: Support 'gpio-reserved-ranges' property Stephen Boyd
@ 2018-03-23 16:34 ` Stephen Boyd
  2018-03-23 23:50 ` [PATCH v4 0/5] Support qcom pinctrl protected pins Timur Tabi
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2018-03-23 16:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Stephen Boyd, linux-kernel, linux-arm-kernel, devicetree,
	linux-arm-msm, Timur Tabi, Bjorn Andersson, Grant Likely,
	linux-gpio, Andy Shevchenko

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 and
reset the device. With a DT/ACPI property to describe the set of
pins that are available for use, parse the available pins and set
the irq valid bits for gpiolib to know what to consider 'valid'.
This should avoid any issues with gpiolib. Furthermore, implement
the pinmux_ops::request function so that pinmux can also make
sure to not use pins that are unavailable.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 65 ++++++++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 495432f3341b..e7abc8ba222b 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -105,6 +105,14 @@ static const struct pinctrl_ops msm_pinctrl_ops = {
 	.dt_free_map		= pinctrl_utils_free_map,
 };
 
+static int msm_pinmux_request(struct pinctrl_dev *pctldev, unsigned offset)
+{
+	struct msm_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	struct gpio_chip *chip = &pctrl->chip;
+
+	return gpiochip_line_is_valid(chip, offset) ? 0 : -EINVAL;
+}
+
 static int msm_get_functions_count(struct pinctrl_dev *pctldev)
 {
 	struct msm_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
@@ -166,6 +174,7 @@ static int msm_pinmux_set_mux(struct pinctrl_dev *pctldev,
 }
 
 static const struct pinmux_ops msm_pinmux_ops = {
+	.request		= msm_pinmux_request,
 	.get_functions_count	= msm_get_functions_count,
 	.get_function_name	= msm_get_function_name,
 	.get_function_groups	= msm_get_function_groups,
@@ -506,6 +515,9 @@ static void msm_gpio_dbg_show_one(struct seq_file *s,
 		"pull up"
 	};
 
+	if (!gpiochip_line_is_valid(chip, offset))
+		return;
+
 	g = &pctrl->soc->groups[offset];
 	ctl_reg = readl(pctrl->regs + g->ctl_reg);
 
@@ -517,6 +529,7 @@ static void msm_gpio_dbg_show_one(struct seq_file *s,
 	seq_printf(s, " %-8s: %-3s %d", g->name, is_out ? "out" : "in", func);
 	seq_printf(s, " %dmA", msm_regval_to_drive(drive));
 	seq_printf(s, " %s", pulls[pull]);
+	seq_puts(s, "\n");
 }
 
 static void msm_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
@@ -524,10 +537,8 @@ static void msm_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 	unsigned gpio = chip->base;
 	unsigned i;
 
-	for (i = 0; i < chip->ngpio; i++, gpio++) {
+	for (i = 0; i < chip->ngpio; i++, gpio++)
 		msm_gpio_dbg_show_one(s, NULL, chip, i, gpio);
-		seq_puts(s, "\n");
-	}
 }
 
 #else
@@ -808,6 +819,46 @@ static void msm_gpio_irq_handler(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
+static int msm_gpio_init_valid_mask(struct gpio_chip *chip,
+				    struct msm_pinctrl *pctrl)
+{
+	int ret;
+	unsigned int len, i;
+	unsigned int max_gpios = pctrl->soc->ngpios;
+	u16 *tmp;
+
+	/* The number of GPIOs in the ACPI tables */
+	len = ret = device_property_read_u16_array(pctrl->dev, "gpios", NULL, 0);
+	if (ret < 0)
+		return 0;
+
+	if (ret > max_gpios)
+		return -EINVAL;
+
+	tmp = kmalloc_array(len, sizeof(*tmp), GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	ret = device_property_read_u16_array(pctrl->dev, "gpios", tmp, len);
+	if (ret < 0) {
+		dev_err(pctrl->dev, "could not read list of GPIOs\n");
+		goto out;
+	}
+
+	bitmap_zero(chip->valid_mask, max_gpios);
+	for (i = 0; i < len; i++)
+		set_bit(tmp[i], chip->valid_mask);
+
+out:
+	kfree(tmp);
+	return ret;
+}
+
+static bool msm_gpio_needs_valid_mask(struct msm_pinctrl *pctrl)
+{
+	return device_property_read_u16_array(pctrl->dev, "gpios", NULL, 0) > 0;
+}
+
 static int msm_gpio_init(struct msm_pinctrl *pctrl)
 {
 	struct gpio_chip *chip;
@@ -824,6 +875,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 	chip->parent = pctrl->dev;
 	chip->owner = THIS_MODULE;
 	chip->of_node = pctrl->dev->of_node;
+	chip->need_valid_mask = msm_gpio_needs_valid_mask(pctrl);
 
 	ret = gpiochip_add_data(&pctrl->chip, pctrl);
 	if (ret) {
@@ -831,6 +883,13 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 		return ret;
 	}
 
+	ret = msm_gpio_init_valid_mask(chip, pctrl);
+	if (ret) {
+		dev_err(pctrl->dev, "Failed to setup irq valid bits\n");
+		gpiochip_remove(&pctrl->chip);
+		return ret;
+	}
+
 	ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev), 0, 0, chip->ngpio);
 	if (ret) {
 		dev_err(pctrl->dev, "Failed to add pin range\n");
-- 
Sent by a computer through tubes

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/5] Support qcom pinctrl protected pins
  2018-03-23 16:34 [PATCH v4 0/5] Support qcom pinctrl protected pins Stephen Boyd
                   ` (4 preceding siblings ...)
  2018-03-23 16:34 ` [PATCH v4 5/5] pinctrl: qcom: Don't allow protected pins to be requested Stephen Boyd
@ 2018-03-23 23:50 ` Timur Tabi
  2018-03-26  8:39 ` Andy Shevchenko
  2018-03-27 13:35 ` Linus Walleij
  7 siblings, 0 replies; 10+ messages in thread
From: Timur Tabi @ 2018-03-23 23:50 UTC (permalink / raw)
  To: Stephen Boyd, Linus Walleij
  Cc: linux-kernel, linux-arm-kernel, devicetree, linux-arm-msm,
	Bjorn Andersson, Grant Likely, linux-gpio, Andy Shevchenko

On 03/23/2018 11:34 AM, Stephen Boyd wrote:
> Stephen Boyd (5):
>    dt-bindings: gpio: Add a gpio-reserved-ranges property
>    gpiolib: Extract mask allocation into subroutine
>    gpiolib: Change bitmap allocation to kmalloc_array
>    gpiolib: Support 'gpio-reserved-ranges' property
>    pinctrl: qcom: Don't allow protected pins to be requested

ACPI parts:

Tested-by: Timur Tabi <timur@codeaurora.org>

I posted a pair of patches that should be applied on top of yours.  The 
first one fixed pinctrl-msm when there is more than one TLMM device. 
The second adds support for my SOC.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/5] Support qcom pinctrl protected pins
  2018-03-23 16:34 [PATCH v4 0/5] Support qcom pinctrl protected pins Stephen Boyd
                   ` (5 preceding siblings ...)
  2018-03-23 23:50 ` [PATCH v4 0/5] Support qcom pinctrl protected pins Timur Tabi
@ 2018-03-26  8:39 ` Andy Shevchenko
  2018-03-27 13:35 ` Linus Walleij
  7 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2018-03-26  8:39 UTC (permalink / raw)
  To: Stephen Boyd, Linus Walleij
  Cc: linux-kernel, linux-arm-kernel, devicetree, linux-arm-msm,
	Timur Tabi, Bjorn Andersson, Grant Likely, linux-gpio

On Fri, 2018-03-23 at 09:34 -0700, Stephen Boyd wrote:
> This patchset proposes a solution to describing the
> valid                                                                 
>           
> pins for a pin controller in a generic way so that
> qcom                                                                  
>       
> platforms can expose the pins that are really
> available.                                                            
>                 
>                                                                       
>                                                               
> Typically, this has been done by having drivers and
> firmware                                                              
>           
> descriptions only use pins they know they have access to, and
> that                                                                  
> still works now because we no longer read the pin direction
> at                                                                    
>   
> boot. But there are still some userspace drivers and debugfs
> facilities                                                            
>  
> that don't know what pins are available and attempt to read
> everything                                                            
>   
> they can. On qcom platforms, this may lead to a system hang, which
> isn't                                                            
> very nice behavior, even if root is the only user that can trigger
> it.                                                              
>                                                                       
>                                                               
> The proposal is to describe the valid pins and then not allow things
> to                                                             
> cause problems by using the invalid pins. Obviously, the firmware
> may                                                               
> mess this up, so this is mostly a nice to have feature or a safety
> net                                                              
> so that things don't blow up
> easily.                                                               

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>


> Changes from v3:
>  * Split out allocation of mask into subroutine
>  * Moved that allocation to kmalloc_array()
>  * Updated qcom driver to simplifiy ACPI logic and fix mem leak
> 
> Changes from v2:
>  * Renamed binding to 'gpio-reserved-ranges'
>  * Reworked gpiolib patch to not use irqdomains
>  * Update qcom driver patch to use new valid_mask field
>                                                                       
>                                                               
> Changes from
> v1:                                                                   
>                                                  
>  * Pushed into gpiolib-of core under irq valid line
> logic                                                                 
>           
>  * Fixed up qcom driver patch to free stuff properly and remove custom
> code                                                         
>  * Dropped export patch as it got picked
> up                                                                    
>                      
>  * Renamed binding to 'reserved-gpio-ranges'   
> 
> 
> Stephen Boyd (5):
>   dt-bindings: gpio: Add a gpio-reserved-ranges property
>   gpiolib: Extract mask allocation into subroutine
>   gpiolib: Change bitmap allocation to kmalloc_array
>   gpiolib: Support 'gpio-reserved-ranges' property
>   pinctrl: qcom: Don't allow protected pins to be requested
> 
>  .../devicetree/bindings/gpio/gpio.txt         |  7 +-
>  drivers/gpio/gpiolib-of.c                     | 24 +++++++
>  drivers/gpio/gpiolib.c                        | 66 +++++++++++++++++-
> -
>  drivers/pinctrl/qcom/pinctrl-msm.c            | 65 +++++++++++++++++-
>  include/linux/gpio/driver.h                   | 16 +++++
>  5 files changed, 167 insertions(+), 11 deletions(-)
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/5] dt-bindings: gpio: Add a gpio-reserved-ranges property
  2018-03-23 16:34 ` [PATCH v4 1/5] dt-bindings: gpio: Add a gpio-reserved-ranges property Stephen Boyd
@ 2018-03-26 22:25   ` Rob Herring
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2018-03-26 22:25 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Linus Walleij, Stephen Boyd, linux-kernel, linux-arm-kernel,
	devicetree, linux-arm-msm, Timur Tabi, Bjorn Andersson,
	Grant Likely, linux-gpio, Andy Shevchenko

On Fri, Mar 23, 2018 at 09:34:49AM -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.
> Introduce 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.
> 
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  Documentation/devicetree/bindings/gpio/gpio.txt | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/5] Support qcom pinctrl protected pins
  2018-03-23 16:34 [PATCH v4 0/5] Support qcom pinctrl protected pins Stephen Boyd
                   ` (6 preceding siblings ...)
  2018-03-26  8:39 ` Andy Shevchenko
@ 2018-03-27 13:35 ` Linus Walleij
  7 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2018-03-27 13:35 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-kernel, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, Timur Tabi, Bjorn Andersson, Grant Likely,
	open list:GPIO SUBSYSTEM, Andy Shevchenko

On Fri, Mar 23, 2018 at 5:34 PM, Stephen Boyd <swboyd@chromium.org> wrote:

> This patchset proposes a solution to describing the valid
> pins for a pin controller in a generic way so that qcom
> platforms can expose the pins that are really available.
>
> Typically, this has been done by having drivers and firmware
> descriptions only use pins they know they have access to, and that
> still works now because we no longer read the pin direction at
> boot. But there are still some userspace drivers and debugfs facilities
> that don't know what pins are available and attempt to read everything
> they can. On qcom platforms, this may lead to a system hang, which isn't
> very nice behavior, even if root is the only user that can trigger it.
>
> The proposal is to describe the valid pins and then not allow things to
> cause problems by using the invalid pins. Obviously, the firmware may
> mess this up, so this is mostly a nice to have feature or a safety net
> so that things don't blow up easily.
>
> Changes from v3:
>  * Split out allocation of mask into subroutine
>  * Moved that allocation to kmalloc_array()
>  * Updated qcom driver to simplifiy ACPI logic and fix mem leak

As both Timur and Andy are happy with this I applied these five
patches for v4.17 and pushed to the test farm. Let's see what
happens!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-03-27 13:36 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-23 16:34 [PATCH v4 0/5] Support qcom pinctrl protected pins Stephen Boyd
2018-03-23 16:34 ` [PATCH v4 1/5] dt-bindings: gpio: Add a gpio-reserved-ranges property Stephen Boyd
2018-03-26 22:25   ` Rob Herring
2018-03-23 16:34 ` [PATCH v4 2/5] gpiolib: Extract mask allocation into subroutine Stephen Boyd
2018-03-23 16:34 ` [PATCH v4 3/5] gpiolib: Change bitmap allocation to kmalloc_array Stephen Boyd
2018-03-23 16:34 ` [PATCH v4 4/5] gpiolib: Support 'gpio-reserved-ranges' property Stephen Boyd
2018-03-23 16:34 ` [PATCH v4 5/5] pinctrl: qcom: Don't allow protected pins to be requested Stephen Boyd
2018-03-23 23:50 ` [PATCH v4 0/5] Support qcom pinctrl protected pins Timur Tabi
2018-03-26  8:39 ` Andy Shevchenko
2018-03-27 13:35 ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).