[PATCH v2 0/3] Support qcom pinctrl protected pins

[PATCH v2 0/3] Support qcom pinctrl protected pins

[Stephen Boyd]

This patchset proposes a solution to describing the valid
pins for a pin controller in a semi-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 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 (3):
  dt-bindings: pinctrl: Add a reserved-gpio-ranges property
  gpiolib-of: Support 'reserved-gpio-ranges' property
  pinctrl: qcom: Don't allow protected pins to be requested

 Documentation/devicetree/bindings/gpio/gpio.txt |  7 +--
 drivers/gpio/gpiolib-of.c                       | 28 ++++++++++
 drivers/gpio/gpiolib.c                          |  9 ++++
 drivers/pinctrl/qcom/pinctrl-msm.c              | 69 +++++++++++++++++++++++--
 4 files changed, 106 insertions(+), 7 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 1/3] dt-bindings: pinctrl: Add a reserved-gpio-ranges property

[Stephen Boyd]

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>
---
 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..c22b56680fc8 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 reserved-gpio-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>;
+	reserved-gpio-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",
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 2/3] gpiolib-of: Support 'reserved-gpio-ranges' property

[Stephen Boyd]

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.

For now, we plumb this into the gpiochip irq APIs so that
GPIO/pinctrl drivers can use the gpiochip_irqchip_irq_valid() to
test validity of GPIOs.

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

Or this can move into a dedicated API and not be tied to the irq code.
Something like of_gpiochip_init_valid_mask?

 drivers/gpio/gpiolib-of.c | 28 ++++++++++++++++++++++++++++
 drivers/gpio/gpiolib.c    |  9 +++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 564bb7a31da4..194b3306ef74 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -498,6 +498,32 @@ void of_mm_gpiochip_remove(struct of_mm_gpio_chip *mm_gc)
 }
 EXPORT_SYMBOL(of_mm_gpiochip_remove);
 
+#ifdef CONFIG_GPIOLIB_IRQCHIP
+static void of_gpiochip_init_irq_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,  "reserved-gpio-ranges");
+	if (len < 0 || len % 2 != 0)
+		return;
+
+	for (i = 0; i < len; i += 2) {
+		of_property_read_u32_index(np, "reserved-gpio-ranges",
+					   i, &start);
+		of_property_read_u32_index(np, "reserved-gpio-ranges",
+					   i + 1, &count);
+		if (start >= chip->ngpio || start + count >= chip->ngpio)
+			continue;
+
+		bitmap_clear(chip->irq.valid_mask, start, count);
+	}
+};
+#else
+static void of_gpiochip_init_irq_valid_mask(struct gpio_chip *chip) { }
+#endif
+
 #ifdef CONFIG_PINCTRL
 static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
 {
@@ -602,6 +628,8 @@ int of_gpiochip_add(struct gpio_chip *chip)
 	if (chip->of_gpio_n_cells > MAX_PHANDLE_ARGS)
 		return -EINVAL;
 
+	of_gpiochip_init_irq_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 930676ec9847..8483850463e6 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1483,6 +1483,15 @@ static struct gpio_chip *find_chip_by_name(const char *name)
 
 static int gpiochip_irqchip_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,  "reserved-gpio-ranges");
+	if (size > 0 && size % 2 == 0)
+		gpiochip->irq.need_valid_mask = true;
+#endif
+
 	if (!gpiochip->irq.need_valid_mask)
 		return 0;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 3/3] pinctrl: qcom: Don't allow protected pins to be requested

[Stephen Boyd]

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>
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 69 +++++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 495432f3341b..c7901def5f2c 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -105,6 +105,17 @@ 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;
+
+	if (gpiochip_irqchip_irq_valid(chip, offset))
+		return 0;
+
+	return -EINVAL;
+}
+
 static int msm_get_functions_count(struct pinctrl_dev *pctldev)
 {
 	struct msm_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
@@ -166,6 +177,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 +518,9 @@ static void msm_gpio_dbg_show_one(struct seq_file *s,
 		"pull up"
 	};
 
+	if (!gpiochip_irqchip_irq_valid(chip, offset))
+		return;
+
 	g = &pctrl->soc->groups[offset];
 	ctl_reg = readl(pctrl->regs + g->ctl_reg);
 
@@ -516,7 +531,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_printf(s, " %s\n", pulls[pull]);
 }
 
 static void msm_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
@@ -524,10 +539,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 +821,46 @@ static void msm_gpio_irq_handler(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
+static int msm_gpio_init_irq_valid_mask(struct gpio_chip *chip,
+					struct msm_pinctrl *pctrl)
+{
+	int ret;
+	unsigned int len, i;
+	unsigned int max_gpios = pctrl->soc->ngpios;
+
+	/* The number of GPIOs in the ACPI tables */
+	ret = device_property_read_u16_array(pctrl->dev, "gpios", NULL, 0);
+	if (ret > 0 && ret < max_gpios) {
+		u16 *tmp;
+
+		len = ret;
+		tmp = kmalloc_array(len, sizeof(tmp[0]), 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");
+			kfree(tmp);
+			return ret;
+		}
+
+		bitmap_zero(chip->irq.valid_mask, max_gpios);
+		for (i = 0; i < len; i++)
+			set_bit(tmp[i], chip->irq.valid_mask);
+
+		return 0;
+	}
+
+	return 0;
+}
+
+static bool msm_gpio_needs_irq_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 +877,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->irq.need_valid_mask = msm_gpio_needs_irq_valid_mask(pctrl);
 
 	ret = gpiochip_add_data(&pctrl->chip, pctrl);
 	if (ret) {
@@ -831,6 +885,13 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 		return ret;
 	}
 
+	ret = msm_gpio_init_irq_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");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 1/3] dt-bindings: pinctrl: Add a reserved-gpio-ranges property

[Andy Shevchenko]

On Thu, 2018-01-25 at 17:13 -0800, Stephen Boyd wrote:
> 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.

>  	gpio-controller;
>  	#gpio-cells = <2>;
>  	ngpios = <18>;
> +	reserved-gpio-ranges = <0 4>, <12 2>;

What about preserving namespace, i.e.

gpio-reserved-ranges vs. your variant?

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

Re: [PATCH v2 2/3] gpiolib-of: Support 'reserved-gpio-ranges' property

[Andy Shevchenko]

On Thu, 2018-01-25 at 17:13 -0800, Stephen Boyd wrote:
> 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.
> 
> For now, we plumb this into the gpiochip irq APIs so that
> GPIO/pinctrl drivers can use the gpiochip_irqchip_irq_valid() to
> test validity of GPIOs.


> +static void of_gpiochip_init_irq_valid_mask(struct gpio_chip *chip)
> +{

> +	int len, i;
> +	u32 start, count;
> +	struct device_node *np = chip->of_node;

Perhaps reversed tree style? (In the following function as well)

> +	len = of_property_count_u32_elems(np,  "reserved-gpio-
> ranges");
> 

> +	for (i = 0; i < len; i += 2) {
> +		of_property_read_u32_index(np, "reserved-gpio-
> ranges",
> +					   i, &start);
> +		of_property_read_u32_index(np, "reserved-gpio-
> ranges",
> +					   i + 1, &count);

of_find_property() + of_prop_next_u32() ?

> +	if (size > 0 && size % 2 == 0)
> +		gpiochip->irq.need_valid_mask = true;

 ffs(size) >= 2 ?


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

Re: [PATCH v2 2/3] gpiolib-of: Support 'reserved-gpio-ranges' property

[Linus Walleij]

Hi Stephen,

nice work!

On Fri, Jan 26, 2018 at 2:13 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:

> For now, we plumb this into the gpiochip irq APIs so that
> GPIO/pinctrl drivers can use the gpiochip_irqchip_irq_valid() to
> test validity of GPIOs.

But is that the right thing to do, given that we just took the
trouble to define a DT binding that is explicitly about
any GPIO, not just IRQ capable ones?

I am worries that the *irq* infix etc on these functions
will be a bit confusing.

Is it a lot of work to make it just generic and maybe bake it
into the gpio_chip so as to refuse already in
gpiod_request_commit() in gpiolib already?

Yours,
Linus Walleij

Re: [PATCH v2 0/3] Support qcom pinctrl protected pins

[Timur Tabi]

On 01/25/2018 07:13 PM, Stephen Boyd wrote:
> This patchset proposes a solution to describing the valid
> pins for a pin controller in a semi-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.

Any progress on this patch set?  Stephen no longer works for Qualcomm, 
so I don't know what the next step is, and I really want this feature in 
4.17 (we've missed so many merge windows already).

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

Re: [PATCH v2 0/3] Support qcom pinctrl protected pins

[Linus Walleij]

On Tue, Feb 20, 2018 at 5:45 PM, Timur Tabi <timur@codeaurora.org> wrote:
> On 01/25/2018 07:13 PM, Stephen Boyd wrote:
>>
>> This patchset proposes a solution to describing the valid
>> pins for a pin controller in a semi-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.
>
> Any progress on this patch set?  Stephen no longer works for Qualcomm, so I
> don't know what the next step is, and I really want this feature in 4.17
> (we've missed so many merge windows already).

I depend on Bjorn as maintainer of the pin control driver to ACK
the solution he likes.

Yours,
Linus Walleij

Re: [PATCH v2 1/3] dt-bindings: pinctrl: Add a reserved-gpio-ranges property

[Bjorn Andersson]

On Thu 25 Jan 17:13 PST 2018, Stephen Boyd wrote:

+ Rob

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

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  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..c22b56680fc8 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 reserved-gpio-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>;
> +	reserved-gpio-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",
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [PATCH v2 0/3] Support qcom pinctrl protected pins

[Bjorn Andersson]

On Fri 23 Feb 06:22 PST 2018, Linus Walleij wrote:

> On Tue, Feb 20, 2018 at 5:45 PM, Timur Tabi <timur@codeaurora.org> wrote:
> > On 01/25/2018 07:13 PM, Stephen Boyd wrote:
> >>
> >> This patchset proposes a solution to describing the valid
> >> pins for a pin controller in a semi-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.
> >
> > Any progress on this patch set?  Stephen no longer works for Qualcomm, so I
> > don't know what the next step is, and I really want this feature in 4.17
> > (we've missed so many merge windows already).
> 
> I depend on Bjorn as maintainer of the pin control driver to ACK
> the solution he likes.
> 

I haven't found the time to review the reuse of the irq valid mask or
the effort needed to replace this, other than that I think the series
looks good.

Regards,
Bjorn

Re: [PATCH v2 0/3] Support qcom pinctrl protected pins

[Timur Tabi]

On 02/23/2018 10:54 AM, Bjorn Andersson wrote:
> I haven't found the time to review the reuse of the irq valid mask or
> the effort needed to replace this, other than that I think the series
> looks good.

So is that an ACK?

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

Re: [PATCH v2 2/3] gpiolib-of: Support 'reserved-gpio-ranges' property

[Stephen Boyd]

Quoting Linus Walleij (2018-02-07 05:34:19)
> Hi Stephen,
> 
> nice work!
> 
> On Fri, Jan 26, 2018 at 2:13 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> 
> > For now, we plumb this into the gpiochip irq APIs so that
> > GPIO/pinctrl drivers can use the gpiochip_irqchip_irq_valid() to
> > test validity of GPIOs.
> 
> But is that the right thing to do, given that we just took the
> trouble to define a DT binding that is explicitly about
> any GPIO, not just IRQ capable ones?
> 
> I am worries that the *irq* infix etc on these functions
> will be a bit confusing.
> 
> Is it a lot of work to make it just generic and maybe bake it
> into the gpio_chip so as to refuse already in
> gpiod_request_commit() in gpiolib already?

I don't think that it will be too much work to tweak the code to treat
these as gpios instead of irq lines. It may end up duplicating a bit of code
that the irq line stuff is already doing, but I'll take a stab at it and
see how bad it comes out.

Re: [PATCH v2 1/3] dt-bindings: pinctrl: Add a reserved-gpio-ranges property

[Linus Walleij]

Hi Stephen,

thanks for the patch and hope you have a good time at your new
workplace!

On Fri, Jan 26, 2018 at 2:13 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:

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

This looks fine except Andy's note to rename this ranges
to gpio-reserved-ranges for namespacing.

Are you reposting this series as v3 with this fixed or does someone
else need to pick it up?

Yours,
Linus Walleij

Re: [PATCH v2 1/3] dt-bindings: pinctrl: Add a reserved-gpio-ranges property

[Timur Tabi]

On 3/19/18 10:36 PM, Linus Walleij wrote:
> This looks fine except Andy's note to rename this ranges
> to gpio-reserved-ranges for namespacing.
> 
> Are you reposting this series as v3 with this fixed or does someone
> else need to pick it up?

I will pick this up if Stephen wants me to.

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

Re: [PATCH v2 1/3] dt-bindings: pinctrl: Add a reserved-gpio-ranges property

[Stephen Boyd]

Quoting Linus Walleij (2018-03-19 20:36:35)
> Hi Stephen,
> 
> thanks for the patch and hope you have a good time at your new
> workplace!
> 
> On Fri, Jan 26, 2018 at 2:13 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> 
> > 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>
> 
> This looks fine except Andy's note to rename this ranges
> to gpio-reserved-ranges for namespacing.

Ok! I'll rename to gpio-reserved-ranges.

> 
> Are you reposting this series as v3 with this fixed or does someone
> else need to pick it up?

Yes I can do it today or tomorrow.