[PATCH 12/20] pinctrl: add i.MXRT driver

[Lukasz Majewski]

On Wed, 11 Dec 2019 13:40:01 +0100
Giulio Benetti <giulio.benetti at benettiengineering.com> wrote:

> Hi Lukasz,
> 
> On 12/10/19 12:46 AM, Lukasz Majewski wrote:
> > On Mon, 9 Dec 2019 12:54:33 +0100
> > Giulio Benetti <giulio.benetti at 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.benetti at benettiengineering.com> wrote:
> >>>      
> >>>> Add i.MXRT pinctrl driver.
> >>>>
> >>>> Signed-off-by: Giulio Benetti
> >>>> <giulio.benetti at 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.  
> 
> Ok
> 
> >>
> >> 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 ?  
> 
> They are already exactly the same structure.

Ok.

> 
> >>
> >> 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.  
> 
> Yes, but what I was asking is about those 0x100b1 that is a bitmask.
> I would like to introduce or'ed short macros to compose that value.
> But if the first goal is to keep in sync with Linux kernel then I
> leave as is and later I will try to introduce this on Linux first and
> later sync here.

Ok.

> 
> >>
> >> 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).  
> 
> Ok
> 
> Thank you
> 
> 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: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20191212/fc1f9255/attachment.sig>