[PATCH 0/2] STM32 pinctrl updates

[PATCH 0/2] STM32 pinctrl updates

[Alexandre Torgue]

Hi, 

This series includes two updates for stm32 pinctrl driver:

 -Fixes the way to map a gpio bank to an EXTi line.
 -Check "status" devicetree entry (in gpio-contriller node) before registering
  a new gpio bank.

Regards
Alex

Alexandre Torgue (2):
  pinctrl: stm32: fix bank io port number
  pinctrl: stm32: check node status before new gpio bank registering

 .../bindings/pinctrl/st,stm32-pinctrl.txt          |  2 ++
 drivers/pinctrl/stm32/pinctrl-stm32.c              | 27 +++++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH 1/2] pinctrl: stm32: fix bank io port number

[Alexandre Torgue]

In case the exti line is not in line with the bank number (that is the case
when there is an hole between two banks, for example GPIOK and then GPIOZ),
use "st,bank-ioport" DT property to get the right exti line.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>

diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
index 9a06e1f..60c678a 100644
--- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
@@ -55,6 +55,8 @@ Optional properties:
    NOTE: If "gpio-ranges" is used for a gpio controller, all gpio-controller
    have to use a "gpio-ranges" entry.
    More details in Documentation/devicetree/bindings/gpio/gpio.txt.
+ - st,bank-ioport: should correspond to the EXTI IOport selection (EXTI line
+   used to select GPIOs as interrupts).
 
 Example 1:
 #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index dfed609..eb6ae14 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -73,6 +73,7 @@ struct stm32_gpio_bank {
 	struct fwnode_handle *fwnode;
 	struct irq_domain *domain;
 	u32 bank_nr;
+	u32 bank_ioport_nr;
 };
 
 struct stm32_pinctrl {
@@ -298,7 +299,7 @@ static int stm32_gpio_domain_activate(struct irq_domain *d,
 	struct stm32_gpio_bank *bank = d->host_data;
 	struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
 
-	regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->bank_nr);
+	regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->bank_ioport_nr);
 	return 0;
 }
 
@@ -948,6 +949,7 @@ static int stm32_gpiolib_register_bank(struct stm32_pinctrl *pctl,
 	struct device_node *np)
 {
 	struct stm32_gpio_bank *bank = &pctl->banks[pctl->nbanks];
+	int bank_ioport_nr;
 	struct pinctrl_gpio_range *range = &bank->range;
 	struct of_phandle_args args;
 	struct device *dev = pctl->dev;
@@ -998,12 +1000,17 @@ static int stm32_gpiolib_register_bank(struct stm32_pinctrl *pctl,
 		pinctrl_add_gpio_range(pctl->pctl_dev,
 				       &pctl->banks[bank_nr].range);
 	}
+
+	if (of_property_read_u32(np, "st,bank-ioport", &bank_ioport_nr))
+		bank_ioport_nr = bank_nr;
+
 	bank->gpio_chip.base = bank_nr * STM32_GPIO_PINS_PER_BANK;
 
 	bank->gpio_chip.ngpio = npins;
 	bank->gpio_chip.of_node = np;
 	bank->gpio_chip.parent = dev;
 	bank->bank_nr = bank_nr;
+	bank->bank_ioport_nr = bank_ioport_nr;
 	spin_lock_init(&bank->lock);
 
 	/* create irq hierarchical domain */
-- 
2.7.4

Re: [PATCH 1/2] pinctrl: stm32: fix bank io port number

[Rob Herring]

On Mon, Jul 16, 2018 at 02:57:36PM +0200, Alexandre Torgue wrote:
> In case the exti line is not in line with the bank number (that is the case
> when there is an hole between two banks, for example GPIOK and then GPIOZ),
> use "st,bank-ioport" DT property to get the right exti line.
> 
> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
> index 9a06e1f..60c678a 100644
> --- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
> @@ -55,6 +55,8 @@ Optional properties:
>     NOTE: If "gpio-ranges" is used for a gpio controller, all gpio-controller
>     have to use a "gpio-ranges" entry.
>     More details in Documentation/devicetree/bindings/gpio/gpio.txt.
> + - st,bank-ioport: should correspond to the EXTI IOport selection (EXTI line
> +   used to select GPIOs as interrupts).

ioport sounds like something else. Perhaps '-exti-line'?

>  
>  Example 1:
>  #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>

Re: [PATCH 1/2] pinctrl: stm32: fix bank io port number

[Alexandre Torgue]

Hi Rob

On 07/20/2018 06:35 PM, Rob Herring wrote:
> On Mon, Jul 16, 2018 at 02:57:36PM +0200, Alexandre Torgue wrote:
>> In case the exti line is not in line with the bank number (that is the case
>> when there is an hole between two banks, for example GPIOK and then GPIOZ),
>> use "st,bank-ioport" DT property to get the right exti line.
>>
>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
>> index 9a06e1f..60c678a 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
>> +++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
>> @@ -55,6 +55,8 @@ Optional properties:
>>      NOTE: If "gpio-ranges" is used for a gpio controller, all gpio-controller
>>      have to use a "gpio-ranges" entry.
>>      More details in Documentation/devicetree/bindings/gpio/gpio.txt.
>> + - st,bank-ioport: should correspond to the EXTI IOport selection (EXTI line
>> +   used to select GPIOs as interrupts).
> 
> ioport sounds like something else. Perhaps '-exti-line'?

IOport is actually what is described in our datasheet. I could change it 
to -exti-line but I would prefer to match with the datasheet.

Regards
Alex
> 
>>   
>>   Example 1:
>>   #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>

Re: [PATCH 1/2] pinctrl: stm32: fix bank io port number

[Rob Herring]

On Tue, Jul 24, 2018 at 10:07 AM Alexandre Torgue
<alexandre.torgue@st.com> wrote:
>
> Hi Rob
>
> On 07/20/2018 06:35 PM, Rob Herring wrote:
> > On Mon, Jul 16, 2018 at 02:57:36PM +0200, Alexandre Torgue wrote:
> >> In case the exti line is not in line with the bank number (that is the case
> >> when there is an hole between two banks, for example GPIOK and then GPIOZ),
> >> use "st,bank-ioport" DT property to get the right exti line.
> >>
> >> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> >> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
> >>
> >> diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
> >> index 9a06e1f..60c678a 100644
> >> --- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
> >> +++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
> >> @@ -55,6 +55,8 @@ Optional properties:
> >>      NOTE: If "gpio-ranges" is used for a gpio controller, all gpio-controller
> >>      have to use a "gpio-ranges" entry.
> >>      More details in Documentation/devicetree/bindings/gpio/gpio.txt.
> >> + - st,bank-ioport: should correspond to the EXTI IOport selection (EXTI line
> >> +   used to select GPIOs as interrupts).
> >
> > ioport sounds like something else. Perhaps '-exti-line'?
>
> IOport is actually what is described in our datasheet. I could change it
> to -exti-line but I would prefer to match with the datasheet.

Okay.

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

Re: [PATCH 1/2] pinctrl: stm32: fix bank io port number

[Linus Walleij]

On Mon, Jul 16, 2018 at 2:57 PM Alexandre Torgue
<alexandre.torgue@st.com> wrote:

> In case the exti line is not in line with the bank number (that is the case
> when there is an hole between two banks, for example GPIOK and then GPIOZ),
> use "st,bank-ioport" DT property to get the right exti line.
>
> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>

Patch applied with Rob's ACK.

Yours,
Linus Walleij

[PATCH 2/2] pinctrl: stm32: check node status before new gpio bank registering

[Alexandre Torgue]

Register a new GPIO bank only if GPIO bank node is enabled. This patch also
adds checks on ranges which are defined only if a bank is registered.

Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>

diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index eb6ae14..111225e 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -639,6 +639,11 @@ static int stm32_pmx_set_mux(struct pinctrl_dev *pctldev,
 	}
 
 	range = pinctrl_find_gpio_range_from_pin(pctldev, g->pin);
+	if (!range) {
+		dev_err(pctl->dev, "No gpio range defined.\n");
+		return -EINVAL;
+	}
+
 	bank = gpiochip_get_data(range->gc);
 	pin = stm32_gpio_pin(g->pin);
 
@@ -807,11 +812,17 @@ static int stm32_pconf_parse_conf(struct pinctrl_dev *pctldev,
 		unsigned int pin, enum pin_config_param param,
 		enum pin_config_param arg)
 {
+	struct stm32_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
 	struct pinctrl_gpio_range *range;
 	struct stm32_gpio_bank *bank;
 	int offset, ret = 0;
 
 	range = pinctrl_find_gpio_range_from_pin(pctldev, pin);
+	if (!range) {
+		dev_err(pctl->dev, "No gpio range defined.\n");
+		return -EINVAL;
+	}
+
 	bank = gpiochip_get_data(range->gc);
 	offset = stm32_gpio_pin(pin);
 
@@ -893,6 +904,9 @@ static void stm32_pconf_dbg_show(struct pinctrl_dev *pctldev,
 	bool val;
 
 	range = pinctrl_find_gpio_range_from_pin_nolock(pctldev, pin);
+	if (!range)
+		return;
+
 	bank = gpiochip_get_data(range->gc);
 	offset = stm32_gpio_pin(pin);
 
@@ -1173,7 +1187,7 @@ int stm32_pctl_probe(struct platform_device *pdev)
 		return PTR_ERR(pctl->pctl_dev);
 	}
 
-	for_each_child_of_node(np, child)
+	for_each_available_child_of_node(np, child)
 		if (of_property_read_bool(child, "gpio-controller"))
 			banks++;
 
@@ -1186,7 +1200,7 @@ int stm32_pctl_probe(struct platform_device *pdev)
 	if (!pctl->banks)
 		return -ENOMEM;
 
-	for_each_child_of_node(np, child) {
+	for_each_available_child_of_node(np, child) {
 		if (of_property_read_bool(child, "gpio-controller")) {
 			ret = stm32_gpiolib_register_bank(pctl, child);
 			if (ret)
-- 
2.7.4

Re: [PATCH 2/2] pinctrl: stm32: check node status before new gpio bank registering

[Linus Walleij]

On Mon, Jul 16, 2018 at 2:57 PM Alexandre Torgue
<alexandre.torgue@st.com> wrote:

> Register a new GPIO bank only if GPIO bank node is enabled. This patch also
> adds checks on ranges which are defined only if a bank is registered.
>
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>

Patch applied.

Alexandre can you check the discussion we've had about using
GPIOLIB_IRQCHIP for multi-bank GPIOs with several IRQ
lines as per drivers/gpio/gpio-tegra186.c?

Is this approach applicable for STM32 so we can pull
more stuff in under GPIOLIB_IRQCHIP?

Yours,
Linus Walleij

Re: [PATCH 2/2] pinctrl: stm32: check node status before new gpio bank registering

[Alexandre Torgue]

Hi Linus

On 07/29/2018 10:11 PM, Linus Walleij wrote:
> On Mon, Jul 16, 2018 at 2:57 PM Alexandre Torgue
> <alexandre.torgue@st.com> wrote:
> 
>> Register a new GPIO bank only if GPIO bank node is enabled. This patch also
>> adds checks on ranges which are defined only if a bank is registered.
>>
>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
> 
> Patch applied.
> 
Thanks

> Alexandre can you check the discussion we've had about using
> GPIOLIB_IRQCHIP for multi-bank GPIOs with several IRQ
> lines as per drivers/gpio/gpio-tegra186.c?
> 
> Is this approach applicable for STM32 so we can pull
> more stuff in under GPIOLIB_IRQCHIP?
> 
Ok. I'm going to check what's possible to do. I let you know soon.

regards
Alex

> Yours,
> Linus Walleij
>