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

[Ramon Fried]

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.

>