diff mbox

[3/4] arm64: add support for i.MX8M EVK board

Message ID 20180117183244.28303-3-l.stach@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Lucas Stach Jan. 17, 2018, 6:32 p.m. UTC
This is the evaluation kit board for the i.MX8M. The current level of
support yields a working console and is able to boot userspace from
SD card or Network.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/nxp/Makefile       |   3 +
 arch/arm64/boot/dts/nxp/imx8mq-evk.dts | 287 +++++++++++++++++++++++++++++++++
 2 files changed, 290 insertions(+)
 create mode 100644 arch/arm64/boot/dts/nxp/Makefile
 create mode 100644 arch/arm64/boot/dts/nxp/imx8mq-evk.dts

Comments

Fabio Estevam Jan. 22, 2018, 11:58 a.m. UTC | #1
On Wed, Jan 17, 2018 at 4:32 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> This is the evaluation kit board for the i.MX8M. The current level of
> support yields a working console and is able to boot userspace from
> SD card or Network.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
Shawn Guo Jan. 23, 2018, 10:39 a.m. UTC | #2
On Wed, Jan 17, 2018 at 07:32:43PM +0100, Lucas Stach wrote:

> +	pinctrl_usdhc2_100mhz: usdhc2grp100mhz {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK			0x85
> +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD			0xc5
> +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0		0xc5
> +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1		0xc5
> +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2		0xc5
> +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3		0xc5
> +			MX8MQ_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0xc1
> +		>;
> +		};

Bad indentation.

Shawn

> +
> +	pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK			0x87
> +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD			0xc7
> +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0		0xc7
> +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1		0xc7
> +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2		0xc7
> +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3		0xc7
> +			MX8MQ_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0xc1
> +		>;
> +	};
> +};
> -- 
> 2.11.0
>
Baruch Siach Jan. 24, 2018, 2:47 p.m. UTC | #3
Hi Lucas,

On Wed, Jan 17, 2018 at 07:32:43PM +0100, Lucas Stach wrote:
> This is the evaluation kit board for the i.MX8M. The current level of
> support yields a working console and is able to boot userspace from
> SD card or Network.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  arch/arm64/boot/dts/nxp/Makefile       |   3 +
>  arch/arm64/boot/dts/nxp/imx8mq-evk.dts | 287 +++++++++++++++++++++++++++++++++
>  2 files changed, 290 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/nxp/Makefile
>  create mode 100644 arch/arm64/boot/dts/nxp/imx8mq-evk.dts
> 
> diff --git a/arch/arm64/boot/dts/nxp/Makefile b/arch/arm64/boot/dts/nxp/Makefile
> new file mode 100644
> index 000000000000..1aebb1f16708
> --- /dev/null
> +++ b/arch/arm64/boot/dts/nxp/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +dtb-$(CONFIG_SOC_IMX8MQ) += imx8m-evk.dtb

Typo, should be imx8mq-evk.dtb. Without this fix I'm getting:

make[2]: *** No rule to make target 'arch/arm64/boot/dts/nxp/imx8m-evk.dtb', needed by '__build'.  Stop.

baruch
Baruch Siach Jan. 25, 2018, 9:52 a.m. UTC | #4
Hi Lucas,

On Wed, Jan 24, 2018 at 04:47:51PM +0200, Baruch Siach wrote:
> On Wed, Jan 17, 2018 at 07:32:43PM +0100, Lucas Stach wrote:
> > This is the evaluation kit board for the i.MX8M. The current level of
> > support yields a working console and is able to boot userspace from
> > SD card or Network.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  arch/arm64/boot/dts/nxp/Makefile       |   3 +
> >  arch/arm64/boot/dts/nxp/imx8mq-evk.dts | 287 +++++++++++++++++++++++++++++++++
> >  2 files changed, 290 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/nxp/Makefile
> >  create mode 100644 arch/arm64/boot/dts/nxp/imx8mq-evk.dts
> > 
> > diff --git a/arch/arm64/boot/dts/nxp/Makefile b/arch/arm64/boot/dts/nxp/Makefile
> > new file mode 100644
> > index 000000000000..1aebb1f16708
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/nxp/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +dtb-$(CONFIG_SOC_IMX8MQ) += imx8m-evk.dtb
> 
> Typo, should be imx8mq-evk.dtb. Without this fix I'm getting:
> 
> make[2]: *** No rule to make target 'arch/arm64/boot/dts/nxp/imx8m-evk.dtb', needed by '__build'.  Stop.

With this fix and CONFIG_CPU_IDLE disabled:

Tested-by: Baruch Siach <baruch@tkos.co.il>

On i.MX8MQ EVK rev B3.

Thanks,
baruch
aisheng.dong@codeaurora.org Jan. 25, 2018, 10:10 a.m. UTC | #5
On 2018-01-23 18:39, Shawn Guo wrote:
> On Wed, Jan 17, 2018 at 07:32:43PM +0100, Lucas Stach wrote:
> 
>> +	pinctrl_usdhc2_100mhz: usdhc2grp100mhz {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK			0x85
>> +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD			0xc5
>> +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0		0xc5
>> +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1		0xc5
>> +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2		0xc5
>> +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3		0xc5
>> +			MX8MQ_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0xc1
>> +		>;
>> +		};
> 
> Bad indentation.
> 
> Shawn
> 
>> +
>> +	pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK			0x87
>> +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD			0xc7
>> +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0		0xc7
>> +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1		0xc7
>> +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2		0xc7
>> +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3		0xc7
>> +			MX8MQ_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0xc1
>> +		>;
>> +	};

AFAIK we switched to generic pinconfig since MX7ULP as maintainer won't
access old binding pinctrl drivers.

So i wonder if you may have some special reasons to insist on old 
binding which
i may overlooked?

(I did not receive your pinctrl patch by accidently. So i can't reply on
it before.)

Regards
Dong Aisheng

>> +};
>> --
>> 2.11.0
>> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Lucas Stach Jan. 25, 2018, 10:31 a.m. UTC | #6
Am Donnerstag, den 25.01.2018, 18:10 +0800 schrieb aisheng.dong@codeaurora.org:
> On 2018-01-23 18:39, Shawn Guo wrote:
> > On Wed, Jan 17, 2018 at 07:32:43PM +0100, Lucas Stach wrote:
> > 
> > > > > > +	pinctrl_usdhc2_100mhz: usdhc2grp100mhz {
> > > > > > +		fsl,pins = <
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK			0x85
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD			0xc5
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0		0xc5
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1		0xc5
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2		0xc5
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3		0xc5
> > > > > > > > > +			MX8MQ_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0xc1
> > > > > > +		>;
> > > +		};
> > 
> > Bad indentation.
> > 
> > Shawn
> > 
> > > +
> > > > > > +	pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
> > > > > > +		fsl,pins = <
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK			0x87
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD			0xc7
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0		0xc7
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1		0xc7
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2		0xc7
> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3		0xc7
> > > > > > > > > +			MX8MQ_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0xc1
> > > > > > +		>;
> > > +	};
> 
> AFAIK we switched to generic pinconfig since MX7ULP as maintainer won't
> access old binding pinctrl drivers.

I'm not convinced that the generic pinconf is good fit. For pingroups
with different configs for some of the pins, like the example above, we
would need to split things into multiple DT nodes. This really hurts
readability, so I'm not going to switch to the generic stuff without
some really convincing arguments.

Regards,
Lucas
aisheng.dong@codeaurora.org Jan. 25, 2018, 10:49 a.m. UTC | #7
On 2018-01-25 18:31, Lucas Stach wrote:
> Am Donnerstag, den 25.01.2018, 18:10 +0800 schrieb 
> aisheng.dong@codeaurora.org:
>> On 2018-01-23 18:39, Shawn Guo wrote:
>> > On Wed, Jan 17, 2018 at 07:32:43PM +0100, Lucas Stach wrote:
>> >
>> > > > > > +	pinctrl_usdhc2_100mhz: usdhc2grp100mhz {
>> > > > > > +		fsl,pins = <
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK			0x85
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD			0xc5
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0		0xc5
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1		0xc5
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2		0xc5
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3		0xc5
>> > > > > > > > > +			MX8MQ_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0xc1
>> > > > > > +		>;
>> > > +		};
>> >
>> > Bad indentation.
>> >
>> > Shawn
>> >
>> > > +
>> > > > > > +	pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
>> > > > > > +		fsl,pins = <
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK			0x87
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD			0xc7
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0		0xc7
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1		0xc7
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2		0xc7
>> > > > > > > > > +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3		0xc7
>> > > > > > > > > +			MX8MQ_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0xc1
>> > > > > > +		>;
>> > > +	};
>> 
>> AFAIK we switched to generic pinconfig since MX7ULP as maintainer 
>> won't
>> access old binding pinctrl drivers.
> 
> I'm not convinced that the generic pinconf is good fit. For pingroups
> with different configs for some of the pins, like the example above, we
> would need to split things into multiple DT nodes. This really hurts
> readability, so I'm not going to switch to the generic stuff without
> some really convincing arguments.
> 

Per my understanding, based on the last discussion with Linus W, we 
actually
did this in order to increase the readability that 1) user does not need 
to
see the 'ugly' unreadable raw data and refer to reference manual 2) 
unified
generic binding format which already exist in kernel and used by many
platforms.

Actually MXS platform already used it for many years in a similar way.

So IMHO a little hurt to add another node for different pad setting in 
the
same group won't be enough reason to stop switching to generic config.

Does it make sense?

Regards
Dong Aisheng

> Regards,
> Lucas
Lucas Stach Jan. 25, 2018, 11:09 a.m. UTC | #8
Am Donnerstag, den 25.01.2018, 18:49 +0800 schrieb Dong Aisheng:
> On 2018-01-25 18:31, Lucas Stach wrote:
> > Am Donnerstag, den 25.01.2018, 18:10 +0800 schrieb 
> > aisheng.dong@codeaurora.org:
[...]
> AFAIK we switched to generic pinconfig since MX7ULP as maintainer 
> > > won't
> > > access old binding pinctrl drivers.
> > 
> > I'm not convinced that the generic pinconf is good fit. For pingroups
> > with different configs for some of the pins, like the example above, we
> > would need to split things into multiple DT nodes. This really hurts
> > readability, so I'm not going to switch to the generic stuff without
> > some really convincing arguments.
> > 
> 
> Per my understanding, based on the last discussion with Linus W, we 
> actually did this in order to increase the readability that 1) user does
> not need to see the 'ugly' unreadable raw data and refer to reference
> manual 2) unified generic binding format which already exist in kernel
> and used by many platforms.
> 
> Actually MXS platform already used it for many years in a similar way.
> So IMHO a little hurt to add another node for different pad setting in 
> the same group won't be enough reason to stop switching to generic config.
> 
> Does it make sense?

I know that Linus W is pushing for this common pinconf thing in the
name of readability. It's just that I don't think it's such a clear
win.

After all you still need to look into the reference manual or binding
to see which values in the common binding correspond to a specific
drive/pull strength, etc.

On the other hand it really bloats the DT description of the pin
configuration. If you want to look at an (IMHO) bad example, go look at
the Tegra DTs. The Tegra pincontrol implements the "separate properties
for each pinconf option" that is pushed by Linus W. This bloated the DT
description to the point that no-one is able/willing to write those
descriptions anymore and the only viable way to get them is to auto-
generate them from some spreadsheets. Not really what I would call an
readable...

Maybe I'm a little stubborn when it comes to this topic, but at
Pengutronix we see a lot of customer designs where we need to come up
with the board DT. Bloating each one of those and making the work of
the developers harder in the name of a readability win that I just
don't see doesn't sound like something I want to support. :)
	
Regards,
Lucas
aisheng.dong@codeaurora.org Jan. 25, 2018, 1:03 p.m. UTC | #9
On 2018-01-25 19:09, Lucas Stach wrote:
> Am Donnerstag, den 25.01.2018, 18:49 +0800 schrieb Dong Aisheng:
>> On 2018-01-25 18:31, Lucas Stach wrote:
>> > Am Donnerstag, den 25.01.2018, 18:10 +0800 schrieb 
>> > aisheng.dong@codeaurora.org:
> [...]
>> AFAIK we switched to generic pinconfig since MX7ULP as maintainer 
>> > > won't
>> > > access old binding pinctrl drivers.
>> >
>> > I'm not convinced that the generic pinconf is good fit. For pingroups
>> > with different configs for some of the pins, like the example above, we
>> > would need to split things into multiple DT nodes. This really hurts
>> > readability, so I'm not going to switch to the generic stuff without
>> > some really convincing arguments.
>> >
>> 
>> Per my understanding, based on the last discussion with Linus W, we 
>> actually did this in order to increase the readability that 1) user 
>> does
>> not need to see the 'ugly' unreadable raw data and refer to reference
>> manual 2) unified generic binding format which already exist in kernel
>> and used by many platforms.
>> 
>> Actually MXS platform already used it for many years in a similar way.
>> So IMHO a little hurt to add another node for different pad setting 
>> in 
>> the same group won't be enough reason to stop switching to generic 
>> config.
>> 
>> Does it make sense?
> 
> I know that Linus W is pushing for this common pinconf thing in the
> name of readability. It's just that I don't think it's such a clear
> win.
> 
> After all you still need to look into the reference manual or binding
> to see which values in the common binding correspond to a specific
> drive/pull strength, etc.
> 

User don't need to look into reference manual and they don't need to
compose the 'ulgy' raw data which is the most tough thing.

With generic binding, it probably can saving ~80% pad setting effort
by refer to the defined generic config properties.
And things can be even better when the reference code is already there
as user becomes know which property supported.

> On the other hand it really bloats the DT description of the pin
> configuration. If you want to look at an (IMHO) bad example, go look at
> the Tegra DTs. The Tegra pincontrol implements the "separate properties
> for each pinconf option" that is pushed by Linus W. This bloated the DT
> description to the point that no-one is able/willing to write those
> descriptions anymore and the only viable way to get them is to auto-
> generate them from some spreadsheets. Not really what I would call an
> readable...
> 

I wonder the worst case you're worrying whether exist in reality.
Take imx6qdl-sabresd as an example, about half of pingroups having the 
same
pad setting while others have two different settings at most.
That means it may not bloat the device tree too much.

> Maybe I'm a little stubborn when it comes to this topic, but at
> Pengutronix we see a lot of customer designs where we need to come up
> with the board DT. Bloating each one of those and making the work of
> the developers harder in the name of a readability win that I just
> don't see doesn't sound like something I want to support. :)
> 

Hmm.. In contrast, what i feel currently is that it may ease the using 
of
pad setting, not make it harder. Not sure if i overlooked something.

Let's listen to Shawn and Linus W if they have some comments.

Regards
Dong Aisheng

> Regards,
> Lucas
Lucas Stach Jan. 25, 2018, 6:03 p.m. UTC | #10
Am Donnerstag, den 25.01.2018, 21:03 +0800 schrieb Dong Aisheng:
> On 2018-01-25 19:09, Lucas Stach wrote:
> > Am Donnerstag, den 25.01.2018, 18:49 +0800 schrieb Dong Aisheng:
> > > On 2018-01-25 18:31, Lucas Stach wrote:
> > > > Am Donnerstag, den 25.01.2018, 18:10 +0800 schrieb 
> > > > aisheng.dong@codeaurora.org:
> > 
> > [...]
> > > AFAIK we switched to generic pinconfig since MX7ULP as
> > > maintainer 
> > > > > won't
> > > > > access old binding pinctrl drivers.
> > > > 
> > > > I'm not convinced that the generic pinconf is good fit. For
> > > > pingroups
> > > > with different configs for some of the pins, like the example
> > > > above, we
> > > > would need to split things into multiple DT nodes. This really
> > > > hurts
> > > > readability, so I'm not going to switch to the generic stuff
> > > > without
> > > > some really convincing arguments.
> > > > 
> > > 
> > > Per my understanding, based on the last discussion with Linus W,
> > > we 
> > > actually did this in order to increase the readability that 1)
> > > user 
> > > does
> > > not need to see the 'ugly' unreadable raw data and refer to
> > > reference
> > > manual 2) unified generic binding format which already exist in
> > > kernel
> > > and used by many platforms.
> > > 
> > > Actually MXS platform already used it for many years in a similar
> > > way.
> > > So IMHO a little hurt to add another node for different pad
> > > setting 
> > > in 
> > > the same group won't be enough reason to stop switching to
> > > generic 
> > > config.
> > > 
> > > Does it make sense?
> > 
> > I know that Linus W is pushing for this common pinconf thing in the
> > name of readability. It's just that I don't think it's such a clear
> > win.
> > 
> > After all you still need to look into the reference manual or
> > binding
> > to see which values in the common binding correspond to a specific
> > drive/pull strength, etc.
> > 
> 
> User don't need to look into reference manual and they don't need to
> compose the 'ulgy' raw data which is the most tough thing.
> 
> With generic binding, it probably can saving ~80% pad setting effort
> by refer to the defined generic config properties.
> And things can be even better when the reference code is already
> there
> as user becomes know which property supported.
> 
> > On the other hand it really bloats the DT description of the pin
> > configuration. If you want to look at an (IMHO) bad example, go
> > look at
> > the Tegra DTs. The Tegra pincontrol implements the "separate
> > properties
> > for each pinconf option" that is pushed by Linus W. This bloated
> > the DT
> > description to the point that no-one is able/willing to write those
> > descriptions anymore and the only viable way to get them is to
> > auto-
> > generate them from some spreadsheets. Not really what I would call
> > an
> > readable...
> > 
> 
> I wonder the worst case you're worrying whether exist in reality.
> Take imx6qdl-sabresd as an example, about half of pingroups having
> the 
> same
> pad setting while others have two different settings at most.
> That means it may not bloat the device tree too much.
> 
> > Maybe I'm a little stubborn when it comes to this topic, but at
> > Pengutronix we see a lot of customer designs where we need to come
> > up
> > with the board DT. Bloating each one of those and making the work
> > of
> > the developers harder in the name of a readability win that I just
> > don't see doesn't sound like something I want to support. :)
> > 
> 
> Hmm.. In contrast, what i feel currently is that it may ease the
> using of pad setting, not make it harder. Not sure if i overlooked
> something.
> 
> Let's listen to Shawn and Linus W if they have some comments.

While I'm still unconvinced that the generic binding actually adds any
value, I'll go and see how converting the MX8M pinctrl will look on a
real board.

This will delay rev 2 of this series a bit, but I hope to have
something to send out tomorrow.

Regards,
Lucas
Aisheng Dong Jan. 26, 2018, 3:06 a.m. UTC | #11
> -----Original Message-----

> From: Lucas Stach [mailto:l.stach@pengutronix.de]

> Sent: Friday, January 26, 2018 2:03 AM

> To: Dong Aisheng <aisheng.dong@codeaurora.org>; Linus Walleij

> <linus.walleij@linaro.org>

> Cc: Shawn Guo <shawnguo@kernel.org>; Mark Rutland

> <mark.rutland@arm.com>; devicetree@vger.kernel.org; Catalin Marinas

> <catalin.marinas@arm.com>; Will Deacon <will.deacon@arm.com>;

> patchwork-lst@pengutronix.de; Rob Herring <robh+dt@kernel.org>;

> kernel@pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>; linux-

> arm-kernel@lists.infradead.org; A.s. Dong <aisheng.dong@nxp.com>; dl-linux-

> imx <linux-imx@nxp.com>

> Subject: Re: [PATCH 3/4] arm64: add support for i.MX8M EVK board

> 

> Am Donnerstag, den 25.01.2018, 21:03 +0800 schrieb Dong Aisheng:

> > On 2018-01-25 19:09, Lucas Stach wrote:

> > > Am Donnerstag, den 25.01.2018, 18:49 +0800 schrieb Dong Aisheng:

> > > > On 2018-01-25 18:31, Lucas Stach wrote:

> > > > > Am Donnerstag, den 25.01.2018, 18:10 +0800 schrieb

> > > > > aisheng.dong@codeaurora.org:

> > >

> > > [...]

> > > > AFAIK we switched to generic pinconfig since MX7ULP as maintainer

> > > > > > won't

> > > > > > access old binding pinctrl drivers.

> > > > >

> > > > > I'm not convinced that the generic pinconf is good fit. For

> > > > > pingroups with different configs for some of the pins, like the

> > > > > example above, we would need to split things into multiple DT

> > > > > nodes. This really hurts readability, so I'm not going to switch

> > > > > to the generic stuff without some really convincing arguments.

> > > > >

> > > >

> > > > Per my understanding, based on the last discussion with Linus W,

> > > > we actually did this in order to increase the readability that 1)

> > > > user does not need to see the 'ugly' unreadable raw data and refer

> > > > to reference manual 2) unified generic binding format which

> > > > already exist in kernel and used by many platforms.

> > > >

> > > > Actually MXS platform already used it for many years in a similar

> > > > way.

> > > > So IMHO a little hurt to add another node for different pad

> > > > setting in the same group won't be enough reason to stop switching

> > > > to generic config.

> > > >

> > > > Does it make sense?

> > >

> > > I know that Linus W is pushing for this common pinconf thing in the

> > > name of readability. It's just that I don't think it's such a clear

> > > win.

> > >

> > > After all you still need to look into the reference manual or

> > > binding to see which values in the common binding correspond to a

> > > specific drive/pull strength, etc.

> > >

> >

> > User don't need to look into reference manual and they don't need to

> > compose the 'ulgy' raw data which is the most tough thing.

> >

> > With generic binding, it probably can saving ~80% pad setting effort

> > by refer to the defined generic config properties.

> > And things can be even better when the reference code is already there

> > as user becomes know which property supported.

> >

> > > On the other hand it really bloats the DT description of the pin

> > > configuration. If you want to look at an (IMHO) bad example, go look

> > > at the Tegra DTs. The Tegra pincontrol implements the "separate

> > > properties for each pinconf option" that is pushed by Linus W. This

> > > bloated the DT description to the point that no-one is able/willing

> > > to write those descriptions anymore and the only viable way to get

> > > them is to

> > > auto-

> > > generate them from some spreadsheets. Not really what I would call

> > > an readable...

> > >

> >

> > I wonder the worst case you're worrying whether exist in reality.

> > Take imx6qdl-sabresd as an example, about half of pingroups having the

> > same pad setting while others have two different settings at most.

> > That means it may not bloat the device tree too much.

> >

> > > Maybe I'm a little stubborn when it comes to this topic, but at

> > > Pengutronix we see a lot of customer designs where we need to come

> > > up with the board DT. Bloating each one of those and making the work

> > > of the developers harder in the name of a readability win that I

> > > just don't see doesn't sound like something I want to support. :)

> > >

> >

> > Hmm.. In contrast, what i feel currently is that it may ease the using

> > of pad setting, not make it harder. Not sure if i overlooked

> > something.

> >

> > Let's listen to Shawn and Linus W if they have some comments.

> 

> While I'm still unconvinced that the generic binding actually adds any value, I'll

> go and see how converting the MX8M pinctrl will look on a real board.

> 


You can refer to what we've done for MX7ULP.

> This will delay rev 2 of this series a bit, but I hope to have something to send

> out tomorrow.

> 


Sorry for making the trouble. 

Regards
Dong Aisheng

> Regards,

> Lucas
Rob Herring (Arm) Jan. 29, 2018, 5:52 p.m. UTC | #12
On Wed, Jan 17, 2018 at 07:32:43PM +0100, Lucas Stach wrote:
> This is the evaluation kit board for the i.MX8M. The current level of
> support yields a working console and is able to boot userspace from
> SD card or Network.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  arch/arm64/boot/dts/nxp/Makefile       |   3 +
>  arch/arm64/boot/dts/nxp/imx8mq-evk.dts | 287 +++++++++++++++++++++++++++++++++
>  2 files changed, 290 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/nxp/Makefile
>  create mode 100644 arch/arm64/boot/dts/nxp/imx8mq-evk.dts
> 
> diff --git a/arch/arm64/boot/dts/nxp/Makefile b/arch/arm64/boot/dts/nxp/Makefile
> new file mode 100644
> index 000000000000..1aebb1f16708
> --- /dev/null
> +++ b/arch/arm64/boot/dts/nxp/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +dtb-$(CONFIG_SOC_IMX8MQ) += imx8m-evk.dtb
> diff --git a/arch/arm64/boot/dts/nxp/imx8mq-evk.dts b/arch/arm64/boot/dts/nxp/imx8mq-evk.dts
> new file mode 100644
> index 000000000000..ff77fdbcbfe3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/nxp/imx8mq-evk.dts
> @@ -0,0 +1,287 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +
> +/dts-v1/;
> +
> +#include "imx8mq.dtsi"
> +
> +/ {
> +	model = "NXP i.MX8MQ EVK";
> +	compatible = "nxp,imx8mq-evk", "nxp,imx8mq";

Documentation?

> +
> +	chosen {
> +		stdout-path = &uart1;
> +	};
> +
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0x00000000 0x40000000 0 0xc0000000>;
> +	};
> +
> +	reg_usdhc2_vmmc: usdhc2_vmmc {

Don't use '_'. Also, ideally this would follow generic node name 
convention, but we don't have anything defined and regulator binding is 
a bit odd. "regulator-3V3" perhaps.

> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_reg_usdhc2>;
> +		compatible = "regulator-fixed";
> +		regulator-name = "VSD_3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +};
> +
> +&fec1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_fec1>;
> +	phy-mode = "rgmii-id";
> +	status = "okay";
> +};
> +
> +&i2c1 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	status = "okay";
> +
> +	pfuze100@8 {

pmic@8

Rob
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/nxp/Makefile b/arch/arm64/boot/dts/nxp/Makefile
new file mode 100644
index 000000000000..1aebb1f16708
--- /dev/null
+++ b/arch/arm64/boot/dts/nxp/Makefile
@@ -0,0 +1,3 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+dtb-$(CONFIG_SOC_IMX8MQ) += imx8m-evk.dtb
diff --git a/arch/arm64/boot/dts/nxp/imx8mq-evk.dts b/arch/arm64/boot/dts/nxp/imx8mq-evk.dts
new file mode 100644
index 000000000000..ff77fdbcbfe3
--- /dev/null
+++ b/arch/arm64/boot/dts/nxp/imx8mq-evk.dts
@@ -0,0 +1,287 @@ 
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+
+/dts-v1/;
+
+#include "imx8mq.dtsi"
+
+/ {
+	model = "NXP i.MX8MQ EVK";
+	compatible = "nxp,imx8mq-evk", "nxp,imx8mq";
+
+	chosen {
+		stdout-path = &uart1;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x00000000 0x40000000 0 0xc0000000>;
+	};
+
+	reg_usdhc2_vmmc: usdhc2_vmmc {
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_reg_usdhc2>;
+		compatible = "regulator-fixed";
+		regulator-name = "VSD_3V3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+};
+
+&fec1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_fec1>;
+	phy-mode = "rgmii-id";
+	status = "okay";
+};
+
+&i2c1 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c1>;
+	status = "okay";
+
+	pfuze100@8 {
+		compatible = "fsl,pfuze100";
+		reg = <0x8>;
+
+		regulators {
+			sw1a_reg: sw1ab {
+				regulator-min-microvolt = <825000>;
+				regulator-max-microvolt = <1100000>;
+			};
+
+			sw1c_reg: sw1c {
+				regulator-min-microvolt = <825000>;
+				regulator-max-microvolt = <1100000>;
+			};
+
+			sw2_reg: sw2 {
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-always-on;
+			};
+
+			sw3a_reg: sw3ab {
+				regulator-min-microvolt = <825000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-always-on;
+			};
+
+			sw4_reg: sw4 {
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+			};
+
+			swbst_reg: swbst {
+				regulator-min-microvolt = <5000000>;
+				regulator-max-microvolt = <5150000>;
+			};
+
+			snvs_reg: vsnvs {
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-always-on;
+			};
+
+			vref_reg: vrefddr {
+				regulator-always-on;
+			};
+
+			vgen1_reg: vgen1 {
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <1550000>;
+			};
+
+			vgen2_reg: vgen2 {
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <975000>;
+				regulator-always-on;
+			};
+
+			vgen3_reg: vgen3 {
+				regulator-min-microvolt = <1675000>;
+				regulator-max-microvolt = <1975000>;
+				regulator-always-on;
+			};
+
+			vgen4_reg: vgen4 {
+				regulator-min-microvolt = <1625000>;
+				regulator-max-microvolt = <1875000>;
+				regulator-always-on;
+			};
+
+			vgen5_reg: vgen5 {
+				regulator-min-microvolt = <3075000>;
+				regulator-max-microvolt = <3625000>;
+				regulator-always-on;
+			};
+
+			vgen6_reg: vgen6 {
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+			};
+		};
+	};
+};
+
+&uart1 {
+	status = "okay";
+};
+
+&usdhc1 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc1>;
+	pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc1_200mhz>;
+	vqmmc-supply = <&sw4_reg>;
+	bus-width = <8>;
+	non-removable;
+	no-sd;
+	no-sdio;
+	status = "okay";
+};
+
+&usdhc2 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc2>;
+	pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc2_200mhz>;
+	cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
+	vmmc-supply = <&reg_usdhc2_vmmc>;
+	status = "okay";
+};
+
+&iomuxc {
+	pinctrl_fec1: fec1grp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_ENET_MDC_ENET1_MDC			0x3
+			MX8MQ_IOMUXC_ENET_MDIO_ENET1_MDIO		0x23
+			MX8MQ_IOMUXC_ENET_TD3_ENET1_RGMII_TD3		0x1f
+			MX8MQ_IOMUXC_ENET_TD2_ENET1_RGMII_TD2		0x1f
+			MX8MQ_IOMUXC_ENET_TD1_ENET1_RGMII_TD1		0x1f
+			MX8MQ_IOMUXC_ENET_TD0_ENET1_RGMII_TD0		0x1f
+			MX8MQ_IOMUXC_ENET_RD3_ENET1_RGMII_RD3		0x91
+			MX8MQ_IOMUXC_ENET_RD2_ENET1_RGMII_RD2		0x91
+			MX8MQ_IOMUXC_ENET_RD1_ENET1_RGMII_RD1		0x91
+			MX8MQ_IOMUXC_ENET_RD0_ENET1_RGMII_RD0		0x91
+			MX8MQ_IOMUXC_ENET_TXC_ENET1_RGMII_TXC		0x1f
+			MX8MQ_IOMUXC_ENET_RXC_ENET1_RGMII_RXC		0x91
+			MX8MQ_IOMUXC_ENET_RX_CTL_ENET1_RGMII_RX_CTL	0x91
+			MX8MQ_IOMUXC_ENET_TX_CTL_ENET1_RGMII_TX_CTL	0x1f
+			MX8MQ_IOMUXC_GPIO1_IO09_GPIO1_IO9		0x19
+		>;
+	};
+
+	pinctrl_i2c1: i2c1grp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_I2C1_SCL_I2C1_SCL			0x4000007f
+			MX8MQ_IOMUXC_I2C1_SDA_I2C1_SDA			0x4000007f
+		>;
+	};
+
+	pinctrl_reg_usdhc2: regusdhc2grpgpio {
+		fsl,pins = <
+			MX8MQ_IOMUXC_SD2_RESET_B_GPIO2_IO19		0x41
+		>;
+	};
+
+	pinctrl_uart1: uart1grp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_UART1_RXD_UART1_DCE_RX		0x49
+			MX8MQ_IOMUXC_UART1_TXD_UART1_DCE_TX		0x49
+		>;
+	};
+
+	pinctrl_usdhc1: usdhc1grp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK			0x83
+			MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD			0xc3
+			MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0		0xc3
+			MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1		0xc3
+			MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2		0xc3
+			MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3		0xc3
+			MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4		0xc3
+			MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5		0xc3
+			MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6		0xc3
+			MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7		0xc3
+			MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE		0x83
+			MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B		0xc1
+			MX8MQ_IOMUXC_SD2_CD_B_GPIO2_IO12		0x41
+		>;
+	};
+
+	pinctrl_usdhc1_100mhz: usdhc1grp100mhz {
+		fsl,pins = <
+			MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK			0x85
+			MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD			0xc5
+			MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0		0xc5
+			MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1		0xc5
+			MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2		0xc5
+			MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3		0xc5
+			MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4		0xc5
+			MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5		0xc5
+			MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6		0xc5
+			MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7		0xc5
+			MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE		0x85
+			MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B		0xc1
+			MX8MQ_IOMUXC_SD2_CD_B_GPIO2_IO12		0x41
+		>;
+	};
+
+	pinctrl_usdhc1_200mhz: usdhc1grp200mhz {
+		fsl,pins = <
+			MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK			0x87
+			MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD			0xc7
+			MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0		0xc7
+			MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1		0xc7
+			MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2		0xc7
+			MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3		0xc7
+			MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4		0xc7
+			MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5		0xc7
+			MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6		0xc7
+			MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7		0xc7
+			MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE		0x87
+			MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B		0xc1
+			MX8MQ_IOMUXC_SD2_CD_B_GPIO2_IO12		0x41
+		>;
+	};
+
+	pinctrl_usdhc2: usdhc2grp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK			0x83
+			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD			0xc3
+			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0		0xc3
+			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1		0xc3
+			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2		0xc3
+			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3		0xc3
+			MX8MQ_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0xc1
+		>;
+	};
+
+	pinctrl_usdhc2_100mhz: usdhc2grp100mhz {
+		fsl,pins = <
+			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK			0x85
+			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD			0xc5
+			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0		0xc5
+			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1		0xc5
+			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2		0xc5
+			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3		0xc5
+			MX8MQ_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0xc1
+		>;
+		};
+
+	pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
+		fsl,pins = <
+			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK			0x87
+			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD			0xc7
+			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0		0xc7
+			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1		0xc7
+			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2		0xc7
+			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3		0xc7
+			MX8MQ_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0xc1
+		>;
+	};
+};