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

[Ramon Fried]

On Tue, May 15, 2018 at 7:15 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Ramon,
>
> On 16 May 2018 at 02:09, Ramon Fried <ramon.fried at gmail.com> wrote:
>>
>>
>> On Tue, May 15, 2018, 7:03 PM Simon Glass <sjg at chromium.org> wrote:
>>>
>>> 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
>>
>> I'm not sure I understand the question. If someone changes the DT to an
>> invalid string? Which is not in the table. Then yes. it will fail.
>
> I am suggesting that the table could be reduced in size since most of
> the entries can be generated with sprintf().
OK. I see.
I will try to fix it.
>
> Regards,
> Simon