| Message ID | 1434548543-22949-5-git-send-email-tomeu.vizoso@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 17, 2015 at 3:42 PM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: > If the gpio DT node has the gpio-ranges property, the range will be > added by the gpio core and doesn't need to be added by the pinctrl > driver. > > By having the gpio-ranges property, we have an explicit dependency from > the gpio node to the pinctrl node and we can stop using the deprecated > pinctrl_add_gpio_range() function. > > Note that when the GPIO device gets probed before the associated > princtrl device, the gpio core actually won't register the gpio range. > Thus, this patch is only safe to be merged after we have in place a way > to assure that gpio devices are probed after their associated pinctrl > devices (such as ordered probing). > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> This doesn't look like it would hurt, but need Stephen's opinion on it, and I think he's on vacation. Would check with next-in-line Tegra maintainer, Thierry/Alexandre? Yours, Linus Walleij
On 13 July 2015 at 22:14, Linus Walleij <linus.walleij@linaro.org> wrote: > On Wed, Jun 17, 2015 at 3:42 PM, Tomeu Vizoso > <tomeu.vizoso@collabora.com> wrote: > >> If the gpio DT node has the gpio-ranges property, the range will be >> added by the gpio core and doesn't need to be added by the pinctrl >> driver. >> >> By having the gpio-ranges property, we have an explicit dependency from >> the gpio node to the pinctrl node and we can stop using the deprecated >> pinctrl_add_gpio_range() function. >> >> Note that when the GPIO device gets probed before the associated >> princtrl device, the gpio core actually won't register the gpio range. >> Thus, this patch is only safe to be merged after we have in place a way >> to assure that gpio devices are probed after their associated pinctrl >> devices (such as ordered probing). >> >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > This doesn't look like it would hurt, but need Stephen's opinion > on it, and I think he's on vacation. Would check with next-in-line > Tegra maintainer, Thierry/Alexandre? Sorry about that, but I have split these changes out into their own series after people complained about it. Have just sent a new version which already has Stephen's ack: https://lkml.kernel.org/g/1436862596-27730-1-git-send-email-tomeu.vizoso@collabora.com Thanks, Tomeu
On Tue, Jul 14, 2015 at 5:34 PM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: > On 13 July 2015 at 22:14, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Wed, Jun 17, 2015 at 3:42 PM, Tomeu Vizoso >> <tomeu.vizoso@collabora.com> wrote: >> >>> If the gpio DT node has the gpio-ranges property, the range will be >>> added by the gpio core and doesn't need to be added by the pinctrl >>> driver. >>> >>> By having the gpio-ranges property, we have an explicit dependency from >>> the gpio node to the pinctrl node and we can stop using the deprecated >>> pinctrl_add_gpio_range() function. >>> >>> Note that when the GPIO device gets probed before the associated >>> princtrl device, the gpio core actually won't register the gpio range. >>> Thus, this patch is only safe to be merged after we have in place a way >>> to assure that gpio devices are probed after their associated pinctrl >>> devices (such as ordered probing). >>> >>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> >> >> This doesn't look like it would hurt, but need Stephen's opinion >> on it, and I think he's on vacation. Would check with next-in-line >> Tegra maintainer, Thierry/Alexandre? > > Sorry about that, but I have split these changes out into their own > series after people complained about it. > > Have just sent a new version which already has Stephen's ack: > > https://lkml.kernel.org/g/1436862596-27730-1-git-send-email-tomeu.vizoso@collabora.com The change looks ok, but why limit it to Tegra? It seems like it could apply to any driver that supports gpio-range. Or is it because this will only work with drivers that use ordered probing?
On 15 July 2015 at 05:17, Alexandre Courbot <gnurou@gmail.com> wrote: > On Tue, Jul 14, 2015 at 5:34 PM, Tomeu Vizoso > <tomeu.vizoso@collabora.com> wrote: >> On 13 July 2015 at 22:14, Linus Walleij <linus.walleij@linaro.org> wrote: >>> On Wed, Jun 17, 2015 at 3:42 PM, Tomeu Vizoso >>> <tomeu.vizoso@collabora.com> wrote: >>> >>>> If the gpio DT node has the gpio-ranges property, the range will be >>>> added by the gpio core and doesn't need to be added by the pinctrl >>>> driver. >>>> >>>> By having the gpio-ranges property, we have an explicit dependency from >>>> the gpio node to the pinctrl node and we can stop using the deprecated >>>> pinctrl_add_gpio_range() function. >>>> >>>> Note that when the GPIO device gets probed before the associated >>>> princtrl device, the gpio core actually won't register the gpio range. >>>> Thus, this patch is only safe to be merged after we have in place a way >>>> to assure that gpio devices are probed after their associated pinctrl >>>> devices (such as ordered probing). >>>> >>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> >>> >>> This doesn't look like it would hurt, but need Stephen's opinion >>> on it, and I think he's on vacation. Would check with next-in-line >>> Tegra maintainer, Thierry/Alexandre? >> >> Sorry about that, but I have split these changes out into their own >> series after people complained about it. >> >> Have just sent a new version which already has Stephen's ack: >> >> https://lkml.kernel.org/g/1436862596-27730-1-git-send-email-tomeu.vizoso@collabora.com > > The change looks ok, but why limit it to Tegra? It seems like it could > apply to any driver that supports gpio-range. Or is it because this > will only work with drivers that use ordered probing? It has to be SoC specific because we need to find out if a node representing the in-SoC gpio controller is present, and has the gpio-ranges property. I guess we could move the code to the pinctrl core and parametrize the property name but given that pinctrl_add_gpio_range is deprecated and that we are doing this only for DT compatibility, I'm not sure it would be a good idea. Regards, Tomeu
On Tue, Jul 14, 2015 at 10:34 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: > On 13 July 2015 at 22:14, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Wed, Jun 17, 2015 at 3:42 PM, Tomeu Vizoso >> <tomeu.vizoso@collabora.com> wrote: >> >>> If the gpio DT node has the gpio-ranges property, the range will be >>> added by the gpio core and doesn't need to be added by the pinctrl >>> driver. >>> >>> By having the gpio-ranges property, we have an explicit dependency from >>> the gpio node to the pinctrl node and we can stop using the deprecated >>> pinctrl_add_gpio_range() function. >>> >>> Note that when the GPIO device gets probed before the associated >>> princtrl device, the gpio core actually won't register the gpio range. >>> Thus, this patch is only safe to be merged after we have in place a way >>> to assure that gpio devices are probed after their associated pinctrl >>> devices (such as ordered probing). >>> >>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> >> >> This doesn't look like it would hurt, but need Stephen's opinion >> on it, and I think he's on vacation. Would check with next-in-line >> Tegra maintainer, Thierry/Alexandre? > > Sorry about that, but I have split these changes out into their own > series after people complained about it. > > Have just sent a new version which already has Stephen's ack: > > https://lkml.kernel.org/g/1436862596-27730-1-git-send-email-tomeu.vizoso@collabora.com I don't see Stephen's ACK on them, where is it? It's not in the patch and I don't see it as response to the patches. Yours, Linus Walleij
On 17 July 2015 at 10:04, Linus Walleij <linus.walleij@linaro.org> wrote: > On Tue, Jul 14, 2015 at 10:34 AM, Tomeu Vizoso > <tomeu.vizoso@collabora.com> wrote: >> On 13 July 2015 at 22:14, Linus Walleij <linus.walleij@linaro.org> wrote: >>> On Wed, Jun 17, 2015 at 3:42 PM, Tomeu Vizoso >>> <tomeu.vizoso@collabora.com> wrote: >>> >>>> If the gpio DT node has the gpio-ranges property, the range will be >>>> added by the gpio core and doesn't need to be added by the pinctrl >>>> driver. >>>> >>>> By having the gpio-ranges property, we have an explicit dependency from >>>> the gpio node to the pinctrl node and we can stop using the deprecated >>>> pinctrl_add_gpio_range() function. >>>> >>>> Note that when the GPIO device gets probed before the associated >>>> princtrl device, the gpio core actually won't register the gpio range. >>>> Thus, this patch is only safe to be merged after we have in place a way >>>> to assure that gpio devices are probed after their associated pinctrl >>>> devices (such as ordered probing). >>>> >>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> >>> >>> This doesn't look like it would hurt, but need Stephen's opinion >>> on it, and I think he's on vacation. Would check with next-in-line >>> Tegra maintainer, Thierry/Alexandre? >> >> Sorry about that, but I have split these changes out into their own >> series after people complained about it. >> >> Have just sent a new version which already has Stephen's ack: >> >> https://lkml.kernel.org/g/1436862596-27730-1-git-send-email-tomeu.vizoso@collabora.com > > I don't see Stephen's ACK on them, where is it? It's not in the > patch and I don't see it as response to the patches. I see it in: https://lkml.kernel.org/g/1436862596-27730-4-git-send-email-tomeu.vizoso@collabora.com Which was given in: https://lkml.kernel.org/g/559D8E36.50209@wwwdotorg.org But we still need an ack for patch 1/3 before both 2/3 and 3/3 can be merged, because if we don't defer the probe when the pinctrl isn't found and just skip adding the gpio-range, there won't be no gpio-range set at all if the pinctrl device probes after the gpiochip device. Merging 2/3 now would be fine as long as we don't merge 3/3 before 1/3. Thanks, Tomeu
On Fri, Jul 17, 2015 at 10:19 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: > On 17 July 2015 at 10:04, Linus Walleij <linus.walleij@linaro.org> wrote: >> I don't see Stephen's ACK on them, where is it? It's not in the >> patch and I don't see it as response to the patches. > > I see it in: > > https://lkml.kernel.org/g/1436862596-27730-4-git-send-email-tomeu.vizoso@collabora.com > > Which was given in: > > https://lkml.kernel.org/g/559D8E36.50209@wwwdotorg.org Hm strange I only see v1, that's why I'm confused.... > But we still need an ack for patch 1/3 before both 2/3 and 3/3 can be > merged, because if we don't defer the probe when the pinctrl isn't > found and just skip adding the gpio-range, there won't be no > gpio-range set at all if the pinctrl device probes after the gpiochip > device. Merging 2/3 now would be fine as long as we don't merge 3/3 > before 1/3. I'll hold it back for now, bug me when you think I can merge 1/3 and 2/3. Yours, Linus Walleij
diff --git a/drivers/pinctrl/pinctrl-tegra.c b/drivers/pinctrl/pinctrl-tegra.c index 0f982b8..0fd7fd2 100644 --- a/drivers/pinctrl/pinctrl-tegra.c +++ b/drivers/pinctrl/pinctrl-tegra.c @@ -624,6 +624,22 @@ static struct pinctrl_desc tegra_pinctrl_desc = { .owner = THIS_MODULE, }; +static bool gpio_node_has_range(void) +{ + struct device_node *np; + bool has_prop = false; + + np = of_find_compatible_node(NULL, NULL, "nvidia,tegra30-gpio"); + if (!np) + return has_prop; + + has_prop = of_find_property(np, "gpio-ranges", NULL); + + of_node_put(np); + + return has_prop; +} + int tegra_pinctrl_probe(struct platform_device *pdev, const struct tegra_pinctrl_soc_data *soc_data) { @@ -708,7 +724,8 @@ int tegra_pinctrl_probe(struct platform_device *pdev, return PTR_ERR(pmx->pctl); } - pinctrl_add_gpio_range(pmx->pctl, &tegra_pinctrl_gpio_range); + if (!gpio_node_has_range()) + pinctrl_add_gpio_range(pmx->pctl, &tegra_pinctrl_gpio_range); platform_set_drvdata(pdev, pmx);
If the gpio DT node has the gpio-ranges property, the range will be added by the gpio core and doesn't need to be added by the pinctrl driver. By having the gpio-ranges property, we have an explicit dependency from the gpio node to the pinctrl node and we can stop using the deprecated pinctrl_add_gpio_range() function. Note that when the GPIO device gets probed before the associated princtrl device, the gpio core actually won't register the gpio range. Thus, this patch is only safe to be merged after we have in place a way to assure that gpio devices are probed after their associated pinctrl devices (such as ordered probing). Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> --- drivers/pinctrl/pinctrl-tegra.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)