Re: [PATCH] imx: imx8mm: Add suppoer for Mettler-Toledo snowflake board.
On 2023-03-16 11:33:47 [+0100], Frieder Schrempf wrote:
> > diff --git a/arch/arm/dts/imx8mm-mt-snowflake-v2.dts
> > b/arch/arm/dts/imx8mm-mt-snowflake-v2.dts
> > new file mode 100644
> > index 0..49761b22afcf0
> > --- /dev/null
> > +++ b/arch/arm/dts/imx8mm-mt-snowflake-v2.dts
> > @@ -0,0 +1,186 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 Mettler Toledo GmbH
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "imx8mm-kontron-sl.dtsi"
> > +#include "imx8mm-u-boot.dtsi"
> > +
> > +/ {
> > + model = "Mettler Toledo i.MX8MM Snowflake V2";
> > + compatible = "mt,imx8mm-snowflake-v2", "fsl,imx8mm";
>
> I think the compatible would normally include the SoM:
>
> compatible = "mt,imx8mm-snowflake-v2", "kontron,imx8mm-sl", "fsl,imx8mm";
This might work. Let me add it.
…
> > +
> > + {
> > + clock-frequency = <40>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <_i2c1>;
> > + status = "okay";
> > +
> > + pca9450: pmic@25 {
…
> > +};
>
> The included SoM DTSI already contains the PMIC node. You can remove
> that here. Or if you need to do adjustments for the board level you
> should only overwrite what is needed instead of duplicating everything.
They appear to identical except for the regulator-name but this does not
matter. Dropping that then.
…
> > + pinctrl_i2c1: i2c1grp {
> > + fsl,pins = <
> > + MX8MM_IOMUXC_I2C1_SCL_I2C1_SCL
> > 0x41c3
> > + MX8MM_IOMUXC_I2C1_SDA_I2C1_SDA
> > 0x41c3
> > + >;
> > + };
>
> Same here, this node is already available from the SoM DTSI.
gone.
…
> > diff --git a/board/mt/snowflake_v2/spl.c b/board/mt/snowflake_v2/spl.c
> > new file mode 100644
> > index 0..3e27256914b32
> > --- /dev/null
> > +++ b/board/mt/snowflake_v2/spl.c
…
> > + dram_timing.fsp_msg[0].fsp_cfg[9].val = 0x110;
> > + dram_timing.fsp_msg[0].fsp_cfg[21].val = 0x1;
> > + dram_timing.fsp_msg[1].fsp_cfg[10].val = 0x110;
> > + dram_timing.fsp_msg[1].fsp_cfg[22].val = 0x1;
> > + dram_timing.fsp_msg[2].fsp_cfg[10].val = 0x110;
> > + dram_timing.fsp_msg[2].fsp_cfg[22].val = 0x1;
> > + dram_timing.fsp_msg[3].fsp_cfg[10].val = 0x110;
> > + dram_timing.fsp_msg[3].fsp_cfg[22].val = 0x1;
>
> As you only support the 1GB DDR on this board, you could integrate these
> values directly in the tables in lpddr4_timing.c and remove them here.
I'm not sure how much of the header file is auto-generated including
these bits here.
Let me merge this now and worry later.
> > +
> > + if (!ddr_init(_timing) && check_ram_available(SZ_1G)) {
> > + size = 1;
> > + puts("1GB DDR RAM\n");
> > + } else {
> > + puts("Init DDR RAM failed\n");
> > + size = 1;
> > + }
> > +
> > + writel(size, M4_BOOTROM_BASE_ADDR);
>
> You could also simplify this and improve the error handling for your case:
>
> if (ddr_init(_timing) || !check_ram_available(SZ_1G)) {
> puts("Init DDR RAM failed\n");
> halt();
> }
>
> puts("1GB DDR RAM\n");
> writel(1, M4_BOOTROM_BASE_ADDR);
Why not. There is not "halt()" in tree so I'm going to substite that
block with panic().
…
> > diff --git a/include/configs/snowflake_v2.h b/include/configs/snowflake_v2.h
> > new file mode 100644
> > index 0..f038dc9e6d7d8
> > --- /dev/null
> > +++ b/include/configs/snowflake_v2.h
> > @@ -0,0 +1,54 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (C) 2019 Kontron Electronics GmbH
> > + * Copyright (C) 2020 Mettler Toledo GmbH
> > + *
> > + * Configuration settings for the MT Snowflake v2 Terminal, based on
> > Kontron N8000 i.MX8MM SMARC module.
>
> It's not a SMARC module. And nowadays the proper product name is
> "Kontron SL i.MX8M Mini".
okay.
That should have been all of your comments.
Sebastian
Re: [PATCH] imx: imx8mm: Add suppoer for Mettler-Toledo snowflake board.
On 16.03.23 11:33, Frieder Schrempf wrote: > On 10.03.23 14:50, Sebastian Andrzej Siewior wrote: >> From: Manuel Traut >> >> The board is similar to Kontron's N801x design. It lacks external eMMC >> support and only supports only 1GiB of main memory. > > Good to see some custom boards being upstreamed! > > It would be worth mentioning here that it is based on the "Kontron SL > i.MX8M Mini" SoM, which is the proper name of the product nowadays. > > What do you mean by "it lacks external eMMC support"? > There is an eMMC on the SoM, so the hardware does support it. Or did I misunderstand the snowflake hardware and it actually doesn't have the SoM onboard but is designed like the SoM?
Re: [PATCH] imx: imx8mm: Add suppoer for Mettler-Toledo snowflake board.
On 10.03.23 14:50, Sebastian Andrzej Siewior wrote:
> From: Manuel Traut
>
> The board is similar to Kontron's N801x design. It lacks external eMMC
> support and only supports only 1GiB of main memory.
Good to see some custom boards being upstreamed!
It would be worth mentioning here that it is based on the "Kontron SL
i.MX8M Mini" SoM, which is the proper name of the product nowadays.
What do you mean by "it lacks external eMMC support"?
There is an eMMC on the SoM, so the hardware does support it.
>
> [ bigeasy: porting and a bit of cleanup ]
>
> Signed-off-by: Manuel Traut
> Signed-off-by: Sebastian Andrzej Siewior
> ---
> arch/arm/dts/Makefile |1 +
> .../dts/imx8mm-mt-snowflake-v2-u-boot.dtsi| 116 ++
> arch/arm/dts/imx8mm-mt-snowflake-v2.dts | 186 ++
> arch/arm/mach-imx/imx8m/Kconfig |8 +
> board/mt/snowflake_v2/Kconfig | 18 +
> board/mt/snowflake_v2/MAINTAINERS |7 +
> board/mt/snowflake_v2/Makefile|9 +
> board/mt/snowflake_v2/imximage.cfg|8 +
> board/mt/snowflake_v2/lpddr4_timing.c | 1844 +
> board/mt/snowflake_v2/snowflake_v2.c | 32 +
> board/mt/snowflake_v2/snowflake_v2.env| 33 +
> board/mt/snowflake_v2/spl.c | 167 ++
> configs/snowflake_v2_emmcboot_defconfig | 134 ++
> include/configs/snowflake_v2.h| 54 +
> 14 files changed, 2617 insertions(+)
> create mode 100644 arch/arm/dts/imx8mm-mt-snowflake-v2-u-boot.dtsi
> create mode 100644 arch/arm/dts/imx8mm-mt-snowflake-v2.dts
> create mode 100644 board/mt/snowflake_v2/Kconfig
> create mode 100644 board/mt/snowflake_v2/MAINTAINERS
> create mode 100644 board/mt/snowflake_v2/Makefile
> create mode 100644 board/mt/snowflake_v2/imximage.cfg
> create mode 100644 board/mt/snowflake_v2/lpddr4_timing.c
> create mode 100644 board/mt/snowflake_v2/snowflake_v2.c
> create mode 100644 board/mt/snowflake_v2/snowflake_v2.env
> create mode 100644 board/mt/snowflake_v2/spl.c
> create mode 100644 configs/snowflake_v2_emmcboot_defconfig
> create mode 100644 include/configs/snowflake_v2.h
>
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 7a577deb5023d..ba0786064847e 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -964,6 +964,7 @@ dtb-$(CONFIG_ARCH_IMX8M) += \
> imx8mm-icore-mx8mm-edimm2.2.dtb \
> imx8mm-kontron-bl.dtb \
> imx8mm-kontron-bl-osm-s.dtb \
> + imx8mm-mt-snowflake-v2.dtb \
> imx8mm-mx8menlo.dtb \
> imx8mm-phg.dtb \
> imx8mm-venice.dtb \
> diff --git a/arch/arm/dts/imx8mm-mt-snowflake-v2-u-boot.dtsi
> b/arch/arm/dts/imx8mm-mt-snowflake-v2-u-boot.dtsi
> new file mode 100644
> index 0..d6476db292b79
> --- /dev/null
> +++ b/arch/arm/dts/imx8mm-mt-snowflake-v2-u-boot.dtsi
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Mettler Toledo GmbH
> + */
> +
> +/ {
> +
> + aliases {
> + usbgadget0 =
> + };
> +
> + usbg1: usbg1 {
> + u-boot,dm-spl;
> + compatible = "fsl,imx27-usb-gadget";
> + dr_mode = "peripheral";
> + chipidea,usb = <>;
> + status = "okay";
> + };
> +
> + firmware {
> + optee {
> + compatible = "linaro,optee-tz";
> + method = "smc";
> + };
> + };
> +};
> +
> + {
> + status = "okay";
> + u-boot,dm-spl;
> + u-boot,dm-pre-reloc;
> +};
> +
> + {
> + status = "okay";
> + u-boot,dm-spl;
> + u-boot,dm-pre-reloc;
> +};
> +
> +_ecspi1 {
> + u-boot,dm-spl;
> +};
> +
> +_i2c1 {
> + u-boot,dm-spl;
> +};
> +
> +_pmic {
> + u-boot,dm-spl;
> +};
> +
> +_uart3 {
> + u-boot,dm-spl;
> + u-boot,dm-pre-reloc;
> +};
> +
> +_usdhc1 {
> + u-boot,dm-spl;
> +};
> +
> +_usdhc1_100mhz {
> + u-boot,dm-spl;
> +};
> +
> +_usdhc1_200mhz {
> + u-boot,dm-spl;
> +};
> +
> + {
> + u-boot,dm-spl;
> +};
> +
> +&{/soc@0/bus@3080/i2c@30a2/pmic@25/regulators} {
> + u-boot,dm-spl;
> +};
> +
> + {
> + u-boot,dm-spl;
> +};
> +
> + {
> + u-boot,dm-spl;
> +};
> +
> + {
> + u-boot,dm-spl;
> +};
> +
> + {
> + u-boot,dm-spl;
> +};
> +
> + {
> + u-boot,dm-spl;
> +};
> +
> + {
> + u-boot,dm-spl;
> +};
> +
> + {
> + u-boot,dm-spl;
> + u-boot,dm-pre-reloc;
> +};
> +
> + {
> + u-boot,dm-spl;
> +};
> +
> + {
> + u-boot,dm-spl;
> +};
> +
> +_wdog {
> + u-boot,dm-spl;
> +};
> diff --git a/arch/arm/dts/imx8mm-mt-snowflake-v2.dts
> b/arch/arm/dts/imx8mm-mt-snowflake-v2.dts
> new file mode 100644
> index 0..49761b22afcf0
> --- /dev/null
> +++ b/arch/arm/dts/imx8mm-mt-snowflake-v2.dts
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Mettler Toledo GmbH
> + */
> +
> +/dts-v1/;
> +
> +#include
