[U-Boot] [PATCH 5/7] mach-snapdragon: Introduce pinctrl driver

[Simon Glass]

Hi Ramon,

On 15 May 2018 at 07:23, Ramon Fried <ramon.fried at gmail.com> wrote:
> On Mon, May 14, 2018 at 10:51 PM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Ramon,
>>
>> On 14 May 2018 at 01:10, Ramon Fried <ramon.fried at gmail.com> wrote:
>>> On Mon, May 14, 2018 at 1:00 AM, Simon Glass <sjg at chromium.org> wrote:
>>>> Hi Ramon,
>>>>
>>>> On 12 May 2018 at 20:15, Ramon Fried <ramon.fried at gmail.com> wrote:
>>>>> This patch adds pinmux and pinctrl driver for TLMM
>>>>> subsystem in snapdragon chipsets.
>>>>> Currently, supporting only 8016, but implementation is
>>>>> generic and 8096 can be added easily.
>>>>>
>>>>> Driver is using the generic dt-bindings and doesn't
>>>>> introduce any new bindings (yet).
>>>>>
>>>>> Signed-off-by: Ramon Fried <ramon.fried at gmail.com>
>>>>> ---
>>>>>  arch/arm/mach-snapdragon/Makefile                |   2 +
>>>>>  arch/arm/mach-snapdragon/pinctrl-apq8016.c       | 162 +++++++++++++++++++++++
>>>>>  arch/arm/mach-snapdragon/pinctrl-snapdragon.c    | 118 +++++++++++++++++
>>>>>  arch/arm/mach-snapdragon/pinctrl-snapdragon.h    |  21 +++
>>>>>  configs/dragonboard410c_defconfig                |   5 +
>>>>>  include/dt-bindings/pinctrl/pinctrl-snapdragon.h |  22 +++
>>>>>  6 files changed, 330 insertions(+)
>>>>>  create mode 100644 arch/arm/mach-snapdragon/pinctrl-apq8016.c
>>>>>  create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.c
>>>>>  create mode 100644 arch/arm/mach-snapdragon/pinctrl-snapdragon.h
>>>>>  create mode 100644 include/dt-bindings/pinctrl/pinctrl-snapdragon.h
>>>>>
>>>>> diff --git a/arch/arm/mach-snapdragon/Makefile b/arch/arm/mach-snapdragon/Makefile
>>>>> index 1c23dc52cf..1d35fea912 100644
>>>>> --- a/arch/arm/mach-snapdragon/Makefile
>>>>> +++ b/arch/arm/mach-snapdragon/Makefile
>>>>> @@ -6,4 +6,6 @@ obj-$(CONFIG_TARGET_DRAGONBOARD820C) += clock-apq8096.o
>>>>>  obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o
>>>>>  obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o
>>>>>  obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o
>>>>> +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-apq8016.o
>>>>> +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += pinctrl-snapdragon.o
>>>>>  obj-y += clock-snapdragon.o
>>>>> diff --git a/arch/arm/mach-snapdragon/pinctrl-apq8016.c b/arch/arm/mach-snapdragon/pinctrl-apq8016.c
>>>>> new file mode 100644
>>>>> index 0000000000..8e57e2338c
>>>>> --- /dev/null
>>>>> +++ b/arch/arm/mach-snapdragon/pinctrl-apq8016.c
>>>>> @@ -0,0 +1,162 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>>> +/*
>>>>> + * Qualcomm APQ8016 pinctrl
>>>>> + *
>>>>> + * (C) Copyright 2018 Ramon Fried <ramon.fried at gmail.com>
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +#include "pinctrl-snapdragon.h"
>>>>> +#include <common.h>
>>>>> +
>>>>> +const char * const msm_pinctrl_pins[] = {
>>>>> +       "GPIO_0",
>>>>> +       "GPIO_1",
>>>>> +       "GPIO_2",
>>>>> +       "GPIO_3",
>>>>> +       "GPIO_4",
>>>>> +       "GPIO_5",
>>>>> +       "GPIO_6",
>>>>> +       "GPIO_7",
>>>>
>>>> This seems inefficient. Could you not sprintf() the name for most of
>>>> these values?
>>> The origin of this table is from the Linux kernel driver.
>>> I'm not sure I understand how sprintf will more efficient, do you want
>>> to fill up this table on runtime ?
>>
>> I think this table is only used in one function, so you could create
>> the string there perhaps?
>>
> Actually, it works the other way around, the generic-pinctrl needs a
> function to translate string to index.
> Basically, it reads strings from the FDT and then go over all indexes
> until it matches that string. this is inefficient IMHO as I think it
> will be easier just
> to be able to provide an index instead of a string in the FDT.

OK, so long as the index is actually a known value. If someone changes
the DT, won't that fail?

Regards,
Simon