[U-Boot] [PATCH 06/10] PINCTRL: stm32f7: add pin control driver

[Vikas MANOCHA]

Thanks Simon,

> -----Original Message-----
> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> Sent: Saturday, February 11, 2017 12:20 PM
> To: Vikas MANOCHA <vikas.manocha at st.com>
> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Beniamino Galvani <b.galvani at gmail.com>; Daniel Schwierzeck
> <daniel.schwierzeck at gmail.com>; Heiko Stübner <heiko at sntech.de>; Kever Yang <kever.yang at rock-chips.com>; Konstantin
> Porotchkin <kostap at marvell.com>; Masahiro Yamada <yamada.masahiro at socionext.com>; Minkyu Kang
> <mk7.kang at samsung.com>; Peng Fan <van.freenix at gmail.com>; Stefan Roese <sr at denx.de>; Thomas Abraham
> <thomas.ab at samsung.com>; Wenyou Yang <wenyou.yang at atmel.com>; Wills Wang <wills.wang at live.com>
> Subject: Re: [PATCH 06/10] PINCTRL: stm32f7: add pin control driver
> 
> Hi Vikas,
> 
> On 10 February 2017 at 14:39, vikas <vikas.manocha at st.com> wrote:
> > Hi Simon,
> >
> > On 02/10/2017 08:22 AM, Simon Glass wrote:
> >> Hi Vikas,
> >>
> >> On 4 February 2017 at 15:43, Vikas Manocha <vikas.manocha at st.com> wrote:
> >>> This driver uses the same pin control binding as that of linux,
> >>> binding document of this patch is copied from linux. One addition
> >>> done is for GPIO input and output mode configuration which was missing.
> >>>
> >>> Signed-off-by: Vikas Manocha <vikas.manocha at st.com>
> >>> ---
> >>>  configs/stm32f746-disco_defconfig                  |   3 +
> >>>  .../pinctrl/st,stm32-pinctrl.txt                   | 133 +++++++++++++++++++++
> >>>  drivers/pinctrl/Kconfig                            |   9 ++
> >>>  drivers/pinctrl/Makefile                           |   1 +
> >>>  drivers/pinctrl/pinctrl_stm32.c                    | 118 ++++++++++++++++++
> >>>  5 files changed, 264 insertions(+)
> >>>  create mode 100644
> >>> doc/device-tree-bindings/pinctrl/st,stm32-pinctrl.txt
> >>>  create mode 100644 drivers/pinctrl/pinctrl_stm32.c
> >>
> >> Reviewed-by: Simon Glass <sjg at chromium.org>
> >>
> >> nits below
> >>
> [..]
> 
> >>
> >>> +               fdt_get_property(gd->fdt_blob, args.node, "pinmux", &len);
> >>> +               len /= 4;
> >>> +               debug("%s: no of pinmux entries= %d\n", __func__, len);
> >>> +               rv = fdtdec_get_int_array(gd->fdt_blob, args.node,
> >>> +                                                "pinmux", pin_mux,
> >>> + len);
> >>
> >> Can you use fdtdec_get_int_array_count() ?
> >
> > Yes but it's almost same, the only difference being xxx_array_count is relaxed on lenght/count.
> > And in this case we are reading length of the property just before.
> >
> > In any case, i will change it to xxx_array_count in v2.
> 
> Yes your code works. I'm only suggesting the change because, in a way,
> fdt_get_property() is quite a low-level function. You can do what you want with an fdtdec function and thus avoid using libfdt
> directly.

Ah..got it, I will remove the length check & use xxx_array_count in v3.

Cheers,
Vikas

> 
> [...]
> 
> Regards,
> Simon