[PATCH 0/5] Add support for the Kosagi Novena board

[PATCH 0/5] Add support for the Kosagi Novena board

[John Watts]

This patch series introduces support for the Novena board.
This support is basic but enough to boot Barebox.

The last patch requires my previous patch series here:
https://lists.infradead.org/pipermail/barebox/2023-January/041270.html

John Watts (5):
  ARM: novena: Add Kosagi Novena board
  ARM: novena: Setup RAM using static configuration
  ARM: novena: Require the PFUZE regulator
  ARM: novena: Read Ethernet MAC address from EEPROM
  ARM: novena: Use DDR3 information from SPD EEPROM

 arch/arm/boards/Makefile                      |   1 +
 arch/arm/boards/novena/Makefile               |   4 +
 arch/arm/boards/novena/board.c                |  81 ++++++++
 arch/arm/boards/novena/ddr_regs.h             | 119 +++++++++++
 .../boards/novena/flash-header-novena.imxcfg  |   6 +
 arch/arm/boards/novena/lowlevel.c             | 195 ++++++++++++++++++
 arch/arm/configs/imx_v7_defconfig             |   1 +
 arch/arm/dts/Makefile                         |   1 +
 arch/arm/dts/imx6q-novena.dts                 |  19 ++
 arch/arm/mach-imx/Kconfig                     |   9 +
 images/Makefile.imx                           |   2 +
 11 files changed, 438 insertions(+)
 create mode 100644 arch/arm/boards/novena/Makefile
 create mode 100644 arch/arm/boards/novena/board.c
 create mode 100644 arch/arm/boards/novena/ddr_regs.h
 create mode 100644 arch/arm/boards/novena/flash-header-novena.imxcfg
 create mode 100644 arch/arm/boards/novena/lowlevel.c
 create mode 100644 arch/arm/dts/imx6q-novena.dts

-- 
2.39.0

[PATCH 1/5] ARM: novena: Add Kosagi Novena board

[John Watts]

The Kosagi Novena is an open source laptop released in 2014.

This patch adds the initial project skeleton for running the PBL
and debugging over the UART2 port (labeled DEBUG on the board.)

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/Makefile                      |  1 +
 arch/arm/boards/novena/Makefile               |  4 +
 arch/arm/boards/novena/board.c                | 13 ++++
 .../boards/novena/flash-header-novena.imxcfg  |  6 ++
 arch/arm/boards/novena/lowlevel.c             | 73 +++++++++++++++++++
 arch/arm/configs/imx_v7_defconfig             |  1 +
 arch/arm/dts/Makefile                         |  1 +
 arch/arm/dts/imx6q-novena.dts                 |  4 +
 arch/arm/mach-imx/Kconfig                     |  6 ++
 images/Makefile.imx                           |  2 +
 10 files changed, 111 insertions(+)
 create mode 100644 arch/arm/boards/novena/Makefile
 create mode 100644 arch/arm/boards/novena/board.c
 create mode 100644 arch/arm/boards/novena/flash-header-novena.imxcfg
 create mode 100644 arch/arm/boards/novena/lowlevel.c
 create mode 100644 arch/arm/dts/imx6q-novena.dts

diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
index 0f4339ebed..177c1da2c5 100644
--- a/arch/arm/boards/Makefile
+++ b/arch/arm/boards/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_MACH_BEAGLEBONE)			+= beaglebone/
 obj-$(CONFIG_MACH_CANON_A1100)			+= canon-a1100/
 obj-$(CONFIG_MACH_CM_FX6)			+= cm-fx6/
 obj-$(CONFIG_MACH_NITROGEN6)			+= boundarydevices-nitrogen6/
+obj-$(CONFIG_MACH_NOVENA)			+= novena/
 obj-$(CONFIG_MACH_CCMX51)			+= ccxmx51/
 obj-$(CONFIG_MACH_CCMX53)			+= ccxmx53/
 obj-$(CONFIG_MACH_CFA10036)			+= crystalfontz-cfa10036/
diff --git a/arch/arm/boards/novena/Makefile b/arch/arm/boards/novena/Makefile
new file mode 100644
index 0000000000..da63d2625f
--- /dev/null
+++ b/arch/arm/boards/novena/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-y += board.o
+lwl-y += lowlevel.o
diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
new file mode 100644
index 0000000000..186f91093f
--- /dev/null
+++ b/arch/arm/boards/novena/board.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// SPDX-FileCopyrightText: 2023 John Watts
+
+#include <common.h>
+
+static int novena_device_init(void)
+{
+	if (!of_machine_is_compatible("kosagi,imx6q-novena"))
+		return 0;
+
+	return 0;
+}
+device_initcall(novena_device_init);
diff --git a/arch/arm/boards/novena/flash-header-novena.imxcfg b/arch/arm/boards/novena/flash-header-novena.imxcfg
new file mode 100644
index 0000000000..61425976ec
--- /dev/null
+++ b/arch/arm/boards/novena/flash-header-novena.imxcfg
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+loadaddr 0x00907000
+soc imx6
+max_load_size 0x11000
+ivtofs 0x400
diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
new file mode 100644
index 0000000000..855855c6b9
--- /dev/null
+++ b/arch/arm/boards/novena/lowlevel.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// SPDX-FileCopyrightText: 2023 John Watts
+
+#include <asm/barebox-arm.h>
+#include <common.h>
+#include <debug_ll.h>
+#include <mach/esdctl.h>
+#include <mach/generic.h>
+#include <mach/imx6.h>
+#include <mach/iomux-mx6.h>
+#include <mach/xload.h>
+#include <soc/fsl/fsl_udc.h>
+
+#define MX6_OCRAM_BASE_ADDR 0x00900000
+#define STACK_TOP (MX6_OCRAM_BASE_ADDR + SZ_128K)
+
+extern char __dtb_imx6q_novena_start[];
+
+static bool running_from_ram(void)
+{
+	return (get_pc() >= MX6_MMDC_PORT01_BASE_ADDR);
+}
+
+static void setup_uart(void)
+{
+	/* NOTE: RX is needed for TX to work on this board */
+	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D26__UART2_RXD);
+	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D27__UART2_TXD);
+	imx6_uart_setup(IOMEM(MX6_UART2_BASE_ADDR));
+	pbl_set_putc(imx_uart_putc, IOMEM(MX6_UART2_BASE_ADDR));
+	pr_debug(">");
+}
+
+static void load_barebox(void)
+{
+	enum bootsource bootsrc;
+	int bootinstance;
+
+	imx6_get_boot_source(&bootsrc, &bootinstance);
+
+	if (bootsrc == BOOTSOURCE_SERIAL) {
+		imx6_barebox_start_usb(IOMEM(MX6_MMDC_PORT01_BASE_ADDR));
+	} else if (bootsrc == BOOTSOURCE_MMC) {
+		imx6_esdhc_start_image(bootinstance);
+	} else {
+		pr_err("Unsupported boot source %x instance %x\n",
+					bootsrc, bootinstance);
+		hang();
+	}
+}
+
+static void boot_barebox(void)
+{
+	void *fdt = __dtb_imx6q_novena_start + get_runtime_offset();
+
+	imx6q_barebox_entry(fdt);
+}
+
+ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
+{
+	imx6_cpu_lowlevel_init();
+	relocate_to_current_adr();
+	setup_c();
+	barrier();
+
+	if (!running_from_ram()) {
+		imx6_ungate_all_peripherals();
+		setup_uart();
+		load_barebox();
+	} else {
+		boot_barebox();
+	}
+}
diff --git a/arch/arm/configs/imx_v7_defconfig b/arch/arm/configs/imx_v7_defconfig
index 3cbec267fc..4d4b286140 100644
--- a/arch/arm/configs/imx_v7_defconfig
+++ b/arch/arm/configs/imx_v7_defconfig
@@ -25,6 +25,7 @@ CONFIG_MACH_SABRELITE=y
 CONFIG_MACH_SABRESD=y
 CONFIG_MACH_FREESCALE_IMX6SX_SABRESDB=y
 CONFIG_MACH_NITROGEN6=y
+CONFIG_MACH_NOVENA=y
 CONFIG_MACH_SOLIDRUN_MICROSOM=y
 CONFIG_MACH_TECHNEXION_PICO_HOBBIT=y
 CONFIG_MACH_TECHNEXION_WANDBOARD=y
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 5ceb97129f..ebb2d6bef8 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -46,6 +46,7 @@ lwl-$(CONFIG_MACH_MYIRTECH_X335X) += am335x-myirtech-myd.dtb.o
 lwl-$(CONFIG_MACH_NETGEAR_RN104) += armada-370-rn104-bb.dtb.o
 lwl-$(CONFIG_MACH_NETGEAR_RN2120) += armada-xp-rn2120-bb.dtb.o
 lwl-$(CONFIG_MACH_NITROGEN6) += imx6q-nitrogen6x.dtb.o imx6dl-nitrogen6x.dtb.o imx6qp-nitrogen6_max.dtb.o
+lwl-$(CONFIG_MACH_NOVENA) += imx6q-novena.dtb.o
 lwl-$(CONFIG_MACH_NVIDIA_BEAVER) += tegra30-beaver.dtb.o
 lwl-$(CONFIG_MACH_NVIDIA_JETSON) += tegra124-jetson-tk1.dtb.o
 lwl-$(CONFIG_MACH_PCA100) += imx27-phytec-phycard-s-rdk-bb.dtb.o
diff --git a/arch/arm/dts/imx6q-novena.dts b/arch/arm/dts/imx6q-novena.dts
new file mode 100644
index 0000000000..07471cb132
--- /dev/null
+++ b/arch/arm/dts/imx6q-novena.dts
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR X11
+// SPDX-FileCopyrightText: 2023 John Watts
+
+#include <arm/imx6q-novena.dts>
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 84b763f83f..a644cbb4df 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -433,6 +433,12 @@ config MACH_NITROGEN6
 	bool "BoundaryDevices Nitrogen6 boards"
 	select ARCH_IMX6
 
+config MACH_NOVENA
+	bool "Kosagi Novena board"
+	select ARCH_IMX6
+	select MCI_IMX_ESDHC_PBL
+	select USB_GADGET_DRIVER_ARC_PBL
+
 config MACH_SOLIDRUN_MICROSOM
 	bool "SolidRun MicroSOM based devices"
 	select ARCH_IMX6
diff --git a/images/Makefile.imx b/images/Makefile.imx
index e9f4ba64e4..65914212bf 100644
--- a/images/Makefile.imx
+++ b/images/Makefile.imx
@@ -258,6 +258,8 @@ $(call build_imx_habv4img, CONFIG_MACH_NITROGEN6, start_imx6dl_nitrogen6x_2g, bo
 
 $(call build_imx_habv4img, CONFIG_MACH_NITROGEN6, start_imx6qp_nitrogen6_max, boundarydevices-nitrogen6/flash-header-nitrogen6qp-max, boundarydevices-imx6qp-nitrogen6_max)
 
+$(call build_imx_habv4img, CONFIG_MACH_NOVENA, start_imx6q_novena, novena/flash-header-novena, imx6q-novena)
+
 $(call build_imx_habv4img, CONFIG_MACH_TX6X, start_imx6dl_tx6x_512m, karo-tx6x/flash-header-tx6dl-512m, karo-imx6dl-tx6x-512m)
 
 $(call build_imx_habv4img, CONFIG_MACH_TX6X, start_imx6dl_tx6x_1g, karo-tx6x/flash-header-tx6dl-1g, karo-imx6dl-tx6x-1g)
-- 
2.39.0

[PATCH 2/5] ARM: novena: Setup RAM using static configuration

[John Watts]

The Novena laptops were shipped with a 4GB stick that can be set up
using this DDR3 configuration.
This isn't ideal as the stick is swappable, but it will work for
most people.

The DDR registers are copied from /board/kosagi/novena/novena_spl.c
from the Novena U-Boot fork (tag novena-r2) with two changes:

- Bank interleaving is disabled: The Novena U-Boot code disables it
  unconditionally during execution anyway
- Page size is set to 1KB: I'm not sure the hardware supports x16 DIMMs

Mainline U-Boot has similar code, but the timing differences don't work
on my board so I'm unable to test or port that code over.

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/novena/ddr_regs.h | 119 ++++++++++++++++++++++++++++++
 arch/arm/boards/novena/lowlevel.c |   9 +++
 2 files changed, 128 insertions(+)
 create mode 100644 arch/arm/boards/novena/ddr_regs.h

diff --git a/arch/arm/boards/novena/ddr_regs.h b/arch/arm/boards/novena/ddr_regs.h
new file mode 100644
index 0000000000..5f18d5e0e4
--- /dev/null
+++ b/arch/arm/boards/novena/ddr_regs.h
@@ -0,0 +1,119 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* SPDX-FileCopyrightText: 2014 Marek Vasut <marex@denx.de> */
+
+#ifndef NOVENA_DDR_REGS_H
+#define NOVENA_DDR_REGS_H
+
+/* MEMORY CONTROLLER CONFIGURATION */
+
+static struct mx6dq_iomux_ddr_regs novena_ddr_regs = {
+	/* SDCLK[0:1], CAS, RAS, Reset: Differential input, 40ohm */
+	.dram_sdclk_0		= 0x00020038,
+	.dram_sdclk_1		= 0x00020038,
+	.dram_cas		= 0x00000038,
+	.dram_ras		= 0x00000038,
+	.dram_reset		= 0x00000038,
+	/* SDCKE[0:1]: 100k pull-up */
+	.dram_sdcke0		= 0x00003000,
+	.dram_sdcke1		= 0x00003000,
+	/* SDBA2: pull-up disabled */
+	.dram_sdba2		= 0x00000000,
+	/* SDODT[0:1]: 100k pull-up, 40 ohm */
+	.dram_sdodt0		= 0x00000038,
+	.dram_sdodt1		= 0x00000038,
+	/* SDQS[0:7]: Differential input, 40 ohm */
+	.dram_sdqs0		= 0x00000038,
+	.dram_sdqs1		= 0x00000038,
+	.dram_sdqs2		= 0x00000038,
+	.dram_sdqs3		= 0x00000038,
+	.dram_sdqs4		= 0x00000038,
+	.dram_sdqs5		= 0x00000038,
+	.dram_sdqs6		= 0x00000038,
+	.dram_sdqs7		= 0x00000038,
+
+	/* DQM[0:7]: Differential input, 40 ohm */
+	.dram_dqm0		= 0x00000038,
+	.dram_dqm1		= 0x00000038,
+	.dram_dqm2		= 0x00000038,
+	.dram_dqm3		= 0x00000038,
+	.dram_dqm4		= 0x00000038,
+	.dram_dqm5		= 0x00000038,
+	.dram_dqm6		= 0x00000038,
+	.dram_dqm7		= 0x00000038,
+};
+
+static struct mx6dq_iomux_grp_regs novena_grp_regs = {
+	/* DDR3 */
+	.grp_ddr_type		= 0x000c0000,
+	.grp_ddrmode_ctl	= 0x00020000,
+	/* Disable DDR pullups */
+	.grp_ddrpke		= 0x00000000,
+	/* ADDR[00:16], SDBA[0:1]: 40 ohm */
+	.grp_addds		= 0x00000038,
+	/* CS0/CS1/SDBA2/CKE0/CKE1/SDWE: 40 ohm */
+	.grp_ctlds		= 0x00000038,
+	/* DATA[00:63]: Differential input, 40 ohm */
+	.grp_ddrmode		= 0x00020000,
+	.grp_b0ds		= 0x00000038,
+	.grp_b1ds		= 0x00000038,
+	.grp_b2ds		= 0x00000038,
+	.grp_b3ds		= 0x00000038,
+	.grp_b4ds		= 0x00000038,
+	.grp_b5ds		= 0x00000038,
+	.grp_b6ds		= 0x00000038,
+	.grp_b7ds		= 0x00000038,
+};
+
+/* MEMORY STICK CONFIGURATION */
+
+static struct mx6_mmdc_calibration novena_mmdc_calib = {
+	/* write leveling calibration determine */
+	.p0_mpwldectrl0		= 0x00420048,
+	.p0_mpwldectrl1		= 0x006f0059,
+	.p1_mpwldectrl0		= 0x005a0104,
+	.p1_mpwldectrl1		= 0x01070113,
+	/* Read DQS Gating calibration */
+	.p0_mpdgctrl0		= 0x437c040b,
+	.p0_mpdgctrl1		= 0x0413040e,
+	.p1_mpdgctrl0		= 0x444f0446,
+	.p1_mpdgctrl1		= 0x044d0422,
+	/* Read Calibration: DQS delay relative to DQ read access */
+	.p0_mprddlctl		= 0x4c424249,
+	.p1_mprddlctl		= 0x4e48414f,
+	/* Write Calibration: DQ/DM delay relative to DQS write access */
+	.p0_mpwrdlctl		= 0x42414641,
+	.p1_mpwrdlctl		= 0x46374b43,
+};
+
+static struct mx6_ddr_sysinfo novena_ddr_info = {
+	/* Width of data bus: 0=16, 1=32, 2=64 */
+	.dsize		= 2,
+	/* Config for full 4GB range so that get_mem_size() works */
+	.cs_density	= 32,	/* 32Gb per CS */
+	/* Single chip select */
+	.ncs		= 1,
+	.cs1_mirror	= 0,
+	.rtt_wr		= 1,	/* RTT_Wr = RZQ/4 */
+	.rtt_nom	= 2,	/* RTT_Nom = RZQ/2 */
+	.walat		= 3,	/* Write additional latency */
+	.ralat		= 7,	/* Read additional latency */
+	.mif3_mode	= 3,	/* Command prediction working mode */
+	.bi_on		= 0,	/* Bank interleaving disabled */
+	.sde_to_rst	= 0x10,	/* 14 cycles, 200us (JEDEC default) */
+	.rst_to_cke	= 0x23,	/* 33 cycles, 500us (JEDEC default) */
+};
+
+static struct mx6_ddr3_cfg novena_ddr_cfg = {
+	.mem_speed	= 1600,
+	.density	= 4,
+	.width		= 64,
+	.banks		= 8,
+	.rowaddr	= 16,
+	.coladdr	= 10,
+	.pagesz		= 1,
+	.trcd		= 1300,
+	.trcmin		= 4900,
+	.trasmin	= 3590,
+};
+
+#endif
diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
index 855855c6b9..ee71d9edd6 100644
--- a/arch/arm/boards/novena/lowlevel.c
+++ b/arch/arm/boards/novena/lowlevel.c
@@ -7,9 +7,11 @@
 #include <mach/esdctl.h>
 #include <mach/generic.h>
 #include <mach/imx6.h>
+#include <mach/imx6-mmdc.h>
 #include <mach/iomux-mx6.h>
 #include <mach/xload.h>
 #include <soc/fsl/fsl_udc.h>
+#include "ddr_regs.h"
 
 #define MX6_OCRAM_BASE_ADDR 0x00900000
 #define STACK_TOP (MX6_OCRAM_BASE_ADDR + SZ_128K)
@@ -31,6 +33,12 @@ static void setup_uart(void)
 	pr_debug(">");
 }
 
+static void setup_ram(void)
+{
+	mx6dq_dram_iocfg(64, &novena_ddr_regs, &novena_grp_regs);
+	mx6_dram_cfg(&novena_ddr_info, &novena_mmdc_calib, &novena_ddr_cfg);
+}
+
 static void load_barebox(void)
 {
 	enum bootsource bootsrc;
@@ -66,6 +74,7 @@ ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
 	if (!running_from_ram()) {
 		imx6_ungate_all_peripherals();
 		setup_uart();
+		setup_ram();
 		load_barebox();
 	} else {
 		boot_barebox();
-- 
2.39.0

[PATCH 3/5] ARM: novena: Require the PFUZE regulator

[John Watts]

The Novena board uses a PF0100 for its power regulation, so select it when
building with support for the board.

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/mach-imx/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index a644cbb4df..5f2d4f4123 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -437,6 +437,7 @@ config MACH_NOVENA
 	bool "Kosagi Novena board"
 	select ARCH_IMX6
 	select MCI_IMX_ESDHC_PBL
+	select REGULATOR_PFUZE
 	select USB_GADGET_DRIVER_ARC_PBL
 
 config MACH_SOLIDRUN_MICROSOM
-- 
2.39.0

[PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

The Novena has an EEPROM used for storing information about the board,
including the Ethernet MAC address.

The reference for the EEPROM fields is the novena-eeprom source code.

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/novena/board.c | 68 ++++++++++++++++++++++++++++++++++
 arch/arm/dts/imx6q-novena.dts  | 15 ++++++++
 2 files changed, 83 insertions(+)

diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
index 186f91093f..0341117c46 100644
--- a/arch/arm/boards/novena/board.c
+++ b/arch/arm/boards/novena/board.c
@@ -2,12 +2,80 @@
 // SPDX-FileCopyrightText: 2023 John Watts
 
 #include <common.h>
+#include <fs.h>
+#include <libfile.h>
+#include <net.h>
+
+struct novena_eeprom {
+	uint8_t signature[6]; /* 'Novena' */
+	uint8_t version; /* 1 or 2, not checked */
+	uint8_t page_size; /* v2 only: EEPROM read/write page */
+	uint32_t serial; /* 32-bit serial number */
+	uint8_t mac[6]; /* Gigabit MAC address */
+	uint16_t features; /* features */
+	/* ... extra fields omitted ... */
+} __packed;
+
+static struct novena_eeprom *novena_read_eeprom(void)
+{
+	size_t read;
+	loff_t max = sizeof(struct novena_eeprom);
+	void *eeprom;
+
+	int rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);
+
+	if (rc < 0 && rc != -EFBIG) {
+		pr_err("Unable to read Novena EEPROM: %s\n", strerror(-rc));
+		return NULL;
+	} else if (read != max) {
+		pr_err("Short read from Novena EEPROM?\n");
+		return NULL;
+	} else {
+		return eeprom;
+	}
+}
+
+static bool novena_check_eeprom(struct novena_eeprom *eeprom)
+{
+	char *sig = eeprom->signature;
+	size_t size = sizeof(eeprom->signature);
+
+	if (memcmp("Novena", sig, size) != 0) {
+		pr_err("Unknown Novena EEPROM signature\n");
+		return false;
+	}
+
+	return true;
+}
+
+static void novena_set_mac(struct novena_eeprom *eeprom)
+{
+	struct device_node *dnode;
+
+	dnode = of_find_node_by_alias(of_get_root_node(), "ethernet0");
+	if (dnode)
+		of_eth_register_ethaddr(dnode, eeprom->mac);
+	else
+		pr_err("Unable to find ethernet node\n");
+}
+
+static void novena_try_eeprom(void)
+{
+	struct novena_eeprom *eeprom = novena_read_eeprom();
+
+	if (!eeprom || !novena_check_eeprom(eeprom))
+		return;
+
+	novena_set_mac(eeprom);
+}
 
 static int novena_device_init(void)
 {
 	if (!of_machine_is_compatible("kosagi,imx6q-novena"))
 		return 0;
 
+	novena_try_eeprom();
+
 	return 0;
 }
 device_initcall(novena_device_init);
diff --git a/arch/arm/dts/imx6q-novena.dts b/arch/arm/dts/imx6q-novena.dts
index 07471cb132..095dcde0d5 100644
--- a/arch/arm/dts/imx6q-novena.dts
+++ b/arch/arm/dts/imx6q-novena.dts
@@ -2,3 +2,18 @@
 // SPDX-FileCopyrightText: 2023 John Watts
 
 #include <arm/imx6q-novena.dts>
+
+/ {
+	aliases {
+		eeprom0 = &eeprom;
+	};
+};
+
+&i2c3 {
+	eeprom: eeprom@56 {
+		compatible = "24c512";
+		reg = <0x56>;
+		pagesize = <128>;
+		status = "okay";
+	};
+};
-- 
2.39.0

[PATCH 5/5] ARM: novena: Use DDR3 information from SPD EEPROM

[John Watts]

The Novena supports swappable memory which means we must query the memory
for configuration details. This patch uses values stored in the SPD EEPROM
to configure the i.MX6 memory controller.

If the SPD can't be read, the default configuration for a 4GB stick is used.

Calibration for the installed stick is run unconditionally.

Verification that this works was done by comparing MMDC registers before and
after and carefully reading other people's source code.

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/novena/lowlevel.c | 113 ++++++++++++++++++++++++++++++
 arch/arm/mach-imx/Kconfig         |   2 +
 2 files changed, 115 insertions(+)

diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
index ee71d9edd6..753fc5dd4a 100644
--- a/arch/arm/boards/novena/lowlevel.c
+++ b/arch/arm/boards/novena/lowlevel.c
@@ -3,6 +3,8 @@
 
 #include <asm/barebox-arm.h>
 #include <common.h>
+#include <ddr_dimms.h>
+#include <ddr_spd.h>
 #include <debug_ll.h>
 #include <mach/esdctl.h>
 #include <mach/generic.h>
@@ -10,6 +12,8 @@
 #include <mach/imx6-mmdc.h>
 #include <mach/iomux-mx6.h>
 #include <mach/xload.h>
+#include <pbl.h>
+#include <pbl/i2c.h>
 #include <soc/fsl/fsl_udc.h>
 #include "ddr_regs.h"
 
@@ -18,6 +22,112 @@
 
 extern char __dtb_imx6q_novena_start[];
 
+static struct spd_eeprom spd_eeprom;
+static struct dimm_params dimm_params;
+
+#if IN_PBL
+/* I2C in the PBL requires udelay, provide our own */
+void udelay(unsigned long us)
+{
+	volatile unsigned long i;
+
+	for (i = 0; i < us * 4; i++);
+}
+#endif
+
+static struct pbl_i2c *setup_spd_i2c(void)
+{
+	void __iomem *iomuxbase = IOMEM(MX6_IOMUXC_BASE_ADDR);
+
+	imx_setup_pad(iomuxbase, MX6Q_PAD_CSI0_DAT8__I2C1_SDA);
+	imx_setup_pad(iomuxbase, MX6Q_PAD_CSI0_DAT9__I2C1_SCL);
+	return imx6_i2c_early_init(IOMEM(MX6_I2C1_BASE_ADDR));
+}
+
+static struct spd_eeprom *read_spd(void)
+{
+	struct spd_eeprom *eeprom = &spd_eeprom;
+	struct pbl_i2c *i2c = setup_spd_i2c();
+	int rc;
+
+	rc = spd_read_eeprom(i2c, 0x50, eeprom, SPD_MEMTYPE_DDR3);
+	if (rc < 0) {
+		pr_err("Couldn't read SPD EEPROM:  %x\n", rc);
+		return NULL;
+	}
+
+	rc = ddr3_spd_check(&eeprom->ddr3);
+	if (rc < 0) {
+		pr_err("Couldn't verify SPD data: %x\n", rc);
+		return NULL;
+	}
+
+	return eeprom;
+}
+
+static bool check_spd_compatible(struct spd_eeprom *eeprom)
+{
+	if (eeprom->ddr3.mem_type != 0x0b) {
+		pr_err("SPD data is not DDR3\n");
+		return false;
+	}
+
+	if ((eeprom->ddr3.bus_width & 0x1f) != 0x03) {
+		pr_err("SPD data is for a 64-bit bus\n");
+		return false;
+	}
+
+	return true;
+}
+
+static void setup_dimm_settings(struct dimm_params *params,
+				struct mx6_ddr_sysinfo *info,
+				struct mx6_ddr3_cfg *cfg)
+{
+	info->ncs = params->n_ranks;
+	info->cs_density = params->capacity / 0x8000000;
+	cfg->mem_speed = params->tckmin_x_ps;
+	cfg->density = info->cs_density / params->n_banks_per_sdram_device;
+	cfg->width = params->data_width;
+	cfg->banks = params->n_banks_per_sdram_device;
+	cfg->rowaddr = params->n_row_addr;
+	cfg->coladdr = params->n_col_addr;
+	cfg->trcd = params->trcd_ps / 10;
+	cfg->trcmin = params->trc_ps / 10;
+	cfg->trasmin = params->tras_ps / 10;
+	cfg->SRT = params->extended_op_srt;
+
+	if (params->device_width >= 16)
+		cfg->pagesz = 2;
+}
+
+static void read_dimm_settings(void)
+{
+	struct spd_eeprom *eeprom = read_spd();
+	struct dimm_params *params = &dimm_params;
+	int rc;
+
+	if (!eeprom) {
+		pr_err("Couldn't read SPD EEPROM, using default settings\n");
+		return;
+	}
+
+	if (!check_spd_compatible(eeprom)) {
+		pr_err("DIMM stick incompatible\n");
+		hang();
+	}
+
+	rc = ddr3_compute_dimm_parameters(&eeprom->ddr3, params);
+	if (rc < 0) {
+		pr_err("Couldn't compute DIMM params: %x\n", rc);
+		return;
+	}
+
+	setup_dimm_settings(params, &novena_ddr_info, &novena_ddr_cfg);
+
+	pr_info("Found DIMM: %s\n", params->mpart);
+}
+
 static bool running_from_ram(void)
 {
 	return (get_pc() >= MX6_MMDC_PORT01_BASE_ADDR);
@@ -37,6 +147,8 @@ static void setup_ram(void)
 {
 	mx6dq_dram_iocfg(64, &novena_ddr_regs, &novena_grp_regs);
 	mx6_dram_cfg(&novena_ddr_info, &novena_mmdc_calib, &novena_ddr_cfg);
+	mmdc_do_write_level_calibration();
+	mmdc_do_dqs_calibration();
 }
 
 static void load_barebox(void)
@@ -74,6 +186,7 @@ ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
 	if (!running_from_ram()) {
 		imx6_ungate_all_peripherals();
 		setup_uart();
+		read_dimm_settings();
 		setup_ram();
 		load_barebox();
 	} else {
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 5f2d4f4123..00c9aacb2b 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -436,6 +436,8 @@ config MACH_NITROGEN6
 config MACH_NOVENA
 	bool "Kosagi Novena board"
 	select ARCH_IMX6
+	select DDR_SPD
+	select I2C_IMX_EARLY
 	select MCI_IMX_ESDHC_PBL
 	select REGULATOR_PFUZE
 	select USB_GADGET_DRIVER_ARC_PBL
-- 
2.39.0

Re: [PATCH 5/5] ARM: novena: Use DDR3 information from SPD EEPROM

[John Watts]

Just a note to myself: After doing some more testing on this code I've found it
doesn't work exactly right:

- It calculates cs_density and density wrong
- I'm still not entirely sure if ddr_spd needs to be changed to aid calibration

Speaking of calibration: I spent a while trying to figure this out but the
Barebox version of mmdc_do_write_level_calibration does not handle calibration
errors. I'll have a patch on the way to fix both of these soon.

With those bugs fixed I can confirm just switching in a 2GB stick works and
Barebox's memtest runs successfully.

On Mon, Jan 23, 2023 at 04:51:41AM +1100, John Watts wrote:
> The Novena supports swappable memory which means we must query the memory
> for configuration details. This patch uses values stored in the SPD EEPROM
> to configure the i.MX6 memory controller.
> 
> If the SPD can't be read, the default configuration for a 4GB stick is used.
> 
> Calibration for the installed stick is run unconditionally.
> 
> Verification that this works was done by comparing MMDC registers before and
> after and carefully reading other people's source code.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  arch/arm/boards/novena/lowlevel.c | 113 ++++++++++++++++++++++++++++++
>  arch/arm/mach-imx/Kconfig         |   2 +
>  2 files changed, 115 insertions(+)
> 
> diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
> index ee71d9edd6..753fc5dd4a 100644
> --- a/arch/arm/boards/novena/lowlevel.c
> +++ b/arch/arm/boards/novena/lowlevel.c
> @@ -3,6 +3,8 @@
>  
>  #include <asm/barebox-arm.h>
>  #include <common.h>
> +#include <ddr_dimms.h>
> +#include <ddr_spd.h>
>  #include <debug_ll.h>
>  #include <mach/esdctl.h>
>  #include <mach/generic.h>
> @@ -10,6 +12,8 @@
>  #include <mach/imx6-mmdc.h>
>  #include <mach/iomux-mx6.h>
>  #include <mach/xload.h>
> +#include <pbl.h>
> +#include <pbl/i2c.h>
>  #include <soc/fsl/fsl_udc.h>
>  #include "ddr_regs.h"
>  
> @@ -18,6 +22,112 @@
>  
>  extern char __dtb_imx6q_novena_start[];
>  
> +static struct spd_eeprom spd_eeprom;
> +static struct dimm_params dimm_params;
> +
> +#if IN_PBL
> +/* I2C in the PBL requires udelay, provide our own */
> +void udelay(unsigned long us)
> +{
> +	volatile unsigned long i;
> +
> +	for (i = 0; i < us * 4; i++);
> +}
> +#endif
> +
> +static struct pbl_i2c *setup_spd_i2c(void)
> +{
> +	void __iomem *iomuxbase = IOMEM(MX6_IOMUXC_BASE_ADDR);
> +
> +	imx_setup_pad(iomuxbase, MX6Q_PAD_CSI0_DAT8__I2C1_SDA);
> +	imx_setup_pad(iomuxbase, MX6Q_PAD_CSI0_DAT9__I2C1_SCL);
> +	return imx6_i2c_early_init(IOMEM(MX6_I2C1_BASE_ADDR));
> +}
> +
> +static struct spd_eeprom *read_spd(void)
> +{
> +	struct spd_eeprom *eeprom = &spd_eeprom;
> +	struct pbl_i2c *i2c = setup_spd_i2c();
> +	int rc;
> +
> +	rc = spd_read_eeprom(i2c, 0x50, eeprom, SPD_MEMTYPE_DDR3);
> +	if (rc < 0) {
> +		pr_err("Couldn't read SPD EEPROM:  %x\n", rc);
> +		return NULL;
> +	}
> +
> +	rc = ddr3_spd_check(&eeprom->ddr3);
> +	if (rc < 0) {
> +		pr_err("Couldn't verify SPD data: %x\n", rc);
> +		return NULL;
> +	}
> +
> +	return eeprom;
> +}
> +
> +static bool check_spd_compatible(struct spd_eeprom *eeprom)
> +{
> +	if (eeprom->ddr3.mem_type != 0x0b) {
> +		pr_err("SPD data is not DDR3\n");
> +		return false;
> +	}
> +
> +	if ((eeprom->ddr3.bus_width & 0x1f) != 0x03) {
> +		pr_err("SPD data is for a 64-bit bus\n");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static void setup_dimm_settings(struct dimm_params *params,
> +				struct mx6_ddr_sysinfo *info,
> +				struct mx6_ddr3_cfg *cfg)
> +{
> +	info->ncs = params->n_ranks;
> +	info->cs_density = params->capacity / 0x8000000;
> +	cfg->mem_speed = params->tckmin_x_ps;
> +	cfg->density = info->cs_density / params->n_banks_per_sdram_device;
> +	cfg->width = params->data_width;
> +	cfg->banks = params->n_banks_per_sdram_device;
> +	cfg->rowaddr = params->n_row_addr;
> +	cfg->coladdr = params->n_col_addr;
> +	cfg->trcd = params->trcd_ps / 10;
> +	cfg->trcmin = params->trc_ps / 10;
> +	cfg->trasmin = params->tras_ps / 10;
> +	cfg->SRT = params->extended_op_srt;
> +
> +	if (params->device_width >= 16)
> +		cfg->pagesz = 2;
> +}
> +
> +static void read_dimm_settings(void)
> +{
> +	struct spd_eeprom *eeprom = read_spd();
> +	struct dimm_params *params = &dimm_params;
> +	int rc;
> +
> +	if (!eeprom) {
> +		pr_err("Couldn't read SPD EEPROM, using default settings\n");
> +		return;
> +	}
> +
> +	if (!check_spd_compatible(eeprom)) {
> +		pr_err("DIMM stick incompatible\n");
> +		hang();
> +	}
> +
> +	rc = ddr3_compute_dimm_parameters(&eeprom->ddr3, params);
> +	if (rc < 0) {
> +		pr_err("Couldn't compute DIMM params: %x\n", rc);
> +		return;
> +	}
> +
> +	setup_dimm_settings(params, &novena_ddr_info, &novena_ddr_cfg);
> +
> +	pr_info("Found DIMM: %s\n", params->mpart);
> +}
> +
>  static bool running_from_ram(void)
>  {
>  	return (get_pc() >= MX6_MMDC_PORT01_BASE_ADDR);
> @@ -37,6 +147,8 @@ static void setup_ram(void)
>  {
>  	mx6dq_dram_iocfg(64, &novena_ddr_regs, &novena_grp_regs);
>  	mx6_dram_cfg(&novena_ddr_info, &novena_mmdc_calib, &novena_ddr_cfg);
> +	mmdc_do_write_level_calibration();
> +	mmdc_do_dqs_calibration();
>  }
>  
>  static void load_barebox(void)
> @@ -74,6 +186,7 @@ ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
>  	if (!running_from_ram()) {
>  		imx6_ungate_all_peripherals();
>  		setup_uart();
> +		read_dimm_settings();
>  		setup_ram();
>  		load_barebox();
>  	} else {
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 5f2d4f4123..00c9aacb2b 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -436,6 +436,8 @@ config MACH_NITROGEN6
>  config MACH_NOVENA
>  	bool "Kosagi Novena board"
>  	select ARCH_IMX6
> +	select DDR_SPD
> +	select I2C_IMX_EARLY
>  	select MCI_IMX_ESDHC_PBL
>  	select REGULATOR_PFUZE
>  	select USB_GADGET_DRIVER_ARC_PBL
> -- 
> 2.39.0
> 

Re: [PATCH 1/5] ARM: novena: Add Kosagi Novena board

[Sascha Hauer]

On Mon, Jan 23, 2023 at 04:51:37AM +1100, John Watts wrote:
> The Kosagi Novena is an open source laptop released in 2014.
> 
> This patch adds the initial project skeleton for running the PBL
> and debugging over the UART2 port (labeled DEBUG on the board.)
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  arch/arm/boards/Makefile                      |  1 +
>  arch/arm/boards/novena/Makefile               |  4 +
>  arch/arm/boards/novena/board.c                | 13 ++++
>  .../boards/novena/flash-header-novena.imxcfg  |  6 ++
>  arch/arm/boards/novena/lowlevel.c             | 73 +++++++++++++++++++
>  arch/arm/configs/imx_v7_defconfig             |  1 +
>  arch/arm/dts/Makefile                         |  1 +
>  arch/arm/dts/imx6q-novena.dts                 |  4 +
>  arch/arm/mach-imx/Kconfig                     |  6 ++
>  images/Makefile.imx                           |  2 +
>  10 files changed, 111 insertions(+)
>  create mode 100644 arch/arm/boards/novena/Makefile
>  create mode 100644 arch/arm/boards/novena/board.c
>  create mode 100644 arch/arm/boards/novena/flash-header-novena.imxcfg
>  create mode 100644 arch/arm/boards/novena/lowlevel.c
>  create mode 100644 arch/arm/dts/imx6q-novena.dts
> 
> diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
> index 0f4339ebed..177c1da2c5 100644
> --- a/arch/arm/boards/Makefile
> +++ b/arch/arm/boards/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_MACH_BEAGLEBONE)			+= beaglebone/
>  obj-$(CONFIG_MACH_CANON_A1100)			+= canon-a1100/
>  obj-$(CONFIG_MACH_CM_FX6)			+= cm-fx6/
>  obj-$(CONFIG_MACH_NITROGEN6)			+= boundarydevices-nitrogen6/
> +obj-$(CONFIG_MACH_NOVENA)			+= novena/
>  obj-$(CONFIG_MACH_CCMX51)			+= ccxmx51/
>  obj-$(CONFIG_MACH_CCMX53)			+= ccxmx53/
>  obj-$(CONFIG_MACH_CFA10036)			+= crystalfontz-cfa10036/
> diff --git a/arch/arm/boards/novena/Makefile b/arch/arm/boards/novena/Makefile
> new file mode 100644
> index 0000000000..da63d2625f
> --- /dev/null
> +++ b/arch/arm/boards/novena/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +obj-y += board.o
> +lwl-y += lowlevel.o
> diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
> new file mode 100644
> index 0000000000..186f91093f
> --- /dev/null
> +++ b/arch/arm/boards/novena/board.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// SPDX-FileCopyrightText: 2023 John Watts
> +
> +#include <common.h>
> +
> +static int novena_device_init(void)
> +{
> +	if (!of_machine_is_compatible("kosagi,imx6q-novena"))
> +		return 0;
> +
> +	return 0;
> +}
> +device_initcall(novena_device_init);

The new way to register board code is to register a driver that matches
to OF root node, see arch/arm/boards/nxp-imx8mm-evk/board.c:74 for an
example.

Also we have a mechanism in place that probes devices on demand when
they are needed and no longer in a fixed order. As this requires some
support by the board it must be enabled per board using:

BAREBOX_DEEP_PROBE_ENABLE(novema_of_match);

It would be great if you could switch to that. As said the board code
needs to support this, I'll comment to that in the next patch.

Sascha

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

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[Sascha Hauer]

On Mon, Jan 23, 2023 at 04:51:40AM +1100, John Watts wrote:
> The Novena has an EEPROM used for storing information about the board,
> including the Ethernet MAC address.
> 
> The reference for the EEPROM fields is the novena-eeprom source code.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  arch/arm/boards/novena/board.c | 68 ++++++++++++++++++++++++++++++++++
>  arch/arm/dts/imx6q-novena.dts  | 15 ++++++++
>  2 files changed, 83 insertions(+)
> 
> diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
> index 186f91093f..0341117c46 100644
> --- a/arch/arm/boards/novena/board.c
> +++ b/arch/arm/boards/novena/board.c
> @@ -2,12 +2,80 @@
>  // SPDX-FileCopyrightText: 2023 John Watts
>  
>  #include <common.h>
> +#include <fs.h>
> +#include <libfile.h>
> +#include <net.h>
> +
> +struct novena_eeprom {
> +	uint8_t signature[6]; /* 'Novena' */
> +	uint8_t version; /* 1 or 2, not checked */
> +	uint8_t page_size; /* v2 only: EEPROM read/write page */
> +	uint32_t serial; /* 32-bit serial number */
> +	uint8_t mac[6]; /* Gigabit MAC address */
> +	uint16_t features; /* features */
> +	/* ... extra fields omitted ... */
> +} __packed;
> +
> +static struct novena_eeprom *novena_read_eeprom(void)
> +{
> +	size_t read;
> +	loff_t max = sizeof(struct novena_eeprom);
> +	void *eeprom;
> +
> +	int rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);

And here is the point where you have to request that the EEPROM is
actually availabe to support the deep probe mechanism. Before reading
the EEPROM add a call to:

	of_device_ensure_probed_by_alias("eeprom0");

Nowadays the read-MAC-address-from-EEPROM thingy would likely be done
using nvmem cells which would boil this code down to device tree changes
only. If you're feeling brave you could change this, I won't insist on
it though.

Sascha

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

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

On Mon, Jan 23, 2023 at 10:33:05AM +0100, Sascha Hauer wrote:
> > +static struct novena_eeprom *novena_read_eeprom(void)
> > +{
> > +	size_t read;
> > +	loff_t max = sizeof(struct novena_eeprom);
> > +	void *eeprom;
> > +
> > +	int rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);
> 
> And here is the point where you have to request that the EEPROM is
> actually availabe to support the deep probe mechanism. Before reading
> the EEPROM add a call to:
> 
> 	of_device_ensure_probed_by_alias("eeprom0");
> 
> Nowadays the read-MAC-address-from-EEPROM thingy would likely be done
> using nvmem cells which would boil this code down to device tree changes
> only. If you're feeling brave you could change this, I won't insist on
> it though.
> 
> Sascha

That makes sense, I'll switch to the new board setup in V2.

Using nvmem cells would work better for this case, but in the future I will
need to read the EEPROM anyway to implement some features specific to the
laptop based on the features flag. I figured I'd get the EEPROM loading done
now rather than later, though if this is jumping the gun I can just change
it to an nvmem patch.

John

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

On Mon, Jan 23, 2023 at 10:33:05AM +0100, Sascha Hauer wrote:
> And here is the point where you have to request that the EEPROM is
> actually availabe to support the deep probe mechanism. Before reading
> the EEPROM add a call to:
> 
> 	of_device_ensure_probed_by_alias("eeprom0");
> 
> Nowadays the read-MAC-address-from-EEPROM thingy would likely be done
> using nvmem cells which would boil this code down to device tree changes
> only. If you're feeling brave you could change this, I won't insist on
> it though.
> 
> Sascha

Hey Sascha,

I did some refactoring today based on your suggestions and now have this code:

        rc = of_device_ensure_probed_by_alias("eeprom0");

        if (rc < 0) {
                pr_err("Unable to probe Novena EEPROM: %s\n", strerror(-rc));
                return NULL;
        }

        rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);

This gets me this bootup sequence:

  of_platform_device_create: register device 21a8000.i2c@21a8000.of, io=0x021a8000
  register_device: 21a8000.i2c@21a8000.of
          probe-> 21a8000.i2c@21a8000.of
  imx-iomuxv3 20e0000.pinctrl@20e0000.of: set state: /soc/bus@2000000/pinctrl@20e0000/i2c3grp-novena
  __request_region ok: 0x021a8000:0x021abfff flags=0x0
  of_get_named_gpio_flags: cannot parse sda-gpios property: -2
  of_get_named_gpio_flags: cannot parse scl-gpios property: -2
  <NULL>: <i2c_fsl_set_clk> I2C_CLK=66000000, REQ DIV=660
  <NULL>: <i2c_fsl_set_clk> IFDR[IC]=0x39, REAL DIV=768
  register_device: i2c0
  register_device: es83280
  es83280: registered on bus 0, chip->addr 0x11
  register_device: 24c5120
              probe-> 24c5120
  of_get_named_gpio_flags: cannot parse wp-gpios property: -2
  at24 24c5120: Registering nvmem device eeprom0
  register_device: eeprom00
  at24 24c5120: registered on bus 0, chip->addr 0x56
  eeprom00: read ofs: 0x00000000 count: 0x00000014
  i2c0: master_xfer[0] W, addr=0x56, len=2
  i2c0: master_xfer[1] R, addr=0x56, len=20
  i2c0: timeout waiting for status set 0x20, cur status: 0x93
  at24 24c5120: read 20@0 --> -95 (1046280961)
  Unable to read Novena EEPROM: Connection timed out

After the shell boots I'm able to read /dev/eeprom0 just fine.

John.

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[Sascha Hauer]

On Wed, Jan 25, 2023 at 05:35:53AM +1100, John Watts wrote:
> On Mon, Jan 23, 2023 at 10:33:05AM +0100, Sascha Hauer wrote:
> > And here is the point where you have to request that the EEPROM is
> > actually availabe to support the deep probe mechanism. Before reading
> > the EEPROM add a call to:
> > 
> > 	of_device_ensure_probed_by_alias("eeprom0");
> > 
> > Nowadays the read-MAC-address-from-EEPROM thingy would likely be done
> > using nvmem cells which would boil this code down to device tree changes
> > only. If you're feeling brave you could change this, I won't insist on
> > it though.
> > 
> > Sascha
> 
> Hey Sascha,
> 
> I did some refactoring today based on your suggestions and now have this code:
> 
>         rc = of_device_ensure_probed_by_alias("eeprom0");
> 
>         if (rc < 0) {
>                 pr_err("Unable to probe Novena EEPROM: %s\n", strerror(-rc));
>                 return NULL;
>         }
> 
>         rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);
> 
> This gets me this bootup sequence:
> 
>   of_platform_device_create: register device 21a8000.i2c@21a8000.of, io=0x021a8000
>   register_device: 21a8000.i2c@21a8000.of
>           probe-> 21a8000.i2c@21a8000.of
>   imx-iomuxv3 20e0000.pinctrl@20e0000.of: set state: /soc/bus@2000000/pinctrl@20e0000/i2c3grp-novena
>   __request_region ok: 0x021a8000:0x021abfff flags=0x0
>   of_get_named_gpio_flags: cannot parse sda-gpios property: -2
>   of_get_named_gpio_flags: cannot parse scl-gpios property: -2

That's expected because you haven't specified sda-gpios or scl-gpios.
The message is only printed at debug level after all.

>   <NULL>: <i2c_fsl_set_clk> I2C_CLK=66000000, REQ DIV=660
>   <NULL>: <i2c_fsl_set_clk> IFDR[IC]=0x39, REAL DIV=768

Shouldn't be <NULL>. I have sent a patch for this a moment ago.
This is not your problem though.

>   register_device: i2c0
>   register_device: es83280
>   es83280: registered on bus 0, chip->addr 0x11
>   register_device: 24c5120
>               probe-> 24c5120
>   of_get_named_gpio_flags: cannot parse wp-gpios property: -2
>   at24 24c5120: Registering nvmem device eeprom0
>   register_device: eeprom00
>   at24 24c5120: registered on bus 0, chip->addr 0x56
>   eeprom00: read ofs: 0x00000000 count: 0x00000014
>   i2c0: master_xfer[0] W, addr=0x56, len=2
>   i2c0: master_xfer[1] R, addr=0x56, len=20
>   i2c0: timeout waiting for status set 0x20, cur status: 0x93
>   at24 24c5120: read 20@0 --> -95 (1046280961)

-95 (Operation not supported) is the return value of i2c_recover_bus()
when no bus recovery is implemented. The return value is confusing, I
sent a patch changing this to -EBUSY instead as the kernel does.
This also doesn't point us to the problem.

>   Unable to read Novena EEPROM: Connection timed out
> 
> After the shell boots I'm able to read /dev/eeprom0 just fine.

It looks like something is not initialized correctly yet. Normally
the usual suspects are pinctrl, clocks or regulators. Each of them
should be tied in by the deep probe mechanism as needed already, at
least when it's properly registered in the device tree. Clock looks
correct (66MHz), for the (pfuze) regulator I can't find anything in the
board code that configures something there later. You could verify
that the pinctrl has been configured already by activating the debugging
in drivers/pinctrl/imx-iomux-v3.c.
It could be something that pulls SDA low, but I don't have an idea why
it should work later.

Sascha

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

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

On Wed, Jan 25, 2023 at 09:04:06AM +0100, Sascha Hauer wrote:
> It looks like something is not initialized correctly yet. Normally
> the usual suspects are pinctrl, clocks or regulators. Each of them
> should be tied in by the deep probe mechanism as needed already, at
> least when it's properly registered in the device tree. Clock looks
> correct (66MHz), for the (pfuze) regulator I can't find anything in the
> board code that configures something there later. You could verify
> that the pinctrl has been configured already by activating the debugging
> in drivers/pinctrl/imx-iomux-v3.c.
> It could be something that pulls SDA low, but I don't have an idea why
> it should work later.

My immediate guess is that it's the pinctrl, but shouldn't this be set up
for me already? Or have I found a device tree bug?

John.

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[Sascha Hauer]

On Wed, Jan 25, 2023 at 07:11:36PM +1100, John Watts wrote:
> On Wed, Jan 25, 2023 at 09:04:06AM +0100, Sascha Hauer wrote:
> > It looks like something is not initialized correctly yet. Normally
> > the usual suspects are pinctrl, clocks or regulators. Each of them
> > should be tied in by the deep probe mechanism as needed already, at
> > least when it's properly registered in the device tree. Clock looks
> > correct (66MHz), for the (pfuze) regulator I can't find anything in the
> > board code that configures something there later. You could verify
> > that the pinctrl has been configured already by activating the debugging
> > in drivers/pinctrl/imx-iomux-v3.c.
> > It could be something that pulls SDA low, but I don't have an idea why
> > it should work later.
> 
> My immediate guess is that it's the pinctrl, but shouldn't this be set up
> for me already? Or have I found a device tree bug?

Yes, normally it should be set up already, unless of course there is
some needed pinctrl configuration in an unrelated device tree node.

Sascha

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

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

On Wed, Jan 25, 2023 at 09:19:23AM +0100, Sascha Hauer wrote:
> Yes, normally it should be set up already, unless of course there is
> some needed pinctrl configuration in an unrelated device tree node.
> 
> Sascha

Hi Sascha,

Here's what I think is going on:

- The I2C line requires 3v3_DELAYED for pull-ups
- The EEPROM requires 3v3_DELAYED for power
- 3v3_DELAYED requires VGEN5 from the PMIC (acts as a gate)
- The PMIC is loaded after the EEPROM

Most things on the board require these delayed rails, so I'm wondering
what the approach should be here. Should I:

- Rewrite the device tree to reference these delayed rails
- Probe the PMIC first

I did try this:

	reg_3p3v: regulator-3p3v {
		compatible = "regulator-fixed";
		regulator-name = "3P3V_DELAYED";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
		regulator-always-on;
		gate-supply = <&reg_vgen6>;
	};

But Barebox doesn't seem to see the 'regulator-always-on' as
'start immediately'. I actually updated the eeprom node to use
reg_3p3v as vcc-supply and things were still probed in the wrong order.

John.

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[Ahmad Fatoum]

On 25.01.23 14:31, John Watts wrote:
> On Wed, Jan 25, 2023 at 09:19:23AM +0100, Sascha Hauer wrote:
>> Yes, normally it should be set up already, unless of course there is
>> some needed pinctrl configuration in an unrelated device tree node.
>>
>> Sascha
> 
> Hi Sascha,
> 
> Here's what I think is going on:
> 
> - The I2C line requires 3v3_DELAYED for pull-ups
> - The EEPROM requires 3v3_DELAYED for power
> - 3v3_DELAYED requires VGEN5 from the PMIC (acts as a gate)
> - The PMIC is loaded after the EEPROM
> 
> Most things on the board require these delayed rails, so I'm wondering
> what the approach should be here. Should I:
> 
> - Rewrite the device tree to reference these delayed rails
> - Probe the PMIC first
> 
> I did try this:
> 
> 	reg_3p3v: regulator-3p3v {
> 		compatible = "regulator-fixed";
> 		regulator-name = "3P3V_DELAYED";
> 		regulator-min-microvolt = <3300000>;
> 		regulator-max-microvolt = <3300000>;
> 		regulator-always-on;
> 		gate-supply = <&reg_vgen6>;

regulator-fixed only understands vin-supply.

> 	};
> 
> But Barebox doesn't seem to see the 'regulator-always-on' as
> 'start immediately'.

For this to work we'd need:

  of_devices_ensure_probed_by_property("regulator-always-on");
  of_devices_ensure_probed_by_property("regulator-boot-on");

somewhere in generic code. Currently, these properties are only
consulted when the device is actually probed.

I actually updated the eeprom node to use
> reg_3p3v as vcc-supply and things were still probed in the wrong order.

That's odd. You can enable CONFIG_DEBUG_PROBES to see in what order
probes happened and at what depth (each probe within a probe is
indented more than its parent).

Cheers,
Ahmad


> 
> John.
> 
> 

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

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

On Wed, Jan 25, 2023 at 02:48:20PM +0100, Ahmad Fatoum wrote:
> regulator-fixed only understands vin-supply.

Right, changed. I was under the assumption that ANY node could specify an
X-supply.

> I actually updated the eeprom node to use
> > reg_3p3v as vcc-supply and things were still probed in the wrong order.
> 
> That's odd. You can enable CONFIG_DEBUG_PROBES to see in what order
> probes happened and at what depth (each probe within a probe is
> indented more than its parent).

Should that even work? The vcc-supply property isn't a real one.

Currently the probe log is this:

detected i.MX6 Quad revision 1.2
i.MX6 Quad unique ID: c1d99d4d8191854
    probe-> 21e8000.serial@21e8000.of
        probe-> 20e0000.pinctrl@20e0000.of
    probe-> 21b0000.memory-controller@21b0000.of
    probe-> ramfs0
    probe-> devfs0
    probe-> mem0
    probe-> mem1
netconsole: port not set
netconsole: registered as netconsole-1
    probe-> machine
        probe-> 21a8000.i2c@21a8000.of
            probe-> 24c5120
Unable to read Novena EEPROM: Connection timed out
    probe-> ldb.of
imx-ldb ldb.of: Neither display timings in ldb node nor remote panel found
imx-ldb ldb.of: dual-channel mode, ignoring second output
    probe-> usbphynop1.of
    probe-> usbphynop2.of
    probe-> 110000.dma-apbh@110000.of
    probe-> 120000.hdmi@120000.of
imx-hdmi 120000.hdmi@120000.of: Detected HDMI controller 0x13:0xa:0xa0:0xc1
    probe-> 2010000.spi@2010000.of
    probe-> 2080000.pwm@2080000.of
    probe-> 209c000.gpio@209c000.of
    probe-> 20a0000.gpio@20a0000.of
    probe-> 20a4000.gpio@20a4000.of
    probe-> 20a8000.gpio@20a8000.of
    probe-> 20ac000.gpio@20ac000.of
    probe-> 20b0000.gpio@20b0000.of
    probe-> 20b4000.gpio@20b4000.of
    probe-> 20bc000.watchdog@20bc000.of
    probe-> 20c9000.usbphy@20c9000.of
    probe-> 20ca000.usbphy@20ca000.of
    probe-> 20d8000.reset-controller@20d8000.of
    probe-> 2184000.usb@2184000.of
        probe-> 2184800.usbmisc@2184800.of
        probe-> regulator-usb-otg-vbus.of
    probe-> 2184200.usb@2184200.of
        probe-> 21a4000.i2c@21a4000.of
            probe-> pfuze1000
imx-usb 2184200.usb@2184200.of: Cannot get vbus regulator: No such device (ignoring)
imx-usb 2184200.usb@2184200.of: USB EHCI 1.00
    probe-> 2188000.ethernet@2188000.of
mdio_bus: miibus0: probed
    probe-> 2194000.mmc@2194000.of
imx-esdhc 2194000.mmc@2194000.of: registered as mmc1
    probe-> 2198000.mmc@2198000.of
imx-esdhc 2198000.mmc@2198000.of: registered as mmc2
    probe-> 21a0000.i2c@21a0000.of
    probe-> 21bc000.efuse@21bc000.of
    probe-> 21ec000.serial@21ec000.of
    probe-> 21f0000.serial@21f0000.of
    probe-> 2400000.ipu@2400000.of
imx-ipuv3 2400000.ipu@2400000.of: IPUv3H probed
    probe-> 2200000.sata@2200000.of
    probe-> 2800000.ipu@2800000.of
imx-ipuv3 2800000.ipu@2800000.of: IPUv3H probed
    probe-> gpio-keys.of
    probe-> leds.of
    probe-> regulator-2p5v.of
    probe-> regulator-3p3v.of
    probe-> regulator-5p0v.of
    probe-> regulator-audio-codec.of
    probe-> regulator-display.of
    probe-> regulator-lvds-lcd.of
    probe-> regulator-pcie.of
    probe-> regulator-sata.of
malloc space: 0xbfcf55c0 -> 0xffcf55bf (size 1 GiB)
    probe-> imx-ipuv3-crtc0
    probe-> imx-ipuv3-crtc1
    probe-> imx-ipuv3-crtc2
    probe-> imx-ipuv3-crtc3

> 
> Cheers,
> Ahmad

John.

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[Ahmad Fatoum]

On 25.01.23 15:04, John Watts wrote:
> On Wed, Jan 25, 2023 at 02:48:20PM +0100, Ahmad Fatoum wrote:
>> regulator-fixed only understands vin-supply.
> 
> Right, changed. I was under the assumption that ANY node could specify an
> X-supply.

vin-supply is handled by regulator core. Other X-supply needs to be
explicitly requested by driver.

>> I actually updated the eeprom node to use
>>> reg_3p3v as vcc-supply and things were still probed in the wrong order.
>>
>> That's odd. You can enable CONFIG_DEBUG_PROBES to see in what order
>> probes happened and at what depth (each probe within a probe is
>> indented more than its parent).
> 
> Should that even work? The vcc-supply property isn't a real one.

It does in Linux :-)

We could add the binding to the barebox AT24/AT25 driver. We'll need
to make sure we don't fail the probe in the error path as not to
break existing users. Fortunately, with deep probe, we will
have no -EPROBE_DEFER that would complicate this.

> 
> Currently the probe log is this:
> 
> detected i.MX6 Quad revision 1.2
> i.MX6 Quad unique ID: c1d99d4d8191854
>     probe-> 21e8000.serial@21e8000.of
>         probe-> 20e0000.pinctrl@20e0000.of
>     probe-> 21b0000.memory-controller@21b0000.of
>     probe-> ramfs0
>     probe-> devfs0
>     probe-> mem0
>     probe-> mem1
> netconsole: port not set
> netconsole: registered as netconsole-1
>     probe-> machine
>         probe-> 21a8000.i2c@21a8000.of
>             probe-> 24c5120
> Unable to read Novena EEPROM: Connection timed out
>     probe-> ldb.of
> imx-ldb ldb.of: Neither display timings in ldb node nor remote panel found
> imx-ldb ldb.of: dual-channel mode, ignoring second output
>     probe-> usbphynop1.of
>     probe-> usbphynop2.of
>     probe-> 110000.dma-apbh@110000.of
>     probe-> 120000.hdmi@120000.of
> imx-hdmi 120000.hdmi@120000.of: Detected HDMI controller 0x13:0xa:0xa0:0xc1
>     probe-> 2010000.spi@2010000.of
>     probe-> 2080000.pwm@2080000.of
>     probe-> 209c000.gpio@209c000.of
>     probe-> 20a0000.gpio@20a0000.of
>     probe-> 20a4000.gpio@20a4000.of
>     probe-> 20a8000.gpio@20a8000.of
>     probe-> 20ac000.gpio@20ac000.of
>     probe-> 20b0000.gpio@20b0000.of
>     probe-> 20b4000.gpio@20b4000.of
>     probe-> 20bc000.watchdog@20bc000.of
>     probe-> 20c9000.usbphy@20c9000.of
>     probe-> 20ca000.usbphy@20ca000.of
>     probe-> 20d8000.reset-controller@20d8000.of
>     probe-> 2184000.usb@2184000.of
>         probe-> 2184800.usbmisc@2184800.of
>         probe-> regulator-usb-otg-vbus.of
>     probe-> 2184200.usb@2184200.of
>         probe-> 21a4000.i2c@21a4000.of
>             probe-> pfuze1000
> imx-usb 2184200.usb@2184200.of: Cannot get vbus regulator: No such device (ignoring)
> imx-usb 2184200.usb@2184200.of: USB EHCI 1.00
>     probe-> 2188000.ethernet@2188000.of
> mdio_bus: miibus0: probed
>     probe-> 2194000.mmc@2194000.of
> imx-esdhc 2194000.mmc@2194000.of: registered as mmc1
>     probe-> 2198000.mmc@2198000.of
> imx-esdhc 2198000.mmc@2198000.of: registered as mmc2
>     probe-> 21a0000.i2c@21a0000.of
>     probe-> 21bc000.efuse@21bc000.of
>     probe-> 21ec000.serial@21ec000.of
>     probe-> 21f0000.serial@21f0000.of
>     probe-> 2400000.ipu@2400000.of
> imx-ipuv3 2400000.ipu@2400000.of: IPUv3H probed
>     probe-> 2200000.sata@2200000.of
>     probe-> 2800000.ipu@2800000.of
> imx-ipuv3 2800000.ipu@2800000.of: IPUv3H probed
>     probe-> gpio-keys.of
>     probe-> leds.of
>     probe-> regulator-2p5v.of
>     probe-> regulator-3p3v.of
>     probe-> regulator-5p0v.of
>     probe-> regulator-audio-codec.of
>     probe-> regulator-display.of
>     probe-> regulator-lvds-lcd.of
>     probe-> regulator-pcie.of
>     probe-> regulator-sata.of
> malloc space: 0xbfcf55c0 -> 0xffcf55bf (size 1 GiB)
>     probe-> imx-ipuv3-crtc0
>     probe-> imx-ipuv3-crtc1
>     probe-> imx-ipuv3-crtc2
>     probe-> imx-ipuv3-crtc3
> 
>>
>> Cheers,
>> Ahmad
> 
> John.
> 

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

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

On Wed, Jan 25, 2023 at 03:16:05PM +0100, Ahmad Fatoum wrote:
> We could add the binding to the barebox AT24/AT25 driver. We'll need
> to make sure we don't fail the probe in the error path as not to
> break existing users. Fortunately, with deep probe, we will
> have no -EPROBE_DEFER that would complicate this.

I'm really not sure if specifying the regulator (that should always be on) for
every possible node is a good idea. I'd need to add -supply properties to:

- USB
- Sata
- HDMI
- SD cards
- Ethernet
- PCIe
- FPGA
- I2C
- UART

Some regulators even require multiple inputs such as 3v and 5v delayed.

John.

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[Ahmad Fatoum]

On 25.01.23 15:28, John Watts wrote:
> On Wed, Jan 25, 2023 at 03:16:05PM +0100, Ahmad Fatoum wrote:
>> We could add the binding to the barebox AT24/AT25 driver. We'll need
>> to make sure we don't fail the probe in the error path as not to
>> break existing users. Fortunately, with deep probe, we will
>> have no -EPROBE_DEFER that would complicate this.
> 
> I'm really not sure if specifying the regulator (that should always be on) for
> every possible node is a good idea. I'd need to add -supply properties to:
> 
> - USB
> - Sata
> - HDMI
> - SD cards
> - Ethernet
> - PCIe
> - FPGA
> - I2C
> - UART

What voltage does your UART require that you don't already have when
your SoC is powered up? anyways, I still see utility in observing
the vcc-supply upstream bindings for EEPROMs.

> 
> Some regulators even require multiple inputs such as 3v and 5v delayed.

Add an of_devices_ensure_probed_by_property("regulator-boot-enabled") or
something to the start of your machine's board driver then.

> 
> John.
> 

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

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

On Wed, Jan 25, 2023 at 03:33:18PM +0100, Ahmad Fatoum wrote:
> On 25.01.23 15:28, John Watts wrote:
> > On Wed, Jan 25, 2023 at 03:16:05PM +0100, Ahmad Fatoum wrote:
> >> We could add the binding to the barebox AT24/AT25 driver. We'll need
> >> to make sure we don't fail the probe in the error path as not to
> >> break existing users. Fortunately, with deep probe, we will
> >> have no -EPROBE_DEFER that would complicate this.
> > 
> > I'm really not sure if specifying the regulator (that should always be on) for
> > every possible node is a good idea. I'd need to add -supply properties to:
> > 
> > - USB
> > - Sata
> > - HDMI
> > - SD cards
> > - Ethernet
> > - PCIe
> > - FPGA
> > - I2C
> > - UART
> 
> What voltage does your UART require that you don't already have when
> your SoC is powered up? anyways, I still see utility in observing
> the vcc-supply upstream bindings for EEPROMs.

These are used for pull-up/pull-down resistors.

> Add an of_devices_ensure_probed_by_property("regulator-boot-enabled") or
> something to the start of your machine's board driver then.

Hmm. Probing for regulator-always-on and regular-boot-on seems to fix my issue.

John.

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[Sascha Hauer]

On Thu, Jan 26, 2023 at 12:31:04AM +1100, John Watts wrote:
> On Wed, Jan 25, 2023 at 09:19:23AM +0100, Sascha Hauer wrote:
> > Yes, normally it should be set up already, unless of course there is
> > some needed pinctrl configuration in an unrelated device tree node.
> > 
> > Sascha
> 
> Hi Sascha,
> 
> Here's what I think is going on:
> 
> - The I2C line requires 3v3_DELAYED for pull-ups
> - The EEPROM requires 3v3_DELAYED for power
> - 3v3_DELAYED requires VGEN5 from the PMIC (acts as a gate)
> - The PMIC is loaded after the EEPROM
> 
> Most things on the board require these delayed rails, so I'm wondering
> what the approach should be here. Should I:
> 
> - Rewrite the device tree to reference these delayed rails
> - Probe the PMIC first

The barebox PFUZE PMIC driver is not a real regulator driver it's just a
no-op driver providing a regmap, so by definition it is not guilty.

My wild guess is that regulator-audio-codec needs to be enabled to let
the EEPROM work. The Audio codec sits on the same I2C bus as the EEPROM
and it could be that the codec just pulls the I2C data line low when
it's not powered up.

You could verify this by doing a

gpio_direction_output 156 0

on the command line and then try to read from the EEPROM. It sould fail
and start working again after a

gpio_direction_output 156 1

Sascha

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

Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

On Wed, Jan 25, 2023 at 04:42:58PM +0100, Sascha Hauer wrote:
> The barebox PFUZE PMIC driver is not a real regulator driver it's just a
> no-op driver providing a regmap, so by definition it is not guilty.
> 
> My wild guess is that regulator-audio-codec needs to be enabled to let
> the EEPROM work. The Audio codec sits on the same I2C bus as the EEPROM
> and it could be that the codec just pulls the I2C data line low when
> it's not powered up.

Hi Sascha,

I was ignoring my email and troubleshooting further and figured out that
the PMIC driver was a no-op and that for some reason I did need the audio
regulator enabled, but I wasn't sure why.

This makes total sense though.

> 
> You could verify this by doing a
> 
> gpio_direction_output 156 0
> 
> on the command line and then try to read from the EEPROM. It sould fail
> and start working again after a
> 
> gpio_direction_output 156 1

I confirm this was the issue with GPIO 145.

> 
> Sascha

Thanks so much. I'll submit V2 of the Novena patches soon.

John.

[PATCH v2 0/4] Add support for the Kosagi Novena board

[John Watts]

This patch series introduces support for the Novena board.
This support is basic but enough to boot Barebox.

The EEPROM code here is included in anticipation for reading the features
flag in the future when more board features are supported.

Changes v1 -> v2:
- The board uses the driver model now
- Memory density is now calculated correctly

John Watts (4):
  ARM: novena: Add Kosagi Novena board
  ARM: novena: Setup RAM using static configuration
  ARM: novena: Read Ethernet MAC address from EEPROM
  ARM: novena: Use DDR3 information from SPD EEPROM

 arch/arm/boards/Makefile                      |   1 +
 arch/arm/boards/novena/Makefile               |   4 +
 arch/arm/boards/novena/board.c                | 114 ++++++++++
 arch/arm/boards/novena/ddr_regs.h             | 119 +++++++++++
 .../boards/novena/flash-header-novena.imxcfg  |   6 +
 arch/arm/boards/novena/lowlevel.c             | 198 ++++++++++++++++++
 arch/arm/configs/imx_v7_defconfig             |   1 +
 arch/arm/dts/Makefile                         |   1 +
 arch/arm/dts/imx6q-novena.dts                 |  19 ++
 arch/arm/mach-imx/Kconfig                     |   8 +
 images/Makefile.imx                           |   2 +
 11 files changed, 473 insertions(+)
 create mode 100644 arch/arm/boards/novena/Makefile
 create mode 100644 arch/arm/boards/novena/board.c
 create mode 100644 arch/arm/boards/novena/ddr_regs.h
 create mode 100644 arch/arm/boards/novena/flash-header-novena.imxcfg
 create mode 100644 arch/arm/boards/novena/lowlevel.c
 create mode 100644 arch/arm/dts/imx6q-novena.dts

-- 
2.39.0

[PATCH v2 1/4] ARM: novena: Add Kosagi Novena board

[John Watts]

The Kosagi Novena is an open source laptop released in 2014.

This patch adds the initial project skeleton for running the PBL
and debugging over the UART2 port (labeled DEBUG on the board.)

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/Makefile                      |  1 +
 arch/arm/boards/novena/Makefile               |  4 +
 arch/arm/boards/novena/board.c                | 26 +++++++
 .../boards/novena/flash-header-novena.imxcfg  |  6 ++
 arch/arm/boards/novena/lowlevel.c             | 73 +++++++++++++++++++
 arch/arm/configs/imx_v7_defconfig             |  1 +
 arch/arm/dts/Makefile                         |  1 +
 arch/arm/dts/imx6q-novena.dts                 |  4 +
 arch/arm/mach-imx/Kconfig                     |  6 ++
 images/Makefile.imx                           |  2 +
 10 files changed, 124 insertions(+)
 create mode 100644 arch/arm/boards/novena/Makefile
 create mode 100644 arch/arm/boards/novena/board.c
 create mode 100644 arch/arm/boards/novena/flash-header-novena.imxcfg
 create mode 100644 arch/arm/boards/novena/lowlevel.c
 create mode 100644 arch/arm/dts/imx6q-novena.dts

diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
index f47aea6602..50088886eb 100644
--- a/arch/arm/boards/Makefile
+++ b/arch/arm/boards/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_MACH_BEAGLEBONE)			+= beaglebone/
 obj-$(CONFIG_MACH_CANON_A1100)			+= canon-a1100/
 obj-$(CONFIG_MACH_CM_FX6)			+= cm-fx6/
 obj-$(CONFIG_MACH_NITROGEN6)			+= boundarydevices-nitrogen6/
+obj-$(CONFIG_MACH_NOVENA)			+= novena/
 obj-$(CONFIG_MACH_CCMX51)			+= ccxmx51/
 obj-$(CONFIG_MACH_CCMX53)			+= ccxmx53/
 obj-$(CONFIG_MACH_CFA10036)			+= crystalfontz-cfa10036/
diff --git a/arch/arm/boards/novena/Makefile b/arch/arm/boards/novena/Makefile
new file mode 100644
index 0000000000..da63d2625f
--- /dev/null
+++ b/arch/arm/boards/novena/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-y += board.o
+lwl-y += lowlevel.o
diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
new file mode 100644
index 0000000000..e247f8b8c3
--- /dev/null
+++ b/arch/arm/boards/novena/board.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// SPDX-FileCopyrightText: 2023 John Watts
+
+#include <common.h>
+#include <deep-probe.h>
+
+static int novena_probe(struct device *dev)
+{
+	return 0;
+}
+
+static const struct of_device_id novena_of_match[] = {
+	{
+		.compatible = "kosagi,imx6q-novena",
+	},
+	{ /* sentinel */ }
+};
+
+static struct driver novena_board_driver = {
+	.name = "board-novena",
+	.probe = novena_probe,
+	.of_compatible = novena_of_match,
+};
+coredevice_platform_driver(novena_board_driver);
+
+BAREBOX_DEEP_PROBE_ENABLE(novena_of_match);
diff --git a/arch/arm/boards/novena/flash-header-novena.imxcfg b/arch/arm/boards/novena/flash-header-novena.imxcfg
new file mode 100644
index 0000000000..61425976ec
--- /dev/null
+++ b/arch/arm/boards/novena/flash-header-novena.imxcfg
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+loadaddr 0x00907000
+soc imx6
+max_load_size 0x11000
+ivtofs 0x400
diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
new file mode 100644
index 0000000000..2052ca9de8
--- /dev/null
+++ b/arch/arm/boards/novena/lowlevel.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// SPDX-FileCopyrightText: 2023 John Watts
+
+#include <asm/barebox-arm.h>
+#include <common.h>
+#include <debug_ll.h>
+#include <mach/esdctl.h>
+#include <mach/generic.h>
+#include <mach/imx6.h>
+#include <mach/iomux-mx6.h>
+#include <mach/xload.h>
+#include <soc/fsl/fsl_udc.h>
+
+#define MX6_OCRAM_BASE_ADDR 0x00900000
+#define STACK_TOP (MX6_OCRAM_BASE_ADDR + SZ_128K)
+
+extern char __dtb_imx6q_novena_start[];
+
+static bool running_from_ram(void)
+{
+	return (get_pc() >= MX6_MMDC_PORT01_BASE_ADDR);
+}
+
+static void setup_uart(void)
+{
+	/* NOTE: RX is needed for TX to work on this board */
+	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D26__UART2_RXD);
+	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D27__UART2_TXD);
+	imx6_uart_setup(IOMEM(MX6_UART2_BASE_ADDR));
+	pbl_set_putc(imx_uart_putc, IOMEM(MX6_UART2_BASE_ADDR));
+	pr_debug(">");
+}
+
+static void load_barebox(void)
+{
+	enum bootsource bootsrc;
+	int bootinstance;
+
+	imx6_get_boot_source(&bootsrc, &bootinstance);
+
+	if (bootsrc == BOOTSOURCE_SERIAL) {
+		imx6_barebox_start_usb(IOMEM(MX6_MMDC_PORT01_BASE_ADDR));
+	} else if (bootsrc == BOOTSOURCE_MMC) {
+		imx6_esdhc_start_image(bootinstance);
+	} else {
+		pr_err("Unsupported boot source %i instance %i\n",
+					bootsrc, bootinstance);
+		hang();
+	}
+}
+
+static void boot_barebox(void)
+{
+	void *fdt = __dtb_imx6q_novena_start + get_runtime_offset();
+
+	imx6q_barebox_entry(fdt);
+}
+
+ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
+{
+	imx6_cpu_lowlevel_init();
+	relocate_to_current_adr();
+	setup_c();
+	barrier();
+
+	if (!running_from_ram()) {
+		imx6_ungate_all_peripherals();
+		setup_uart();
+		load_barebox();
+	} else {
+		boot_barebox();
+	}
+}
diff --git a/arch/arm/configs/imx_v7_defconfig b/arch/arm/configs/imx_v7_defconfig
index 6110a52e48..f8deca324a 100644
--- a/arch/arm/configs/imx_v7_defconfig
+++ b/arch/arm/configs/imx_v7_defconfig
@@ -25,6 +25,7 @@ CONFIG_MACH_SABRELITE=y
 CONFIG_MACH_SABRESD=y
 CONFIG_MACH_FREESCALE_IMX6SX_SABRESDB=y
 CONFIG_MACH_NITROGEN6=y
+CONFIG_MACH_NOVENA=y
 CONFIG_MACH_SOLIDRUN_MICROSOM=y
 CONFIG_MACH_TECHNEXION_PICO_HOBBIT=y
 CONFIG_MACH_TECHNEXION_WANDBOARD=y
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 26c4a8ab99..a01ee19f68 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -46,6 +46,7 @@ lwl-$(CONFIG_MACH_MYIRTECH_X335X) += am335x-myirtech-myd.dtb.o
 lwl-$(CONFIG_MACH_NETGEAR_RN104) += armada-370-rn104-bb.dtb.o
 lwl-$(CONFIG_MACH_NETGEAR_RN2120) += armada-xp-rn2120-bb.dtb.o
 lwl-$(CONFIG_MACH_NITROGEN6) += imx6q-nitrogen6x.dtb.o imx6dl-nitrogen6x.dtb.o imx6qp-nitrogen6_max.dtb.o
+lwl-$(CONFIG_MACH_NOVENA) += imx6q-novena.dtb.o
 lwl-$(CONFIG_MACH_NVIDIA_BEAVER) += tegra30-beaver.dtb.o
 lwl-$(CONFIG_MACH_NVIDIA_JETSON) += tegra124-jetson-tk1.dtb.o
 lwl-$(CONFIG_MACH_PCA100) += imx27-phytec-phycard-s-rdk-bb.dtb.o
diff --git a/arch/arm/dts/imx6q-novena.dts b/arch/arm/dts/imx6q-novena.dts
new file mode 100644
index 0000000000..07471cb132
--- /dev/null
+++ b/arch/arm/dts/imx6q-novena.dts
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR X11
+// SPDX-FileCopyrightText: 2023 John Watts
+
+#include <arm/imx6q-novena.dts>
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 774c4cacb7..7993835be7 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -434,6 +434,12 @@ config MACH_NITROGEN6
 	bool "BoundaryDevices Nitrogen6 boards"
 	select ARCH_IMX6
 
+config MACH_NOVENA
+	bool "Kosagi Novena board"
+	select ARCH_IMX6
+	select MCI_IMX_ESDHC_PBL
+	select USB_GADGET_DRIVER_ARC_PBL
+
 config MACH_SOLIDRUN_MICROSOM
 	bool "SolidRun MicroSOM based devices"
 	select ARCH_IMX6
diff --git a/images/Makefile.imx b/images/Makefile.imx
index e9f4ba64e4..65914212bf 100644
--- a/images/Makefile.imx
+++ b/images/Makefile.imx
@@ -258,6 +258,8 @@ $(call build_imx_habv4img, CONFIG_MACH_NITROGEN6, start_imx6dl_nitrogen6x_2g, bo
 
 $(call build_imx_habv4img, CONFIG_MACH_NITROGEN6, start_imx6qp_nitrogen6_max, boundarydevices-nitrogen6/flash-header-nitrogen6qp-max, boundarydevices-imx6qp-nitrogen6_max)
 
+$(call build_imx_habv4img, CONFIG_MACH_NOVENA, start_imx6q_novena, novena/flash-header-novena, imx6q-novena)
+
 $(call build_imx_habv4img, CONFIG_MACH_TX6X, start_imx6dl_tx6x_512m, karo-tx6x/flash-header-tx6dl-512m, karo-imx6dl-tx6x-512m)
 
 $(call build_imx_habv4img, CONFIG_MACH_TX6X, start_imx6dl_tx6x_1g, karo-tx6x/flash-header-tx6dl-1g, karo-imx6dl-tx6x-1g)
-- 
2.39.0

[PATCH v2 2/4] ARM: novena: Setup RAM using static configuration

[John Watts]

The Novena laptops were shipped with a 4GB stick that can be set up
using this DDR3 configuration.
This isn't ideal as the stick is swappable, but it will work for
most people.

The DDR registers are copied from /board/kosagi/novena/novena_spl.c
from the Novena U-Boot fork (tag novena-r2) with two changes:

- Bank interleaving is disabled: The Novena U-Boot code disables it
  unconditionally during execution anyway
- Page size is set to 1KB: I'm not sure the hardware supports x16 DIMMs

Mainline U-Boot has similar code, but the timing differences don't work
on my board so I'm unable to test or port that code over.

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/novena/ddr_regs.h | 119 ++++++++++++++++++++++++++++++
 arch/arm/boards/novena/lowlevel.c |   9 +++
 2 files changed, 128 insertions(+)
 create mode 100644 arch/arm/boards/novena/ddr_regs.h

diff --git a/arch/arm/boards/novena/ddr_regs.h b/arch/arm/boards/novena/ddr_regs.h
new file mode 100644
index 0000000000..5f18d5e0e4
--- /dev/null
+++ b/arch/arm/boards/novena/ddr_regs.h
@@ -0,0 +1,119 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* SPDX-FileCopyrightText: 2014 Marek Vasut <marex@denx.de> */
+
+#ifndef NOVENA_DDR_REGS_H
+#define NOVENA_DDR_REGS_H
+
+/* MEMORY CONTROLLER CONFIGURATION */
+
+static struct mx6dq_iomux_ddr_regs novena_ddr_regs = {
+	/* SDCLK[0:1], CAS, RAS, Reset: Differential input, 40ohm */
+	.dram_sdclk_0		= 0x00020038,
+	.dram_sdclk_1		= 0x00020038,
+	.dram_cas		= 0x00000038,
+	.dram_ras		= 0x00000038,
+	.dram_reset		= 0x00000038,
+	/* SDCKE[0:1]: 100k pull-up */
+	.dram_sdcke0		= 0x00003000,
+	.dram_sdcke1		= 0x00003000,
+	/* SDBA2: pull-up disabled */
+	.dram_sdba2		= 0x00000000,
+	/* SDODT[0:1]: 100k pull-up, 40 ohm */
+	.dram_sdodt0		= 0x00000038,
+	.dram_sdodt1		= 0x00000038,
+	/* SDQS[0:7]: Differential input, 40 ohm */
+	.dram_sdqs0		= 0x00000038,
+	.dram_sdqs1		= 0x00000038,
+	.dram_sdqs2		= 0x00000038,
+	.dram_sdqs3		= 0x00000038,
+	.dram_sdqs4		= 0x00000038,
+	.dram_sdqs5		= 0x00000038,
+	.dram_sdqs6		= 0x00000038,
+	.dram_sdqs7		= 0x00000038,
+
+	/* DQM[0:7]: Differential input, 40 ohm */
+	.dram_dqm0		= 0x00000038,
+	.dram_dqm1		= 0x00000038,
+	.dram_dqm2		= 0x00000038,
+	.dram_dqm3		= 0x00000038,
+	.dram_dqm4		= 0x00000038,
+	.dram_dqm5		= 0x00000038,
+	.dram_dqm6		= 0x00000038,
+	.dram_dqm7		= 0x00000038,
+};
+
+static struct mx6dq_iomux_grp_regs novena_grp_regs = {
+	/* DDR3 */
+	.grp_ddr_type		= 0x000c0000,
+	.grp_ddrmode_ctl	= 0x00020000,
+	/* Disable DDR pullups */
+	.grp_ddrpke		= 0x00000000,
+	/* ADDR[00:16], SDBA[0:1]: 40 ohm */
+	.grp_addds		= 0x00000038,
+	/* CS0/CS1/SDBA2/CKE0/CKE1/SDWE: 40 ohm */
+	.grp_ctlds		= 0x00000038,
+	/* DATA[00:63]: Differential input, 40 ohm */
+	.grp_ddrmode		= 0x00020000,
+	.grp_b0ds		= 0x00000038,
+	.grp_b1ds		= 0x00000038,
+	.grp_b2ds		= 0x00000038,
+	.grp_b3ds		= 0x00000038,
+	.grp_b4ds		= 0x00000038,
+	.grp_b5ds		= 0x00000038,
+	.grp_b6ds		= 0x00000038,
+	.grp_b7ds		= 0x00000038,
+};
+
+/* MEMORY STICK CONFIGURATION */
+
+static struct mx6_mmdc_calibration novena_mmdc_calib = {
+	/* write leveling calibration determine */
+	.p0_mpwldectrl0		= 0x00420048,
+	.p0_mpwldectrl1		= 0x006f0059,
+	.p1_mpwldectrl0		= 0x005a0104,
+	.p1_mpwldectrl1		= 0x01070113,
+	/* Read DQS Gating calibration */
+	.p0_mpdgctrl0		= 0x437c040b,
+	.p0_mpdgctrl1		= 0x0413040e,
+	.p1_mpdgctrl0		= 0x444f0446,
+	.p1_mpdgctrl1		= 0x044d0422,
+	/* Read Calibration: DQS delay relative to DQ read access */
+	.p0_mprddlctl		= 0x4c424249,
+	.p1_mprddlctl		= 0x4e48414f,
+	/* Write Calibration: DQ/DM delay relative to DQS write access */
+	.p0_mpwrdlctl		= 0x42414641,
+	.p1_mpwrdlctl		= 0x46374b43,
+};
+
+static struct mx6_ddr_sysinfo novena_ddr_info = {
+	/* Width of data bus: 0=16, 1=32, 2=64 */
+	.dsize		= 2,
+	/* Config for full 4GB range so that get_mem_size() works */
+	.cs_density	= 32,	/* 32Gb per CS */
+	/* Single chip select */
+	.ncs		= 1,
+	.cs1_mirror	= 0,
+	.rtt_wr		= 1,	/* RTT_Wr = RZQ/4 */
+	.rtt_nom	= 2,	/* RTT_Nom = RZQ/2 */
+	.walat		= 3,	/* Write additional latency */
+	.ralat		= 7,	/* Read additional latency */
+	.mif3_mode	= 3,	/* Command prediction working mode */
+	.bi_on		= 0,	/* Bank interleaving disabled */
+	.sde_to_rst	= 0x10,	/* 14 cycles, 200us (JEDEC default) */
+	.rst_to_cke	= 0x23,	/* 33 cycles, 500us (JEDEC default) */
+};
+
+static struct mx6_ddr3_cfg novena_ddr_cfg = {
+	.mem_speed	= 1600,
+	.density	= 4,
+	.width		= 64,
+	.banks		= 8,
+	.rowaddr	= 16,
+	.coladdr	= 10,
+	.pagesz		= 1,
+	.trcd		= 1300,
+	.trcmin		= 4900,
+	.trasmin	= 3590,
+};
+
+#endif
diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
index 2052ca9de8..ef8acc8600 100644
--- a/arch/arm/boards/novena/lowlevel.c
+++ b/arch/arm/boards/novena/lowlevel.c
@@ -7,9 +7,11 @@
 #include <mach/esdctl.h>
 #include <mach/generic.h>
 #include <mach/imx6.h>
+#include <mach/imx6-mmdc.h>
 #include <mach/iomux-mx6.h>
 #include <mach/xload.h>
 #include <soc/fsl/fsl_udc.h>
+#include "ddr_regs.h"
 
 #define MX6_OCRAM_BASE_ADDR 0x00900000
 #define STACK_TOP (MX6_OCRAM_BASE_ADDR + SZ_128K)
@@ -31,6 +33,12 @@ static void setup_uart(void)
 	pr_debug(">");
 }
 
+static void setup_ram(void)
+{
+	mx6dq_dram_iocfg(64, &novena_ddr_regs, &novena_grp_regs);
+	mx6_dram_cfg(&novena_ddr_info, &novena_mmdc_calib, &novena_ddr_cfg);
+}
+
 static void load_barebox(void)
 {
 	enum bootsource bootsrc;
@@ -66,6 +74,7 @@ ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
 	if (!running_from_ram()) {
 		imx6_ungate_all_peripherals();
 		setup_uart();
+		setup_ram();
 		load_barebox();
 	} else {
 		boot_barebox();
-- 
2.39.0

[PATCH v2 3/4] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

The Novena has an EEPROM used for storing information about the board,
including the Ethernet MAC address.

The reference for the EEPROM fields is the novena-eeprom source code.

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/novena/board.c | 88 ++++++++++++++++++++++++++++++++++
 arch/arm/dts/imx6q-novena.dts  | 15 ++++++
 2 files changed, 103 insertions(+)

diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
index e247f8b8c3..24cde1c0d4 100644
--- a/arch/arm/boards/novena/board.c
+++ b/arch/arm/boards/novena/board.c
@@ -3,9 +3,97 @@
 
 #include <common.h>
 #include <deep-probe.h>
+#include <fs.h>
+#include <libfile.h>
+#include <net.h>
+
+struct novena_eeprom {
+	uint8_t signature[6]; /* 'Novena' */
+	uint8_t version; /* 1 or 2, not checked */
+	uint8_t page_size; /* v2 only: EEPROM read/write page */
+	uint32_t serial; /* 32-bit serial number */
+	uint8_t mac[6]; /* Gigabit MAC address */
+	uint16_t features; /* features */
+	/* ... extra fields omitted ... */
+} __packed;
+
+static void power_on_audio_codec(void)
+{
+	int rc = of_devices_ensure_probed_by_name("regulator-audio-codec");
+
+	if (rc < 0)
+		pr_err("Unable to power on audio codec: %s\n", strerror(-rc));
+}
+
+static struct novena_eeprom *novena_read_eeprom(void)
+{
+	size_t read;
+	loff_t max = sizeof(struct novena_eeprom);
+	void *eeprom;
+	int rc;
+
+	/*
+	 * When powered off the audio codec pulls down the EEPROM's I2C line.
+	 * Power it on so we can actually read data.
+	 */
+	power_on_audio_codec();
+
+	rc = of_device_ensure_probed_by_alias("eeprom0");
+	if (rc < 0) {
+		pr_err("Unable to probe eeprom0: %s\n", strerror(-rc));
+		return NULL;
+	}
+
+	rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);
+
+	if (rc < 0 && rc != -EFBIG) {
+		pr_err("Unable to read Novena EEPROM: %s\n", strerror(-rc));
+		return NULL;
+	} else if (read != max) {
+		pr_err("Short read from Novena EEPROM?\n");
+		return NULL;
+	} else {
+		return eeprom;
+	}
+}
+
+static bool novena_check_eeprom(struct novena_eeprom *eeprom)
+{
+	char *sig = eeprom->signature;
+	size_t size = sizeof(eeprom->signature);
+
+	if (memcmp("Novena", sig, size) != 0) {
+		pr_err("Unknown Novena EEPROM signature\n");
+		return false;
+	}
+
+	return true;
+}
+
+static void novena_set_mac(struct novena_eeprom *eeprom)
+{
+	struct device_node *dnode;
+
+	dnode = of_find_node_by_alias(of_get_root_node(), "ethernet0");
+	if (dnode)
+		of_eth_register_ethaddr(dnode, eeprom->mac);
+	else
+		pr_err("Unable to find ethernet node\n");
+}
+
+static void novena_try_eeprom(void)
+{
+	struct novena_eeprom *eeprom = novena_read_eeprom();
+
+	if (!eeprom || !novena_check_eeprom(eeprom))
+		return;
+
+	novena_set_mac(eeprom);
+}
 
 static int novena_probe(struct device *dev)
 {
+	novena_try_eeprom();
+
 	return 0;
 }
 
diff --git a/arch/arm/dts/imx6q-novena.dts b/arch/arm/dts/imx6q-novena.dts
index 07471cb132..095dcde0d5 100644
--- a/arch/arm/dts/imx6q-novena.dts
+++ b/arch/arm/dts/imx6q-novena.dts
@@ -2,3 +2,18 @@
 // SPDX-FileCopyrightText: 2023 John Watts
 
 #include <arm/imx6q-novena.dts>
+
+/ {
+	aliases {
+		eeprom0 = &eeprom;
+	};
+};
+
+&i2c3 {
+	eeprom: eeprom@56 {
+		compatible = "24c512";
+		reg = <0x56>;
+		pagesize = <128>;
+		status = "okay";
+	};
+};
-- 
2.39.0

[PATCH v2 4/4] ARM: novena: Use DDR3 information from SPD EEPROM

[John Watts]

The Novena supports swappable memory which means we must query the memory
for configuration details. This patch uses values stored in the SPD EEPROM
to configure the i.MX6 memory controller.

If the SPD can't be read, the default configuration for a 4GB stick is used.

Calibration for the installed stick is run unconditionally.

Verification that this works was done by comparing MMDC registers before and
after and carefully reading other people's source code.

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/novena/lowlevel.c | 116 ++++++++++++++++++++++++++++++
 arch/arm/mach-imx/Kconfig         |   2 +
 2 files changed, 118 insertions(+)

diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
index ef8acc8600..3765f00f83 100644
--- a/arch/arm/boards/novena/lowlevel.c
+++ b/arch/arm/boards/novena/lowlevel.c
@@ -3,6 +3,8 @@
 
 #include <asm/barebox-arm.h>
 #include <common.h>
+#include <ddr_dimms.h>
+#include <ddr_spd.h>
 #include <debug_ll.h>
 #include <mach/esdctl.h>
 #include <mach/generic.h>
@@ -10,6 +12,8 @@
 #include <mach/imx6-mmdc.h>
 #include <mach/iomux-mx6.h>
 #include <mach/xload.h>
+#include <pbl.h>
+#include <pbl/i2c.h>
 #include <soc/fsl/fsl_udc.h>
 #include "ddr_regs.h"
 
@@ -18,6 +22,115 @@
 
 extern char __dtb_imx6q_novena_start[];
 
+static struct spd_eeprom spd_eeprom;
+static struct dimm_params dimm_params;
+
+#if IN_PBL
+/* I2C in the PBL requires udelay, provide our own */
+void udelay(unsigned long us)
+{
+	volatile unsigned long i;
+
+	for (i = 0; i < us * 4; i++);
+}
+#endif
+
+static struct pbl_i2c *setup_spd_i2c(void)
+{
+	void __iomem *iomuxbase = IOMEM(MX6_IOMUXC_BASE_ADDR);
+
+	imx_setup_pad(iomuxbase, MX6Q_PAD_CSI0_DAT8__I2C1_SDA);
+	imx_setup_pad(iomuxbase, MX6Q_PAD_CSI0_DAT9__I2C1_SCL);
+	return imx6_i2c_early_init(IOMEM(MX6_I2C1_BASE_ADDR));
+}
+
+static struct spd_eeprom *read_spd(void)
+{
+	struct spd_eeprom *eeprom = &spd_eeprom;
+	struct pbl_i2c *i2c = setup_spd_i2c();
+	int rc;
+
+	rc = spd_read_eeprom(i2c, 0x50, eeprom, SPD_MEMTYPE_DDR3);
+	if (rc < 0) {
+		pr_err("Couldn't read SPD EEPROM: %i\n", rc);
+		return NULL;
+	}
+
+	rc = ddr3_spd_check(&eeprom->ddr3);
+	if (rc < 0) {
+		pr_err("Couldn't verify SPD data: %i\n", rc);
+		return NULL;
+	}
+
+	return eeprom;
+}
+
+static bool check_spd_compatible(struct spd_eeprom *eeprom)
+{
+	if (eeprom->ddr3.mem_type != 0x0b) {
+		pr_err("SPD data is not DDR3\n");
+		return false;
+	}
+
+	if ((eeprom->ddr3.bus_width & 0x1f) != 0x03) {
+		pr_err("SPD data is for a 64-bit bus\n");
+		return false;
+	}
+
+	return true;
+}
+
+static void setup_dimm_settings(struct dimm_params *params,
+				struct mx6_ddr_sysinfo *info,
+				struct mx6_ddr3_cfg *cfg)
+{
+	int capacity_gbit = params->capacity / 0x8000000;
+	int density_rank = capacity_gbit / params->n_ranks;
+
+	info->ncs = params->n_ranks;
+	info->cs_density = density_rank;
+	cfg->mem_speed = params->tckmin_x_ps;
+	cfg->density = density_rank / params->n_banks_per_sdram_device;
+	cfg->width = params->data_width;
+	cfg->banks = params->n_banks_per_sdram_device;
+	cfg->rowaddr = params->n_row_addr;
+	cfg->coladdr = params->n_col_addr;
+	cfg->trcd = params->trcd_ps / 10;
+	cfg->trcmin = params->trc_ps / 10;
+	cfg->trasmin = params->tras_ps / 10;
+	cfg->SRT = params->extended_op_srt;
+
+	if (params->device_width >= 16)
+		cfg->pagesz = 2;
+}
+
+static void read_dimm_settings(void)
+{
+	struct spd_eeprom *eeprom = read_spd();
+	struct dimm_params *params = &dimm_params;
+	int rc;
+
+	if (!eeprom) {
+		pr_err("Couldn't read SPD EEPROM, using default settings\n");
+		return;
+	}
+
+	if (!check_spd_compatible(eeprom)) {
+		pr_err("DIMM stick incompatible\n");
+		hang();
+	}
+
+	rc = ddr3_compute_dimm_parameters(&eeprom->ddr3, params);
+	if (rc < 0) {
+		pr_err("Couldn't compute DIMM params: %i\n", rc);
+		return;
+	}
+
+	setup_dimm_settings(params, &novena_ddr_info, &novena_ddr_cfg);
+
+	pr_info("Found DIMM: %s\n", params->mpart);
+}
+
 static bool running_from_ram(void)
 {
 	return (get_pc() >= MX6_MMDC_PORT01_BASE_ADDR);
@@ -37,6 +150,8 @@ static void setup_ram(void)
 {
 	mx6dq_dram_iocfg(64, &novena_ddr_regs, &novena_grp_regs);
 	mx6_dram_cfg(&novena_ddr_info, &novena_mmdc_calib, &novena_ddr_cfg);
+	mmdc_do_write_level_calibration();
+	mmdc_do_dqs_calibration();
 }
 
 static void load_barebox(void)
@@ -74,6 +189,7 @@ ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
 	if (!running_from_ram()) {
 		imx6_ungate_all_peripherals();
 		setup_uart();
+		read_dimm_settings();
 		setup_ram();
 		load_barebox();
 	} else {
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 7993835be7..aa7a86da66 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -437,6 +437,8 @@ config MACH_NITROGEN6
 config MACH_NOVENA
 	bool "Kosagi Novena board"
 	select ARCH_IMX6
+	select DDR_SPD
+	select I2C_IMX_EARLY
 	select MCI_IMX_ESDHC_PBL
 	select USB_GADGET_DRIVER_ARC_PBL
 
-- 
2.39.0

Re: [PATCH v2 1/4] ARM: novena: Add Kosagi Novena board

[Marco Felsch]

Hi John,

thanks for your patch.

On 23-01-26, John Watts wrote:
> The Kosagi Novena is an open source laptop released in 2014.
> 
> This patch adds the initial project skeleton for running the PBL
> and debugging over the UART2 port (labeled DEBUG on the board.)
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  arch/arm/boards/Makefile                      |  1 +
>  arch/arm/boards/novena/Makefile               |  4 +
>  arch/arm/boards/novena/board.c                | 26 +++++++
>  .../boards/novena/flash-header-novena.imxcfg  |  6 ++
>  arch/arm/boards/novena/lowlevel.c             | 73 +++++++++++++++++++
>  arch/arm/configs/imx_v7_defconfig             |  1 +
>  arch/arm/dts/Makefile                         |  1 +
>  arch/arm/dts/imx6q-novena.dts                 |  4 +
>  arch/arm/mach-imx/Kconfig                     |  6 ++
>  images/Makefile.imx                           |  2 +
>  10 files changed, 124 insertions(+)
>  create mode 100644 arch/arm/boards/novena/Makefile
>  create mode 100644 arch/arm/boards/novena/board.c
>  create mode 100644 arch/arm/boards/novena/flash-header-novena.imxcfg
>  create mode 100644 arch/arm/boards/novena/lowlevel.c
>  create mode 100644 arch/arm/dts/imx6q-novena.dts
> 
> diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
> index f47aea6602..50088886eb 100644
> --- a/arch/arm/boards/Makefile
> +++ b/arch/arm/boards/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_MACH_BEAGLEBONE)			+= beaglebone/
>  obj-$(CONFIG_MACH_CANON_A1100)			+= canon-a1100/
>  obj-$(CONFIG_MACH_CM_FX6)			+= cm-fx6/
>  obj-$(CONFIG_MACH_NITROGEN6)			+= boundarydevices-nitrogen6/
> +obj-$(CONFIG_MACH_NOVENA)			+= novena/
>  obj-$(CONFIG_MACH_CCMX51)			+= ccxmx51/
>  obj-$(CONFIG_MACH_CCMX53)			+= ccxmx53/
>  obj-$(CONFIG_MACH_CFA10036)			+= crystalfontz-cfa10036/
> diff --git a/arch/arm/boards/novena/Makefile b/arch/arm/boards/novena/Makefile
> new file mode 100644
> index 0000000000..da63d2625f
> --- /dev/null
> +++ b/arch/arm/boards/novena/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +obj-y += board.o
> +lwl-y += lowlevel.o
> diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
> new file mode 100644
> index 0000000000..e247f8b8c3
> --- /dev/null
> +++ b/arch/arm/boards/novena/board.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// SPDX-FileCopyrightText: 2023 John Watts
						^
I'm not sure if this is required, but can you add your mail address
here as well?

> +
> +#include <common.h>
> +#include <deep-probe.h>
> +
> +static int novena_probe(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static const struct of_device_id novena_of_match[] = {
> +	{
> +		.compatible = "kosagi,imx6q-novena",
> +	},

Nit: could be a oneliner.

> +	{ /* sentinel */ }
> +};
> +
> +static struct driver novena_board_driver = {
> +	.name = "board-novena",
> +	.probe = novena_probe,
> +	.of_compatible = novena_of_match,
> +};
> +coredevice_platform_driver(novena_board_driver);
> +
> +BAREBOX_DEEP_PROBE_ENABLE(novena_of_match);
> diff --git a/arch/arm/boards/novena/flash-header-novena.imxcfg b/arch/arm/boards/novena/flash-header-novena.imxcfg
> new file mode 100644
> index 0000000000..61425976ec
> --- /dev/null
> +++ b/arch/arm/boards/novena/flash-header-novena.imxcfg
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +loadaddr 0x00907000
> +soc imx6
> +max_load_size 0x11000
> +ivtofs 0x400
> diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
> new file mode 100644
> index 0000000000..2052ca9de8
> --- /dev/null
> +++ b/arch/arm/boards/novena/lowlevel.c
> @@ -0,0 +1,73 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// SPDX-FileCopyrightText: 2023 John Watts
> +
> +#include <asm/barebox-arm.h>
> +#include <common.h>
> +#include <debug_ll.h>
> +#include <mach/esdctl.h>
> +#include <mach/generic.h>
> +#include <mach/imx6.h>
> +#include <mach/iomux-mx6.h>
> +#include <mach/xload.h>
> +#include <soc/fsl/fsl_udc.h>
> +
> +#define MX6_OCRAM_BASE_ADDR 0x00900000
> +#define STACK_TOP (MX6_OCRAM_BASE_ADDR + SZ_128K)
> +
> +extern char __dtb_imx6q_novena_start[];
> +
> +static bool running_from_ram(void)
> +{
> +	return (get_pc() >= MX6_MMDC_PORT01_BASE_ADDR);
> +}
> +
> +static void setup_uart(void)
> +{
> +	/* NOTE: RX is needed for TX to work on this board */
> +	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D26__UART2_RXD);
> +	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D27__UART2_TXD);

Can we add a newline in between to make it more readable?

> +	imx6_uart_setup(IOMEM(MX6_UART2_BASE_ADDR));
> +	pbl_set_putc(imx_uart_putc, IOMEM(MX6_UART2_BASE_ADDR));

Here as well.

> +	pr_debug(">");
> +}
> +
> +static void load_barebox(void)
> +{
> +	enum bootsource bootsrc;
> +	int bootinstance;
> +
> +	imx6_get_boot_source(&bootsrc, &bootinstance);
> +
> +	if (bootsrc == BOOTSOURCE_SERIAL) {
> +		imx6_barebox_start_usb(IOMEM(MX6_MMDC_PORT01_BASE_ADDR));
> +	} else if (bootsrc == BOOTSOURCE_MMC) {
> +		imx6_esdhc_start_image(bootinstance);
> +	} else {
> +		pr_err("Unsupported boot source %i instance %i\n",
> +					bootsrc, bootinstance);
> +		hang();
> +	}

We could rewrite this to:

	if (bootsrc == BOOTSOURCE_SERIAL)
		imx6_barebox_start_usb(IOMEM(MX6_MMDC_PORT01_BASE_ADDR));
	else if (bootsrc == BOOTSOURCE_MMC)
		imx6_esdhc_start_image(bootinstance);

	pr_err("Unsupported boot source %i instance %i\n", bootsrc, bootinstance);
	
	hang();

or use switch-case.

> +}
> +
> +static void boot_barebox(void)
> +{
> +	void *fdt = __dtb_imx6q_novena_start + get_runtime_offset();

The get_runtime_offset() can be dropped here since we already relocated.
Also we could move this function into the entry_function.

> +
> +	imx6q_barebox_entry(fdt);
> +}
> +
> +ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
> +{
> +	imx6_cpu_lowlevel_init();
> +	relocate_to_current_adr();
> +	setup_c();
> +	barrier();

After reading the setup_c() and the cache-armv7.S code I think we don't
need the barrier() here.

> +	if (!running_from_ram()) {
> +		imx6_ungate_all_peripherals();
> +		setup_uart();
> +		load_barebox();
> +	} else {
> +		boot_barebox();
> +	}

This could be re-written to:

	imx6_ungate_all_peripherals();

	if (IS_ENABLED(CONFIG_DEBUG_LL))
		setup_uart();

	if (!running_from_ram())
		load_barebox();
	
	boot_barebox;

> +}
> diff --git a/arch/arm/configs/imx_v7_defconfig b/arch/arm/configs/imx_v7_defconfig
> index 6110a52e48..f8deca324a 100644
> --- a/arch/arm/configs/imx_v7_defconfig
> +++ b/arch/arm/configs/imx_v7_defconfig
> @@ -25,6 +25,7 @@ CONFIG_MACH_SABRELITE=y
>  CONFIG_MACH_SABRESD=y
>  CONFIG_MACH_FREESCALE_IMX6SX_SABRESDB=y
>  CONFIG_MACH_NITROGEN6=y
> +CONFIG_MACH_NOVENA=y
>  CONFIG_MACH_SOLIDRUN_MICROSOM=y
>  CONFIG_MACH_TECHNEXION_PICO_HOBBIT=y
>  CONFIG_MACH_TECHNEXION_WANDBOARD=y
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 26c4a8ab99..a01ee19f68 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -46,6 +46,7 @@ lwl-$(CONFIG_MACH_MYIRTECH_X335X) += am335x-myirtech-myd.dtb.o
>  lwl-$(CONFIG_MACH_NETGEAR_RN104) += armada-370-rn104-bb.dtb.o
>  lwl-$(CONFIG_MACH_NETGEAR_RN2120) += armada-xp-rn2120-bb.dtb.o
>  lwl-$(CONFIG_MACH_NITROGEN6) += imx6q-nitrogen6x.dtb.o imx6dl-nitrogen6x.dtb.o imx6qp-nitrogen6_max.dtb.o
> +lwl-$(CONFIG_MACH_NOVENA) += imx6q-novena.dtb.o
>  lwl-$(CONFIG_MACH_NVIDIA_BEAVER) += tegra30-beaver.dtb.o
>  lwl-$(CONFIG_MACH_NVIDIA_JETSON) += tegra124-jetson-tk1.dtb.o
>  lwl-$(CONFIG_MACH_PCA100) += imx27-phytec-phycard-s-rdk-bb.dtb.o
> diff --git a/arch/arm/dts/imx6q-novena.dts b/arch/arm/dts/imx6q-novena.dts
> new file mode 100644
> index 0000000000..07471cb132
> --- /dev/null
> +++ b/arch/arm/dts/imx6q-novena.dts
> @@ -0,0 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR X11
> +// SPDX-FileCopyrightText: 2023 John Watts
> +
> +#include <arm/imx6q-novena.dts>
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 774c4cacb7..7993835be7 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -434,6 +434,12 @@ config MACH_NITROGEN6
>  	bool "BoundaryDevices Nitrogen6 boards"
>  	select ARCH_IMX6
>  
> +config MACH_NOVENA
> +	bool "Kosagi Novena board"
> +	select ARCH_IMX6
> +	select MCI_IMX_ESDHC_PBL
> +	select USB_GADGET_DRIVER_ARC_PBL
> +
>  config MACH_SOLIDRUN_MICROSOM
>  	bool "SolidRun MicroSOM based devices"
>  	select ARCH_IMX6
> diff --git a/images/Makefile.imx b/images/Makefile.imx
> index e9f4ba64e4..65914212bf 100644
> --- a/images/Makefile.imx
> +++ b/images/Makefile.imx
> @@ -258,6 +258,8 @@ $(call build_imx_habv4img, CONFIG_MACH_NITROGEN6, start_imx6dl_nitrogen6x_2g, bo
>  
>  $(call build_imx_habv4img, CONFIG_MACH_NITROGEN6, start_imx6qp_nitrogen6_max, boundarydevices-nitrogen6/flash-header-nitrogen6qp-max, boundarydevices-imx6qp-nitrogen6_max)
>  
> +$(call build_imx_habv4img, CONFIG_MACH_NOVENA, start_imx6q_novena, novena/flash-header-novena, imx6q-novena)
> +
>  $(call build_imx_habv4img, CONFIG_MACH_TX6X, start_imx6dl_tx6x_512m, karo-tx6x/flash-header-tx6dl-512m, karo-imx6dl-tx6x-512m)
>  
>  $(call build_imx_habv4img, CONFIG_MACH_TX6X, start_imx6dl_tx6x_1g, karo-tx6x/flash-header-tx6dl-1g, karo-imx6dl-tx6x-1g)
> -- 
> 2.39.0
> 
> 
> 

Re: [PATCH v2 2/4] ARM: novena: Setup RAM using static configuration

[Marco Felsch]

Hi John,

On 23-01-26, John Watts wrote:
> The Novena laptops were shipped with a 4GB stick that can be set up
> using this DDR3 configuration.
> This isn't ideal as the stick is swappable, but it will work for
> most people.
> 
> The DDR registers are copied from /board/kosagi/novena/novena_spl.c
> from the Novena U-Boot fork (tag novena-r2) with two changes:
> 
> - Bank interleaving is disabled: The Novena U-Boot code disables it
>   unconditionally during execution anyway
> - Page size is set to 1KB: I'm not sure the hardware supports x16 DIMMs
> 
> Mainline U-Boot has similar code, but the timing differences don't work
> on my board so I'm unable to test or port that code over.
> 
> Signed-off-by: John Watts <contact@jookia.org>

this patch should be part of patch1 since without this one you can't
boot the board, right?

> ---
>  arch/arm/boards/novena/ddr_regs.h | 119 ++++++++++++++++++++++++++++++
>  arch/arm/boards/novena/lowlevel.c |   9 +++
>  2 files changed, 128 insertions(+)
>  create mode 100644 arch/arm/boards/novena/ddr_regs.h
> 
> diff --git a/arch/arm/boards/novena/ddr_regs.h b/arch/arm/boards/novena/ddr_regs.h
> new file mode 100644
> index 0000000000..5f18d5e0e4
> --- /dev/null
> +++ b/arch/arm/boards/novena/ddr_regs.h
> @@ -0,0 +1,119 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* SPDX-FileCopyrightText: 2014 Marek Vasut <marex@denx.de> */
> +
> +#ifndef NOVENA_DDR_REGS_H
> +#define NOVENA_DDR_REGS_H
> +
> +/* MEMORY CONTROLLER CONFIGURATION */
> +
> +static struct mx6dq_iomux_ddr_regs novena_ddr_regs = {
> +	/* SDCLK[0:1], CAS, RAS, Reset: Differential input, 40ohm */
> +	.dram_sdclk_0		= 0x00020038,
> +	.dram_sdclk_1		= 0x00020038,
> +	.dram_cas		= 0x00000038,
> +	.dram_ras		= 0x00000038,
> +	.dram_reset		= 0x00000038,
> +	/* SDCKE[0:1]: 100k pull-up */
> +	.dram_sdcke0		= 0x00003000,
> +	.dram_sdcke1		= 0x00003000,
> +	/* SDBA2: pull-up disabled */
> +	.dram_sdba2		= 0x00000000,
> +	/* SDODT[0:1]: 100k pull-up, 40 ohm */
> +	.dram_sdodt0		= 0x00000038,
> +	.dram_sdodt1		= 0x00000038,
> +	/* SDQS[0:7]: Differential input, 40 ohm */
> +	.dram_sdqs0		= 0x00000038,
> +	.dram_sdqs1		= 0x00000038,
> +	.dram_sdqs2		= 0x00000038,
> +	.dram_sdqs3		= 0x00000038,
> +	.dram_sdqs4		= 0x00000038,
> +	.dram_sdqs5		= 0x00000038,
> +	.dram_sdqs6		= 0x00000038,
> +	.dram_sdqs7		= 0x00000038,
> +
> +	/* DQM[0:7]: Differential input, 40 ohm */
> +	.dram_dqm0		= 0x00000038,
> +	.dram_dqm1		= 0x00000038,
> +	.dram_dqm2		= 0x00000038,
> +	.dram_dqm3		= 0x00000038,
> +	.dram_dqm4		= 0x00000038,
> +	.dram_dqm5		= 0x00000038,
> +	.dram_dqm6		= 0x00000038,
> +	.dram_dqm7		= 0x00000038,
> +};
> +
> +static struct mx6dq_iomux_grp_regs novena_grp_regs = {
> +	/* DDR3 */
> +	.grp_ddr_type		= 0x000c0000,
> +	.grp_ddrmode_ctl	= 0x00020000,
> +	/* Disable DDR pullups */
> +	.grp_ddrpke		= 0x00000000,
> +	/* ADDR[00:16], SDBA[0:1]: 40 ohm */
> +	.grp_addds		= 0x00000038,
> +	/* CS0/CS1/SDBA2/CKE0/CKE1/SDWE: 40 ohm */
> +	.grp_ctlds		= 0x00000038,
> +	/* DATA[00:63]: Differential input, 40 ohm */
> +	.grp_ddrmode		= 0x00020000,
> +	.grp_b0ds		= 0x00000038,
> +	.grp_b1ds		= 0x00000038,
> +	.grp_b2ds		= 0x00000038,
> +	.grp_b3ds		= 0x00000038,
> +	.grp_b4ds		= 0x00000038,
> +	.grp_b5ds		= 0x00000038,
> +	.grp_b6ds		= 0x00000038,
> +	.grp_b7ds		= 0x00000038,
> +};
> +
> +/* MEMORY STICK CONFIGURATION */
> +
> +static struct mx6_mmdc_calibration novena_mmdc_calib = {
> +	/* write leveling calibration determine */
> +	.p0_mpwldectrl0		= 0x00420048,
> +	.p0_mpwldectrl1		= 0x006f0059,
> +	.p1_mpwldectrl0		= 0x005a0104,
> +	.p1_mpwldectrl1		= 0x01070113,
> +	/* Read DQS Gating calibration */
> +	.p0_mpdgctrl0		= 0x437c040b,
> +	.p0_mpdgctrl1		= 0x0413040e,
> +	.p1_mpdgctrl0		= 0x444f0446,
> +	.p1_mpdgctrl1		= 0x044d0422,
> +	/* Read Calibration: DQS delay relative to DQ read access */
> +	.p0_mprddlctl		= 0x4c424249,
> +	.p1_mprddlctl		= 0x4e48414f,
> +	/* Write Calibration: DQ/DM delay relative to DQS write access */
> +	.p0_mpwrdlctl		= 0x42414641,
> +	.p1_mpwrdlctl		= 0x46374b43,
> +};
> +
> +static struct mx6_ddr_sysinfo novena_ddr_info = {
> +	/* Width of data bus: 0=16, 1=32, 2=64 */
> +	.dsize		= 2,
> +	/* Config for full 4GB range so that get_mem_size() works */
> +	.cs_density	= 32,	/* 32Gb per CS */
> +	/* Single chip select */
> +	.ncs		= 1,
> +	.cs1_mirror	= 0,
> +	.rtt_wr		= 1,	/* RTT_Wr = RZQ/4 */
> +	.rtt_nom	= 2,	/* RTT_Nom = RZQ/2 */
> +	.walat		= 3,	/* Write additional latency */
> +	.ralat		= 7,	/* Read additional latency */
> +	.mif3_mode	= 3,	/* Command prediction working mode */
> +	.bi_on		= 0,	/* Bank interleaving disabled */
> +	.sde_to_rst	= 0x10,	/* 14 cycles, 200us (JEDEC default) */
> +	.rst_to_cke	= 0x23,	/* 33 cycles, 500us (JEDEC default) */
> +};
> +
> +static struct mx6_ddr3_cfg novena_ddr_cfg = {
> +	.mem_speed	= 1600,
> +	.density	= 4,
> +	.width		= 64,
> +	.banks		= 8,
> +	.rowaddr	= 16,
> +	.coladdr	= 10,
> +	.pagesz		= 1,
> +	.trcd		= 1300,
> +	.trcmin		= 4900,
> +	.trasmin	= 3590,
> +};
> +
> +#endif
> diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
> index 2052ca9de8..ef8acc8600 100644
> --- a/arch/arm/boards/novena/lowlevel.c
> +++ b/arch/arm/boards/novena/lowlevel.c
> @@ -7,9 +7,11 @@
>  #include <mach/esdctl.h>
>  #include <mach/generic.h>
>  #include <mach/imx6.h>
> +#include <mach/imx6-mmdc.h>
>  #include <mach/iomux-mx6.h>
>  #include <mach/xload.h>
>  #include <soc/fsl/fsl_udc.h>
> +#include "ddr_regs.h"
>  
>  #define MX6_OCRAM_BASE_ADDR 0x00900000
>  #define STACK_TOP (MX6_OCRAM_BASE_ADDR + SZ_128K)
> @@ -31,6 +33,12 @@ static void setup_uart(void)
>  	pr_debug(">");
>  }
>  
> +static void setup_ram(void)
> +{
> +	mx6dq_dram_iocfg(64, &novena_ddr_regs, &novena_grp_regs);
> +	mx6_dram_cfg(&novena_ddr_info, &novena_mmdc_calib, &novena_ddr_cfg);
> +}
> +
>  static void load_barebox(void)
>  {
>  	enum bootsource bootsrc;
> @@ -66,6 +74,7 @@ ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
>  	if (!running_from_ram()) {
>  		imx6_ungate_all_peripherals();
>  		setup_uart();
> +		setup_ram();

Nit: You can also move the setup_ram() into the load_barebox() function.

Regards,
  Marco

>  		load_barebox();
>  	} else {
>  		boot_barebox();
> -- 
> 2.39.0
> 
> 
> 

Re: [PATCH v2 3/4] ARM: novena: Read Ethernet MAC address from EEPROM

[Marco Felsch]

On 23-01-26, John Watts wrote:
> The Novena has an EEPROM used for storing information about the board,
> including the Ethernet MAC address.
> 
> The reference for the EEPROM fields is the novena-eeprom source code.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  arch/arm/boards/novena/board.c | 88 ++++++++++++++++++++++++++++++++++
>  arch/arm/dts/imx6q-novena.dts  | 15 ++++++
>  2 files changed, 103 insertions(+)
> 
> diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
> index e247f8b8c3..24cde1c0d4 100644
> --- a/arch/arm/boards/novena/board.c
> +++ b/arch/arm/boards/novena/board.c
> @@ -3,9 +3,97 @@
>  
>  #include <common.h>
>  #include <deep-probe.h>
> +#include <fs.h>
> +#include <libfile.h>
> +#include <net.h>
> +
> +struct novena_eeprom {
> +	uint8_t signature[6]; /* 'Novena' */
> +	uint8_t version; /* 1 or 2, not checked */
> +	uint8_t page_size; /* v2 only: EEPROM read/write page */
> +	uint32_t serial; /* 32-bit serial number */
> +	uint8_t mac[6]; /* Gigabit MAC address */
> +	uint16_t features; /* features */
> +	/* ... extra fields omitted ... */
> +} __packed;
> +
> +static void power_on_audio_codec(void)
> +{
> +	int rc = of_devices_ensure_probed_by_name("regulator-audio-codec");
> +
> +	if (rc < 0)
> +		pr_err("Unable to power on audio codec: %s\n", strerror(-rc));
> +}
> +
> +static struct novena_eeprom *novena_read_eeprom(void)
> +{
> +	size_t read;
> +	loff_t max = sizeof(struct novena_eeprom);
> +	void *eeprom;
> +	int rc;
> +
> +	/*
> +	 * When powered off the audio codec pulls down the EEPROM's I2C line.
> +	 * Power it on so we can actually read data.
> +	 */
> +	power_on_audio_codec();
> +
> +	rc = of_device_ensure_probed_by_alias("eeprom0");
> +	if (rc < 0) {
> +		pr_err("Unable to probe eeprom0: %s\n", strerror(-rc));
> +		return NULL;
> +	}
> +
> +	rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);

You never free the eeprom buffer.

> +
> +	if (rc < 0 && rc != -EFBIG) {
> +		pr_err("Unable to read Novena EEPROM: %s\n", strerror(-rc));
> +		return NULL;
> +	} else if (read != max) {
> +		pr_err("Short read from Novena EEPROM?\n");
> +		return NULL;
> +	} else {
> +		return eeprom;
> +	}

Last else can be dropped and instad:

	return eeprom;
> +}
> +
> +static bool novena_check_eeprom(struct novena_eeprom *eeprom)
> +{
> +	char *sig = eeprom->signature;
> +	size_t size = sizeof(eeprom->signature);
> +
> +	if (memcmp("Novena", sig, size) != 0) {
> +		pr_err("Unknown Novena EEPROM signature\n");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static void novena_set_mac(struct novena_eeprom *eeprom)
> +{
> +	struct device_node *dnode;
> +
> +	dnode = of_find_node_by_alias(of_get_root_node(), "ethernet0");
> +	if (dnode)
> +		of_eth_register_ethaddr(dnode, eeprom->mac);
> +	else
> +		pr_err("Unable to find ethernet node\n");
> +}
> +
> +static void novena_try_eeprom(void)
> +{
> +	struct novena_eeprom *eeprom = novena_read_eeprom();
> +
> +	if (!eeprom || !novena_check_eeprom(eeprom))
> +		return;
> +
> +	novena_set_mac(eeprom);
> +}
>  
>  static int novena_probe(struct device *dev)
>  {
> +	novena_try_eeprom();

It's not wrong to move it into a sub-function but IMO at least this
function can be part of the probe() function. Also I would do the
device_ensured_probe function here and just have one function which I
would call: novena_set_mac(). This way it's a bit easier to follow the
code. But that's just my oppinion.

> +
>  	return 0;
>  }
>  
> diff --git a/arch/arm/dts/imx6q-novena.dts b/arch/arm/dts/imx6q-novena.dts
> index 07471cb132..095dcde0d5 100644
> --- a/arch/arm/dts/imx6q-novena.dts
> +++ b/arch/arm/dts/imx6q-novena.dts
> @@ -2,3 +2,18 @@
>  // SPDX-FileCopyrightText: 2023 John Watts
>  
>  #include <arm/imx6q-novena.dts>
> +
> +/ {
> +	aliases {
> +		eeprom0 = &eeprom;
> +	};
> +};
> +
> +&i2c3 {
> +	eeprom: eeprom@56 {
> +		compatible = "24c512";
> +		reg = <0x56>;
> +		pagesize = <128>;
> +		status = "okay";

status can be dropped, "okay" is the default for the dtc.

Regards,
  Marco

> +	};
> +};
> -- 
> 2.39.0
> 
> 
> 

Re: [PATCH v2 1/4] ARM: novena: Add Kosagi Novena board

[Sascha Hauer]

On Wed, Jan 25, 2023 at 08:33:57PM +0100, Marco Felsch wrote:
> Hi John,
> 
> thanks for your patch.
> 
> On 23-01-26, John Watts wrote:
> > The Kosagi Novena is an open source laptop released in 2014.
> > 
> > This patch adds the initial project skeleton for running the PBL
> > and debugging over the UART2 port (labeled DEBUG on the board.)
> > 
> > Signed-off-by: John Watts <contact@jookia.org>
> > ---
> >  arch/arm/boards/Makefile                      |  1 +
> >  arch/arm/boards/novena/Makefile               |  4 +
> >  arch/arm/boards/novena/board.c                | 26 +++++++
> >  .../boards/novena/flash-header-novena.imxcfg  |  6 ++
> >  arch/arm/boards/novena/lowlevel.c             | 73 +++++++++++++++++++
> >  arch/arm/configs/imx_v7_defconfig             |  1 +
> >  arch/arm/dts/Makefile                         |  1 +
> >  arch/arm/dts/imx6q-novena.dts                 |  4 +
> >  arch/arm/mach-imx/Kconfig                     |  6 ++
> >  images/Makefile.imx                           |  2 +
> >  10 files changed, 124 insertions(+)
> >  create mode 100644 arch/arm/boards/novena/Makefile
> >  create mode 100644 arch/arm/boards/novena/board.c
> >  create mode 100644 arch/arm/boards/novena/flash-header-novena.imxcfg
> >  create mode 100644 arch/arm/boards/novena/lowlevel.c
> >  create mode 100644 arch/arm/dts/imx6q-novena.dts
> > 
> > diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
> > index f47aea6602..50088886eb 100644
> > --- a/arch/arm/boards/Makefile
> > +++ b/arch/arm/boards/Makefile
> > @@ -19,6 +19,7 @@ obj-$(CONFIG_MACH_BEAGLEBONE)			+= beaglebone/
> >  obj-$(CONFIG_MACH_CANON_A1100)			+= canon-a1100/
> >  obj-$(CONFIG_MACH_CM_FX6)			+= cm-fx6/
> >  obj-$(CONFIG_MACH_NITROGEN6)			+= boundarydevices-nitrogen6/
> > +obj-$(CONFIG_MACH_NOVENA)			+= novena/
> >  obj-$(CONFIG_MACH_CCMX51)			+= ccxmx51/
> >  obj-$(CONFIG_MACH_CCMX53)			+= ccxmx53/
> >  obj-$(CONFIG_MACH_CFA10036)			+= crystalfontz-cfa10036/
> > +	if (!running_from_ram()) {
> > +		imx6_ungate_all_peripherals();
> > +		setup_uart();
> > +		load_barebox();
> > +	} else {
> > +		boot_barebox();
> > +	}
> 
> This could be re-written to:
> 
> 	imx6_ungate_all_peripherals();
> 
> 	if (IS_ENABLED(CONFIG_DEBUG_LL))
> 		setup_uart();

setup_uart() also calls pbl_set_putc() to support PBL console. This is
useful without CONFIG_DEBUG_LL enabled, so please call it
unconditionally.

Sascha

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

Re: [PATCH v2 1/4] ARM: novena: Add Kosagi Novena board

[John Watts]

On Wed, Jan 25, 2023 at 08:33:57PM +0100, Marco Felsch wrote:
> Hi John,

Hello,

> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +// SPDX-FileCopyrightText: 2023 John Watts
> 						^
> I'm not sure if this is required, but can you add your mail address
> here as well?

Sure!

> 
> > +
> > +#include <common.h>
> > +#include <deep-probe.h>
> > +
> > +static int novena_probe(struct device *dev)
> > +{
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id novena_of_match[] = {
> > +	{
> > +		.compatible = "kosagi,imx6q-novena",
> > +	},
> 
> Nit: could be a oneliner.

I'm not sure I understand. I copied this from another board.

> > +	/* NOTE: RX is needed for TX to work on this board */
> > +	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D26__UART2_RXD);
> > +	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D27__UART2_TXD);
> 
> Can we add a newline in between to make it more readable?
> 
> > +	imx6_uart_setup(IOMEM(MX6_UART2_BASE_ADDR));
> > +	pbl_set_putc(imx_uart_putc, IOMEM(MX6_UART2_BASE_ADDR));
> 
> Here as well.
> 
> > +	pr_debug(">");
> > +}

Okay.

> We could rewrite this to:
> 
> 	if (bootsrc == BOOTSOURCE_SERIAL)
> 		imx6_barebox_start_usb(IOMEM(MX6_MMDC_PORT01_BASE_ADDR));
> 	else if (bootsrc == BOOTSOURCE_MMC)
> 		imx6_esdhc_start_image(bootinstance);
> 
> 	pr_err("Unsupported boot source %i instance %i\n", bootsrc, bootinstance);
> 	
> 	hang();
> 
> or use switch-case.

I'll do a fix with a switch case. I'm not a fan of having the style you just
described because it's not immediately clear that _start_usb and _start_image
are the end of the function. :)

> 
> > +}
> > +
> > +static void boot_barebox(void)
> > +{
> > +	void *fdt = __dtb_imx6q_novena_start + get_runtime_offset();
> 
> The get_runtime_offset() can be dropped here since we already relocated.

Ah okay, that helps.

> Also we could move this function into the entry_function.

I like the symmetry and verbosity of having the two code paths both named,
it makes reading the entry a bit easier.

I have a habit of moving things to small functions that describe what they
do instead of writing comments.

> > +
> > +	imx6q_barebox_entry(fdt);
> > +}
> > +
> > +ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
> > +{
> > +	imx6_cpu_lowlevel_init();
> > +	relocate_to_current_adr();
> > +	setup_c();
> > +	barrier();
> 
> After reading the setup_c() and the cache-armv7.S code I think we don't
> need the barrier() here.

Lots of other i.MX6 platforms use this. Shall remove though.

> 
> > +	if (!running_from_ram()) {
> > +		imx6_ungate_all_peripherals();
> > +		setup_uart();
> > +		load_barebox();
> > +	} else {
> > +		boot_barebox();
> > +	}
> 
> This could be re-written to:
> 
> 	imx6_ungate_all_peripherals();
> 
> 	if (IS_ENABLED(CONFIG_DEBUG_LL))
> 		setup_uart();
> 
> 	if (!running_from_ram())
> 		load_barebox();
> 	
> 	boot_barebox;

As Sascha said, this isn't the DEBUG_LL, but I again have kind of the same
function where you can't tell that load_barebox is an exit.

I wouldn't mind something like this:

 	imx6_ungate_all_peripherals();
        setup_uart();
 
 	if (!running_from_ram())
 		load_barebox();
 	else
 		boot_barebox();

But do we want to setup uart twice? I guess it doesn't matter.

Thanks for the review,

John.

Re: [PATCH v2 2/4] ARM: novena: Setup RAM using static configuration

[John Watts]

On Wed, Jan 25, 2023 at 08:39:40PM +0100, Marco Felsch wrote:
> Hi John,
> 
> this patch should be part of patch1 since without this one you can't
> boot the board, right?

Yes, but I wanted to do a separate commit so I can document the origin of the
code for the Git history and make it easier to review.

If this is inappropriate I'd like to move some of the git description to the
header file instead.

> >  	enum bootsource bootsrc;
> > @@ -66,6 +74,7 @@ ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
> >  	if (!running_from_ram()) {
> >  		imx6_ungate_all_peripherals();
> >  		setup_uart();
> > +		setup_ram();
> 
> Nit: You can also move the setup_ram() into the load_barebox() function.

Hmm. I'm not too sure how I feel about this as the two tasks are unrelated
code-wise. I can change it if it makes things easier.

> 
> Regards,
>   Marco

John

Re: [PATCH v2 3/4] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

On Wed, Jan 25, 2023 at 09:07:14PM +0100, Marco Felsch wrote:
> > +	rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);
> 
> You never free the eeprom buffer.

Argh, I explicitly remember writing down that I need to do this then promptly
got distracted when searching for the correct free function to use.

> > +
> > +	if (rc < 0 && rc != -EFBIG) {
> > +		pr_err("Unable to read Novena EEPROM: %s\n", strerror(-rc));
> > +		return NULL;
> > +	} else if (read != max) {
> > +		pr_err("Short read from Novena EEPROM?\n");
> > +		return NULL;
> > +	} else {
> > +		return eeprom;
> > +	}
> 
> Last else can be dropped and instad:
> 
> 	return eeprom;
> > +}

Okay.

> > +
> > +static bool novena_check_eeprom(struct novena_eeprom *eeprom)
> > +{
> > +	char *sig = eeprom->signature;
> > +	size_t size = sizeof(eeprom->signature);
> > +
> > +	if (memcmp("Novena", sig, size) != 0) {
> > +		pr_err("Unknown Novena EEPROM signature\n");
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static void novena_set_mac(struct novena_eeprom *eeprom)
> > +{
> > +	struct device_node *dnode;
> > +
> > +	dnode = of_find_node_by_alias(of_get_root_node(), "ethernet0");
> > +	if (dnode)
> > +		of_eth_register_ethaddr(dnode, eeprom->mac);
> > +	else
> > +		pr_err("Unable to find ethernet node\n");
> > +}
> > +
> > +static void novena_try_eeprom(void)
> > +{
> > +	struct novena_eeprom *eeprom = novena_read_eeprom();
> > +
> > +	if (!eeprom || !novena_check_eeprom(eeprom))
> > +		return;
> > +
> > +	novena_set_mac(eeprom);
> > +}
> >  
> >  static int novena_probe(struct device *dev)
> >  {
> > +	novena_try_eeprom();
> 
> It's not wrong to move it into a sub-function but IMO at least this
> function can be part of the probe() function. Also I would do the
> device_ensured_probe function here and just have one function which I
> would call: novena_set_mac(). This way it's a bit easier to follow the
> code. But that's just my oppinion.

I'm not quite sure what you mean, could you provide an example?

> > +	eeprom: eeprom@56 {
> > +		compatible = "24c512";
> > +		reg = <0x56>;
> > +		pagesize = <128>;
> > +		status = "okay";
> 
> status can be dropped, "okay" is the default for the dtc.

Gotcha.

> 
> Regards,
>   Marco

John.

Re: [PATCH v2 2/4] ARM: novena: Setup RAM using static configuration

[Sascha Hauer]

On Thu, Jan 26, 2023 at 06:54:06PM +1100, John Watts wrote:
> On Wed, Jan 25, 2023 at 08:39:40PM +0100, Marco Felsch wrote:
> > Hi John,
> > 
> > this patch should be part of patch1 since without this one you can't
> > boot the board, right?
> 
> Yes, but I wanted to do a separate commit so I can document the origin of the
> code for the Git history and make it easier to review.

Normally you can put everything needed for a new board into a single
commit, but splitting it up in two patches as you did gives you the
opportunity to add a few lines to the commit message specific to the
SDRAM setup. Therefore I think it's good the way you did it.

Sascha


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

Re: [PATCH v2 3/4] ARM: novena: Read Ethernet MAC address from EEPROM

[Marco Felsch]

Hi John,

On 23-01-26, John Watts wrote:
> On Wed, Jan 25, 2023 at 09:07:14PM +0100, Marco Felsch wrote:
> > > +	rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);
> > 
> > You never free the eeprom buffer.
> 
> Argh, I explicitly remember writing down that I need to do this then promptly
> got distracted when searching for the correct free function to use.
> 
> > > +
> > > +	if (rc < 0 && rc != -EFBIG) {
> > > +		pr_err("Unable to read Novena EEPROM: %s\n", strerror(-rc));
> > > +		return NULL;

...

> > >  static int novena_probe(struct device *dev)
> > >  {
> > > +	novena_try_eeprom();
> > 
> > It's not wrong to move it into a sub-function but IMO at least this
> > function can be part of the probe() function. Also I would do the
> > device_ensured_probe function here and just have one function which I
> > would call: novena_set_mac(). This way it's a bit easier to follow the
> > code. But that's just my oppinion.
> 
> I'm not quite sure what you mean, could you provide an example?

I meant that the content of novena_try_eeprom() can be part of the
probe() function :) Furthermore I mean that having to much sub-functions
isn't really helpful, but that is just my oppinion.

Regards,
  Marco

Re: [PATCH v2 1/4] ARM: novena: Add Kosagi Novena board

[Marco Felsch]

Hi John,

On 23-01-26, John Watts wrote:
> On Wed, Jan 25, 2023 at 08:33:57PM +0100, Marco Felsch wrote:
> > Hi John,

...

> > > +static const struct of_device_id novena_of_match[] = {
> > > +	{
> > > +		.compatible = "kosagi,imx6q-novena",
> > > +	},
> > 
> > Nit: could be a oneliner.
> 
> I'm not sure I understand. I copied this from another board.

I meant this:

static const struct of_device_id novena_of_match[] = {
	{ .compatible = "kosagi,imx6q-novena", },

> > > +	/* NOTE: RX is needed for TX to work on this board */
> > > +	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D26__UART2_RXD);
> > > +	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D27__UART2_TXD);
> > 
> > Can we add a newline in between to make it more readable?
> > 
> > > +	imx6_uart_setup(IOMEM(MX6_UART2_BASE_ADDR));
> > > +	pbl_set_putc(imx_uart_putc, IOMEM(MX6_UART2_BASE_ADDR));
> > 
> > Here as well.
> > 
> > > +	pr_debug(">");
> > > +}
> 
> Okay.
> 
> > We could rewrite this to:
> > 
> > 	if (bootsrc == BOOTSOURCE_SERIAL)
> > 		imx6_barebox_start_usb(IOMEM(MX6_MMDC_PORT01_BASE_ADDR));
> > 	else if (bootsrc == BOOTSOURCE_MMC)
> > 		imx6_esdhc_start_image(bootinstance);
> > 
> > 	pr_err("Unsupported boot source %i instance %i\n", bootsrc, bootinstance);
> > 	
> > 	hang();
> > 
> > or use switch-case.
> 
> I'll do a fix with a switch case. I'm not a fan of having the style you just
> described because it's not immediately clear that _start_usb and _start_image
> are the end of the function. :)

Then an comment like:

/* This should never be reached */

helps :)

> > > +}
> > > +
> > > +static void boot_barebox(void)
> > > +{
> > > +	void *fdt = __dtb_imx6q_novena_start + get_runtime_offset();
> > 
> > The get_runtime_offset() can be dropped here since we already relocated.
> 
> Ah okay, that helps.
> 
> > Also we could move this function into the entry_function.
> 
> I like the symmetry and verbosity of having the two code paths both named,
> it makes reading the entry a bit easier.
> 
> I have a habit of moving things to small functions that describe what they
> do instead of writing comments.
> 
> > > +
> > > +	imx6q_barebox_entry(fdt);
> > > +}
> > > +
> > > +ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
> > > +{
> > > +	imx6_cpu_lowlevel_init();
> > > +	relocate_to_current_adr();
> > > +	setup_c();
> > > +	barrier();
> > 
> > After reading the setup_c() and the cache-armv7.S code I think we don't
> > need the barrier() here.
> 
> Lots of other i.MX6 platforms use this. Shall remove though.

I think so. I wondered because I saw boards not having a barrier() call
and some having. This triggered my curiosity and I checked the setup_c()
code.

> > > +	if (!running_from_ram()) {
> > > +		imx6_ungate_all_peripherals();
> > > +		setup_uart();
> > > +		load_barebox();
> > > +	} else {
> > > +		boot_barebox();
> > > +	}
> > 
> > This could be re-written to:
> > 
> > 	imx6_ungate_all_peripherals();
> > 
> > 	if (IS_ENABLED(CONFIG_DEBUG_LL))
> > 		setup_uart();
> > 
> > 	if (!running_from_ram())
> > 		load_barebox();
> > 	
> > 	boot_barebox;
> 
> As Sascha said, this isn't the DEBUG_LL, but I again have kind of the same
> function where you can't tell that load_barebox is an exit.
> 
> I wouldn't mind something like this:
> 
>  	imx6_ungate_all_peripherals();
>         setup_uart();
>  
>  	if (!running_from_ram())
>  		load_barebox();
>  	else
>  		boot_barebox();
> 
> But do we want to setup uart twice? I guess it doesn't matter.

I think that correct :)

> Thanks for the review,

You're welcome.

Regards,
  Marco

Re: [PATCH v2 2/4] ARM: novena: Setup RAM using static configuration

[Marco Felsch]

On 23-01-26, Sascha Hauer wrote:
> On Thu, Jan 26, 2023 at 06:54:06PM +1100, John Watts wrote:
> > On Wed, Jan 25, 2023 at 08:39:40PM +0100, Marco Felsch wrote:
> > > Hi John,
> > > 
> > > this patch should be part of patch1 since without this one you can't
> > > boot the board, right?
> > 
> > Yes, but I wanted to do a separate commit so I can document the origin of the
> > code for the Git history and make it easier to review.
> 
> Normally you can put everything needed for a new board into a single
> commit, but splitting it up in two patches as you did gives you the
> opportunity to add a few lines to the commit message specific to the
> SDRAM setup. Therefore I think it's good the way you did it.

Okay, if Sascha is fine with this style :)

Regards,
  Marco

[PATCH v3 0/4] Add support for the Kosagi Novena board

[John Watts]

This patch series introduces support for the Novena board.
This support is basic but enough to boot Barebox.

The EEPROM code here is included in anticipation for reading the features
flag in the future when more board features are supported.

Changes v2 -> v3:
- EEPROM data is freed correctly
- Add email to copyright headers
- Switched license from GPL2 to GPL2+
- Dropped get_runtime_offset for device tree
- Removed "okay" status from EEPROM in device tree
- Moved novena_try_eeprom code in to novena_probe
- Other small code re-organizations
- Moved OCRAM address to its own separate patch
- Moved udelay fix to its own patch

Changes v1 -> v2:
- The board uses the driver model now
- Memory density is now calculated correctly

John Watts (4):
  ARM: novena: Add Kosagi Novena board
  ARM: novena: Setup RAM using static configuration
  ARM: novena: Read Ethernet MAC address from EEPROM
  ARM: novena: Use DDR3 information from SPD EEPROM

 arch/arm/boards/Makefile                      |   1 +
 arch/arm/boards/novena/Makefile               |   4 +
 arch/arm/boards/novena/board.c                | 109 +++++++++++
 arch/arm/boards/novena/ddr_regs.h             | 119 ++++++++++++
 .../boards/novena/flash-header-novena.imxcfg  |   6 +
 arch/arm/boards/novena/lowlevel.c             | 183 ++++++++++++++++++
 arch/arm/configs/imx_v7_defconfig             |   1 +
 arch/arm/dts/Makefile                         |   1 +
 arch/arm/dts/imx6q-novena.dts                 |  18 ++
 arch/arm/mach-imx/Kconfig                     |   8 +
 images/Makefile.imx                           |   2 +
 11 files changed, 452 insertions(+)
 create mode 100644 arch/arm/boards/novena/Makefile
 create mode 100644 arch/arm/boards/novena/board.c
 create mode 100644 arch/arm/boards/novena/ddr_regs.h
 create mode 100644 arch/arm/boards/novena/flash-header-novena.imxcfg
 create mode 100644 arch/arm/boards/novena/lowlevel.c
 create mode 100644 arch/arm/dts/imx6q-novena.dts

-- 
2.39.1

[PATCH v3 1/4] ARM: novena: Add Kosagi Novena board

[John Watts]

The Kosagi Novena is an open source laptop released in 2014.

This patch adds the initial project skeleton for running the PBL
and debugging over the UART2 port (labeled DEBUG on the board.)

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/Makefile                      |  1 +
 arch/arm/boards/novena/Makefile               |  4 ++
 arch/arm/boards/novena/board.c                | 24 +++++++
 .../boards/novena/flash-header-novena.imxcfg  |  6 ++
 arch/arm/boards/novena/lowlevel.c             | 65 +++++++++++++++++++
 arch/arm/configs/imx_v7_defconfig             |  1 +
 arch/arm/dts/Makefile                         |  1 +
 arch/arm/dts/imx6q-novena.dts                 |  4 ++
 arch/arm/mach-imx/Kconfig                     |  6 ++
 images/Makefile.imx                           |  2 +
 10 files changed, 114 insertions(+)
 create mode 100644 arch/arm/boards/novena/Makefile
 create mode 100644 arch/arm/boards/novena/board.c
 create mode 100644 arch/arm/boards/novena/flash-header-novena.imxcfg
 create mode 100644 arch/arm/boards/novena/lowlevel.c
 create mode 100644 arch/arm/dts/imx6q-novena.dts

diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
index f47aea6602..50088886eb 100644
--- a/arch/arm/boards/Makefile
+++ b/arch/arm/boards/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_MACH_BEAGLEBONE)			+= beaglebone/
 obj-$(CONFIG_MACH_CANON_A1100)			+= canon-a1100/
 obj-$(CONFIG_MACH_CM_FX6)			+= cm-fx6/
 obj-$(CONFIG_MACH_NITROGEN6)			+= boundarydevices-nitrogen6/
+obj-$(CONFIG_MACH_NOVENA)			+= novena/
 obj-$(CONFIG_MACH_CCMX51)			+= ccxmx51/
 obj-$(CONFIG_MACH_CCMX53)			+= ccxmx53/
 obj-$(CONFIG_MACH_CFA10036)			+= crystalfontz-cfa10036/
diff --git a/arch/arm/boards/novena/Makefile b/arch/arm/boards/novena/Makefile
new file mode 100644
index 0000000000..3111392bf9
--- /dev/null
+++ b/arch/arm/boards/novena/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+obj-y += board.o
+lwl-y += lowlevel.o
diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
new file mode 100644
index 0000000000..e41ea10f8d
--- /dev/null
+++ b/arch/arm/boards/novena/board.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// SPDX-FileCopyrightText: 2023 John Watts <contact@jookia.org>
+
+#include <common.h>
+#include <deep-probe.h>
+
+static int novena_probe(struct device *dev)
+{
+	return 0;
+}
+
+static const struct of_device_id novena_of_match[] = {
+	{ .compatible = "kosagi,imx6q-novena", },
+	{ /* sentinel */ }
+};
+
+static struct driver novena_board_driver = {
+	.name = "board-novena",
+	.probe = novena_probe,
+	.of_compatible = novena_of_match,
+};
+coredevice_platform_driver(novena_board_driver);
+
+BAREBOX_DEEP_PROBE_ENABLE(novena_of_match);
diff --git a/arch/arm/boards/novena/flash-header-novena.imxcfg b/arch/arm/boards/novena/flash-header-novena.imxcfg
new file mode 100644
index 0000000000..0612542c19
--- /dev/null
+++ b/arch/arm/boards/novena/flash-header-novena.imxcfg
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+loadaddr 0x00907000
+soc imx6
+max_load_size 0x11000
+ivtofs 0x400
diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
new file mode 100644
index 0000000000..58b925c311
--- /dev/null
+++ b/arch/arm/boards/novena/lowlevel.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// SPDX-FileCopyrightText: 2023 John Watts <contact@jookia.org>
+
+#include <asm/barebox-arm.h>
+#include <common.h>
+#include <debug_ll.h>
+#include <mach/esdctl.h>
+#include <mach/generic.h>
+#include <mach/imx6.h>
+#include <mach/iomux-mx6.h>
+#include <mach/xload.h>
+#include <soc/fsl/fsl_udc.h>
+
+#define STACK_TOP (MX6_OCRAM_BASE_ADDR + MX6_OCRAM_MAX_SIZE)
+
+extern char __dtb_imx6q_novena_start[];
+
+static bool running_from_ram(void)
+{
+	return (get_pc() >= MX6_MMDC_PORT01_BASE_ADDR);
+}
+
+static void setup_uart(void)
+{
+	/* NOTE: RX is needed for TX to work on this board */
+	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D26__UART2_RXD);
+	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D27__UART2_TXD);
+
+	imx6_uart_setup(IOMEM(MX6_UART2_BASE_ADDR));
+	pbl_set_putc(imx_uart_putc, IOMEM(MX6_UART2_BASE_ADDR));
+
+	pr_debug(">");
+}
+
+static void load_barebox(void)
+{
+	enum bootsource bootsrc;
+	int bootinstance;
+
+	imx6_get_boot_source(&bootsrc, &bootinstance);
+
+	if (bootsrc == BOOTSOURCE_SERIAL)
+		imx6_barebox_start_usb(IOMEM(MX6_MMDC_PORT01_BASE_ADDR));
+	else if (bootsrc == BOOTSOURCE_MMC)
+		imx6_esdhc_start_image(bootinstance);
+
+	pr_err("Unsupported boot source %i instance %i\n",
+				bootsrc, bootinstance);
+	hang();
+}
+
+ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
+{
+	imx6_cpu_lowlevel_init();
+	relocate_to_current_adr();
+	setup_c();
+
+	imx6_ungate_all_peripherals();
+	setup_uart();
+
+	if (!running_from_ram())
+		load_barebox();
+	else
+		imx6q_barebox_entry(__dtb_imx6q_novena_start);
+}
diff --git a/arch/arm/configs/imx_v7_defconfig b/arch/arm/configs/imx_v7_defconfig
index 6110a52e48..f8deca324a 100644
--- a/arch/arm/configs/imx_v7_defconfig
+++ b/arch/arm/configs/imx_v7_defconfig
@@ -25,6 +25,7 @@ CONFIG_MACH_SABRELITE=y
 CONFIG_MACH_SABRESD=y
 CONFIG_MACH_FREESCALE_IMX6SX_SABRESDB=y
 CONFIG_MACH_NITROGEN6=y
+CONFIG_MACH_NOVENA=y
 CONFIG_MACH_SOLIDRUN_MICROSOM=y
 CONFIG_MACH_TECHNEXION_PICO_HOBBIT=y
 CONFIG_MACH_TECHNEXION_WANDBOARD=y
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 26c4a8ab99..a01ee19f68 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -46,6 +46,7 @@ lwl-$(CONFIG_MACH_MYIRTECH_X335X) += am335x-myirtech-myd.dtb.o
 lwl-$(CONFIG_MACH_NETGEAR_RN104) += armada-370-rn104-bb.dtb.o
 lwl-$(CONFIG_MACH_NETGEAR_RN2120) += armada-xp-rn2120-bb.dtb.o
 lwl-$(CONFIG_MACH_NITROGEN6) += imx6q-nitrogen6x.dtb.o imx6dl-nitrogen6x.dtb.o imx6qp-nitrogen6_max.dtb.o
+lwl-$(CONFIG_MACH_NOVENA) += imx6q-novena.dtb.o
 lwl-$(CONFIG_MACH_NVIDIA_BEAVER) += tegra30-beaver.dtb.o
 lwl-$(CONFIG_MACH_NVIDIA_JETSON) += tegra124-jetson-tk1.dtb.o
 lwl-$(CONFIG_MACH_PCA100) += imx27-phytec-phycard-s-rdk-bb.dtb.o
diff --git a/arch/arm/dts/imx6q-novena.dts b/arch/arm/dts/imx6q-novena.dts
new file mode 100644
index 0000000000..12bade849c
--- /dev/null
+++ b/arch/arm/dts/imx6q-novena.dts
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR X11
+// SPDX-FileCopyrightText: 2023 John Watts <contact@jookia.org>
+
+#include <arm/imx6q-novena.dts>
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 774c4cacb7..7993835be7 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -434,6 +434,12 @@ config MACH_NITROGEN6
 	bool "BoundaryDevices Nitrogen6 boards"
 	select ARCH_IMX6
 
+config MACH_NOVENA
+	bool "Kosagi Novena board"
+	select ARCH_IMX6
+	select MCI_IMX_ESDHC_PBL
+	select USB_GADGET_DRIVER_ARC_PBL
+
 config MACH_SOLIDRUN_MICROSOM
 	bool "SolidRun MicroSOM based devices"
 	select ARCH_IMX6
diff --git a/images/Makefile.imx b/images/Makefile.imx
index e9f4ba64e4..65914212bf 100644
--- a/images/Makefile.imx
+++ b/images/Makefile.imx
@@ -258,6 +258,8 @@ $(call build_imx_habv4img, CONFIG_MACH_NITROGEN6, start_imx6dl_nitrogen6x_2g, bo
 
 $(call build_imx_habv4img, CONFIG_MACH_NITROGEN6, start_imx6qp_nitrogen6_max, boundarydevices-nitrogen6/flash-header-nitrogen6qp-max, boundarydevices-imx6qp-nitrogen6_max)
 
+$(call build_imx_habv4img, CONFIG_MACH_NOVENA, start_imx6q_novena, novena/flash-header-novena, imx6q-novena)
+
 $(call build_imx_habv4img, CONFIG_MACH_TX6X, start_imx6dl_tx6x_512m, karo-tx6x/flash-header-tx6dl-512m, karo-imx6dl-tx6x-512m)
 
 $(call build_imx_habv4img, CONFIG_MACH_TX6X, start_imx6dl_tx6x_1g, karo-tx6x/flash-header-tx6dl-1g, karo-imx6dl-tx6x-1g)
-- 
2.39.1

[PATCH v3 2/4] ARM: novena: Setup RAM using static configuration

[John Watts]

The Novena laptops were shipped with a 4GB stick that can be set up
using this DDR3 configuration.
This isn't ideal as the stick is swappable, but it will work for
most people.

The DDR registers are copied from /board/kosagi/novena/novena_spl.c
from the Novena U-Boot fork (tag novena-r2) with two changes:

- Bank interleaving is disabled: The Novena U-Boot code disables it
  unconditionally during execution anyway
- Page size is set to 1KB: I'm not sure the hardware supports x16 DIMMs

Mainline U-Boot has similar code, but the timing differences don't work
on my board so I'm unable to test or port that code over.

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/novena/ddr_regs.h | 119 ++++++++++++++++++++++++++++++
 arch/arm/boards/novena/lowlevel.c |  14 +++-
 2 files changed, 131 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/boards/novena/ddr_regs.h

diff --git a/arch/arm/boards/novena/ddr_regs.h b/arch/arm/boards/novena/ddr_regs.h
new file mode 100644
index 0000000000..5f18d5e0e4
--- /dev/null
+++ b/arch/arm/boards/novena/ddr_regs.h
@@ -0,0 +1,119 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* SPDX-FileCopyrightText: 2014 Marek Vasut <marex@denx.de> */
+
+#ifndef NOVENA_DDR_REGS_H
+#define NOVENA_DDR_REGS_H
+
+/* MEMORY CONTROLLER CONFIGURATION */
+
+static struct mx6dq_iomux_ddr_regs novena_ddr_regs = {
+	/* SDCLK[0:1], CAS, RAS, Reset: Differential input, 40ohm */
+	.dram_sdclk_0		= 0x00020038,
+	.dram_sdclk_1		= 0x00020038,
+	.dram_cas		= 0x00000038,
+	.dram_ras		= 0x00000038,
+	.dram_reset		= 0x00000038,
+	/* SDCKE[0:1]: 100k pull-up */
+	.dram_sdcke0		= 0x00003000,
+	.dram_sdcke1		= 0x00003000,
+	/* SDBA2: pull-up disabled */
+	.dram_sdba2		= 0x00000000,
+	/* SDODT[0:1]: 100k pull-up, 40 ohm */
+	.dram_sdodt0		= 0x00000038,
+	.dram_sdodt1		= 0x00000038,
+	/* SDQS[0:7]: Differential input, 40 ohm */
+	.dram_sdqs0		= 0x00000038,
+	.dram_sdqs1		= 0x00000038,
+	.dram_sdqs2		= 0x00000038,
+	.dram_sdqs3		= 0x00000038,
+	.dram_sdqs4		= 0x00000038,
+	.dram_sdqs5		= 0x00000038,
+	.dram_sdqs6		= 0x00000038,
+	.dram_sdqs7		= 0x00000038,
+
+	/* DQM[0:7]: Differential input, 40 ohm */
+	.dram_dqm0		= 0x00000038,
+	.dram_dqm1		= 0x00000038,
+	.dram_dqm2		= 0x00000038,
+	.dram_dqm3		= 0x00000038,
+	.dram_dqm4		= 0x00000038,
+	.dram_dqm5		= 0x00000038,
+	.dram_dqm6		= 0x00000038,
+	.dram_dqm7		= 0x00000038,
+};
+
+static struct mx6dq_iomux_grp_regs novena_grp_regs = {
+	/* DDR3 */
+	.grp_ddr_type		= 0x000c0000,
+	.grp_ddrmode_ctl	= 0x00020000,
+	/* Disable DDR pullups */
+	.grp_ddrpke		= 0x00000000,
+	/* ADDR[00:16], SDBA[0:1]: 40 ohm */
+	.grp_addds		= 0x00000038,
+	/* CS0/CS1/SDBA2/CKE0/CKE1/SDWE: 40 ohm */
+	.grp_ctlds		= 0x00000038,
+	/* DATA[00:63]: Differential input, 40 ohm */
+	.grp_ddrmode		= 0x00020000,
+	.grp_b0ds		= 0x00000038,
+	.grp_b1ds		= 0x00000038,
+	.grp_b2ds		= 0x00000038,
+	.grp_b3ds		= 0x00000038,
+	.grp_b4ds		= 0x00000038,
+	.grp_b5ds		= 0x00000038,
+	.grp_b6ds		= 0x00000038,
+	.grp_b7ds		= 0x00000038,
+};
+
+/* MEMORY STICK CONFIGURATION */
+
+static struct mx6_mmdc_calibration novena_mmdc_calib = {
+	/* write leveling calibration determine */
+	.p0_mpwldectrl0		= 0x00420048,
+	.p0_mpwldectrl1		= 0x006f0059,
+	.p1_mpwldectrl0		= 0x005a0104,
+	.p1_mpwldectrl1		= 0x01070113,
+	/* Read DQS Gating calibration */
+	.p0_mpdgctrl0		= 0x437c040b,
+	.p0_mpdgctrl1		= 0x0413040e,
+	.p1_mpdgctrl0		= 0x444f0446,
+	.p1_mpdgctrl1		= 0x044d0422,
+	/* Read Calibration: DQS delay relative to DQ read access */
+	.p0_mprddlctl		= 0x4c424249,
+	.p1_mprddlctl		= 0x4e48414f,
+	/* Write Calibration: DQ/DM delay relative to DQS write access */
+	.p0_mpwrdlctl		= 0x42414641,
+	.p1_mpwrdlctl		= 0x46374b43,
+};
+
+static struct mx6_ddr_sysinfo novena_ddr_info = {
+	/* Width of data bus: 0=16, 1=32, 2=64 */
+	.dsize		= 2,
+	/* Config for full 4GB range so that get_mem_size() works */
+	.cs_density	= 32,	/* 32Gb per CS */
+	/* Single chip select */
+	.ncs		= 1,
+	.cs1_mirror	= 0,
+	.rtt_wr		= 1,	/* RTT_Wr = RZQ/4 */
+	.rtt_nom	= 2,	/* RTT_Nom = RZQ/2 */
+	.walat		= 3,	/* Write additional latency */
+	.ralat		= 7,	/* Read additional latency */
+	.mif3_mode	= 3,	/* Command prediction working mode */
+	.bi_on		= 0,	/* Bank interleaving disabled */
+	.sde_to_rst	= 0x10,	/* 14 cycles, 200us (JEDEC default) */
+	.rst_to_cke	= 0x23,	/* 33 cycles, 500us (JEDEC default) */
+};
+
+static struct mx6_ddr3_cfg novena_ddr_cfg = {
+	.mem_speed	= 1600,
+	.density	= 4,
+	.width		= 64,
+	.banks		= 8,
+	.rowaddr	= 16,
+	.coladdr	= 10,
+	.pagesz		= 1,
+	.trcd		= 1300,
+	.trcmin		= 4900,
+	.trasmin	= 3590,
+};
+
+#endif
diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
index 58b925c311..3acf983ee0 100644
--- a/arch/arm/boards/novena/lowlevel.c
+++ b/arch/arm/boards/novena/lowlevel.c
@@ -7,9 +7,11 @@
 #include <mach/esdctl.h>
 #include <mach/generic.h>
 #include <mach/imx6.h>
+#include <mach/imx6-mmdc.h>
 #include <mach/iomux-mx6.h>
 #include <mach/xload.h>
 #include <soc/fsl/fsl_udc.h>
+#include "ddr_regs.h"
 
 #define STACK_TOP (MX6_OCRAM_BASE_ADDR + MX6_OCRAM_MAX_SIZE)
 
@@ -32,6 +34,12 @@ static void setup_uart(void)
 	pr_debug(">");
 }
 
+static void setup_ram(void)
+{
+	mx6dq_dram_iocfg(64, &novena_ddr_regs, &novena_grp_regs);
+	mx6_dram_cfg(&novena_ddr_info, &novena_mmdc_calib, &novena_ddr_cfg);
+}
+
 static void load_barebox(void)
 {
 	enum bootsource bootsrc;
@@ -58,8 +66,10 @@ ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
 	imx6_ungate_all_peripherals();
 	setup_uart();
 
-	if (!running_from_ram())
+	if (!running_from_ram()) {
+		setup_ram();
 		load_barebox();
-	else
+	} else {
 		imx6q_barebox_entry(__dtb_imx6q_novena_start);
+	}
 }
-- 
2.39.1

[PATCH v3 3/4] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

The Novena has an EEPROM used for storing information about the board,
including the Ethernet MAC address.

The reference for the EEPROM fields is the novena-eeprom source code.

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/novena/board.c | 85 ++++++++++++++++++++++++++++++++++
 arch/arm/dts/imx6q-novena.dts  | 14 ++++++
 2 files changed, 99 insertions(+)

diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
index e41ea10f8d..b6c59aff44 100644
--- a/arch/arm/boards/novena/board.c
+++ b/arch/arm/boards/novena/board.c
@@ -3,9 +3,94 @@
 
 #include <common.h>
 #include <deep-probe.h>
+#include <fs.h>
+#include <libfile.h>
+#include <net.h>
+
+struct novena_eeprom {
+	uint8_t signature[6]; /* 'Novena' */
+	uint8_t version; /* 1 or 2, not checked */
+	uint8_t page_size; /* v2 only: EEPROM read/write page */
+	uint32_t serial; /* 32-bit serial number */
+	uint8_t mac[6]; /* Gigabit MAC address */
+	uint16_t features; /* features */
+	/* ... extra fields omitted ... */
+} __packed;
+
+static void power_on_audio_codec(void)
+{
+	int rc = of_devices_ensure_probed_by_name("regulator-audio-codec");
+
+	if (rc < 0)
+		pr_err("Unable to power on audio codec: %s\n", strerror(-rc));
+}
+
+static struct novena_eeprom *novena_read_eeprom(void)
+{
+	size_t read;
+	loff_t max = sizeof(struct novena_eeprom);
+	void *eeprom;
+	int rc;
+
+	/*
+	 * When powered off the audio codec pulls down the EEPROM's I2C line.
+	 * Power it on so we can actually read data.
+	 */
+	power_on_audio_codec();
+
+	rc = of_device_ensure_probed_by_alias("eeprom0");
+	if (rc < 0) {
+		pr_err("Unable to probe eeprom0: %s\n", strerror(-rc));
+		return NULL;
+	}
+
+	rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);
+
+	if (rc < 0 && rc != -EFBIG) {
+		pr_err("Unable to read Novena EEPROM: %s\n", strerror(-rc));
+		return NULL;
+	} else if (read != max) {
+		pr_err("Short read from Novena EEPROM?\n");
+		free(eeprom);
+		return NULL;
+	}
+
+	return eeprom;
+}
+
+static bool novena_check_eeprom(struct novena_eeprom *eeprom)
+{
+	char *sig = eeprom->signature;
+	size_t size = sizeof(eeprom->signature);
+
+	if (memcmp("Novena", sig, size) != 0) {
+		pr_err("Unknown Novena EEPROM signature\n");
+		return false;
+	}
+
+	return true;
+}
+
+static void novena_set_mac(struct novena_eeprom *eeprom)
+{
+	struct device_node *dnode;
+
+	dnode = of_find_node_by_alias(of_get_root_node(), "ethernet0");
+	if (dnode)
+		of_eth_register_ethaddr(dnode, eeprom->mac);
+	else
+		pr_err("Unable to find ethernet node\n");
+}
 
 static int novena_probe(struct device *dev)
 {
+	struct novena_eeprom *eeprom = novena_read_eeprom();
+
+	if (eeprom && novena_check_eeprom(eeprom))
+		novena_set_mac(eeprom);
+
+	free(eeprom);
+
 	return 0;
 }
 
diff --git a/arch/arm/dts/imx6q-novena.dts b/arch/arm/dts/imx6q-novena.dts
index 12bade849c..c614ed3cbb 100644
--- a/arch/arm/dts/imx6q-novena.dts
+++ b/arch/arm/dts/imx6q-novena.dts
@@ -2,3 +2,17 @@
 // SPDX-FileCopyrightText: 2023 John Watts <contact@jookia.org>
 
 #include <arm/imx6q-novena.dts>
+
+/ {
+	aliases {
+		eeprom0 = &eeprom;
+	};
+};
+
+&i2c3 {
+	eeprom: eeprom@56 {
+		compatible = "24c512";
+		reg = <0x56>;
+		pagesize = <128>;
+	};
+};
-- 
2.39.1

[PATCH v3 4/4] ARM: novena: Use DDR3 information from SPD EEPROM

[John Watts]

The Novena supports swappable memory which means we must query the memory
for configuration details. This patch uses values stored in the SPD EEPROM
to configure the i.MX6 memory controller.

If the SPD can't be read, the default configuration for a 4GB stick is used.

Calibration for the installed stick is run unconditionally.

Verification that this works was done by comparing MMDC registers before and
after and carefully reading other people's source code.

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/novena/lowlevel.c | 108 ++++++++++++++++++++++++++++++
 arch/arm/mach-imx/Kconfig         |   2 +
 2 files changed, 110 insertions(+)

diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
index 3acf983ee0..54980bda4e 100644
--- a/arch/arm/boards/novena/lowlevel.c
+++ b/arch/arm/boards/novena/lowlevel.c
@@ -3,6 +3,8 @@
 
 #include <asm/barebox-arm.h>
 #include <common.h>
+#include <ddr_dimms.h>
+#include <ddr_spd.h>
 #include <debug_ll.h>
 #include <mach/esdctl.h>
 #include <mach/generic.h>
@@ -10,6 +12,7 @@
 #include <mach/imx6-mmdc.h>
 #include <mach/iomux-mx6.h>
 #include <mach/xload.h>
+#include <pbl/i2c.h>
 #include <soc/fsl/fsl_udc.h>
 #include "ddr_regs.h"
 
@@ -17,6 +20,106 @@
 
 extern char __dtb_imx6q_novena_start[];
 
+static struct spd_eeprom spd_eeprom;
+static struct dimm_params dimm_params;
+
+static struct pbl_i2c *setup_spd_i2c(void)
+{
+	void __iomem *iomuxbase = IOMEM(MX6_IOMUXC_BASE_ADDR);
+
+	imx_setup_pad(iomuxbase, MX6Q_PAD_CSI0_DAT8__I2C1_SDA);
+	imx_setup_pad(iomuxbase, MX6Q_PAD_CSI0_DAT9__I2C1_SCL);
+
+	return imx6_i2c_early_init(IOMEM(MX6_I2C1_BASE_ADDR));
+}
+
+static struct spd_eeprom *read_spd(void)
+{
+	struct spd_eeprom *eeprom = &spd_eeprom;
+	struct pbl_i2c *i2c = setup_spd_i2c();
+	int rc;
+
+	rc = spd_read_eeprom(i2c, 0x50, eeprom, SPD_MEMTYPE_DDR3);
+	if (rc < 0) {
+		pr_err("Couldn't read SPD EEPROM: %i\n", rc);
+		return NULL;
+	}
+
+	rc = ddr3_spd_check(&eeprom->ddr3);
+	if (rc < 0) {
+		pr_err("Couldn't verify SPD data: %i\n", rc);
+		return NULL;
+	}
+
+	return eeprom;
+}
+
+static bool check_spd_compatible(struct spd_eeprom *eeprom)
+{
+	if (eeprom->ddr3.mem_type != 0x0b) {
+		pr_err("SPD data is not DDR3\n");
+		return false;
+	}
+
+	if ((eeprom->ddr3.bus_width & 0x1f) != 0x03) {
+		pr_err("SPD data is for a 64-bit bus\n");
+		return false;
+	}
+
+	return true;
+}
+
+static void setup_dimm_settings(struct dimm_params *params,
+				struct mx6_ddr_sysinfo *info,
+				struct mx6_ddr3_cfg *cfg)
+{
+	int capacity_gbit = params->capacity / 0x8000000;
+	int density_rank = capacity_gbit / params->n_ranks;
+
+	info->ncs = params->n_ranks;
+	info->cs_density = density_rank;
+	cfg->mem_speed = params->tckmin_x_ps;
+	cfg->density = density_rank / params->n_banks_per_sdram_device;
+	cfg->width = params->data_width;
+	cfg->banks = params->n_banks_per_sdram_device;
+	cfg->rowaddr = params->n_row_addr;
+	cfg->coladdr = params->n_col_addr;
+	cfg->trcd = params->trcd_ps / 10;
+	cfg->trcmin = params->trc_ps / 10;
+	cfg->trasmin = params->tras_ps / 10;
+	cfg->SRT = params->extended_op_srt;
+
+	if (params->device_width >= 16)
+		cfg->pagesz = 2;
+}
+
+static void read_dimm_settings(void)
+{
+	struct spd_eeprom *eeprom = read_spd();
+	struct dimm_params *params = &dimm_params;
+	int rc;
+
+	if (!eeprom) {
+		pr_err("Couldn't read SPD EEPROM, using default settings\n");
+		return;
+	}
+
+	if (!check_spd_compatible(eeprom)) {
+		pr_err("DIMM stick incompatible\n");
+		hang();
+	}
+
+	rc = ddr3_compute_dimm_parameters(&eeprom->ddr3, params);
+	if (rc < 0) {
+		pr_err("Couldn't compute DIMM params: %i\n", rc);
+		return;
+	}
+
+	setup_dimm_settings(params, &novena_ddr_info, &novena_ddr_cfg);
+
+	pr_info("Found DIMM: %s\n", params->mpart);
+}
+
 static bool running_from_ram(void)
 {
 	return (get_pc() >= MX6_MMDC_PORT01_BASE_ADDR);
@@ -36,8 +139,13 @@ static void setup_uart(void)
 
 static void setup_ram(void)
 {
+	read_dimm_settings();
+
 	mx6dq_dram_iocfg(64, &novena_ddr_regs, &novena_grp_regs);
 	mx6_dram_cfg(&novena_ddr_info, &novena_mmdc_calib, &novena_ddr_cfg);
+
+	mmdc_do_write_level_calibration();
+	mmdc_do_dqs_calibration();
 }
 
 static void load_barebox(void)
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 7993835be7..aa7a86da66 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -437,6 +437,8 @@ config MACH_NITROGEN6
 config MACH_NOVENA
 	bool "Kosagi Novena board"
 	select ARCH_IMX6
+	select DDR_SPD
+	select I2C_IMX_EARLY
 	select MCI_IMX_ESDHC_PBL
 	select USB_GADGET_DRIVER_ARC_PBL
 
-- 
2.39.1

Re: [PATCH v3 3/4] ARM: novena: Read Ethernet MAC address from EEPROM

[Marco Felsch]

Hi John,

thanks for the patch.

On 23-01-30, John Watts wrote:
> The Novena has an EEPROM used for storing information about the board,
> including the Ethernet MAC address.
> 
> The reference for the EEPROM fields is the novena-eeprom source code.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  arch/arm/boards/novena/board.c | 85 ++++++++++++++++++++++++++++++++++
>  arch/arm/dts/imx6q-novena.dts  | 14 ++++++
>  2 files changed, 99 insertions(+)
> 
> diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
> index e41ea10f8d..b6c59aff44 100644
> --- a/arch/arm/boards/novena/board.c
> +++ b/arch/arm/boards/novena/board.c
> @@ -3,9 +3,94 @@
>  
>  #include <common.h>
>  #include <deep-probe.h>
> +#include <fs.h>
> +#include <libfile.h>
> +#include <net.h>
> +
> +struct novena_eeprom {
> +	uint8_t signature[6]; /* 'Novena' */
> +	uint8_t version; /* 1 or 2, not checked */
> +	uint8_t page_size; /* v2 only: EEPROM read/write page */
> +	uint32_t serial; /* 32-bit serial number */
> +	uint8_t mac[6]; /* Gigabit MAC address */
> +	uint16_t features; /* features */
> +	/* ... extra fields omitted ... */
> +} __packed;
> +
> +static void power_on_audio_codec(void)
> +{
> +	int rc = of_devices_ensure_probed_by_name("regulator-audio-codec");
> +
> +	if (rc < 0)
> +		pr_err("Unable to power on audio codec: %s\n", strerror(-rc));
> +}
> +
> +static struct novena_eeprom *novena_read_eeprom(void)
> +{
> +	size_t read;
> +	loff_t max = sizeof(struct novena_eeprom);
> +	void *eeprom;
> +	int rc;
> +
> +	/*
> +	 * When powered off the audio codec pulls down the EEPROM's I2C line.
> +	 * Power it on so we can actually read data.
> +	 */
> +	power_on_audio_codec();
> +
> +	rc = of_device_ensure_probed_by_alias("eeprom0");
> +	if (rc < 0) {
> +		pr_err("Unable to probe eeprom0: %s\n", strerror(-rc));
> +		return NULL;
> +	}
> +
> +	rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);
> +
> +	if (rc < 0 && rc != -EFBIG) {
> +		pr_err("Unable to read Novena EEPROM: %s\n", strerror(-rc));
> +		return NULL;
> +	} else if (read != max) {
> +		pr_err("Short read from Novena EEPROM?\n");
> +		free(eeprom);

This will trigger a double free since you're use of an unconditional
free() during probe().

Regards,
  Marco

> +		return NULL;
> +	}
> +
> +	return eeprom;
> +}
> +
> +static bool novena_check_eeprom(struct novena_eeprom *eeprom)
> +{
> +	char *sig = eeprom->signature;
> +	size_t size = sizeof(eeprom->signature);
> +
> +	if (memcmp("Novena", sig, size) != 0) {
> +		pr_err("Unknown Novena EEPROM signature\n");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static void novena_set_mac(struct novena_eeprom *eeprom)
> +{
> +	struct device_node *dnode;
> +
> +	dnode = of_find_node_by_alias(of_get_root_node(), "ethernet0");
> +	if (dnode)
> +		of_eth_register_ethaddr(dnode, eeprom->mac);
> +	else
> +		pr_err("Unable to find ethernet node\n");
> +}
>  
>  static int novena_probe(struct device *dev)
>  {
> +	struct novena_eeprom *eeprom = novena_read_eeprom();
> +
> +	if (eeprom && novena_check_eeprom(eeprom))
> +		novena_set_mac(eeprom);
> +
> +	free(eeprom);
> +
>  	return 0;
>  }
>  
> diff --git a/arch/arm/dts/imx6q-novena.dts b/arch/arm/dts/imx6q-novena.dts
> index 12bade849c..c614ed3cbb 100644
> --- a/arch/arm/dts/imx6q-novena.dts
> +++ b/arch/arm/dts/imx6q-novena.dts
> @@ -2,3 +2,17 @@
>  // SPDX-FileCopyrightText: 2023 John Watts <contact@jookia.org>
>  
>  #include <arm/imx6q-novena.dts>
> +
> +/ {
> +	aliases {
> +		eeprom0 = &eeprom;
> +	};
> +};
> +
> +&i2c3 {
> +	eeprom: eeprom@56 {
> +		compatible = "24c512";
> +		reg = <0x56>;
> +		pagesize = <128>;
> +	};
> +};
> -- 
> 2.39.1
> 
> 
> 

Re: [PATCH v3 1/4] ARM: novena: Add Kosagi Novena board

[Marco Felsch]

Hi John,

thanks for the patch.

On 23-01-30, John Watts wrote:
> The Kosagi Novena is an open source laptop released in 2014.
> 
> This patch adds the initial project skeleton for running the PBL
> and debugging over the UART2 port (labeled DEBUG on the board.)
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  arch/arm/boards/Makefile                      |  1 +
>  arch/arm/boards/novena/Makefile               |  4 ++
>  arch/arm/boards/novena/board.c                | 24 +++++++
>  .../boards/novena/flash-header-novena.imxcfg  |  6 ++
>  arch/arm/boards/novena/lowlevel.c             | 65 +++++++++++++++++++
>  arch/arm/configs/imx_v7_defconfig             |  1 +
>  arch/arm/dts/Makefile                         |  1 +
>  arch/arm/dts/imx6q-novena.dts                 |  4 ++
>  arch/arm/mach-imx/Kconfig                     |  6 ++
>  images/Makefile.imx                           |  2 +
>  10 files changed, 114 insertions(+)
>  create mode 100644 arch/arm/boards/novena/Makefile
>  create mode 100644 arch/arm/boards/novena/board.c
>  create mode 100644 arch/arm/boards/novena/flash-header-novena.imxcfg
>  create mode 100644 arch/arm/boards/novena/lowlevel.c
>  create mode 100644 arch/arm/dts/imx6q-novena.dts
> 
> diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
> index f47aea6602..50088886eb 100644
> --- a/arch/arm/boards/Makefile
> +++ b/arch/arm/boards/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_MACH_BEAGLEBONE)			+= beaglebone/
>  obj-$(CONFIG_MACH_CANON_A1100)			+= canon-a1100/
>  obj-$(CONFIG_MACH_CM_FX6)			+= cm-fx6/
>  obj-$(CONFIG_MACH_NITROGEN6)			+= boundarydevices-nitrogen6/
> +obj-$(CONFIG_MACH_NOVENA)			+= novena/
>  obj-$(CONFIG_MACH_CCMX51)			+= ccxmx51/
>  obj-$(CONFIG_MACH_CCMX53)			+= ccxmx53/
>  obj-$(CONFIG_MACH_CFA10036)			+= crystalfontz-cfa10036/
> diff --git a/arch/arm/boards/novena/Makefile b/arch/arm/boards/novena/Makefile
> new file mode 100644
> index 0000000000..3111392bf9
> --- /dev/null
> +++ b/arch/arm/boards/novena/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +obj-y += board.o
> +lwl-y += lowlevel.o
> diff --git a/arch/arm/boards/novena/board.c b/arch/arm/boards/novena/board.c
> new file mode 100644
> index 0000000000..e41ea10f8d
> --- /dev/null
> +++ b/arch/arm/boards/novena/board.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// SPDX-FileCopyrightText: 2023 John Watts <contact@jookia.org>
> +
> +#include <common.h>
> +#include <deep-probe.h>
> +
> +static int novena_probe(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static const struct of_device_id novena_of_match[] = {
> +	{ .compatible = "kosagi,imx6q-novena", },
> +	{ /* sentinel */ }
> +};
> +
> +static struct driver novena_board_driver = {
> +	.name = "board-novena",
> +	.probe = novena_probe,
> +	.of_compatible = novena_of_match,
> +};
> +coredevice_platform_driver(novena_board_driver);
> +
> +BAREBOX_DEEP_PROBE_ENABLE(novena_of_match);
> diff --git a/arch/arm/boards/novena/flash-header-novena.imxcfg b/arch/arm/boards/novena/flash-header-novena.imxcfg
> new file mode 100644
> index 0000000000..0612542c19
> --- /dev/null
> +++ b/arch/arm/boards/novena/flash-header-novena.imxcfg
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +loadaddr 0x00907000
> +soc imx6
> +max_load_size 0x11000
> +ivtofs 0x400
> diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
> new file mode 100644
> index 0000000000..58b925c311
> --- /dev/null
> +++ b/arch/arm/boards/novena/lowlevel.c
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// SPDX-FileCopyrightText: 2023 John Watts <contact@jookia.org>
> +
> +#include <asm/barebox-arm.h>
> +#include <common.h>
> +#include <debug_ll.h>
> +#include <mach/esdctl.h>
> +#include <mach/generic.h>
> +#include <mach/imx6.h>
> +#include <mach/iomux-mx6.h>
> +#include <mach/xload.h>
> +#include <soc/fsl/fsl_udc.h>
> +
> +#define STACK_TOP (MX6_OCRAM_BASE_ADDR + MX6_OCRAM_MAX_SIZE)
> +
> +extern char __dtb_imx6q_novena_start[];

Can we use compressed device-tree's instead? For this feature you only
need to rename this variable to: __dtb_z_imx6q_novena_start[] and select
'ARM_USE_COMPRESSED_DTB' within the Kconfig.

An example can be found here:
 - arch/arm/boards/zii-imx6q-rdu2/lowlevel.c
 - arch/arm/mach-imx/Kconfig

Sorry for not recognize it the first time.

> +
> +static bool running_from_ram(void)
> +{
> +	return (get_pc() >= MX6_MMDC_PORT01_BASE_ADDR);
> +}
> +
> +static void setup_uart(void)
> +{
> +	/* NOTE: RX is needed for TX to work on this board */
> +	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D26__UART2_RXD);
> +	imx_setup_pad(IOMEM(MX6_IOMUXC_BASE_ADDR), MX6Q_PAD_EIM_D27__UART2_TXD);
> +
> +	imx6_uart_setup(IOMEM(MX6_UART2_BASE_ADDR));
> +	pbl_set_putc(imx_uart_putc, IOMEM(MX6_UART2_BASE_ADDR));
> +
> +	pr_debug(">");
> +}
> +
> +static void load_barebox(void)
> +{
> +	enum bootsource bootsrc;
> +	int bootinstance;
> +
> +	imx6_get_boot_source(&bootsrc, &bootinstance);
> +
> +	if (bootsrc == BOOTSOURCE_SERIAL)
> +		imx6_barebox_start_usb(IOMEM(MX6_MMDC_PORT01_BASE_ADDR));
> +	else if (bootsrc == BOOTSOURCE_MMC)
> +		imx6_esdhc_start_image(bootinstance);
> +
> +	pr_err("Unsupported boot source %i instance %i\n",
> +				bootsrc, bootinstance);

Nit: Please align it correctly.

Regards,
  Marco

> +	hang();
> +}
> +
> +ENTRY_FUNCTION_WITHSTACK(start_imx6q_novena, STACK_TOP, r0, r1, r2)
> +{
> +	imx6_cpu_lowlevel_init();
> +	relocate_to_current_adr();
> +	setup_c();
> +
> +	imx6_ungate_all_peripherals();
> +	setup_uart();
> +
> +	if (!running_from_ram())
> +		load_barebox();
> +	else
> +		imx6q_barebox_entry(__dtb_imx6q_novena_start);
> +}
> diff --git a/arch/arm/configs/imx_v7_defconfig b/arch/arm/configs/imx_v7_defconfig
> index 6110a52e48..f8deca324a 100644
> --- a/arch/arm/configs/imx_v7_defconfig
> +++ b/arch/arm/configs/imx_v7_defconfig
> @@ -25,6 +25,7 @@ CONFIG_MACH_SABRELITE=y
>  CONFIG_MACH_SABRESD=y
>  CONFIG_MACH_FREESCALE_IMX6SX_SABRESDB=y
>  CONFIG_MACH_NITROGEN6=y
> +CONFIG_MACH_NOVENA=y
>  CONFIG_MACH_SOLIDRUN_MICROSOM=y
>  CONFIG_MACH_TECHNEXION_PICO_HOBBIT=y
>  CONFIG_MACH_TECHNEXION_WANDBOARD=y
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 26c4a8ab99..a01ee19f68 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -46,6 +46,7 @@ lwl-$(CONFIG_MACH_MYIRTECH_X335X) += am335x-myirtech-myd.dtb.o
>  lwl-$(CONFIG_MACH_NETGEAR_RN104) += armada-370-rn104-bb.dtb.o
>  lwl-$(CONFIG_MACH_NETGEAR_RN2120) += armada-xp-rn2120-bb.dtb.o
>  lwl-$(CONFIG_MACH_NITROGEN6) += imx6q-nitrogen6x.dtb.o imx6dl-nitrogen6x.dtb.o imx6qp-nitrogen6_max.dtb.o
> +lwl-$(CONFIG_MACH_NOVENA) += imx6q-novena.dtb.o
>  lwl-$(CONFIG_MACH_NVIDIA_BEAVER) += tegra30-beaver.dtb.o
>  lwl-$(CONFIG_MACH_NVIDIA_JETSON) += tegra124-jetson-tk1.dtb.o
>  lwl-$(CONFIG_MACH_PCA100) += imx27-phytec-phycard-s-rdk-bb.dtb.o
> diff --git a/arch/arm/dts/imx6q-novena.dts b/arch/arm/dts/imx6q-novena.dts
> new file mode 100644
> index 0000000000..12bade849c
> --- /dev/null
> +++ b/arch/arm/dts/imx6q-novena.dts
> @@ -0,0 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR X11
> +// SPDX-FileCopyrightText: 2023 John Watts <contact@jookia.org>
> +
> +#include <arm/imx6q-novena.dts>
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 774c4cacb7..7993835be7 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -434,6 +434,12 @@ config MACH_NITROGEN6
>  	bool "BoundaryDevices Nitrogen6 boards"
>  	select ARCH_IMX6
>  
> +config MACH_NOVENA
> +	bool "Kosagi Novena board"
> +	select ARCH_IMX6
> +	select MCI_IMX_ESDHC_PBL
> +	select USB_GADGET_DRIVER_ARC_PBL
> +
>  config MACH_SOLIDRUN_MICROSOM
>  	bool "SolidRun MicroSOM based devices"
>  	select ARCH_IMX6
> diff --git a/images/Makefile.imx b/images/Makefile.imx
> index e9f4ba64e4..65914212bf 100644
> --- a/images/Makefile.imx
> +++ b/images/Makefile.imx
> @@ -258,6 +258,8 @@ $(call build_imx_habv4img, CONFIG_MACH_NITROGEN6, start_imx6dl_nitrogen6x_2g, bo
>  
>  $(call build_imx_habv4img, CONFIG_MACH_NITROGEN6, start_imx6qp_nitrogen6_max, boundarydevices-nitrogen6/flash-header-nitrogen6qp-max, boundarydevices-imx6qp-nitrogen6_max)
>  
> +$(call build_imx_habv4img, CONFIG_MACH_NOVENA, start_imx6q_novena, novena/flash-header-novena, imx6q-novena)
> +
>  $(call build_imx_habv4img, CONFIG_MACH_TX6X, start_imx6dl_tx6x_512m, karo-tx6x/flash-header-tx6dl-512m, karo-imx6dl-tx6x-512m)
>  
>  $(call build_imx_habv4img, CONFIG_MACH_TX6X, start_imx6dl_tx6x_1g, karo-tx6x/flash-header-tx6dl-1g, karo-imx6dl-tx6x-1g)
> -- 
> 2.39.1
> 
> 
> 

Re: [PATCH v3 4/4] ARM: novena: Use DDR3 information from SPD EEPROM

[Marco Felsch]

Hi John,

thanks for your patch.

On 23-01-30, John Watts wrote:
> The Novena supports swappable memory which means we must query the memory
> for configuration details. This patch uses values stored in the SPD EEPROM
> to configure the i.MX6 memory controller.
> 
> If the SPD can't be read, the default configuration for a 4GB stick is used.
> 
> Calibration for the installed stick is run unconditionally.
> 
> Verification that this works was done by comparing MMDC registers before and
> after and carefully reading other people's source code.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  arch/arm/boards/novena/lowlevel.c | 108 ++++++++++++++++++++++++++++++
>  arch/arm/mach-imx/Kconfig         |   2 +
>  2 files changed, 110 insertions(+)
> 
> diff --git a/arch/arm/boards/novena/lowlevel.c b/arch/arm/boards/novena/lowlevel.c
> index 3acf983ee0..54980bda4e 100644
> --- a/arch/arm/boards/novena/lowlevel.c
> +++ b/arch/arm/boards/novena/lowlevel.c
> @@ -3,6 +3,8 @@
>  
>  #include <asm/barebox-arm.h>
>  #include <common.h>
> +#include <ddr_dimms.h>
> +#include <ddr_spd.h>
>  #include <debug_ll.h>
>  #include <mach/esdctl.h>
>  #include <mach/generic.h>
> @@ -10,6 +12,7 @@
>  #include <mach/imx6-mmdc.h>
>  #include <mach/iomux-mx6.h>
>  #include <mach/xload.h>
> +#include <pbl/i2c.h>
>  #include <soc/fsl/fsl_udc.h>
>  #include "ddr_regs.h"
>  
> @@ -17,6 +20,106 @@
>  
>  extern char __dtb_imx6q_novena_start[];
>  
> +static struct spd_eeprom spd_eeprom;
> +static struct dimm_params dimm_params;
> +
> +static struct pbl_i2c *setup_spd_i2c(void)
> +{
> +	void __iomem *iomuxbase = IOMEM(MX6_IOMUXC_BASE_ADDR);
> +
> +	imx_setup_pad(iomuxbase, MX6Q_PAD_CSI0_DAT8__I2C1_SDA);
> +	imx_setup_pad(iomuxbase, MX6Q_PAD_CSI0_DAT9__I2C1_SCL);
> +
> +	return imx6_i2c_early_init(IOMEM(MX6_I2C1_BASE_ADDR));
> +}
> +
> +static struct spd_eeprom *read_spd(void)
> +{
> +	struct spd_eeprom *eeprom = &spd_eeprom;
> +	struct pbl_i2c *i2c = setup_spd_i2c();
> +	int rc;
> +
> +	rc = spd_read_eeprom(i2c, 0x50, eeprom, SPD_MEMTYPE_DDR3);
> +	if (rc < 0) {
> +		pr_err("Couldn't read SPD EEPROM: %i\n", rc);
> +		return NULL;
> +	}
> +
> +	rc = ddr3_spd_check(&eeprom->ddr3);
> +	if (rc < 0) {
> +		pr_err("Couldn't verify SPD data: %i\n", rc);
> +		return NULL;
> +	}
> +
> +	return eeprom;
> +}
> +
> +static bool check_spd_compatible(struct spd_eeprom *eeprom)
> +{
> +	if (eeprom->ddr3.mem_type != 0x0b) {

Is this parameter and value standarized?

> +		pr_err("SPD data is not DDR3\n");
> +		return false;
> +	}
> +
> +	if ((eeprom->ddr3.bus_width & 0x1f) != 0x03) {

Same question as above. Would be nice to avoid magic numbers if this is
the case.

Regards,
  Marco

> +		pr_err("SPD data is for a 64-bit bus\n");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static void setup_dimm_settings(struct dimm_params *params,
> +				struct mx6_ddr_sysinfo *info,
> +				struct mx6_ddr3_cfg *cfg)
> +{
> +	int capacity_gbit = params->capacity / 0x8000000;
> +	int density_rank = capacity_gbit / params->n_ranks;
> +
> +	info->ncs = params->n_ranks;
> +	info->cs_density = density_rank;
> +	cfg->mem_speed = params->tckmin_x_ps;
> +	cfg->density = density_rank / params->n_banks_per_sdram_device;
> +	cfg->width = params->data_width;
> +	cfg->banks = params->n_banks_per_sdram_device;
> +	cfg->rowaddr = params->n_row_addr;
> +	cfg->coladdr = params->n_col_addr;
> +	cfg->trcd = params->trcd_ps / 10;
> +	cfg->trcmin = params->trc_ps / 10;
> +	cfg->trasmin = params->tras_ps / 10;
> +	cfg->SRT = params->extended_op_srt;
> +
> +	if (params->device_width >= 16)
> +		cfg->pagesz = 2;
> +}
> +
> +static void read_dimm_settings(void)
> +{
> +	struct spd_eeprom *eeprom = read_spd();
> +	struct dimm_params *params = &dimm_params;
> +	int rc;
> +
> +	if (!eeprom) {
> +		pr_err("Couldn't read SPD EEPROM, using default settings\n");
> +		return;
> +	}
> +
> +	if (!check_spd_compatible(eeprom)) {
> +		pr_err("DIMM stick incompatible\n");
> +		hang();
> +	}
> +
> +	rc = ddr3_compute_dimm_parameters(&eeprom->ddr3, params);
> +	if (rc < 0) {
> +		pr_err("Couldn't compute DIMM params: %i\n", rc);
> +		return;
> +	}
> +
> +	setup_dimm_settings(params, &novena_ddr_info, &novena_ddr_cfg);
> +
> +	pr_info("Found DIMM: %s\n", params->mpart);
> +}
> +
>  static bool running_from_ram(void)
>  {
>  	return (get_pc() >= MX6_MMDC_PORT01_BASE_ADDR);
> @@ -36,8 +139,13 @@ static void setup_uart(void)
>  
>  static void setup_ram(void)
>  {
> +	read_dimm_settings();
> +
>  	mx6dq_dram_iocfg(64, &novena_ddr_regs, &novena_grp_regs);
>  	mx6_dram_cfg(&novena_ddr_info, &novena_mmdc_calib, &novena_ddr_cfg);
> +
> +	mmdc_do_write_level_calibration();
> +	mmdc_do_dqs_calibration();
>  }
>  
>  static void load_barebox(void)
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 7993835be7..aa7a86da66 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -437,6 +437,8 @@ config MACH_NITROGEN6
>  config MACH_NOVENA
>  	bool "Kosagi Novena board"
>  	select ARCH_IMX6
> +	select DDR_SPD
> +	select I2C_IMX_EARLY
>  	select MCI_IMX_ESDHC_PBL
>  	select USB_GADGET_DRIVER_ARC_PBL
>  
> -- 
> 2.39.1
> 
> 
> 

Re: [PATCH v3 3/4] ARM: novena: Read Ethernet MAC address from EEPROM

[John Watts]

On Mon, Jan 30, 2023 at 09:04:43PM +0100, Marco Felsch wrote:
> Hi John,
> 
> thanks for the patch.

Thanks for the review as always :)

> This will trigger a double free since you're use of an unconditional
> free() during probe().

It returns NULL, so probe will do a no-op.

> 
> Regards,
>   Marco

John.

Re: [PATCH v3 1/4] ARM: novena: Add Kosagi Novena board

[John Watts]

On Mon, Jan 30, 2023 at 09:13:08PM +0100, Marco Felsch wrote:
> > +#define STACK_TOP (MX6_OCRAM_BASE_ADDR + MX6_OCRAM_MAX_SIZE)
> > +
> > +extern char __dtb_imx6q_novena_start[];
> 
> Can we use compressed device-tree's instead? For this feature you only
> need to rename this variable to: __dtb_z_imx6q_novena_start[] and select
> 'ARM_USE_COMPRESSED_DTB' within the Kconfig.
> 
> An example can be found here:
>  - arch/arm/boards/zii-imx6q-rdu2/lowlevel.c
>  - arch/arm/mach-imx/Kconfig
> 
> Sorry for not recognize it the first time.

Sure!

> > +	pr_err("Unsupported boot source %i instance %i\n",
> > +				bootsrc, bootinstance);
> 
> Nit: Please align it correctly.

Just a single tab?

> Regards,
>   Marco

John.

Re: [PATCH v3 4/4] ARM: novena: Use DDR3 information from SPD EEPROM

[John Watts]

On Mon, Jan 30, 2023 at 09:18:24PM +0100, Marco Felsch wrote:
> > +	if (eeprom->ddr3.mem_type != 0x0b) {
> 
> Is this parameter and value standarized?
> 
> > +		pr_err("SPD data is not DDR3\n");
> > +		return false;
> > +	}
> > +
> > +	if ((eeprom->ddr3.bus_width & 0x1f) != 0x03) {
> 
> Same question as above. Would be nice to avoid magic numbers if this is
> the case.

I believe it's standard. I could add some comments but the standard itself
isn't publicly available unless you can find it re-uploaded online.

> 
> Regards,
>   Marco

John.

Re: [PATCH v3 3/4] ARM: novena: Read Ethernet MAC address from EEPROM

[Marco Felsch]

On 23-01-31, John Watts wrote:
> On Mon, Jan 30, 2023 at 09:04:43PM +0100, Marco Felsch wrote:
> > Hi John,
> > 
> > thanks for the patch.
> 
> Thanks for the review as always :)
> 
> > This will trigger a double free since you're use of an unconditional
> > free() during probe().
> 
> It returns NULL, so probe will do a no-op.

Arg.. you're right. I shouldn't review something that late ^^

Regards,
  Marco

> > Regards,
> >   Marco
> 
> John.
> 

Re: [PATCH v3 1/4] ARM: novena: Add Kosagi Novena board

[Marco Felsch]

On 23-01-31, John Watts wrote:
> On Mon, Jan 30, 2023 at 09:13:08PM +0100, Marco Felsch wrote:
> > > +#define STACK_TOP (MX6_OCRAM_BASE_ADDR + MX6_OCRAM_MAX_SIZE)
> > > +
> > > +extern char __dtb_imx6q_novena_start[];
> > 
> > Can we use compressed device-tree's instead? For this feature you only
> > need to rename this variable to: __dtb_z_imx6q_novena_start[] and select
> > 'ARM_USE_COMPRESSED_DTB' within the Kconfig.
> > 
> > An example can be found here:
> >  - arch/arm/boards/zii-imx6q-rdu2/lowlevel.c
> >  - arch/arm/mach-imx/Kconfig
> > 
> > Sorry for not recognize it the first time.
> 
> Sure!
> 
> > > +	pr_err("Unsupported boot source %i instance %i\n",
> > > +				bootsrc, bootinstance);
> > 
> > Nit: Please align it correctly.
> 
> Just a single tab?

Alligend with the opening parenthesis so like this:

| pr_err("Unsupported boot source %i instance %i\n",
|        bootsrc, bootinstance);

Regards,
  Marco