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

[Tomeu Vizoso <tomeu.vizoso-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>]

On 2 June 2015 at 17:40, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 06/02/2015 05:28 AM, Linus Walleij wrote:
>>
>> On Tue, May 26, 2015 at 9:41 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.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-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
>>>> ---
>>>>    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?
>>
>>
>> Agreed.
>>
>>> 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.
>>
>>
>> That sounds like duplication indeed, I would expect that first a patch
>> adds the ranges to the dts[i] files and then a second patch delete the
>> same ranges from the pinctrl driver then, if these shall come in from
>> the device tree.
>
>
> We can't delete the gpio-range-registration code from the Tegra pinmux
> driver, or old DTs won't work correctly. We could make it conditional based
> upon whether the DT contains the property or not.

I've been looking at this and haven't found a good solution. From what
I see, the pinctrl driver doesn't have a reference to the gpio device
node so cannot tell if it needs to add the range or not.

The gpio driver can tell whether it should add the range or not, but
if it has to because the gpio-ranges property isn't there, then it
doesn't have the reference to the pinctrl device to set the range to.

So, given that pinctrl_add_gpio_range is deprecated already, wonder if
the lesser evil isn't leaving the duplicated entries for now. On newer
SoC revisions such as T210 we can stop calling pinctrl_add_gpio_range
at all.

Or, we can accept that nobody is going to boot a newer kernel with an
older DT on the affected boards and just rely on the presence of the
gpio-ranges property :)

Regards,

Tomeu
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html