[PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[Lars Pedersen]

This adds support for Kamstrup mx7 concentrator board

Signed-off-by: Lars Pedersen <lapeddk@gmail.com>
Reviewed-by: Bruno Thomsen <bruno.thomsen@gmail.com>
---
 arch/arm/boards/Makefile                      |   1 +
 .../boards/kamstrup-mx7-concentrator/Makefile |   2 +
 .../boards/kamstrup-mx7-concentrator/board.c  |  51 +++++++++
 .../flash-header-tqma7d.imxcfg                |  79 +++++++++++++
 .../kamstrup-mx7-concentrator/lowlevel.c      |  35 ++++++
 arch/arm/dts/Makefile                         |   1 +
 arch/arm/dts/imx7d-flex-concentrator-mfg.dts  | 108 ++++++++++++++++++
 arch/arm/mach-imx/Kconfig                     |   5 +
 images/Makefile.imx                           |   5 +
 9 files changed, 287 insertions(+)
 create mode 100644 arch/arm/boards/kamstrup-mx7-concentrator/Makefile
 create mode 100644 arch/arm/boards/kamstrup-mx7-concentrator/board.c
 create mode 100644 arch/arm/boards/kamstrup-mx7-concentrator/flash-header-tqma7d.imxcfg
 create mode 100644 arch/arm/boards/kamstrup-mx7-concentrator/lowlevel.c
 create mode 100644 arch/arm/dts/imx7d-flex-concentrator-mfg.dts

diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
index 9ccb75e27..0e59d6a77 100644
--- a/arch/arm/boards/Makefile
+++ b/arch/arm/boards/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_MACH_HABA_KNX_LITE)		+= haba-knx/
 obj-$(CONFIG_MACH_IMX21ADS)			+= freescale-mx21-ads/
 obj-$(CONFIG_MACH_IMX233_OLINUXINO)		+= imx233-olinuxino/
 obj-$(CONFIG_MACH_IMX27ADS)			+= freescale-mx27-ads/
+obj-$(CONFIG_MACH_KAMSTRUP_MX7_CONCENTRATOR)	+= kamstrup-mx7-concentrator/
 obj-$(CONFIG_MACH_KINDLE3)			+= kindle3/
 obj-$(CONFIG_MACH_KONTRON_SAMX6I)		+= kontron-samx6i/
 obj-$(CONFIG_MACH_LENOVO_IX4_300D)		+= lenovo-ix4-300d/
diff --git a/arch/arm/boards/kamstrup-mx7-concentrator/Makefile b/arch/arm/boards/kamstrup-mx7-concentrator/Makefile
new file mode 100644
index 000000000..a8b45be12
--- /dev/null
+++ b/arch/arm/boards/kamstrup-mx7-concentrator/Makefile
@@ -0,0 +1,2 @@
+obj-y += board.o
+lwl-y += lowlevel.o
\ No newline at end of file
diff --git a/arch/arm/boards/kamstrup-mx7-concentrator/board.c b/arch/arm/boards/kamstrup-mx7-concentrator/board.c
new file mode 100644
index 000000000..ac32e9b05
--- /dev/null
+++ b/arch/arm/boards/kamstrup-mx7-concentrator/board.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// SPDX-FileCopyrightText: 2021 Kamstrup A/S
+
+/* Author: Lars Pedersen <lapeddk@gmail.com> */
+
+#include <common.h>
+#include <init.h>
+#include <io.h>
+#include <gpio.h>
+#include <restart.h>
+#include <mach/imx7-regs.h>
+#include <mach/iomux-mx7.h>
+#include <mach/generic.h>
+#include <mfd/imx7-iomuxc-gpr.h>
+
+#define BOARD_RESTART_GPIO IMX_GPIO_NR(7, 12)
+#define TPM_RESET_GPIO IMX_GPIO_NR(3, 8)
+
+static void kamstrup_mx7_tpm_reset(void)
+{
+	imx7_setup_pad(MX7D_PAD_LCD_DATA03__GPIO3_IO8);
+
+	gpio_request(TPM_RESET_GPIO, "tpm-reset");
+	gpio_direction_output(TPM_RESET_GPIO, 1);
+	mdelay(100);
+	gpio_set_value(TPM_RESET_GPIO, 0);
+	mdelay(100);
+	gpio_set_value(TPM_RESET_GPIO, 1);
+	gpio_free(TPM_RESET_GPIO);
+}
+
+static void __noreturn kamstrup_mx7_board_restart_gpio(struct restart_handler *rst)
+{
+	imx7_setup_pad(MX7D_PAD_ENET1_TX_CLK__GPIO7_IO12);
+
+	gpio_direction_output(BOARD_RESTART_GPIO, 0);
+	mdelay(1);
+	gpio_set_value(BOARD_RESTART_GPIO, 0);
+
+	hang();
+}
+
+static int kamstrup_mx7_concentrator_coredevices_init(void)
+{
+	kamstrup_mx7_tpm_reset();
+	restart_handler_register_fn("board", kamstrup_mx7_board_restart_gpio);
+	barebox_set_model("Kamstrup OMNIA Concentrator");
+
+	return 0;
+}
+coredevice_initcall(kamstrup_mx7_concentrator_coredevices_init);
diff --git a/arch/arm/boards/kamstrup-mx7-concentrator/flash-header-tqma7d.imxcfg b/arch/arm/boards/kamstrup-mx7-concentrator/flash-header-tqma7d.imxcfg
new file mode 100644
index 000000000..4b3632411
--- /dev/null
+++ b/arch/arm/boards/kamstrup-mx7-concentrator/flash-header-tqma7d.imxcfg
@@ -0,0 +1,79 @@
+soc imx7
+loadaddr 0xbfbff000
+ivtofs 0x400
+
+#include <mach/imx7-ddr-regs.h>
+
+wm 32 0x30340004 0x4F400005 /* IOMUXC_GPR_GPR1 */
+/* Clear then set bit30 to ensure exit from DDR retention */
+wm 32 0x30360388 0x40000000
+wm 32 0x30360384 0x40000000
+
+/* TQMa7x DRAM Timing REV0100 */
+/* DCD Code i.MX7D/S 528 MHz 1 GByte Samsung K4B4G1646D */
+wm 32 0x30360070 0x0070302C /* CCM_ANALOG_PLL_DDRx */
+wm 32  0x30360090 0x00000000 /* CCM_ANALOG_PLL_NUM */
+wm 32  0x30360070 0x0060302C /* CCM_ANALOG_PLL_DDRx */
+check 32 until_all_bits_set 0x30360070 0x80000000
+wm 32 0x30391000 0x00000002 /* SRC_DDRC_RCR */
+
+wm 32 MX7_DDRC_MSTR 0x01040001
+wm 32 MX7_DDRC_DFIUPD0 0x80400003
+wm 32 MX7_DDRC_DFIUPD1 0x00100020
+wm 32 MX7_DDRC_DFIUPD2 0x80100004
+wm 32 MX7_DDRC_RFSHTMG 0x00200045
+wm 32 MX7_DDRC_MP_PCTRL_0 0x00000001
+wm 32 MX7_DDRC_INIT0 0x00020081
+wm 32 MX7_DDRC_INIT1 0x00680000
+wm 32 MX7_DDRC_INIT3 0x09300004
+wm 32 MX7_DDRC_INIT4 0x00480000
+wm 32 MX7_DDRC_INIT5 0x00100004
+wm 32 MX7_DDRC_RANKCTL 0x0000033F
+wm 32 MX7_DDRC_DRAMTMG0 0x090E0809
+wm 32 MX7_DDRC_DRAMTMG1 0x0007020E
+wm 32 MX7_DDRC_DRAMTMG2 0x03040407
+wm 32 MX7_DDRC_DRAMTMG3 0x00002006
+wm 32 MX7_DDRC_DRAMTMG4 0x04020304
+wm 32 MX7_DDRC_DRAMTMG5 0x03030202
+wm 32 MX7_DDRC_DRAMTMG8 0x00000803
+wm 32 MX7_DDRC_ZQCTL0 0x00800020
+wm 32 MX7_DDRC_DFITMG0 0x02098204
+wm 32 MX7_DDRC_DFITMG1 0x00030303
+wm 32 MX7_DDRC_ADDRMAP0 0x00000016
+wm 32 MX7_DDRC_ADDRMAP1 0x00171717
+wm 32 MX7_DDRC_ADDRMAP4 0x00000F0F
+wm 32 MX7_DDRC_ADDRMAP5 0x04040404
+wm 32 MX7_DDRC_ADDRMAP6 0x0F040404
+wm 32 MX7_DDRC_ODTCFG 0x06000604
+wm 32 MX7_DDRC_ODTMAP 0x00000001
+wm 32 0x30391000 0x00000000 /* SRC_DDRC_RCR */
+wm 32 MX7_DDR_PHY_PHY_CON0 0x17420F40
+wm 32 MX7_DDR_PHY_PHY_CON1 0x10210100
+wm 32 MX7_DDR_PHY_PHY_CON4 0x00060807
+wm 32 MX7_DDR_PHY_MDLL_CON0 0x1010007E
+wm 32 MX7_DDR_PHY_DRVDS_CON0 0x00000924
+/* DDR_PHY_CMD_DESKEW_CON0 not set */
+/* DDR_PHY_CMD_DESKEW_CON1 not set */
+/* DDR_PHY_CMD_DESKEW_CON2 not set */
+/* DDR_PHY_CMD_DESKEW_CON3 not set */
+/* DDR_PHY_LVL_CON0 not set */
+wm 32 MX7_DDR_PHY_OFFSET_RD_CON0 0x0B0B0B0B
+wm 32 MX7_DDR_PHY_OFFSET_WR_CON0 0x06060606
+wm 32 MX7_DDR_PHY_CMD_SDLL_CON0 0x01000010
+wm 32 MX7_DDR_PHY_CMD_SDLL_CON0 0x00000010
+
+wm 32 MX7_DDR_PHY_ZQ_CON0 0x0C407304
+wm 32 MX7_DDR_PHY_ZQ_CON0 0x0C447304
+wm 32 MX7_DDR_PHY_ZQ_CON0 0x0C447306
+check 32 until_all_bits_set MX7_DDR_PHY_ZQ_CON1 0x1 /* ZQ Calibration is finished */
+wm 32 MX7_DDR_PHY_ZQ_CON0 0x0C447304
+wm 32 MX7_DDR_PHY_ZQ_CON0 0x0C407304
+
+wm 32 0x30384130 0x00000000 /* CCM_CCGRn */
+wm 32 0x30340020 0x00000178 /* IOMUXC_GPR_GPR8 */
+wm 32 0x30384130 0x00000002 /* CCM_CCGRn */
+wm 32 0x30790018 0x0000000f /* DDR_PHY_LP_CON0 */
+
+/* DDRC_STAT */
+check 32 until_all_bits_set 0x307a0004 0x1
+
diff --git a/arch/arm/boards/kamstrup-mx7-concentrator/lowlevel.c b/arch/arm/boards/kamstrup-mx7-concentrator/lowlevel.c
new file mode 100644
index 000000000..8cd6d67f7
--- /dev/null
+++ b/arch/arm/boards/kamstrup-mx7-concentrator/lowlevel.c
@@ -0,0 +1,35 @@
+#include <debug_ll.h>
+#include <io.h>
+#include <common.h>
+#include <linux/sizes.h>
+#include <mach/generic.h>
+#include <asm/barebox-arm-head.h>
+#include <asm/barebox-arm.h>
+#include <mach/imx7-ccm-regs.h>
+#include <mach/iomux-mx7.h>
+#include <mach/debug_ll.h>
+#include <asm/cache.h>
+#include <mach/esdctl.h>
+
+extern char __dtb_z_imx7d_flex_concentrator_mfg_start[];
+
+static inline void setup_uart(void)
+{
+	imx7_early_setup_uart_clock();
+
+	imx7_setup_pad(MX7D_PAD_SAI2_TX_BCLK__UART4_DCE_TX);
+
+	imx7_uart_setup_ll();
+
+	putc_ll('>');
+}
+
+ENTRY_FUNCTION(start_kamstrup_mx7_concentrator, r0, r1, r2)
+{
+	imx7_cpu_lowlevel_init();
+
+	if (IS_ENABLED(CONFIG_DEBUG_LL))
+		setup_uart();
+
+	imx7d_barebox_entry(__dtb_z_imx7d_flex_concentrator_mfg_start + get_runtime_offset());
+}
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index d5f61768a..d66b6db07 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -30,6 +30,7 @@ lwl-$(CONFIG_MACH_GRINN_LITEBOARD) += imx6ul-liteboard.dtb.o
 lwl-$(CONFIG_MACH_GUF_SANTARO) += imx6q-guf-santaro.dtb.o
 lwl-$(CONFIG_MACH_GUF_VINCELL) += imx53-guf-vincell.dtb.o imx53-guf-vincell-lt.dtb.o
 lwl-$(CONFIG_MACH_GW_VENTANA) += imx6q-gw54xx.dtb.o
+lwl-$(CONFIG_MACH_KAMSTRUP_MX7_CONCENTRATOR) += imx7d-flex-concentrator-mfg.dtb.o
 lwl-$(CONFIG_MACH_KONTRON_SAMX6I) += imx6q-samx6i.dtb.o \
 					imx6dl-samx6i.dtb.o
 lwl-$(CONFIG_MACH_LENOVO_IX4_300D) += armada-xp-lenovo-ix4-300d-bb.dtb.o
diff --git a/arch/arm/dts/imx7d-flex-concentrator-mfg.dts b/arch/arm/dts/imx7d-flex-concentrator-mfg.dts
new file mode 100644
index 000000000..d174ef725
--- /dev/null
+++ b/arch/arm/dts/imx7d-flex-concentrator-mfg.dts
@@ -0,0 +1,108 @@
+/*
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <arm/imx7d-flex-concentrator-mfg.dts>
+
+/ {
+	chosen {
+		environment {
+			compatible = "barebox,environment";
+			device-path = &environment_emmc;
+		};
+	};
+
+	aliases {
+		state = &state_emmc;
+	};
+
+	state_emmc: state {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "barebox,state";
+		magic = <0x4b414d31>;
+		backend-type = "raw";
+		backend = <&backend_state_emmc>;
+		backend-stridesize = <0x200>;
+
+		bootstate {
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			system0 {
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				remaining_attempts@0 {
+					reg = <0x0 0x4>;
+					type = "uint32";
+					default = <10>;
+				};
+
+				priority@4 {
+					reg = <0x4 0x4>;
+					type = "uint32";
+					default = <21>;
+				};
+			};
+
+			system1 {
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				remaining_attempts@8 {
+					reg = <0x8 0x4>;
+					type = "uint32";
+					default = <0>;
+				};
+
+				priority@c {
+					reg = <0xc 0x4>;
+					type = "uint32";
+					default = <20>;
+				};
+			};
+
+			last_chosen@10 {
+				reg = <0x10 0x4>;
+				type = "uint32";
+			};
+		};
+	};
+};
+
+/* eMMC */
+&usdhc3 {
+	partitions {
+		compatible = "fixed-partitions";
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		partition@0 {
+			label = "barebox";
+			reg = <0x0 0x100000>;
+		};
+
+		environment_emmc: partition@100000 {
+			label = "barebox-environment";
+			reg = <0x100000 0x100000>;
+		};
+
+		backend_state_emmc: partition@200000 {
+			label = "barebox-state";
+			reg = <0x200000 0x100000>;
+		};
+	};
+};
+
+/* FIXME: barebox serial is broken when barebox applies requested reparenting */
+&uart4 {
+	/delete-property/ assigned-clocks;
+	/delete-property/ assigned-clock-parents;
+};
+
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 3f8012c73..cc73d0a0d 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -526,6 +526,11 @@ config MACH_FREESCALE_MX7_SABRESD
 
 	  https://goo.gl/6EKGdk
 
+config MACH_KAMSTRUP_MX7_CONCENTRATOR
+	bool "Kamstrup i.MX7 Concentrator"
+	select ARCH_IMX7
+	select ARM_USE_COMPRESSED_DTB
+
 config MACH_NXP_IMX6ULL_EVK
 	bool "NXP i.MX6ull EVK Board"
 	select ARCH_IMX6UL
diff --git a/images/Makefile.imx b/images/Makefile.imx
index b892ec019..bc5ad1c7c 100644
--- a/images/Makefile.imx
+++ b/images/Makefile.imx
@@ -403,6 +403,11 @@ CFG_start_zii_imx7d_dev.pblb.imximg = $(board)/zii-imx7d-dev/flash-header-zii-im
 FILE_barebox-zii-imx7d-dev.img = start_zii_imx7d_dev.pblb.imximg
 image-$(CONFIG_MACH_ZII_IMX7D_DEV) += barebox-zii-imx7d-dev.img
 
+pblb-$(CONFIG_MACH_KAMSTRUP_MX7_CONCENTRATOR) += start_kamstrup_mx7_concentrator
+CFG_start_kamstrup_mx7_concentrator.pblb.imximg = $(board)/kamstrup-mx7-concentrator/flash-header-tqma7d.imxcfg
+FILE_barebox-kamstrup-mx7-concentrator.img = start_kamstrup_mx7_concentrator.pblb.imximg
+image-$(CONFIG_MACH_KAMSTRUP_MX7_CONCENTRATOR) += barebox-kamstrup-mx7-concentrator.img
+
 # ----------------------- i.MX8mm based boards --------------------------
 pblb-$(CONFIG_MACH_NXP_IMX8MM_EVK) += start_nxp_imx8mm_evk
 CFG_start_nxp_imx8mm_evk.pblb.imximg = $(board)/nxp-imx8mm-evk/flash-header-imx8mm-evk.imxcfg
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[Ahmad Fatoum]

Hello Lars,

On 09.04.21 15:20, Lars Pedersen wrote:
> diff --git a/arch/arm/boards/kamstrup-mx7-concentrator/board.c b/arch/arm/boards/kamstrup-mx7-concentrator/board.c
> new file mode 100644
> index 000000000..ac32e9b05
> --- /dev/null
> +++ b/arch/arm/boards/kamstrup-mx7-concentrator/board.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// SPDX-FileCopyrightText: 2021 Kamstrup A/S
> +
> +/* Author: Lars Pedersen <lapeddk@gmail.com> */
> +
> +#include <common.h>
> +#include <init.h>
> +#include <io.h>
> +#include <gpio.h>
> +#include <restart.h>
> +#include <mach/imx7-regs.h>
> +#include <mach/iomux-mx7.h>
> +#include <mach/generic.h>
> +#include <mfd/imx7-iomuxc-gpr.h>
> +
> +#define BOARD_RESTART_GPIO IMX_GPIO_NR(7, 12)
> +#define TPM_RESET_GPIO IMX_GPIO_NR(3, 8)
> +
> +static void kamstrup_mx7_tpm_reset(void)
> +{
> +	imx7_setup_pad(MX7D_PAD_LCD_DATA03__GPIO3_IO8);
> +
> +	gpio_request(TPM_RESET_GPIO, "tpm-reset");
> +	gpio_direction_output(TPM_RESET_GPIO, 1);
> +	mdelay(100);
> +	gpio_set_value(TPM_RESET_GPIO, 0);
> +	mdelay(100);
> +	gpio_set_value(TPM_RESET_GPIO, 1);
> +	gpio_free(TPM_RESET_GPIO);

We are trying to cut down on code that doesn't use the driver model.
Couldn't this be represented as a gpio-hog in the device tree or
a reset line for the SPI device?

> +}
> +
> +static void __noreturn kamstrup_mx7_board_restart_gpio(struct restart_handler *rst)
> +{
> +	imx7_setup_pad(MX7D_PAD_ENET1_TX_CLK__GPIO7_IO12);
> +
> +	gpio_direction_output(BOARD_RESTART_GPIO, 0);
> +	mdelay(1);
> +	gpio_set_value(BOARD_RESTART_GPIO, 0);

I just sent out a patch[1] with a driver implementing the "gpio-restart" device
tree binding. Could you test that one and use it here instead?

> +
> +	hang();
> +}
> +
> +static int kamstrup_mx7_concentrator_coredevices_init(void)
> +{
> +	kamstrup_mx7_tpm_reset();
> +	restart_handler_register_fn("board", kamstrup_mx7_board_restart_gpio);
> +	barebox_set_model("Kamstrup OMNIA Concentrator");

The default model name is "Kamstrup OMNIA Flex Concentrator".
If that's too long, you could override /model in the barebox device tree.
With the changes suggested above, you could drop board.c then altogether.

The rest of the patch looks good to me. With the feedback addressed:
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

[1]: https://lore.pengutronix.de/barebox/20210410103511.2073504-1-ahmad@a3f.at/T/#mc4dbda46e6bd868fc9ecfcfa42f3dbfe943bef47
 
Cheers,
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[Lars Pedersen]

Hello Ahmad.

Thanks for the feedback.

On Sat, 10 Apr 2021 at 12:43, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Lars,
>
> On 09.04.21 15:20, Lars Pedersen wrote:
> > diff --git a/arch/arm/boards/kamstrup-mx7-concentrator/board.c b/arch/arm/boards/kamstrup-mx7-concentrator/board.c
> > new file mode 100644
> > index 000000000..ac32e9b05
> > --- /dev/null
> > +++ b/arch/arm/boards/kamstrup-mx7-concentrator/board.c
> > @@ -0,0 +1,51 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +// SPDX-FileCopyrightText: 2021 Kamstrup A/S
> > +
> > +/* Author: Lars Pedersen <lapeddk@gmail.com> */
> > +
> > +#include <common.h>
> > +#include <init.h>
> > +#include <io.h>
> > +#include <gpio.h>
> > +#include <restart.h>
> > +#include <mach/imx7-regs.h>
> > +#include <mach/iomux-mx7.h>
> > +#include <mach/generic.h>
> > +#include <mfd/imx7-iomuxc-gpr.h>
> > +
> > +#define BOARD_RESTART_GPIO IMX_GPIO_NR(7, 12)
> > +#define TPM_RESET_GPIO IMX_GPIO_NR(3, 8)
> > +
> > +static void kamstrup_mx7_tpm_reset(void)
> > +{
> > +     imx7_setup_pad(MX7D_PAD_LCD_DATA03__GPIO3_IO8);
> > +
> > +     gpio_request(TPM_RESET_GPIO, "tpm-reset");
> > +     gpio_direction_output(TPM_RESET_GPIO, 1);
> > +     mdelay(100);
> > +     gpio_set_value(TPM_RESET_GPIO, 0);
> > +     mdelay(100);
> > +     gpio_set_value(TPM_RESET_GPIO, 1);
> > +     gpio_free(TPM_RESET_GPIO);
>
> We are trying to cut down on code that doesn't use the driver model.
> Couldn't this be represented as a gpio-hog in the device tree or
> a reset line for the SPI device?

I can't find anything in the DT binding for the SPI/TPM driver to use
a reset line. Can a DT gpio-hog toggle the pin? Don't you need a
driver for this? I see the following options:

1) gpio-hog with a label and use new gpiolib in board.c.
2) gpio-hog and control the pin in a boot script.

>
> > +}
> > +
> > +static void __noreturn kamstrup_mx7_board_restart_gpio(struct restart_handler *rst)
> > +{
> > +     imx7_setup_pad(MX7D_PAD_ENET1_TX_CLK__GPIO7_IO12);
> > +
> > +     gpio_direction_output(BOARD_RESTART_GPIO, 0);
> > +     mdelay(1);
> > +     gpio_set_value(BOARD_RESTART_GPIO, 0);
>
> I just sent out a patch[1] with a driver implementing the "gpio-restart" device
> tree binding. Could you test that one and use it here instead?
>

I have applied the patches on a master branch (Last patch failed to
apply but was only watchdog) and the gpio-restart works perfectly.

> > +
> > +     hang();
> > +}
> > +
> > +static int kamstrup_mx7_concentrator_coredevices_init(void)
> > +{
> > +     kamstrup_mx7_tpm_reset();
> > +     restart_handler_register_fn("board", kamstrup_mx7_board_restart_gpio);
> > +     barebox_set_model("Kamstrup OMNIA Concentrator");
>
> The default model name is "Kamstrup OMNIA Flex Concentrator".
> If that's too long, you could override /model in the barebox device tree.
> With the changes suggested above, you could drop board.c then altogether.
>

I'll move this into the device tree or delete it entirely.

> The rest of the patch looks good to me. With the feedback addressed:
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>
> [1]: https://lore.pengutronix.de/barebox/20210410103511.2073504-1-ahmad@a3f.at/T/#mc4dbda46e6bd868fc9ecfcfa42f3dbfe943bef47
>
> Cheers,
> Ahmad
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[Ahmad Fatoum]

Hello Lars,

On 12.04.21 12:45, Lars Pedersen wrote:
>>> +#define BOARD_RESTART_GPIO IMX_GPIO_NR(7, 12)
>>> +#define TPM_RESET_GPIO IMX_GPIO_NR(3, 8)
>>> +
>>> +static void kamstrup_mx7_tpm_reset(void)
>>> +{
>>> +     imx7_setup_pad(MX7D_PAD_LCD_DATA03__GPIO3_IO8);
>>> +
>>> +     gpio_request(TPM_RESET_GPIO, "tpm-reset");
>>> +     gpio_direction_output(TPM_RESET_GPIO, 1);
>>> +     mdelay(100);
>>> +     gpio_set_value(TPM_RESET_GPIO, 0);
>>> +     mdelay(100);
>>> +     gpio_set_value(TPM_RESET_GPIO, 1);
>>> +     gpio_free(TPM_RESET_GPIO);
>>
>> We are trying to cut down on code that doesn't use the driver model.
>> Couldn't this be represented as a gpio-hog in the device tree or
>> a reset line for the SPI device?
> 
> I can't find anything in the DT binding for the SPI/TPM driver to use
> a reset line.

Proper way would be for this to be added into Linux then,
but that's out of scope for the patch here.

> Can a DT gpio-hog toggle the pin? Don't you need a
> driver for this?

You can't pulse with a gpio-hog. I assumed the TPM is in reset by
default.

> I see the following options:
> 
> 1) gpio-hog with a label and use new gpiolib in board.c.
> 2) gpio-hog and control the pin in a boot script.

If there's no straight-forward way to do it without board code,
it's ok the way it is with one change though: I missed it the first
time, but your board breaks multi-image support because you don't
check whether the initcall is indeed running on your board (See for
example imx_v7_defconfig, which builds over a hundred boards at once).

Easiest way to get this right is to write a board driver.
See arch/arm/boards/lxa-mc1/board.c for an example.

>>> +}
>>> +
>>> +static void __noreturn kamstrup_mx7_board_restart_gpio(struct restart_handler *rst)
>>> +{
>>> +     imx7_setup_pad(MX7D_PAD_ENET1_TX_CLK__GPIO7_IO12);
>>> +
>>> +     gpio_direction_output(BOARD_RESTART_GPIO, 0);
>>> +     mdelay(1);
>>> +     gpio_set_value(BOARD_RESTART_GPIO, 0);
>>
>> I just sent out a patch[1] with a driver implementing the "gpio-restart" device
>> tree binding. Could you test that one and use it here instead?
>>
> 
> I have applied the patches on a master branch (Last patch failed to
> apply but was only watchdog) and the gpio-restart works perfectly.

Thanks for testing! You can add your Tested-By on the patch in question
if you like.

> 
>>> +
>>> +     hang();
>>> +}
>>> +
>>> +static int kamstrup_mx7_concentrator_coredevices_init(void)
>>> +{
>>> +     kamstrup_mx7_tpm_reset();
>>> +     restart_handler_register_fn("board", kamstrup_mx7_board_restart_gpio);
>>> +     barebox_set_model("Kamstrup OMNIA Concentrator");
>>
>> The default model name is "Kamstrup OMNIA Flex Concentrator".
>> If that's too long, you could override /model in the barebox device tree.
>> With the changes suggested above, you could drop board.c then altogether.
>>
> 
> I'll move this into the device tree or delete it entirely.

Great. :-)


Cheers,
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[Lars Pedersen]

Hello again. Thanks for the quick response :)

On Mon, 12 Apr 2021 at 13:24, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Lars,
>
> On 12.04.21 12:45, Lars Pedersen wrote:
> >>> +#define BOARD_RESTART_GPIO IMX_GPIO_NR(7, 12)
> >>> +#define TPM_RESET_GPIO IMX_GPIO_NR(3, 8)
> >>> +
> >>> +static void kamstrup_mx7_tpm_reset(void)
> >>> +{
> >>> +     imx7_setup_pad(MX7D_PAD_LCD_DATA03__GPIO3_IO8);
> >>> +
> >>> +     gpio_request(TPM_RESET_GPIO, "tpm-reset");
> >>> +     gpio_direction_output(TPM_RESET_GPIO, 1);
> >>> +     mdelay(100);
> >>> +     gpio_set_value(TPM_RESET_GPIO, 0);
> >>> +     mdelay(100);
> >>> +     gpio_set_value(TPM_RESET_GPIO, 1);
> >>> +     gpio_free(TPM_RESET_GPIO);
> >>
> >> We are trying to cut down on code that doesn't use the driver model.
> >> Couldn't this be represented as a gpio-hog in the device tree or
> >> a reset line for the SPI device?
> >
> > I can't find anything in the DT binding for the SPI/TPM driver to use
> > a reset line.
>
> Proper way would be for this to be added into Linux then,
> but that's out of scope for the patch here.
>
> > Can a DT gpio-hog toggle the pin? Don't you need a
> > driver for this?
>
> You can't pulse with a gpio-hog. I assumed the TPM is in reset by
> default.
>
> > I see the following options:
> >
> > 1) gpio-hog with a label and use new gpiolib in board.c.
> > 2) gpio-hog and control the pin in a boot script.
>
> If there's no straight-forward way to do it without board code,
> it's ok the way it is with one change though: I missed it the first
> time, but your board breaks multi-image support because you don't
> check whether the initcall is indeed running on your board (See for
> example imx_v7_defconfig, which builds over a hundred boards at once).
>
> Easiest way to get this right is to write a board driver.
> See arch/arm/boards/lxa-mc1/board.c for an example.

In arch/arm/boards/freescale-mx7-sabresd/board.c they do something like:

if (!of_machine_is_compatible("fsl,imx7d-sdb"))
  return 0;

Can't I just do the same?

if (!of_machine_is_compatible("kam,imx7d-flex-concentrator-mfg"))
  return 0;

The board driver in arch/arm/boards/lxa-mc1/board.c seems more complicated.

>
> >>> +}
> >>> +
> >>> +static void __noreturn kamstrup_mx7_board_restart_gpio(struct restart_handler *rst)
> >>> +{
> >>> +     imx7_setup_pad(MX7D_PAD_ENET1_TX_CLK__GPIO7_IO12);
> >>> +
> >>> +     gpio_direction_output(BOARD_RESTART_GPIO, 0);
> >>> +     mdelay(1);
> >>> +     gpio_set_value(BOARD_RESTART_GPIO, 0);
> >>
> >> I just sent out a patch[1] with a driver implementing the "gpio-restart" device
> >> tree binding. Could you test that one and use it here instead?
> >>
> >
> > I have applied the patches on a master branch (Last patch failed to
> > apply but was only watchdog) and the gpio-restart works perfectly.
>
> Thanks for testing! You can add your Tested-By on the patch in question
> if you like.
>
> >
> >>> +
> >>> +     hang();
> >>> +}
> >>> +
> >>> +static int kamstrup_mx7_concentrator_coredevices_init(void)
> >>> +{
> >>> +     kamstrup_mx7_tpm_reset();
> >>> +     restart_handler_register_fn("board", kamstrup_mx7_board_restart_gpio);
> >>> +     barebox_set_model("Kamstrup OMNIA Concentrator");
> >>
> >> The default model name is "Kamstrup OMNIA Flex Concentrator".
> >> If that's too long, you could override /model in the barebox device tree.
> >> With the changes suggested above, you could drop board.c then altogether.
> >>
> >
> > I'll move this into the device tree or delete it entirely.
>
> Great. :-)
>
>
> Cheers,
> Ahmad
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[Rouven Czerwinski]

On Mon, 2021-04-12 at 13:24 +0200, Ahmad Fatoum wrote:
> Hello Lars,
> 
> On 12.04.21 12:45, Lars Pedersen wrote:
> > > > +#define BOARD_RESTART_GPIO IMX_GPIO_NR(7, 12)
> > > > +#define TPM_RESET_GPIO IMX_GPIO_NR(3, 8)
> > > > +
> > > > +static void kamstrup_mx7_tpm_reset(void)
> > > > +{
> > > > +     imx7_setup_pad(MX7D_PAD_LCD_DATA03__GPIO3_IO8);
> > > > +
> > > > +     gpio_request(TPM_RESET_GPIO, "tpm-reset");
> > > > +     gpio_direction_output(TPM_RESET_GPIO, 1);
> > > > +     mdelay(100);
> > > > +     gpio_set_value(TPM_RESET_GPIO, 0);
> > > > +     mdelay(100);
> > > > +     gpio_set_value(TPM_RESET_GPIO, 1);
> > > > +     gpio_free(TPM_RESET_GPIO);
> > > 
> > > We are trying to cut down on code that doesn't use the driver model.
> > > Couldn't this be represented as a gpio-hog in the device tree or
> > > a reset line for the SPI device?
> > 
> > I can't find anything in the DT binding for the SPI/TPM driver to use
> > a reset line.
> 
> Proper way would be for this to be added into Linux then,
> but that's out of scope for the patch here.
> 
> > Can a DT gpio-hog toggle the pin? Don't you need a
> > driver for this?
> 
> You can't pulse with a gpio-hog. I assumed the TPM is in reset by
> default.
> 
> > I see the following options:
> > 
> > 1) gpio-hog with a label and use new gpiolib in board.c.
> > 2) gpio-hog and control the pin in a boot script.
> 
> If there's no straight-forward way to do it without board code,
> it's ok the way it is with one change though: I missed it the first
> time, but your board breaks multi-image support because you don't
> check whether the initcall is indeed running on your board (See for
> example imx_v7_defconfig, which builds over a hundred boards at once).
> 
> Easiest way to get this right is to write a board driver.
> See arch/arm/boards/lxa-mc1/board.c for an example.

TPMs usually don't have a reset line since it is a hardware misdesign
if the reset needs to be done within the bootloader. This opens up the
TPM to an attack where the system is properly booted, unlocking keys
which are only accessible if the correct PCR sequence is send to the
TPM. Than the attacker resets the hardware/CPU, but potentially loads
up a different bootloader or tricks the bootloader into skipping the
TPM reset. This will leave the TPM keys accessible even if the system
has not been booted with the correct measured boot values. Boards with
TPMs should be designed so that a CPU-Reset always results in a TPM
Reset as well.

Regards,
Rouven 


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[Lars Pedersen]

Hello Rouven.

Thanks for your response.

We will need another hardware update before production so we will try
to get this added.

Regards,
Lars Pedersen

On Mon, 12 Apr 2021 at 14:22, Rouven Czerwinski
<r.czerwinski@pengutronix.de> wrote:
>
> On Mon, 2021-04-12 at 13:24 +0200, Ahmad Fatoum wrote:
> > Hello Lars,
> >
> > On 12.04.21 12:45, Lars Pedersen wrote:
> > > > > +#define BOARD_RESTART_GPIO IMX_GPIO_NR(7, 12)
> > > > > +#define TPM_RESET_GPIO IMX_GPIO_NR(3, 8)
> > > > > +
> > > > > +static void kamstrup_mx7_tpm_reset(void)
> > > > > +{
> > > > > +     imx7_setup_pad(MX7D_PAD_LCD_DATA03__GPIO3_IO8);
> > > > > +
> > > > > +     gpio_request(TPM_RESET_GPIO, "tpm-reset");
> > > > > +     gpio_direction_output(TPM_RESET_GPIO, 1);
> > > > > +     mdelay(100);
> > > > > +     gpio_set_value(TPM_RESET_GPIO, 0);
> > > > > +     mdelay(100);
> > > > > +     gpio_set_value(TPM_RESET_GPIO, 1);
> > > > > +     gpio_free(TPM_RESET_GPIO);
> > > >
> > > > We are trying to cut down on code that doesn't use the driver model.
> > > > Couldn't this be represented as a gpio-hog in the device tree or
> > > > a reset line for the SPI device?
> > >
> > > I can't find anything in the DT binding for the SPI/TPM driver to use
> > > a reset line.
> >
> > Proper way would be for this to be added into Linux then,
> > but that's out of scope for the patch here.
> >
> > > Can a DT gpio-hog toggle the pin? Don't you need a
> > > driver for this?
> >
> > You can't pulse with a gpio-hog. I assumed the TPM is in reset by
> > default.
> >
> > > I see the following options:
> > >
> > > 1) gpio-hog with a label and use new gpiolib in board.c.
> > > 2) gpio-hog and control the pin in a boot script.
> >
> > If there's no straight-forward way to do it without board code,
> > it's ok the way it is with one change though: I missed it the first
> > time, but your board breaks multi-image support because you don't
> > check whether the initcall is indeed running on your board (See for
> > example imx_v7_defconfig, which builds over a hundred boards at once).
> >
> > Easiest way to get this right is to write a board driver.
> > See arch/arm/boards/lxa-mc1/board.c for an example.
>
> TPMs usually don't have a reset line since it is a hardware misdesign
> if the reset needs to be done within the bootloader. This opens up the
> TPM to an attack where the system is properly booted, unlocking keys
> which are only accessible if the correct PCR sequence is send to the
> TPM. Than the attacker resets the hardware/CPU, but potentially loads
> up a different bootloader or tricks the bootloader into skipping the
> TPM reset. This will leave the TPM keys accessible even if the system
> has not been booted with the correct measured boot values. Boards with
> TPMs should be designed so that a CPU-Reset always results in a TPM
> Reset as well.
>
> Regards,
> Rouven
>

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[Ahmad Fatoum]

Hello Lars,

On 12.04.21 14:09, Lars Pedersen wrote:
> Hello again. Thanks for the quick response :)
>> Easiest way to get this right is to write a board driver.
>> See arch/arm/boards/lxa-mc1/board.c for an example.
> 
> In arch/arm/boards/freescale-mx7-sabresd/board.c they do something like:
> 
> if (!of_machine_is_compatible("fsl,imx7d-sdb"))
>   return 0;
> 
> Can't I just do the same?
> 
> if (!of_machine_is_compatible("kam,imx7d-flex-concentrator-mfg"))
>   return 0;
> 
> The board driver in arch/arm/boards/lxa-mc1/board.c seems more complicated.

The board driver support is recent and not many boards use it.
Doing of_machine_is_compatible check manually is error-prone (inverted logic,
sometimes it's forgotten, like in v1 here) and doesn't scale once you
have multiple compatibles you want to match against. Drivers can also
EPROBE_DEFER if they can't yet acquire a resource.

For the simple case of toggling a reset, likely only difference now is that you
can verify your driver availability with drvinfo. I'd prefer new code uses board
from the get-go though if board code is really needed.

Cheers,
Ahmad

> 
>>
>>>>> +}
>>>>> +
>>>>> +static void __noreturn kamstrup_mx7_board_restart_gpio(struct restart_handler *rst)
>>>>> +{
>>>>> +     imx7_setup_pad(MX7D_PAD_ENET1_TX_CLK__GPIO7_IO12);
>>>>> +
>>>>> +     gpio_direction_output(BOARD_RESTART_GPIO, 0);
>>>>> +     mdelay(1);
>>>>> +     gpio_set_value(BOARD_RESTART_GPIO, 0);
>>>>
>>>> I just sent out a patch[1] with a driver implementing the "gpio-restart" device
>>>> tree binding. Could you test that one and use it here instead?
>>>>
>>>
>>> I have applied the patches on a master branch (Last patch failed to
>>> apply but was only watchdog) and the gpio-restart works perfectly.
>>
>> Thanks for testing! You can add your Tested-By on the patch in question
>> if you like.
>>
>>>
>>>>> +
>>>>> +     hang();
>>>>> +}
>>>>> +
>>>>> +static int kamstrup_mx7_concentrator_coredevices_init(void)
>>>>> +{
>>>>> +     kamstrup_mx7_tpm_reset();
>>>>> +     restart_handler_register_fn("board", kamstrup_mx7_board_restart_gpio);
>>>>> +     barebox_set_model("Kamstrup OMNIA Concentrator");
>>>>
>>>> The default model name is "Kamstrup OMNIA Flex Concentrator".
>>>> If that's too long, you could override /model in the barebox device tree.
>>>> With the changes suggested above, you could drop board.c then altogether.
>>>>
>>>
>>> I'll move this into the device tree or delete it entirely.
>>
>> Great. :-)
>>
>>
>> Cheers,
>> Ahmad
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[Lars Pedersen]

Hello Ahmad

On Mon, 12 Apr 2021 at 14:43, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Lars,
>
> On 12.04.21 14:09, Lars Pedersen wrote:
> > Hello again. Thanks for the quick response :)
> >> Easiest way to get this right is to write a board driver.
> >> See arch/arm/boards/lxa-mc1/board.c for an example.
> >
> > In arch/arm/boards/freescale-mx7-sabresd/board.c they do something like:
> >
> > if (!of_machine_is_compatible("fsl,imx7d-sdb"))
> >   return 0;
> >
> > Can't I just do the same?
> >
> > if (!of_machine_is_compatible("kam,imx7d-flex-concentrator-mfg"))
> >   return 0;
> >
> > The board driver in arch/arm/boards/lxa-mc1/board.c seems more complicated.
>
> The board driver support is recent and not many boards use it.
> Doing of_machine_is_compatible check manually is error-prone (inverted logic,
> sometimes it's forgotten, like in v1 here) and doesn't scale once you
> have multiple compatibles you want to match against. Drivers can also
> EPROBE_DEFER if they can't yet acquire a resource.
>
> For the simple case of toggling a reset, likely only difference now is that you
> can verify your driver availability with drvinfo. I'd prefer new code uses board
> from the get-go though if board code is really needed.
>

I got it working but we have decided to update the hardware to reset
the TPM after your colleague mentioned how it should be done correctly
:)

Just to be clear. I can now remove board.c as you first mentioned
since I don't need any special stuff.

Also I need to update imx_v7_defconfig with this board with default value yes.

> Cheers,
> Ahmad
>
> >
> >>
> >>>>> +}
> >>>>> +
> >>>>> +static void __noreturn kamstrup_mx7_board_restart_gpio(struct restart_handler *rst)
> >>>>> +{
> >>>>> +     imx7_setup_pad(MX7D_PAD_ENET1_TX_CLK__GPIO7_IO12);
> >>>>> +
> >>>>> +     gpio_direction_output(BOARD_RESTART_GPIO, 0);
> >>>>> +     mdelay(1);
> >>>>> +     gpio_set_value(BOARD_RESTART_GPIO, 0);
> >>>>
> >>>> I just sent out a patch[1] with a driver implementing the "gpio-restart" device
> >>>> tree binding. Could you test that one and use it here instead?
> >>>>
> >>>
> >>> I have applied the patches on a master branch (Last patch failed to
> >>> apply but was only watchdog) and the gpio-restart works perfectly.
> >>
> >> Thanks for testing! You can add your Tested-By on the patch in question
> >> if you like.
> >>
> >>>
> >>>>> +
> >>>>> +     hang();
> >>>>> +}
> >>>>> +
> >>>>> +static int kamstrup_mx7_concentrator_coredevices_init(void)
> >>>>> +{
> >>>>> +     kamstrup_mx7_tpm_reset();
> >>>>> +     restart_handler_register_fn("board", kamstrup_mx7_board_restart_gpio);
> >>>>> +     barebox_set_model("Kamstrup OMNIA Concentrator");
> >>>>
> >>>> The default model name is "Kamstrup OMNIA Flex Concentrator".
> >>>> If that's too long, you could override /model in the barebox device tree.
> >>>> With the changes suggested above, you could drop board.c then altogether.
> >>>>
> >>>
> >>> I'll move this into the device tree or delete it entirely.
> >>
> >> Great. :-)
> >>
> >>
> >> Cheers,
> >> Ahmad
> >>
> >> --
> >> Pengutronix e.K.                           |                             |
> >> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> >> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: i.MX: Kamstrup mx7 concentrator board support

[Ahmad Fatoum]

Hello,

On 12.04.21 16:17, Lars Pedersen wrote:
>> For the simple case of toggling a reset, likely only difference now is that you
>> can verify your driver availability with drvinfo. I'd prefer new code uses board
>> from the get-go though if board code is really needed.
>>
> 
> I got it working but we have decided to update the hardware to reset
> the TPM after your colleague mentioned how it should be done correctly
> :)
> 
> Just to be clear. I can now remove board.c as you first mentioned
> since I don't need any special stuff.

Great!

> Also I need to update imx_v7_defconfig with this board with default value yes.

This is optional. I'd recommend you do it, so you benefit from build-time coverage for
your board-specific code.

Cheers,
Ahmad

> 
>> Cheers,
>> Ahmad
>>
>>>
>>>>
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void __noreturn kamstrup_mx7_board_restart_gpio(struct restart_handler *rst)
>>>>>>> +{
>>>>>>> +     imx7_setup_pad(MX7D_PAD_ENET1_TX_CLK__GPIO7_IO12);
>>>>>>> +
>>>>>>> +     gpio_direction_output(BOARD_RESTART_GPIO, 0);
>>>>>>> +     mdelay(1);
>>>>>>> +     gpio_set_value(BOARD_RESTART_GPIO, 0);
>>>>>>
>>>>>> I just sent out a patch[1] with a driver implementing the "gpio-restart" device
>>>>>> tree binding. Could you test that one and use it here instead?
>>>>>>
>>>>>
>>>>> I have applied the patches on a master branch (Last patch failed to
>>>>> apply but was only watchdog) and the gpio-restart works perfectly.
>>>>
>>>> Thanks for testing! You can add your Tested-By on the patch in question
>>>> if you like.
>>>>
>>>>>
>>>>>>> +
>>>>>>> +     hang();
>>>>>>> +}
>>>>>>> +
>>>>>>> +static int kamstrup_mx7_concentrator_coredevices_init(void)
>>>>>>> +{
>>>>>>> +     kamstrup_mx7_tpm_reset();
>>>>>>> +     restart_handler_register_fn("board", kamstrup_mx7_board_restart_gpio);
>>>>>>> +     barebox_set_model("Kamstrup OMNIA Concentrator");
>>>>>>
>>>>>> The default model name is "Kamstrup OMNIA Flex Concentrator".
>>>>>> If that's too long, you could override /model in the barebox device tree.
>>>>>> With the changes suggested above, you could drop board.c then altogether.
>>>>>>
>>>>>
>>>>> I'll move this into the device tree or delete it entirely.
>>>>
>>>> Great. :-)
>>>>
>>>>
>>>> Cheers,
>>>> Ahmad
>>>>
>>>> --
>>>> Pengutronix e.K.                           |                             |
>>>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>>>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>>
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox