On Mon, 9 Dec 2019 12:54:33 +0100 Giulio Benetti <giulio.bene...@benettiengineering.com> wrote:
> Hi Lukasz, Stefano, Fabio, all,
>
> On 12/8/19 3:45 PM, Lukasz Majewski wrote:
> > On Wed, 4 Dec 2019 18:44:31 +0100
> > Giulio Benetti <giulio.bene...@benettiengineering.com> wrote:
> >
> >> Add i.MXRT pinctrl driver.
> >>
> >> Signed-off-by: Giulio Benetti
> >> <giulio.bene...@benettiengineering.com> ---
> >> drivers/pinctrl/nxp/Kconfig | 14 ++++++++++
> >> drivers/pinctrl/nxp/Makefile | 1 +
> >> drivers/pinctrl/nxp/pinctrl-imxrt.c | 40
> >> +++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+)
> >> create mode 100644 drivers/pinctrl/nxp/pinctrl-imxrt.c
> >>
> >> diff --git a/drivers/pinctrl/nxp/Kconfig
> >> b/drivers/pinctrl/nxp/Kconfig index f2e67ca231..ec55351e61 100644
> >> --- a/drivers/pinctrl/nxp/Kconfig
> >> +++ b/drivers/pinctrl/nxp/Kconfig
> >> @@ -99,6 +99,20 @@ config PINCTRL_MXS
> >> familiy, e.g. i.MX28. This feature depends on device
> >> tree configuration.
> >>
> >> +config PINCTRL_IMXRT
> >> + bool "IMXRT pinctrl driver"
> >> + depends on ARCH_IMXRT && PINCTRL_FULL
> >> + select DEVRES
> >> + select PINCTRL_IMX
> >> + help
> >> + Say Y here to enable the imxrt pinctrl driver
> >> +
> >> + This provides a simple pinctrl driver for i.MXRT SoC
> >> familiy.
> >> + This feature depends on device tree configuration. This
> >> driver
> >> + is different from the linux one, this is a simple
> >> implementation,
> >
> > Could you add proper documentation entry (in ./doc/*) in which you
> > would point out the differences between the full blown Linux driver
> > and this U-Boot driver (I do guess that "only parsing 'fsl,pins'"
> > is not the only difference - more details are welcome).
>
> Sure, but isn't maybe something to be done in the document below?...
+1
>
> > And a bit more generic request - it is also nice to add some kind of
> > ./doc/README.* documentation in which one can put some hints (or
> > usage patterns) for in-uboot boards (like during development
> > discovered HW issues, etc.). Such information is priceless when
> > other community member wants to use this code/board (and NXP is
> > very often silence about them :-) ).
>
> Sure, I'm going to provide a
> ./doc/device-tree-bindings/pinctrl/fsl,imx-pinctrl.txt unique for
> every imx at this point, ok?
I think that it would be ok, to first focus on iMXRT.
>
> And what do you think to provide defines in
> ./include/dt-bindings/pinctrl/pinctrl-imxrt1050.h for every setting
> in PAD? Or better, in a pinctrl-imxrt.h to be included in
> pinctrl-imxrt1050.h(since they are the same)
> I mean for pull-up/down, drive strength etc.
I would prefer to have the same files as are (or would be) in the Linux
kernel.
Moreover, there are already added
./include/dt-bindings/pinctrl/pads-imx8q* files in this directory.
Maybe you could provide similar structure ?
>
> Because, even for me that I'm developing, I see it very difficult to
> recognize every setting from hexadecimal in dts file, and I loose the
> overview. I've already used IMX_PAD_SION to set pad as input.
User readable defines DO HELP a lot.
IMX6Q uses for example:
fsl,pins = <
MX6QDL_PAD_CSI0_DAT10__ECSPI2_MISO 0x100b1
MX6QDL_PAD_CSI0_DAT9__ECSPI2_MOSI 0x100b1
MX6QDL_PAD_CSI0_DAT8__ECSPI2_SCLK 0x100b1
>;
which is in sync with Linux kernel.
>
> Then I would be happy, to add all definitions for every imx and
> modify every dts file, but I'd need everyone to re-test its board.
> What about this?
Let's keep things in sync with Linux kernel as much as possible. This
is one of U-Boot's recent goals to:
1. Reduce maintenance effort
2. Ease porting of Linux drivers.
What we do need to focus on (in U-Boot) to trim those drivers to keep
U-Boot's footprint _really_ small or at least as small as it was
before DTS was introduced (in SPL for e.g. i.MX).
>
> Best regards
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de
pgpi7m8GWz6sN.pgp
Description: OpenPGP digital signature
