Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1568479 > unrolled thread

[PATCH 2/8] pinctrl: stm32: use gpio-ranges to declare bank range

Started byAlexandre TORGUE <alexandre.torgue@st.com>
First post2017-01-27 17:20 +0100
Last post2017-02-01 16:10 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/8] pinctrl: stm32: use gpio-ranges to declare bank range Alexandre TORGUE <alexandre.torgue@st.com> - 2017-01-27 17:20 +0100
    Re: [PATCH 2/8] pinctrl: stm32: use gpio-ranges to declare bank range Linus Walleij <linus.walleij@linaro.org> - 2017-01-30 16:30 +0100
      Re: [PATCH 2/8] pinctrl: stm32: use gpio-ranges to declare bank range Alexandre Torgue <alexandre.torgue@st.com> - 2017-01-30 17:40 +0100
        Re: [PATCH 2/8] pinctrl: stm32: use gpio-ranges to declare bank range Linus Walleij <linus.walleij@linaro.org> - 2017-02-01 16:10 +0100

#1568479 — [PATCH 2/8] pinctrl: stm32: use gpio-ranges to declare bank range

FromAlexandre TORGUE <alexandre.torgue@st.com>
Date2017-01-27 17:20 +0100
Subject[PATCH 2/8] pinctrl: stm32: use gpio-ranges to declare bank range
Message-ID<t4hpM-6ni-15@gated-at.bofh.it>
Use device tree entries to declare gpio range. It will allow to use
no contiguous gpio bank and holes inside a bank.

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 b145431..d0a968a 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -589,7 +589,7 @@ static int stm32_pmx_set_mux(struct pinctrl_dev *pctldev,
 	}
 
 	range = pinctrl_find_gpio_range_from_pin(pctldev, g->pin);
-	bank = gpio_range_to_bank(range);
+	bank = gpiochip_get_data(range->gc);
 	pin = stm32_gpio_pin(g->pin);
 
 	mode = stm32_gpio_get_mode(function);
@@ -604,7 +604,7 @@ static int stm32_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
 			struct pinctrl_gpio_range *range, unsigned gpio,
 			bool input)
 {
-	struct stm32_gpio_bank *bank = gpio_range_to_bank(range);
+	struct stm32_gpio_bank *bank = gpiochip_get_data(range->gc);
 	int pin = stm32_gpio_pin(gpio);
 
 	stm32_pmx_set_mode(bank, pin, !input, 0);
@@ -761,7 +761,7 @@ static int stm32_pconf_parse_conf(struct pinctrl_dev *pctldev,
 	int offset, ret = 0;
 
 	range = pinctrl_find_gpio_range_from_pin(pctldev, pin);
-	bank = gpio_range_to_bank(range);
+	bank = gpiochip_get_data(range->gc);
 	offset = stm32_gpio_pin(pin);
 
 	switch (param) {
@@ -842,7 +842,7 @@ static void stm32_pconf_dbg_show(struct pinctrl_dev *pctldev,
 	bool val;
 
 	range = pinctrl_find_gpio_range_from_pin_nolock(pctldev, pin);
-	bank = gpio_range_to_bank(range);
+	bank = gpiochip_get_data(range->gc);
 	offset = stm32_gpio_pin(pin);
 
 	stm32_pmx_get_mode(bank, offset, &mode, &alt);
@@ -900,10 +900,12 @@ static int stm32_gpiolib_register_bank(struct stm32_pinctrl *pctl,
 	int bank_nr = pctl->nbanks;
 	struct stm32_gpio_bank *bank = &pctl->banks[bank_nr];
 	struct pinctrl_gpio_range *range = &bank->range;
+	struct of_phandle_args args;
 	struct device *dev = pctl->dev;
 	struct resource res;
 	struct reset_control *rstc;
-	int err, npins;
+	unsigned int npins;
+	int err;
 
 	rstc = of_reset_control_get(np, NULL);
 	if (!IS_ERR(rstc))
@@ -928,28 +930,32 @@ static int stm32_gpiolib_register_bank(struct stm32_pinctrl *pctl,
 		return err;
 	}
 
-	npins = pctl->match_data->npins;
-	npins -= bank_nr * STM32_GPIO_PINS_PER_BANK;
-	if (npins < 0)
-		return -EINVAL;
-	else if (npins > STM32_GPIO_PINS_PER_BANK)
+	if (of_property_read_u32(np, "ngpios", &npins))
 		npins = STM32_GPIO_PINS_PER_BANK;
 
 	bank->gpio_chip = stm32_gpio_template;
+
+	of_property_read_string(np, "st,bank-name", &bank->gpio_chip.label);
+
+	if (!of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, 0, &args))
+		bank_nr = args.args[1] / STM32_GPIO_PINS_PER_BANK;
+	else {
+		range->name = bank->gpio_chip.label;
+		range->id = bank_nr;
+		range->pin_base = range->id * STM32_GPIO_PINS_PER_BANK;
+		range->base = range->id * STM32_GPIO_PINS_PER_BANK;
+		range->npins = npins;
+		range->gc = &bank->gpio_chip;
+		pinctrl_add_gpio_range(pctl->pctl_dev,
+				       &pctl->banks[bank_nr].range);
+	}
+
 	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;
 	spin_lock_init(&bank->lock);
 
-	of_property_read_string(np, "st,bank-name", &range->name);
-	bank->gpio_chip.label = range->name;
-
-	range->id = bank_nr;
-	range->pin_base = range->base = range->id * STM32_GPIO_PINS_PER_BANK;
-	range->npins = bank->gpio_chip.ngpio;
-	range->gc = &bank->gpio_chip;
-
 	/* create irq hierarchical domain */
 	bank->fwnode = of_node_to_fwnode(np);
 
@@ -966,7 +972,7 @@ static int stm32_gpiolib_register_bank(struct stm32_pinctrl *pctl,
 		return err;
 	}
 
-	dev_info(dev, "%s bank added\n", range->name);
+	dev_info(dev, "%s bank added\n", bank->gpio_chip.label);
 	return 0;
 }
 
@@ -1085,30 +1091,6 @@ int stm32_pctl_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	for_each_child_of_node(np, child)
-		if (of_property_read_bool(child, "gpio-controller"))
-			banks++;
-
-	if (!banks) {
-		dev_err(dev, "at least one GPIO bank is required\n");
-		return -EINVAL;
-	}
-
-	pctl->banks = devm_kcalloc(dev, banks, sizeof(*pctl->banks),
-			GFP_KERNEL);
-	if (!pctl->banks)
-		return -ENOMEM;
-
-	for_each_child_of_node(np, child) {
-		if (of_property_read_bool(child, "gpio-controller")) {
-			ret = stm32_gpiolib_register_bank(pctl, child);
-			if (ret)
-				return ret;
-
-			pctl->nbanks++;
-		}
-	}
-
 	pins = devm_kcalloc(&pdev->dev, pctl->match_data->npins, sizeof(*pins),
 			    GFP_KERNEL);
 	if (!pins)
@@ -1133,8 +1115,28 @@ int stm32_pctl_probe(struct platform_device *pdev)
 		return PTR_ERR(pctl->pctl_dev);
 	}
 
-	for (i = 0; i < pctl->nbanks; i++)
-		pinctrl_add_gpio_range(pctl->pctl_dev, &pctl->banks[i].range);
+	for_each_child_of_node(np, child)
+		if (of_property_read_bool(child, "gpio-controller"))
+			banks++;
+
+	if (!banks) {
+		dev_err(dev, "at least one GPIO bank is required\n");
+		return -EINVAL;
+	}
+	pctl->banks = devm_kcalloc(dev, banks, sizeof(*pctl->banks),
+			GFP_KERNEL);
+	if (!pctl->banks)
+		return -ENOMEM;
+
+	for_each_child_of_node(np, child) {
+		if (of_property_read_bool(child, "gpio-controller")) {
+			ret = stm32_gpiolib_register_bank(pctl, child);
+			if (ret)
+				return ret;
+
+			pctl->nbanks++;
+		}
+	}
 
 	dev_info(dev, "Pinctrl STM32 initialized\n");
 
-- 
1.9.1

[toc] | [next] | [standalone]

#1569831

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-01-30 16:30 +0100
Message-ID<t5m42-5BN-33@gated-at.bofh.it>
In reply to#1568479
On Fri, Jan 27, 2017 at 5:15 PM, Alexandre TORGUE
<alexandre.torgue@st.com> wrote:

> Use device tree entries to declare gpio range. It will allow to use
> no contiguous gpio bank and holes inside a bank.
>
> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>

(...)
> +       of_property_read_string(np, "st,bank-name", &bank->gpio_chip.label);
> +
> +       if (!of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, 0, &args))
> +               bank_nr = args.args[1] / STM32_GPIO_PINS_PER_BANK;
> +       else {
> +               range->name = bank->gpio_chip.label;
> +               range->id = bank_nr;
> +               range->pin_base = range->id * STM32_GPIO_PINS_PER_BANK;
> +               range->base = range->id * STM32_GPIO_PINS_PER_BANK;
> +               range->npins = npins;
> +               range->gc = &bank->gpio_chip;
> +               pinctrl_add_gpio_range(pctl->pctl_dev,
> +                                      &pctl->banks[bank_nr].range);
> +       }

Why are you doing this?

There is already code in drivers/gpio/gpiolib-of.c to pick ranges
from the device tree and add when you're adding the GPIO chips.

Please use that or figure out what is needed to make it work for
you instead of reimplementing it.

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1569879

FromAlexandre Torgue <alexandre.torgue@st.com>
Date2017-01-30 17:40 +0100
Message-ID<t5n9L-6e5-23@gated-at.bofh.it>
In reply to#1569831
Hi Linus,

On 01/30/2017 04:19 PM, Linus Walleij wrote:
> On Fri, Jan 27, 2017 at 5:15 PM, Alexandre TORGUE
> <alexandre.torgue@st.com> wrote:
>
>> Use device tree entries to declare gpio range. It will allow to use
>> no contiguous gpio bank and holes inside a bank.
>>
>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>
>
> (...)
>> +       of_property_read_string(np, "st,bank-name", &bank->gpio_chip.label);
>> +
>> +       if (!of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, 0, &args))
>> +               bank_nr = args.args[1] / STM32_GPIO_PINS_PER_BANK;
>> +       else {
>> +               range->name = bank->gpio_chip.label;
>> +               range->id = bank_nr;
>> +               range->pin_base = range->id * STM32_GPIO_PINS_PER_BANK;
>> +               range->base = range->id * STM32_GPIO_PINS_PER_BANK;
>> +               range->npins = npins;
>> +               range->gc = &bank->gpio_chip;
>> +               pinctrl_add_gpio_range(pctl->pctl_dev,
>> +                                      &pctl->banks[bank_nr].range);
>> +       }

I Keep the old way to get range in order to keep compatibility with 
older device tree (if no gpio ranges are defined).

>
> Why are you doing this?
>
> There is already code in drivers/gpio/gpiolib-of.c to pick ranges
> from the device tree and add when you're adding the GPIO chips.

Yes, If I declare gpio-ranges inside DT node, the range is well handled 
by the gpiolib. But in this condition I use gpio-ranges entry to get the 
pin base offset for the bank we are registering. For example, if there 
an hole between 2 banks (GPIOA, GPIOD, ...) I need to know at which pin 
offset the bank D starts. It is the only way I found.

The "else" statement is here to get compatibility with older device tree 
(which don't use gpio-ranges entry).

Regards
Alex


>
> Please use that or figure out what is needed to make it work for
> you instead of reimplementing it.
>
> Yours,
> Linus Walleij
>

[toc] | [prev] | [next] | [standalone]

#1571643

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-02-01 16:10 +0100
Message-ID<t64HN-7x7-33@gated-at.bofh.it>
In reply to#1569879
On Mon, Jan 30, 2017 at 5:29 PM, Alexandre Torgue
<alexandre.torgue@st.com> wrote:
> On 01/30/2017 04:19 PM, Linus Walleij wrote:

>>> +       if (!of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, 0,
>>> &args))
>>> +               bank_nr = args.args[1] / STM32_GPIO_PINS_PER_BANK;
>>> +       else {
>>> +               range->name = bank->gpio_chip.label;
>>> +               range->id = bank_nr;
>>> +               range->pin_base = range->id * STM32_GPIO_PINS_PER_BANK;
>>> +               range->base = range->id * STM32_GPIO_PINS_PER_BANK;
>>> +               range->npins = npins;
>>> +               range->gc = &bank->gpio_chip;
>>> +               pinctrl_add_gpio_range(pctl->pctl_dev,
>>> +                                      &pctl->banks[bank_nr].range);
>>> +       }
>
>
> I Keep the old way to get range in order to keep compatibility with older
> device tree (if no gpio ranges are defined).

Aha I understand. Sorry for my stupidity.

Go ahead with this!

Yours,
Linus Walleij

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web