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

[Giulio Benetti]

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.

>>
>> 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.

>>
>> 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
-- 
Giulio Benetti
Benetti Engineering sas

>>
>> 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
>