Re: [PATCH 02/21] ARM: tegra: Add gpio-ranges property

From: Tomeu Vizoso <tomeu.vizoso@collabora.com>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: "Linus Walleij" <linus.walleij@linaro.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"Stéphane Marchesin" <stephane.marchesin@gmail.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
	"Alexander Holler" <holler@ahsoftware.de>,
	"Grant Likely" <grant.likely@linaro.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Pawel Moll" <pawel.moll@arm.com>,
	"Ian Campbell" <ijc+devicetree@hellion.org.uk>,
	"Kumar Gala" <galak@codeaurora.org>,
	"Russell King" <linux@arm.linux.org.uk>,
	"Alexandre Courbot" <gnurou@gmail.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 02/21] ARM: tegra: Add gpio-ranges property
Date: Wed, 27 May 2015 16:18:29 +0200	[thread overview]
Message-ID: <CAAObsKD7YbZX01A=SS7z_PxAMPweHy6sw5ut=50h50C=j9y0zA@mail.gmail.com> (raw)
In-Reply-To: <5564CC84.1030700@wwwdotorg.org>

On 26 May 2015 at 21:41, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/25/2015 08:53 AM, Tomeu Vizoso wrote:
>>
>> Specify how the GPIOs map to the pins in T124, so the dependency is
>> explicit.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> ---
>>   arch/arm/boot/dts/tegra124.dtsi | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>> b/arch/arm/boot/dts/tegra124.dtsi
>> index 13cc7ca..5d1d35f 100644
>> --- a/arch/arm/boot/dts/tegra124.dtsi
>> +++ b/arch/arm/boot/dts/tegra124.dtsi
>> @@ -254,6 +254,7 @@
>>                 gpio-controller;
>>                 #interrupt-cells = <2>;
>>                 interrupt-controller;
>> +               gpio-ranges = <&pinmux 0 0 250>;
>
>
> We should be consistent between SoCs. Why not make the same change for all
> Tegra SoCs?
>
> I think this change will cause the GPIO subsystem to call into the pinctrl
> subsystem and create/add/register a new GPIO<->pinctrl range structure. The
> pinctrl driver already does this, so I think we'll end up with two duplicate
> entries in the pinctrl device's gpio_ranges list. This probably won't cause
> a problem, but I wanted to make sure you'd thought about it to make sure.

Actually, I have checked and see that gpio-tegra.c registers 256
gpios, but pinctrl-tegra124.c adds a range of only 251. I don't really
remember where I got the 250 value from, sorry :(

I don't see how that would cause any concrete problems, but maybe we
should have a single authoritative source (not sure we can do so
without breaking DT ABI though).

> Right now, I think we get lucky and pinctrl ends up probing first (or at
> least very early) anyway. Somewhat related to this series, I wonder if we
> shouldn't add pinctrl client properties to every node in the Tegra DT that
> describes a controller that makes use of external pins that are affected by
> the pinmux. Such a change would guarantee this desired probing order. In
> order to preserve the "program the entire pinmux at once" semantics, these
> new pinctrl client properties would all need to reference empty states, yet
> would still need to exist to represent the dependency.

I think so, but aren't almost all those pins used as gpios? If so,
then such a controller's driver will request the gpio it wants which
will cause the gpio driver to be registered (and hopefully probed) if
needed, which in turn will check that the corresponding pinctrl device
has been registered. Or am I missing something?

Regards,

Tomeu

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/