[U-Boot] [PATCH v1 00/14] dm: Convert TPC70 to use DM and DTS in SPL and u-boot proper

This patch series converts imx6q based tpc70 board to use driver model and device tree description in SPL and u-boot proper.
All the non-DM parts of the code has been removed.
This patch series also has several early DM issues fixed for imx6q - for example the pinctrl static definitions.
Also, the anatop clock is now setup in a generic imx6q clock code.
Lukasz Majewski (14): tpc70: config: Add script commands to update u-boot and OE's wic tpc70: config: Update TPC70 config to support eMMC's boot0 SPL update tpc70: Provide board_boot_order() function to distinct between eMMC and SD boot DTS: imx: Remove not needed '#address-cells' and '#size-cells' properties board: cosmetic: Use define to set ENET clock selection mask on TPC70 DM: tpc70: led: Enable LED default state pinctrl: imx: Replace static soc info definitions with run time allocations DTS: imx: Add "u-boot,dm-pre-reloc" property to relevant imx6qdl nodes imx: serial: dm: Enable DM_FLAG_PRE_RELOC in the IMX uart driver imx: clock: Introduce set_fec_clock() to configure ETH clock (imx6) DM: net: imx: Provide weak function to initialize fec clocks imx: mmc: Use 'fsl,usdhc-index' property to provide esdhc controller number DTS: imx: tpc70: Add TPC70 board (imx6q based) device tree description imx: tpc70: Convert TPC70 (imx6q based) board to use DM/DTS in SPL and u-boot
arch/arm/dts/imx6q-kp.dts | 227 ++++++++++++++++++++++++++++++ arch/arm/dts/imx6q.dtsi | 4 - arch/arm/dts/imx6qdl.dtsi | 14 +- arch/arm/include/asm/arch-mx6/clock.h | 1 + arch/arm/mach-imx/mx6/Kconfig | 10 ++ arch/arm/mach-imx/mx6/clock.c | 17 +++ board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c | 177 +---------------------- board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c | 137 ++---------------- configs/kp_imx6q_tpc_defconfig | 31 +++- drivers/mmc/fsl_esdhc.c | 17 ++- drivers/net/fec_mxc.c | 25 ++++ drivers/net/fec_mxc.h | 2 + drivers/pinctrl/nxp/pinctrl-imx6.c | 39 +++-- drivers/serial/serial_mxc.c | 2 - include/configs/kp_imx6q_tpc.h | 47 ++++--- 15 files changed, 393 insertions(+), 357 deletions(-) create mode 100644 arch/arm/dts/imx6q-kp.dts

Signed-off-by: Lukasz Majewski lukma@denx.de ---
include/configs/kp_imx6q_tpc.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/include/configs/kp_imx6q_tpc.h b/include/configs/kp_imx6q_tpc.h index b6b27ee1d5..ee9c56bc21 100644 --- a/include/configs/kp_imx6q_tpc.h +++ b/include/configs/kp_imx6q_tpc.h @@ -89,11 +89,32 @@ "rdinit=/sbin/init\0" \ "addinitrd=setenv bootargs ${bootargs} rdinit=${rdinit} ${debug} \0" \ "fit_config=mx6q_tpc70_conf\0" \ + "uboot_file=u-boot.img\0" \ + "SPL_file=SPL\0" \ + "wic_file=kp-image-kpimx6qtpc.wic\0" \ "upd_image=st.4k\0" \ "updargs=setenv bootargs console=${console} ${smp}"\ "rdinit=${rdinit} ${debug} ${displayargs}\0" \ "loadusb=usb start; " \ "fatload usb 0 ${loadaddr} ${upd_image}\0" \ + "upd_uboot_sd=" \ + "if tftp ${loadaddr} ${uboot_file}; then " \ + "setexpr blkc ${filesize} / 0x200;" \ + "setexpr blkc ${blkc} + 1;" \ + "mmc write ${loadaddr} 0x8A ${blkc};" \ + "fi;\0" \ + "upd_SPL_sd=" \ + "if tftp ${loadaddr} ${SPL_file}; then " \ + "setexpr blkc ${filesize} / 0x200;" \ + "setexpr blkc ${blkc} + 1;" \ + "mmc write ${loadaddr} 0x2 ${blkc};" \ + "fi;\0" \ + "upd_wic=" \ + "if tftp ${loadaddr} ${wic_file}; then " \ + "setexpr blkc ${filesize} / 0x200;" \ + "setexpr blkc ${blkc} + 1;" \ + "mmc write ${loadaddr} 0x0 ${blkc};" \ + "fi;\0" \ "usbupd=echo Booting update from usb ...; " \ "setenv bootargs; " \ "run updargs; " \

On 1/2/19 12:37 AM, Lukasz Majewski wrote:
Signed-off-by: Lukasz Majewski lukma@denx.de
The tags should be ARM: imx: ...
The commit message is missing.
include/configs/kp_imx6q_tpc.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/include/configs/kp_imx6q_tpc.h b/include/configs/kp_imx6q_tpc.h index b6b27ee1d5..ee9c56bc21 100644 --- a/include/configs/kp_imx6q_tpc.h +++ b/include/configs/kp_imx6q_tpc.h @@ -89,11 +89,32 @@ "rdinit=/sbin/init\0" \ "addinitrd=setenv bootargs ${bootargs} rdinit=${rdinit} ${debug} \0" \ "fit_config=mx6q_tpc70_conf\0" \
- "uboot_file=u-boot.img\0" \
- "SPL_file=SPL\0" \
- "wic_file=kp-image-kpimx6qtpc.wic\0" \
Why don't you just generate a fitImage and use the fitupd for this ?
"upd_image=st.4k\0" \ "updargs=setenv bootargs console=${console} ${smp}"\ "rdinit=${rdinit} ${debug} ${displayargs}\0" \ "loadusb=usb start; " \ "fatload usb 0 ${loadaddr} ${upd_image}\0" \
- "upd_uboot_sd=" \
"if tftp ${loadaddr} ${uboot_file}; then " \
"setexpr blkc ${filesize} / 0x200;" \
"setexpr blkc ${blkc} + 1;" \
"mmc write ${loadaddr} 0x8A ${blkc};" \
"fi;\0" \
- "upd_SPL_sd=" \
"if tftp ${loadaddr} ${SPL_file}; then " \
"setexpr blkc ${filesize} / 0x200;" \
"setexpr blkc ${blkc} + 1;" \
"mmc write ${loadaddr} 0x2 ${blkc};" \
"fi;\0" \
- "upd_wic=" \
"if tftp ${loadaddr} ${wic_file}; then " \
"setexpr blkc ${filesize} / 0x200;" \
"setexpr blkc ${blkc} + 1;" \
"mmc write ${loadaddr} 0x0 ${blkc};" \
"usbupd=echo Booting update from usb ...; " \ "setenv bootargs; " \ "run updargs; " \"fi;\0" \

Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
Signed-off-by: Lukasz Majewski lukma@denx.de
The tags should be ARM: imx: ...
The commit message is missing.
include/configs/kp_imx6q_tpc.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/include/configs/kp_imx6q_tpc.h b/include/configs/kp_imx6q_tpc.h index b6b27ee1d5..ee9c56bc21 100644 --- a/include/configs/kp_imx6q_tpc.h +++ b/include/configs/kp_imx6q_tpc.h @@ -89,11 +89,32 @@ "rdinit=/sbin/init\0" \ "addinitrd=setenv bootargs ${bootargs} rdinit=${rdinit} ${debug} \0" \ "fit_config=mx6q_tpc70_conf\0" \
- "uboot_file=u-boot.img\0" \
- "SPL_file=SPL\0" \
- "wic_file=kp-image-kpimx6qtpc.wic\0" \
Why don't you just generate a fitImage and use the fitupd for this ?
This is one of possible solutions - yes.
"upd_image=st.4k\0" \ "updargs=setenv bootargs console=${console} ${smp}"\ "rdinit=${rdinit} ${debug} ${displayargs}\0" \ "loadusb=usb start; " \ "fatload usb 0 ${loadaddr} ${upd_image}\0" \
- "upd_uboot_sd=" \
"if tftp ${loadaddr} ${uboot_file}; then " \
"setexpr blkc ${filesize} / 0x200;" \
"setexpr blkc ${blkc} + 1;" \
"mmc write ${loadaddr} 0x8A ${blkc};" \
"fi;\0" \
- "upd_SPL_sd=" \
"if tftp ${loadaddr} ${SPL_file}; then " \
"setexpr blkc ${filesize} / 0x200;" \
"setexpr blkc ${blkc} + 1;" \
"mmc write ${loadaddr} 0x2 ${blkc};" \
"fi;\0" \
- "upd_wic=" \
"if tftp ${loadaddr} ${wic_file}; then " \
"setexpr blkc ${filesize} / 0x200;" \
"setexpr blkc ${blkc} + 1;" \
"mmc write ${loadaddr} 0x0 ${blkc};" \
"usbupd=echo Booting update from usb ...; " \ "setenv bootargs; " \ "run updargs; " \"fi;\0" \
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On 1/2/19 10:50 AM, Lukasz Majewski wrote:
Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
Signed-off-by: Lukasz Majewski lukma@denx.de
The tags should be ARM: imx: ...
The commit message is missing.
include/configs/kp_imx6q_tpc.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/include/configs/kp_imx6q_tpc.h b/include/configs/kp_imx6q_tpc.h index b6b27ee1d5..ee9c56bc21 100644 --- a/include/configs/kp_imx6q_tpc.h +++ b/include/configs/kp_imx6q_tpc.h @@ -89,11 +89,32 @@ "rdinit=/sbin/init\0" \ "addinitrd=setenv bootargs ${bootargs} rdinit=${rdinit} ${debug} \0" \ "fit_config=mx6q_tpc70_conf\0" \
- "uboot_file=u-boot.img\0" \
- "SPL_file=SPL\0" \
- "wic_file=kp-image-kpimx6qtpc.wic\0" \
Why don't you just generate a fitImage and use the fitupd for this ?
This is one of possible solutions - yes.
It's the only solution if you want to prevent SPL and U-Boot from being out of sync.

The TPC70 can boot from eMMC's boot0. This patch allows it to update this HW partition's SPL.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
include/configs/kp_imx6q_tpc.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/configs/kp_imx6q_tpc.h b/include/configs/kp_imx6q_tpc.h index ee9c56bc21..f26b18442b 100644 --- a/include/configs/kp_imx6q_tpc.h +++ b/include/configs/kp_imx6q_tpc.h @@ -49,6 +49,7 @@ #define CONFIG_SYS_FSL_ESDHC_ADDR 0 #define CONFIG_SYS_FSL_USDHC_NUM 2 #define CONFIG_SYS_MMC_ENV_DEV 1 /* 0 = SDHC2, 1 = SDHC4 (eMMC) */ +#define CONFIG_SUPPORT_EMMC_BOOT
/* UART */ #define CONFIG_MXC_UART @@ -109,6 +110,10 @@ "setexpr blkc ${blkc} + 1;" \ "mmc write ${loadaddr} 0x2 ${blkc};" \ "fi;\0" \ + "upd_SPL_mmc=mmc dev 1; mmc partconf 1 0 1 1; run upd_SPL_sd\0" \ + "upd_uboot_mmc=mmc dev 1; mmc partconf 1 0 1 1; run upd_uboot_sd\0" \ + "up_mmc=run upd_SPL_mmc; run upd_uboot_mmc\0" \ + "up_sd=run upd_SPL_sd; run upd_uboot_sd\0" \ "upd_wic=" \ "if tftp ${loadaddr} ${wic_file}; then " \ "setexpr blkc ${filesize} / 0x200;" \

On 1/2/19 12:37 AM, Lukasz Majewski wrote:
The TPC70 can boot from eMMC's boot0. This patch allows it to update this HW partition's SPL.
Signed-off-by: Lukasz Majewski lukma@denx.de
include/configs/kp_imx6q_tpc.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/configs/kp_imx6q_tpc.h b/include/configs/kp_imx6q_tpc.h index ee9c56bc21..f26b18442b 100644 --- a/include/configs/kp_imx6q_tpc.h +++ b/include/configs/kp_imx6q_tpc.h @@ -49,6 +49,7 @@ #define CONFIG_SYS_FSL_ESDHC_ADDR 0 #define CONFIG_SYS_FSL_USDHC_NUM 2 #define CONFIG_SYS_MMC_ENV_DEV 1 /* 0 = SDHC2, 1 = SDHC4 (eMMC) */ +#define CONFIG_SUPPORT_EMMC_BOOT
/* UART */ #define CONFIG_MXC_UART @@ -109,6 +110,10 @@ "setexpr blkc ${blkc} + 1;" \ "mmc write ${loadaddr} 0x2 ${blkc};" \ "fi;\0" \
- "upd_SPL_mmc=mmc dev 1; mmc partconf 1 0 1 1; run upd_SPL_sd\0" \
If mmc dev 1 fails, this will randomly rewrite or even damage some SD/MMC card that was selected before. Use && ...
- "upd_uboot_mmc=mmc dev 1; mmc partconf 1 0 1 1; run upd_uboot_sd\0" \
Deduplicate these repeated commands.
- "up_mmc=run upd_SPL_mmc; run upd_uboot_mmc\0" \
- "up_sd=run upd_SPL_sd; run upd_uboot_sd\0" \ "upd_wic=" \ "if tftp ${loadaddr} ${wic_file}; then " \ "setexpr blkc ${filesize} / 0x200;" \

Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
The TPC70 can boot from eMMC's boot0. This patch allows it to update this HW partition's SPL.
Signed-off-by: Lukasz Majewski lukma@denx.de
include/configs/kp_imx6q_tpc.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/configs/kp_imx6q_tpc.h b/include/configs/kp_imx6q_tpc.h index ee9c56bc21..f26b18442b 100644 --- a/include/configs/kp_imx6q_tpc.h +++ b/include/configs/kp_imx6q_tpc.h @@ -49,6 +49,7 @@ #define CONFIG_SYS_FSL_ESDHC_ADDR 0 #define CONFIG_SYS_FSL_USDHC_NUM 2 #define CONFIG_SYS_MMC_ENV_DEV 1 /* 0 = SDHC2, 1 = SDHC4 (eMMC) */ +#define CONFIG_SUPPORT_EMMC_BOOT
/* UART */ #define CONFIG_MXC_UART @@ -109,6 +110,10 @@ "setexpr blkc ${blkc} + 1;" \ "mmc write ${loadaddr} 0x2 ${blkc};" \ "fi;\0" \
- "upd_SPL_mmc=mmc dev 1; mmc partconf 1 0 1 1; run
upd_SPL_sd\0" \
If mmc dev 1 fails, this will randomly rewrite or even damage some SD/MMC card that was selected before . Use && ...
Ok - good point.
- "upd_uboot_mmc=mmc dev 1; mmc partconf 1 0 1 1; run
upd_uboot_sd\0" \
Deduplicate these repeated commands.
Could you be more specific here?
Without mmc dev 1 and partconf I cannot access boot0 eMMC area. This particular board has the SD as mmc0 (rescue/devel) and eMMC as mmc1 (and eMMC is a production boot medium).
- "up_mmc=run upd_SPL_mmc; run upd_uboot_mmc\0" \
- "up_sd=run upd_SPL_sd; run upd_uboot_sd\0" \ "upd_wic=" \ "if tftp ${loadaddr} ${wic_file}; then " \ "setexpr blkc ${filesize} / 0x200;" \
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On 1/2/19 10:47 AM, Lukasz Majewski wrote:
Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
The TPC70 can boot from eMMC's boot0. This patch allows it to update this HW partition's SPL.
Signed-off-by: Lukasz Majewski lukma@denx.de
include/configs/kp_imx6q_tpc.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/configs/kp_imx6q_tpc.h b/include/configs/kp_imx6q_tpc.h index ee9c56bc21..f26b18442b 100644 --- a/include/configs/kp_imx6q_tpc.h +++ b/include/configs/kp_imx6q_tpc.h @@ -49,6 +49,7 @@ #define CONFIG_SYS_FSL_ESDHC_ADDR 0 #define CONFIG_SYS_FSL_USDHC_NUM 2 #define CONFIG_SYS_MMC_ENV_DEV 1 /* 0 = SDHC2, 1 = SDHC4 (eMMC) */ +#define CONFIG_SUPPORT_EMMC_BOOT
/* UART */ #define CONFIG_MXC_UART @@ -109,6 +110,10 @@ "setexpr blkc ${blkc} + 1;" \ "mmc write ${loadaddr} 0x2 ${blkc};" \ "fi;\0" \
- "upd_SPL_mmc=mmc dev 1; mmc partconf 1 0 1 1; run
upd_SPL_sd\0" \
If mmc dev 1 fails, this will randomly rewrite or even damage some SD/MMC card that was selected before . Use && ...
Ok - good point.
- "upd_uboot_mmc=mmc dev 1; mmc partconf 1 0 1 1; run
upd_uboot_sd\0" \
Deduplicate these repeated commands.
Could you be more specific here?
Without mmc dev 1 and partconf I cannot access boot0 eMMC area. This particular board has the SD as mmc0 (rescue/devel) and eMMC as mmc1 (and eMMC is a production boot medium).
The same commands are called from multiple scripts.
- "up_mmc=run upd_SPL_mmc; run upd_uboot_mmc\0" \
- "up_sd=run upd_SPL_sd; run upd_uboot_sd\0" \ "upd_wic=" \ "if tftp ${loadaddr} ${wic_file}; then " \ "setexpr blkc ${filesize} / 0x200;" \
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

The TPC70 can boot from SD card (debug/development) and eMMC (production). The board_boot_order() function provides a run time check for the device from which one wants to boot (it is selected by GPIO pins setup).
Moreover, a fallback to SD card is provided if the detection is not possible or working properly.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c index d89e1120a5..07414431f3 100644 --- a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c +++ b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c @@ -308,6 +308,26 @@ int board_mmc_init(bd_t *bd) return fsl_esdhc_initialize(bd, &usdhc_cfg[0]); }
+void board_boot_order(u32 *spl_boot_list) +{ + u32 boot_device = spl_boot_device(); + u32 reg = imx6_src_get_boot_mode(); + + reg = (reg & IMX6_BMODE_MASK) >> IMX6_BMODE_SHIFT; + + debug("%s: boot device: 0x%x (0x4 SD, 0x6 eMMC)\n", __func__, reg); + if (boot_device == BOOT_DEVICE_MMC1) + if (reg == IMX6_BMODE_MMC || reg == IMX6_BMODE_EMMC) + boot_device = BOOT_DEVICE_MMC2; + + spl_boot_list[0] = boot_device; + /* + * Below boot device is a 'fallback' - it shall always be possible to + * boot from SD card + */ + spl_boot_list[1] = BOOT_DEVICE_MMC1; +} + void board_init_f(ulong dummy) { /* setup AIPS and disable watchdog */

On Tue, 1 Jan 2019 at 16:38, Lukasz Majewski lukma@denx.de wrote:
The TPC70 can boot from SD card (debug/development) and eMMC (production). The board_boot_order() function provides a run time check for the device from which one wants to boot (it is selected by GPIO pins setup).
Moreover, a fallback to SD card is provided if the detection is not possible or working properly.
Signed-off-by: Lukasz Majewski lukma@denx.de
board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

This commit fixes warnings produced by newest in u-boot DTC compiler: 'unnecessary #address-cells/#size-cells without "ranges" or child "reg" property'
Signed-off-by: Lukasz Majewski lukma@denx.de ---
arch/arm/dts/imx6q.dtsi | 4 ---- arch/arm/dts/imx6qdl.dtsi | 9 --------- 2 files changed, 13 deletions(-)
diff --git a/arch/arm/dts/imx6q.dtsi b/arch/arm/dts/imx6q.dtsi index c30c8368ca..7f5d91d7f0 100644 --- a/arch/arm/dts/imx6q.dtsi +++ b/arch/arm/dts/imx6q.dtsi @@ -150,8 +150,6 @@ };
ipu2_di0: port@2 { - #address-cells = <1>; - #size-cells = <0>; reg = <2>;
ipu2_di0_disp0: disp0-endpoint { @@ -175,8 +173,6 @@ };
ipu2_di1: port@3 { - #address-cells = <1>; - #size-cells = <0>; reg = <3>;
ipu2_di1_hdmi: hdmi-endpoint { diff --git a/arch/arm/dts/imx6qdl.dtsi b/arch/arm/dts/imx6qdl.dtsi index b13b0b2db8..476cf93445 100644 --- a/arch/arm/dts/imx6qdl.dtsi +++ b/arch/arm/dts/imx6qdl.dtsi @@ -49,9 +49,6 @@ };
clocks { - #address-cells = <1>; - #size-cells = <0>; - ckil { compatible = "fsl,imx-ckil", "fixed-clock"; #clock-cells = <0>; @@ -1125,8 +1122,6 @@ };
mipi_dsi: mipi@021e0000 { - #address-cells = <1>; - #size-cells = <0>; reg = <0x021e0000 0x4000>; status = "disabled";
@@ -1228,8 +1223,6 @@ };
ipu1_di0: port@2 { - #address-cells = <1>; - #size-cells = <0>; reg = <2>;
ipu1_di0_disp0: disp0-endpoint { @@ -1253,8 +1246,6 @@ };
ipu1_di1: port@3 { - #address-cells = <1>; - #size-cells = <0>; reg = <3>;
ipu1_di1_disp1: disp1-endpoint {

On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This commit fixes warnings produced by newest in u-boot DTC compiler: 'unnecessary #address-cells/#size-cells without "ranges" or child "reg" property'
Signed-off-by: Lukasz Majewski lukma@denx.de
I presume the DTSIs are taken from Linux, is this a backport from Linux then ?
arch/arm/dts/imx6q.dtsi | 4 ---- arch/arm/dts/imx6qdl.dtsi | 9 --------- 2 files changed, 13 deletions(-)
diff --git a/arch/arm/dts/imx6q.dtsi b/arch/arm/dts/imx6q.dtsi index c30c8368ca..7f5d91d7f0 100644 --- a/arch/arm/dts/imx6q.dtsi +++ b/arch/arm/dts/imx6q.dtsi @@ -150,8 +150,6 @@ };
ipu2_di0: port@2 {
#address-cells = <1>;
#size-cells = <0>; reg = <2>; ipu2_di0_disp0: disp0-endpoint {
@@ -175,8 +173,6 @@ };
ipu2_di1: port@3 {
#address-cells = <1>;
#size-cells = <0>; reg = <3>; ipu2_di1_hdmi: hdmi-endpoint {
diff --git a/arch/arm/dts/imx6qdl.dtsi b/arch/arm/dts/imx6qdl.dtsi index b13b0b2db8..476cf93445 100644 --- a/arch/arm/dts/imx6qdl.dtsi +++ b/arch/arm/dts/imx6qdl.dtsi @@ -49,9 +49,6 @@ };
clocks {
#address-cells = <1>;
#size-cells = <0>;
- ckil { compatible = "fsl,imx-ckil", "fixed-clock"; #clock-cells = <0>;
@@ -1125,8 +1122,6 @@ };
mipi_dsi: mipi@021e0000 {
#address-cells = <1>;
#size-cells = <0>; reg = <0x021e0000 0x4000>; status = "disabled";
@@ -1228,8 +1223,6 @@ };
ipu1_di0: port@2 {
#address-cells = <1>;
#size-cells = <0>; reg = <2>; ipu1_di0_disp0: disp0-endpoint {
@@ -1253,8 +1246,6 @@ };
ipu1_di1: port@3 {
#address-cells = <1>;
#size-cells = <0>; reg = <3>; ipu1_di1_disp1: disp1-endpoint {

Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This commit fixes warnings produced by newest in u-boot DTC compiler: 'unnecessary #address-cells/#size-cells without "ranges" or child "reg" property'
Signed-off-by: Lukasz Majewski lukma@denx.de
I presume the DTSIs are taken from Linux, is this a backport from Linux then ?
Regarding Linux - with Version: DTC 1.4.7-gc86da84d and make W=1 I do not see those warnings as in scripts/Makefile.lib :
# Disable noisy checks by default ifeq ($(findstring 1,$(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) DTC_FLAGS += -Wno-unit_address_vs_reg \ -Wno-unit_address_format \ -Wno-avoid_unnecessary_addr_size \
those warnings are disabled (-Wno-avoid_unnecessary_addr_size).
On the other hand - in u-boot those warnings are checked (maybe are necessary for OF_PLATDATA correct operation?).
arch/arm/dts/imx6q.dtsi | 4 ---- arch/arm/dts/imx6qdl.dtsi | 9 --------- 2 files changed, 13 deletions(-)
diff --git a/arch/arm/dts/imx6q.dtsi b/arch/arm/dts/imx6q.dtsi index c30c8368ca..7f5d91d7f0 100644 --- a/arch/arm/dts/imx6q.dtsi +++ b/arch/arm/dts/imx6q.dtsi @@ -150,8 +150,6 @@ };
ipu2_di0: port@2 {
#address-cells = <1>;
#size-cells = <0>; reg = <2>; ipu2_di0_disp0: disp0-endpoint {
@@ -175,8 +173,6 @@ };
ipu2_di1: port@3 {
#address-cells = <1>;
#size-cells = <0>; reg = <3>; ipu2_di1_hdmi: hdmi-endpoint {
diff --git a/arch/arm/dts/imx6qdl.dtsi b/arch/arm/dts/imx6qdl.dtsi index b13b0b2db8..476cf93445 100644 --- a/arch/arm/dts/imx6qdl.dtsi +++ b/arch/arm/dts/imx6qdl.dtsi @@ -49,9 +49,6 @@ };
clocks {
#address-cells = <1>;
#size-cells = <0>;
- ckil { compatible = "fsl,imx-ckil", "fixed-clock"; #clock-cells = <0>;
@@ -1125,8 +1122,6 @@ };
mipi_dsi: mipi@021e0000 {
#address-cells = <1>;
#size-cells = <0>; reg = <0x021e0000 0x4000>; status = "disabled";
@@ -1228,8 +1223,6 @@ };
ipu1_di0: port@2 {
#address-cells = <1>;
#size-cells = <0>; reg = <2>; ipu1_di0_disp0: disp0-endpoint {
@@ -1253,8 +1246,6 @@ };
ipu1_di1: port@3 {
#address-cells = <1>;
#size-cells = <0>; reg = <3>; ipu1_di1_disp1: disp1-endpoint {
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On 1/2/19 10:43 AM, Lukasz Majewski wrote:
Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This commit fixes warnings produced by newest in u-boot DTC compiler: 'unnecessary #address-cells/#size-cells without "ranges" or child "reg" property'
Signed-off-by: Lukasz Majewski lukma@denx.de
I presume the DTSIs are taken from Linux, is this a backport from Linux then ?
Regarding Linux - with Version: DTC 1.4.7-gc86da84d and make W=1 I do not see those warnings as in scripts/Makefile.lib :
# Disable noisy checks by default ifeq ($(findstring 1,$(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) DTC_FLAGS += -Wno-unit_address_vs_reg \ -Wno-unit_address_format \ -Wno-avoid_unnecessary_addr_size \
those warnings are disabled (-Wno-avoid_unnecessary_addr_size).
On the other hand - in u-boot those warnings are checked (maybe are necessary for OF_PLATDATA correct operation?).
No, keep the DTs synced and fix things upstream (in Linux) if needed, then backport the fixes.

Hi Marek,
On 1/2/19 10:43 AM, Lukasz Majewski wrote:
Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This commit fixes warnings produced by newest in u-boot DTC compiler: 'unnecessary #address-cells/#size-cells without "ranges" or child "reg" property'
Signed-off-by: Lukasz Majewski lukma@denx.de
I presume the DTSIs are taken from Linux, is this a backport from Linux then ?
Regarding Linux - with Version: DTC 1.4.7-gc86da84d and make W=1 I do not see those warnings as in scripts/Makefile.lib :
# Disable noisy checks by default ifeq ($(findstring 1,$(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) DTC_FLAGS += -Wno-unit_address_vs_reg \ -Wno-unit_address_format \ -Wno-avoid_unnecessary_addr_size \
those warnings are disabled (-Wno-avoid_unnecessary_addr_size).
On the other hand - in u-boot those warnings are checked (maybe are necessary for OF_PLATDATA correct operation?).
No, keep the DTs synced and fix things upstream (in Linux) if needed, then backport the fixes.
Please correct my understanding if I get this wrong.
Do you recommend to send this patch to Linux upstream to fix warnings, which are disabled (no checked and not present) by default in Linux ?
After this patch is accepted in Linux - I shall backport it to u-boot ?
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

This is a cosmetic change, just to use proper define.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c index ace986fa05..bcc22b1aa8 100644 --- a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c +++ b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c @@ -118,7 +118,8 @@ static int setup_fec_clock(void) struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR;
/* set gpr1[21] to select anatop clock */ - clrsetbits_le32(&iomuxc_regs->gpr[1], 0x1 << 21, 0x1 << 21); + clrsetbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_ENET_CLK_SEL_MASK, + IOMUXC_GPR1_ENET_CLK_SEL_MASK);
return enable_fec_anatop_clock(0, ENET_50MHZ); }

On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This is a cosmetic change, just to use proper define.
Signed-off-by: Lukasz Majewski lukma@denx.de
The subject tags are wrong, fix globally.
board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c index ace986fa05..bcc22b1aa8 100644 --- a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c +++ b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c @@ -118,7 +118,8 @@ static int setup_fec_clock(void) struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR;
/* set gpr1[21] to select anatop clock */
- clrsetbits_le32(&iomuxc_regs->gpr[1], 0x1 << 21, 0x1 << 21);
clrsetbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_ENET_CLK_SEL_MASK,
IOMUXC_GPR1_ENET_CLK_SEL_MASK);
return enable_fec_anatop_clock(0, ENET_50MHZ);
}

Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This is a cosmetic change, just to use proper define.
Signed-off-by: Lukasz Majewski lukma@denx.de
The subject tags are wrong, fix globally.
Is there any doc which describes the allowed/not allowed tags (either with Linux or U-boot)?
From my experience it is a some kind of a convention depending on the maintainer (which helps him to spot relevant patches on ML).
In [1] - point 14) there is: The canonical patch subject line is: "Subject: [PATCH 001/123] subsystem: summary phrase"
and the "subsystem: " is the tag.
In the case of this patch -> I do fix stuff in "board(s)" which is "cosmetic".
Or will we only stick to tags/mails from ./doc/git-mailrc ?
[1] - https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c index ace986fa05..bcc22b1aa8 100644 --- a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c +++ b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c @@ -118,7 +118,8 @@ static int setup_fec_clock(void) struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR; /* set gpr1[21] to select anatop clock */
- clrsetbits_le32(&iomuxc_regs->gpr[1], 0x1 << 21, 0x1 <<
21);
- clrsetbits_le32(&iomuxc_regs->gpr[1],
IOMUXC_GPR1_ENET_CLK_SEL_MASK,
IOMUXC_GPR1_ENET_CLK_SEL_MASK);
return enable_fec_anatop_clock(0, ENET_50MHZ);
}
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On 1/2/19 11:06 AM, Lukasz Majewski wrote:
Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This is a cosmetic change, just to use proper define.
Signed-off-by: Lukasz Majewski lukma@denx.de
The subject tags are wrong, fix globally.
Is there any doc which describes the allowed/not allowed tags (either with Linux or U-boot)?
I am not aware of one, however you can write one if you feel inclined to do so.
From my experience it is a some kind of a convention depending on the maintainer (which helps him to spot relevant patches on ML).
Right, and this is ARM imx board, hence those tags should be included.
In [1] - point 14) there is: The canonical patch subject line is: "Subject: [PATCH 001/123] subsystem: summary phrase"
and the "subsystem: " is the tag.
In the case of this patch -> I do fix stuff in "board(s)" which is "cosmetic".
Or will we only stick to tags/mails from ./doc/git-mailrc ?
[1] - https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
[...]

Hi Marek,
On 1/2/19 11:06 AM, Lukasz Majewski wrote:
Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This is a cosmetic change, just to use proper define.
Signed-off-by: Lukasz Majewski lukma@denx.de
The subject tags are wrong, fix globally.
Is there any doc which describes the allowed/not allowed tags (either with Linux or U-boot)?
I am not aware of one, however you can write one if you feel inclined to do so.
I was just curious if I have overlooked something.
From my experience it is a some kind of a convention depending on the maintainer (which helps him to spot relevant patches on ML).
Right, and this is ARM imx board, hence those tags should be included.
In [1] - point 14) there is: The canonical patch subject line is: "Subject: [PATCH 001/123] subsystem: summary phrase"
and the "subsystem: " is the tag.
In the case of this patch -> I do fix stuff in "board(s)" which is "cosmetic".
Or will we only stick to tags/mails from ./doc/git-mailrc ?
[1] - https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
[...]
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Signed-off-by: Lukasz Majewski lukma@denx.de ---
board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c index bcc22b1aa8..f0c97ba368 100644 --- a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c +++ b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c @@ -27,6 +27,7 @@ #include <netdev.h> #include <usb.h> #include <usb/ehci-ci.h> +#include <led.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -290,6 +291,9 @@ int board_late_init(void) add_board_boot_modes(board_boot_modes); #endif
+ if (IS_ENABLED(CONFIG_LED)) + led_default_state(); + env_set("boardname", "kp-tpc"); env_set("boardsoc", "imx6q"); return 0;

This commit is necessary to be able to re-use the pinctrl code in early SPL to properly configure pins.
The problem is that those "static" structures are placed in the SDRAM area, which corresponds to u-boot proper (not even SPL). Hence, when one wants to configure pins before relocation via DTS/DM, the board hangs (imx6q SoC powered one) as only OCRAM area is available (0x009xxxxx).
It is also safe to use calloc in this case, as the early SPL code provides it - either full or trimmed version. The allocated memory is also correct in respect to the memory area in which the SoC is currently running (OCRAM vs. SDRAM).
Signed-off-by: Lukasz Majewski lukma@denx.de ---
drivers/pinctrl/nxp/pinctrl-imx6.c | 39 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/pinctrl/nxp/pinctrl-imx6.c b/drivers/pinctrl/nxp/pinctrl-imx6.c index d7c95bb738..876049b39b 100644 --- a/drivers/pinctrl/nxp/pinctrl-imx6.c +++ b/drivers/pinctrl/nxp/pinctrl-imx6.c @@ -10,34 +10,33 @@
#include "pinctrl-imx.h"
-static struct imx_pinctrl_soc_info imx6_pinctrl_soc_info; - -/* FIXME Before reloaction, BSS is overlapped with DT area */ -static struct imx_pinctrl_soc_info imx6ul_pinctrl_soc_info = { - .flags = ZERO_OFFSET_VALID, -}; - -static struct imx_pinctrl_soc_info imx6_snvs_pinctrl_soc_info = { - .flags = ZERO_OFFSET_VALID, -}; - static int imx6_pinctrl_probe(struct udevice *dev) { struct imx_pinctrl_soc_info *info = - (struct imx_pinctrl_soc_info *)dev_get_driver_data(dev); + calloc(1, sizeof(struct imx_pinctrl_soc_info)); + + if (!info) { + printf("%s: Not enough memory!\n", __func__); + return -ENOMEM; + } + + if (device_is_compatible(dev, "fsl,imx6sll-iomuxc-snvs") || + device_is_compatible(dev, "fsl,imx6ull-iomuxc-snvs") || + device_is_compatible(dev, "fsl,imx6ul-iomuxc")) + info->flags = ZERO_OFFSET_VALID;
return imx_pinctrl_probe(dev, info); }
static const struct udevice_id imx6_pinctrl_match[] = { - { .compatible = "fsl,imx6q-iomuxc", .data = (ulong)&imx6_pinctrl_soc_info }, - { .compatible = "fsl,imx6dl-iomuxc", .data = (ulong)&imx6_pinctrl_soc_info }, - { .compatible = "fsl,imx6sl-iomuxc", .data = (ulong)&imx6_pinctrl_soc_info }, - { .compatible = "fsl,imx6sll-iomuxc-snvs", .data = (ulong)&imx6_snvs_pinctrl_soc_info }, - { .compatible = "fsl,imx6sll-iomuxc", .data = (ulong)&imx6_pinctrl_soc_info }, - { .compatible = "fsl,imx6sx-iomuxc", .data = (ulong)&imx6_pinctrl_soc_info }, - { .compatible = "fsl,imx6ul-iomuxc", .data = (ulong)&imx6ul_pinctrl_soc_info }, - { .compatible = "fsl,imx6ull-iomuxc-snvs", .data = (ulong)&imx6_snvs_pinctrl_soc_info }, + { .compatible = "fsl,imx6q-iomuxc" }, + { .compatible = "fsl,imx6dl-iomuxc" }, + { .compatible = "fsl,imx6sl-iomuxc" }, + { .compatible = "fsl,imx6sll-iomuxc-snvs" }, + { .compatible = "fsl,imx6sll-iomuxc" }, + { .compatible = "fsl,imx6sx-iomuxc" }, + { .compatible = "fsl,imx6ul-iomuxc" }, + { .compatible = "fsl,imx6ull-iomuxc-snvs" }, { /* sentinel */ } };

On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This commit is necessary to be able to re-use the pinctrl code in early SPL to properly configure pins.
The problem is that those "static" structures are placed in the SDRAM area, which corresponds to u-boot proper (not even SPL). Hence, when one wants to configure pins before relocation via DTS/DM, the board hangs (imx6q SoC powered one) as only OCRAM area is available (0x009xxxxx).
It is also safe to use calloc in this case, as the early SPL code provides it - either full or trimmed version. The allocated memory is also correct in respect to the memory area in which the SoC is currently running (OCRAM vs. SDRAM).
You should be able to access static data in early environment, many other drivers have no problem with it. I believe there is some more fundamental problem that needs to be fixed, hacking around it like this is just hiding the real issue.
Signed-off-by: Lukasz Majewski lukma@denx.de
drivers/pinctrl/nxp/pinctrl-imx6.c | 39 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/pinctrl/nxp/pinctrl-imx6.c b/drivers/pinctrl/nxp/pinctrl-imx6.c index d7c95bb738..876049b39b 100644 --- a/drivers/pinctrl/nxp/pinctrl-imx6.c +++ b/drivers/pinctrl/nxp/pinctrl-imx6.c @@ -10,34 +10,33 @@
#include "pinctrl-imx.h"
-static struct imx_pinctrl_soc_info imx6_pinctrl_soc_info;
-/* FIXME Before reloaction, BSS is overlapped with DT area */ -static struct imx_pinctrl_soc_info imx6ul_pinctrl_soc_info = {
- .flags = ZERO_OFFSET_VALID,
-};
-static struct imx_pinctrl_soc_info imx6_snvs_pinctrl_soc_info = {
- .flags = ZERO_OFFSET_VALID,
-};
static int imx6_pinctrl_probe(struct udevice *dev) { struct imx_pinctrl_soc_info *info =
(struct imx_pinctrl_soc_info *)dev_get_driver_data(dev);
calloc(1, sizeof(struct imx_pinctrl_soc_info));
if (!info) {
printf("%s: Not enough memory!\n", __func__);
return -ENOMEM;
}
if (device_is_compatible(dev, "fsl,imx6sll-iomuxc-snvs") ||
device_is_compatible(dev, "fsl,imx6ull-iomuxc-snvs") ||
device_is_compatible(dev, "fsl,imx6ul-iomuxc"))
info->flags = ZERO_OFFSET_VALID;
return imx_pinctrl_probe(dev, info);
}
static const struct udevice_id imx6_pinctrl_match[] = {
- { .compatible = "fsl,imx6q-iomuxc", .data = (ulong)&imx6_pinctrl_soc_info },
- { .compatible = "fsl,imx6dl-iomuxc", .data = (ulong)&imx6_pinctrl_soc_info },
- { .compatible = "fsl,imx6sl-iomuxc", .data = (ulong)&imx6_pinctrl_soc_info },
- { .compatible = "fsl,imx6sll-iomuxc-snvs", .data = (ulong)&imx6_snvs_pinctrl_soc_info },
- { .compatible = "fsl,imx6sll-iomuxc", .data = (ulong)&imx6_pinctrl_soc_info },
- { .compatible = "fsl,imx6sx-iomuxc", .data = (ulong)&imx6_pinctrl_soc_info },
- { .compatible = "fsl,imx6ul-iomuxc", .data = (ulong)&imx6ul_pinctrl_soc_info },
- { .compatible = "fsl,imx6ull-iomuxc-snvs", .data = (ulong)&imx6_snvs_pinctrl_soc_info },
- { .compatible = "fsl,imx6q-iomuxc" },
- { .compatible = "fsl,imx6dl-iomuxc" },
- { .compatible = "fsl,imx6sl-iomuxc" },
- { .compatible = "fsl,imx6sll-iomuxc-snvs" },
- { .compatible = "fsl,imx6sll-iomuxc" },
- { .compatible = "fsl,imx6sx-iomuxc" },
- { .compatible = "fsl,imx6ul-iomuxc" },
- { .compatible = "fsl,imx6ull-iomuxc-snvs" }, { /* sentinel */ }
};

Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This commit is necessary to be able to re-use the pinctrl code in early SPL to properly configure pins.
The problem is that those "static" structures are placed in the SDRAM area, which corresponds to u-boot proper (not even SPL). Hence, when one wants to configure pins before relocation via DTS/DM, the board hangs (imx6q SoC powered one) as only OCRAM area is available (0x009xxxxx).
It is also safe to use calloc in this case, as the early SPL code provides it - either full or trimmed version. The allocated memory is also correct in respect to the memory area in which the SoC is currently running (OCRAM vs. SDRAM).
You should be able to access static data in early environment, many other drivers have no problem with it.
Have you used pinctrl IMX6Q driver with early DM at SPL?
I believe there is some more fundamental problem that needs to be fixed,
With the current code - which was not used in OCRAM only available memory - the statics are placed in the SDRAM area (0x178x xxxx).
Instead of changing linker script (and validate it for all drivers and boards for SPL and u-boot proper) - the code has been rewritten to use malloc (which is available in the early code - either full blown or simple version).
hacking around it like this is just hiding the real issue.
The real issue is that IMX6Q uses SDRAM memory for statics/globals from the very beginning.
Signed-off-by: Lukasz Majewski lukma@denx.de
drivers/pinctrl/nxp/pinctrl-imx6.c | 39 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/pinctrl/nxp/pinctrl-imx6.c b/drivers/pinctrl/nxp/pinctrl-imx6.c index d7c95bb738..876049b39b 100644 --- a/drivers/pinctrl/nxp/pinctrl-imx6.c +++ b/drivers/pinctrl/nxp/pinctrl-imx6.c @@ -10,34 +10,33 @@
#include "pinctrl-imx.h"
-static struct imx_pinctrl_soc_info imx6_pinctrl_soc_info;
-/* FIXME Before reloaction, BSS is overlapped with DT area */ -static struct imx_pinctrl_soc_info imx6ul_pinctrl_soc_info = {
- .flags = ZERO_OFFSET_VALID,
-};
-static struct imx_pinctrl_soc_info imx6_snvs_pinctrl_soc_info = {
- .flags = ZERO_OFFSET_VALID,
-};
static int imx6_pinctrl_probe(struct udevice *dev) { struct imx_pinctrl_soc_info *info =
(struct imx_pinctrl_soc_info
*)dev_get_driver_data(dev);
calloc(1, sizeof(struct imx_pinctrl_soc_info));
if (!info) {
printf("%s: Not enough memory!\n", __func__);
return -ENOMEM;
}
if (device_is_compatible(dev, "fsl,imx6sll-iomuxc-snvs") ||
device_is_compatible(dev, "fsl,imx6ull-iomuxc-snvs") ||
device_is_compatible(dev, "fsl,imx6ul-iomuxc"))
info->flags = ZERO_OFFSET_VALID;
return imx_pinctrl_probe(dev, info);
}
static const struct udevice_id imx6_pinctrl_match[] = {
- { .compatible = "fsl,imx6q-iomuxc", .data =
(ulong)&imx6_pinctrl_soc_info },
- { .compatible = "fsl,imx6dl-iomuxc", .data =
(ulong)&imx6_pinctrl_soc_info },
- { .compatible = "fsl,imx6sl-iomuxc", .data =
(ulong)&imx6_pinctrl_soc_info },
- { .compatible = "fsl,imx6sll-iomuxc-snvs", .data =
(ulong)&imx6_snvs_pinctrl_soc_info },
- { .compatible = "fsl,imx6sll-iomuxc", .data =
(ulong)&imx6_pinctrl_soc_info },
- { .compatible = "fsl,imx6sx-iomuxc", .data =
(ulong)&imx6_pinctrl_soc_info },
- { .compatible = "fsl,imx6ul-iomuxc", .data =
(ulong)&imx6ul_pinctrl_soc_info },
- { .compatible = "fsl,imx6ull-iomuxc-snvs", .data =
(ulong)&imx6_snvs_pinctrl_soc_info },
- { .compatible = "fsl,imx6q-iomuxc" },
- { .compatible = "fsl,imx6dl-iomuxc" },
- { .compatible = "fsl,imx6sl-iomuxc" },
- { .compatible = "fsl,imx6sll-iomuxc-snvs" },
- { .compatible = "fsl,imx6sll-iomuxc" },
- { .compatible = "fsl,imx6sx-iomuxc" },
- { .compatible = "fsl,imx6ul-iomuxc" },
- { .compatible = "fsl,imx6ull-iomuxc-snvs" }, { /* sentinel */ }
};
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On 1/2/19 9:26 AM, Lukasz Majewski wrote:
Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This commit is necessary to be able to re-use the pinctrl code in early SPL to properly configure pins.
The problem is that those "static" structures are placed in the SDRAM area, which corresponds to u-boot proper (not even SPL). Hence, when one wants to configure pins before relocation via DTS/DM, the board hangs (imx6q SoC powered one) as only OCRAM area is available (0x009xxxxx).
It is also safe to use calloc in this case, as the early SPL code provides it - either full or trimmed version. The allocated memory is also correct in respect to the memory area in which the SoC is currently running (OCRAM vs. SDRAM).
You should be able to access static data in early environment, many other drivers have no problem with it.
Have you used pinctrl IMX6Q driver with early DM at SPL?
I believe there is some more fundamental problem that needs to be fixed,
With the current code - which was not used in OCRAM only available memory - the statics are placed in the SDRAM area (0x178x xxxx).
Instead of changing linker script (and validate it for all drivers and boards for SPL and u-boot proper) - the code has been rewritten to use malloc (which is available in the early code - either full blown or simple version).
hacking around it like this is just hiding the real issue.
The real issue is that IMX6Q uses SDRAM memory for statics/globals from the very beginning.
Fix it ? What you're implying here is that rodata (preinitialized static variables) are misplaced.

On Wed, 2 Jan 2019 15:14:22 +0100 Marek Vasut marex@denx.de wrote:
On 1/2/19 9:26 AM, Lukasz Majewski wrote:
Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This commit is necessary to be able to re-use the pinctrl code in early SPL to properly configure pins.
The problem is that those "static" structures are placed in the SDRAM area, which corresponds to u-boot proper (not even SPL). Hence, when one wants to configure pins before relocation via DTS/DM, the board hangs (imx6q SoC powered one) as only OCRAM area is available (0x009xxxxx).
It is also safe to use calloc in this case, as the early SPL code provides it - either full or trimmed version. The allocated memory is also correct in respect to the memory area in which the SoC is currently running (OCRAM vs. SDRAM).
You should be able to access static data in early environment, many other drivers have no problem with it.
Have you used pinctrl IMX6Q driver with early DM at SPL?
I believe there is some more fundamental problem that needs to be fixed,
With the current code - which was not used in OCRAM only available memory - the statics are placed in the SDRAM area (0x178x xxxx).
Instead of changing linker script (and validate it for all drivers and boards for SPL and u-boot proper) - the code has been rewritten to use malloc (which is available in the early code - either full blown or simple version).
hacking around it like this is just hiding the real issue.
The real issue is that IMX6Q uses SDRAM memory for statics/globals from the very beginning.
Fix it ? What you're implying here is that rodata (preinitialized static variables) are misplaced.
The not initialized (as going to be zeroed)
static struct imx_pinctrl_soc_info imx6_pinctrl_soc_info; goes to bss section, which is located in SDRAM (not available yet).
.bss.imx6_pinctrl_soc_info 0x0000000018200050 0x10 drivers/built-in.o
The other structs in this file, as they have assigned values, are placed correctly in data (OCRAM area).
I could add __attribute__((section("data"))) in front of this definition, but this may be a problematic for u-boot proper where this code is reused (and correctly this data is placed in bss).
Hence, the idea to use calloc explicitly - and with it the correct (available in the moment) memory is provided.
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

This patch is a preparation for the imx6q to use DTS in the SPL for very early configuration, as 'u-boot,dm-pre-reloc;' is necessary to initialize uart and SD/eMMC controllers in SPL.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
arch/arm/dts/imx6qdl.dtsi | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/arch/arm/dts/imx6qdl.dtsi b/arch/arm/dts/imx6qdl.dtsi index 476cf93445..61cb59cc94 100644 --- a/arch/arm/dts/imx6qdl.dtsi +++ b/arch/arm/dts/imx6qdl.dtsi @@ -72,6 +72,7 @@ #address-cells = <1>; #size-cells = <1>; compatible = "simple-bus"; + u-boot,dm-pre-reloc; interrupt-parent = <&gpc>; ranges;
@@ -218,6 +219,7 @@
aips-bus@02000000 { /* AIPS1 */ compatible = "fsl,aips-bus", "simple-bus"; + u-boot,dm-pre-reloc; #address-cells = <1>; #size-cells = <1>; reg = <0x02000000 0x100000>; @@ -225,6 +227,7 @@
spba-bus@02000000 { compatible = "fsl,spba-bus", "simple-bus"; + u-boot,dm-pre-reloc; #address-cells = <1>; #size-cells = <1>; reg = <0x02000000 0x40000>; @@ -801,6 +804,7 @@
iomuxc: iomuxc@020e0000 { compatible = "fsl,imx6dl-iomuxc", "fsl,imx6q-iomuxc"; + u-boot,dm-pre-reloc; reg = <0x020e0000 0x4000>; };
@@ -882,6 +886,7 @@
aips-bus@02100000 { /* AIPS2 */ compatible = "fsl,aips-bus", "simple-bus"; + u-boot,dm-pre-reloc; #address-cells = <1>; #size-cells = <1>; reg = <0x02100000 0x100000>;

On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This patch is a preparation for the imx6q to use DTS in the SPL for very early configuration, as 'u-boot,dm-pre-reloc;' is necessary to initialize uart and SD/eMMC controllers in SPL.
Signed-off-by: Lukasz Majewski lukma@denx.de
Should be in U-Boot specific DTSI to allow easy resync of DTSIs with Linux.
arch/arm/dts/imx6qdl.dtsi | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/arch/arm/dts/imx6qdl.dtsi b/arch/arm/dts/imx6qdl.dtsi index 476cf93445..61cb59cc94 100644 --- a/arch/arm/dts/imx6qdl.dtsi +++ b/arch/arm/dts/imx6qdl.dtsi @@ -72,6 +72,7 @@ #address-cells = <1>; #size-cells = <1>; compatible = "simple-bus";
interrupt-parent = <&gpc>; ranges;u-boot,dm-pre-reloc;
@@ -218,6 +219,7 @@
aips-bus@02000000 { /* AIPS1 */ compatible = "fsl,aips-bus", "simple-bus";
u-boot,dm-pre-reloc; #address-cells = <1>; #size-cells = <1>; reg = <0x02000000 0x100000>;
@@ -225,6 +227,7 @@
spba-bus@02000000 { compatible = "fsl,spba-bus", "simple-bus";
u-boot,dm-pre-reloc; #address-cells = <1>; #size-cells = <1>; reg = <0x02000000 0x40000>;
@@ -801,6 +804,7 @@
iomuxc: iomuxc@020e0000 { compatible = "fsl,imx6dl-iomuxc", "fsl,imx6q-iomuxc";
u-boot,dm-pre-reloc; reg = <0x020e0000 0x4000>; };
@@ -882,6 +886,7 @@
aips-bus@02100000 { /* AIPS2 */ compatible = "fsl,aips-bus", "simple-bus";
u-boot,dm-pre-reloc; #address-cells = <1>; #size-cells = <1>; reg = <0x02100000 0x100000>;

Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This patch is a preparation for the imx6q to use DTS in the SPL for very early configuration, as 'u-boot,dm-pre-reloc;' is necessary to initialize uart and SD/eMMC controllers in SPL.
Signed-off-by: Lukasz Majewski lukma@denx.de
Should be in U-Boot specific DTSI to allow easy resync of DTSIs with Linux.
Good point. I will do this in a way similar to arch/arm/dts/unipher-v7-u-boot.dtsi
arch/arm/dts/imx6qdl.dtsi | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/arch/arm/dts/imx6qdl.dtsi b/arch/arm/dts/imx6qdl.dtsi index 476cf93445..61cb59cc94 100644 --- a/arch/arm/dts/imx6qdl.dtsi +++ b/arch/arm/dts/imx6qdl.dtsi @@ -72,6 +72,7 @@ #address-cells = <1>; #size-cells = <1>; compatible = "simple-bus";
interrupt-parent = <&gpc>; ranges;u-boot,dm-pre-reloc;
@@ -218,6 +219,7 @@
aips-bus@02000000 { /* AIPS1 */ compatible = "fsl,aips-bus", "simple-bus";
u-boot,dm-pre-reloc; #address-cells = <1>; #size-cells = <1>; reg = <0x02000000 0x100000>;
@@ -225,6 +227,7 @@
spba-bus@02000000 { compatible = "fsl,spba-bus",
"simple-bus";
u-boot,dm-pre-reloc; #address-cells = <1>; #size-cells = <1>; reg = <0x02000000 0x40000>;
@@ -801,6 +804,7 @@
iomuxc: iomuxc@020e0000 { compatible = "fsl,imx6dl-iomuxc",
"fsl,imx6q-iomuxc";
u-boot,dm-pre-reloc; reg = <0x020e0000 0x4000>; };
@@ -882,6 +886,7 @@
aips-bus@02100000 { /* AIPS2 */ compatible = "fsl,aips-bus", "simple-bus";
u-boot,dm-pre-reloc; #address-cells = <1>; #size-cells = <1>; reg = <0x02100000 0x100000>;
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

The DM_FLAG_PRE_RELOC shall be enabled as this driver is going to be re-used in the i.MX based SoCs.
It is crucial to have running the serial console before relocation.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
drivers/serial/serial_mxc.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 7e4e6d36b8..e586c18cf0 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -354,9 +354,7 @@ U_BOOT_DRIVER(serial_mxc) = { #endif .probe = mxc_serial_probe, .ops = &mxc_serial_ops, -#if !CONFIG_IS_ENABLED(OF_CONTROL) .flags = DM_FLAG_PRE_RELOC, -#endif }; #endif

On 1/2/19 12:37 AM, Lukasz Majewski wrote:
The DM_FLAG_PRE_RELOC shall be enabled as this driver is going to be re-used in the i.MX based SoCs.
It is crucial to have running the serial console before relocation.
But the patch doesn't do what the subject/commit message claims is does. If I understand it correctly, it sets the PRE_RELOC flag unconditionally.
Signed-off-by: Lukasz Majewski lukma@denx.de
drivers/serial/serial_mxc.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 7e4e6d36b8..e586c18cf0 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -354,9 +354,7 @@ U_BOOT_DRIVER(serial_mxc) = { #endif .probe = mxc_serial_probe, .ops = &mxc_serial_ops, -#if !CONFIG_IS_ENABLED(OF_CONTROL) .flags = DM_FLAG_PRE_RELOC, -#endif }; #endif

Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
The DM_FLAG_PRE_RELOC shall be enabled as this driver is going to be re-used in the i.MX based SoCs.
It is crucial to have running the serial console before relocation.
But the patch doesn't do what the subject/commit message claims is does. If I understand it correctly, it sets the PRE_RELOC flag unconditionally.
The !CONFIG_IS_ENABLED(OF_CONTROL) [*] was set as a "workaround" for DM problem: SHA1: 4687919684e0e4390b9fc20d1809ecaa9dc3cb81
Let's look on this check [*] - we add this flag if the board doesn't support OF_CONTROL. This is a bit strange as the serial_mxc.c can be used with CONFIG_DM_SERIAL but without corresponding device tree description (OF_CONTROL).
Other boards/SoCs have this flag set unconditionally.
And one more remark - with OF_CONTROL one needs to declare 'u-boot,dm-pre-reloc;' property in DTS to have the device probed in the SPL's pre relocation stage.
Signed-off-by: Lukasz Majewski lukma@denx.de
drivers/serial/serial_mxc.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 7e4e6d36b8..e586c18cf0 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -354,9 +354,7 @@ U_BOOT_DRIVER(serial_mxc) = { #endif .probe = mxc_serial_probe, .ops = &mxc_serial_ops, -#if !CONFIG_IS_ENABLED(OF_CONTROL) .flags = DM_FLAG_PRE_RELOC, -#endif }; #endif
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On 1/2/19 9:42 AM, Lukasz Majewski wrote:
Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
The DM_FLAG_PRE_RELOC shall be enabled as this driver is going to be re-used in the i.MX based SoCs.
It is crucial to have running the serial console before relocation.
But the patch doesn't do what the subject/commit message claims is does. If I understand it correctly, it sets the PRE_RELOC flag unconditionally.
The !CONFIG_IS_ENABLED(OF_CONTROL) [*] was set as a "workaround" for DM problem: SHA1: 4687919684e0e4390b9fc20d1809ecaa9dc3cb81
Let's look on this check [*] - we add this flag if the board doesn't support OF_CONTROL. This is a bit strange as the serial_mxc.c can be used with CONFIG_DM_SERIAL but without corresponding device tree description (OF_CONTROL).
Other boards/SoCs have this flag set unconditionally.
And one more remark - with OF_CONTROL one needs to declare 'u-boot,dm-pre-reloc;' property in DTS to have the device probed in the SPL's pre relocation stage.
Explain it in the patch description and send V2.

This patch provides a generic way to setup ENET (ETH) clocks for imx6(q) based boards. Previously this was performed per board in the board_eth_init() function.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
arch/arm/include/asm/arch-mx6/clock.h | 1 + arch/arm/mach-imx/mx6/clock.c | 17 +++++++++++++++++ 2 files changed, 18 insertions(+)
diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h index a9481a5fea..9a217349f5 100644 --- a/arch/arm/include/asm/arch-mx6/clock.h +++ b/arch/arm/include/asm/arch-mx6/clock.h @@ -72,6 +72,7 @@ int enable_i2c_clk(unsigned char enable, unsigned i2c_num); int enable_spi_clk(unsigned char enable, unsigned spi_num); void enable_ipu_clock(void); int enable_fec_anatop_clock(int fec_id, enum enet_freq freq); +int set_fec_clock(int fec_id, enum enet_freq freq); void enable_enet_clk(unsigned char enable); int enable_lcdif_clock(u32 base_addr, bool enable); void enable_qspi_clk(int qspi_num); diff --git a/arch/arm/mach-imx/mx6/clock.c b/arch/arm/mach-imx/mx6/clock.c index 366a4e3c6b..8a4fb23090 100644 --- a/arch/arm/mach-imx/mx6/clock.c +++ b/arch/arm/mach-imx/mx6/clock.c @@ -902,6 +902,17 @@ void enable_qspi_clk(int qspi_num) #endif
#ifdef CONFIG_FEC_MXC +static void select_fec_clock_source(int fec_id) +{ + struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR; + + if (is_mx6dq()) { + /* set gpr1[21] to select anatop clock */ + setbits_le32(&iomuxc_regs->gpr[1], + IOMUXC_GPR1_ENET_CLK_SEL_MASK); + } +} + int enable_fec_anatop_clock(int fec_id, enum enet_freq freq) { u32 reg = 0; @@ -976,6 +987,12 @@ int enable_fec_anatop_clock(int fec_id, enum enet_freq freq) #endif return 0; } + +int set_fec_clock(int fec_id, enum enet_freq freq) +{ + select_fec_clock_source(fec_id); + return enable_fec_anatop_clock(fec_id, freq); +} #endif
static u32 get_usdhc_clk(u32 port)

On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This patch provides a generic way to setup ENET (ETH) clocks for imx6(q) based boards. Previously this was performed per board in the board_eth_init() function.
Signed-off-by: Lukasz Majewski lukma@denx.de
arch/arm/include/asm/arch-mx6/clock.h | 1 + arch/arm/mach-imx/mx6/clock.c | 17 +++++++++++++++++ 2 files changed, 18 insertions(+)
diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h index a9481a5fea..9a217349f5 100644 --- a/arch/arm/include/asm/arch-mx6/clock.h +++ b/arch/arm/include/asm/arch-mx6/clock.h @@ -72,6 +72,7 @@ int enable_i2c_clk(unsigned char enable, unsigned i2c_num); int enable_spi_clk(unsigned char enable, unsigned spi_num); void enable_ipu_clock(void); int enable_fec_anatop_clock(int fec_id, enum enet_freq freq); +int set_fec_clock(int fec_id, enum enet_freq freq); void enable_enet_clk(unsigned char enable); int enable_lcdif_clock(u32 base_addr, bool enable); void enable_qspi_clk(int qspi_num); diff --git a/arch/arm/mach-imx/mx6/clock.c b/arch/arm/mach-imx/mx6/clock.c index 366a4e3c6b..8a4fb23090 100644 --- a/arch/arm/mach-imx/mx6/clock.c +++ b/arch/arm/mach-imx/mx6/clock.c @@ -902,6 +902,17 @@ void enable_qspi_clk(int qspi_num) #endif
#ifdef CONFIG_FEC_MXC +static void select_fec_clock_source(int fec_id)
How is the fec_id() used in here ? Shouldn't this be part of enable_fec_anatop_clock() ?
+{
- struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR;
- if (is_mx6dq()) {
/* set gpr1[21] to select anatop clock */
setbits_le32(&iomuxc_regs->gpr[1],
IOMUXC_GPR1_ENET_CLK_SEL_MASK);
- }
+}
int enable_fec_anatop_clock(int fec_id, enum enet_freq freq) { u32 reg = 0; @@ -976,6 +987,12 @@ int enable_fec_anatop_clock(int fec_id, enum enet_freq freq) #endif return 0; }
+int set_fec_clock(int fec_id, enum enet_freq freq) +{
- select_fec_clock_source(fec_id);
- return enable_fec_anatop_clock(fec_id, freq);
+} #endif
static u32 get_usdhc_clk(u32 port)

Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This patch provides a generic way to setup ENET (ETH) clocks for imx6(q) based boards. Previously this was performed per board in the board_eth_init() function.
Signed-off-by: Lukasz Majewski lukma@denx.de
arch/arm/include/asm/arch-mx6/clock.h | 1 + arch/arm/mach-imx/mx6/clock.c | 17 +++++++++++++++++ 2 files changed, 18 insertions(+)
diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h index a9481a5fea..9a217349f5 100644 --- a/arch/arm/include/asm/arch-mx6/clock.h +++ b/arch/arm/include/asm/arch-mx6/clock.h @@ -72,6 +72,7 @@ int enable_i2c_clk(unsigned char enable, unsigned i2c_num); int enable_spi_clk(unsigned char enable, unsigned spi_num); void enable_ipu_clock(void); int enable_fec_anatop_clock(int fec_id, enum enet_freq freq); +int set_fec_clock(int fec_id, enum enet_freq freq); void enable_enet_clk(unsigned char enable); int enable_lcdif_clock(u32 base_addr, bool enable); void enable_qspi_clk(int qspi_num); diff --git a/arch/arm/mach-imx/mx6/clock.c b/arch/arm/mach-imx/mx6/clock.c index 366a4e3c6b..8a4fb23090 100644 --- a/arch/arm/mach-imx/mx6/clock.c +++ b/arch/arm/mach-imx/mx6/clock.c @@ -902,6 +902,17 @@ void enable_qspi_clk(int qspi_num) #endif
#ifdef CONFIG_FEC_MXC +static void select_fec_clock_source(int fec_id)
How is the fec_id() used in here ?
I guess that you refer to "int fec_id."
Shouldn't this be part of enable_fec_anatop_clock() ?
The enable_fec_anatop_clock() function is used on several board files - for example: http://git.denx.de/?p=u-boot.git;a=blob;f=board/dhelectronics/dh_imx6/dh_imx...
And changing it could break some boards.
The select_fec_clock_source() shall be used to replace several: clrsetbits_le32(&iomuxc_regs->gpr[1], 0x1 << 21, 0x1 << 21);
clauses for imx6 variants.
Moreover, I'd pass 'fec_id' parameter anyway - it may be needed by other imx6 variants.
+{
- struct iomuxc *iomuxc_regs = (struct iomuxc
*)IOMUXC_BASE_ADDR; +
- if (is_mx6dq()) {
/* set gpr1[21] to select anatop clock */
setbits_le32(&iomuxc_regs->gpr[1],
IOMUXC_GPR1_ENET_CLK_SEL_MASK);
- }
+}
int enable_fec_anatop_clock(int fec_id, enum enet_freq freq) { u32 reg = 0; @@ -976,6 +987,12 @@ int enable_fec_anatop_clock(int fec_id, enum enet_freq freq) #endif return 0; }
+int set_fec_clock(int fec_id, enum enet_freq freq) +{
- select_fec_clock_source(fec_id);
- return enable_fec_anatop_clock(fec_id, freq);
+} #endif
static u32 get_usdhc_clk(u32 port)
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On 1/2/19 10:00 AM, Lukasz Majewski wrote:
Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This patch provides a generic way to setup ENET (ETH) clocks for imx6(q) based boards. Previously this was performed per board in the board_eth_init() function.
Signed-off-by: Lukasz Majewski lukma@denx.de
arch/arm/include/asm/arch-mx6/clock.h | 1 + arch/arm/mach-imx/mx6/clock.c | 17 +++++++++++++++++ 2 files changed, 18 insertions(+)
diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h index a9481a5fea..9a217349f5 100644 --- a/arch/arm/include/asm/arch-mx6/clock.h +++ b/arch/arm/include/asm/arch-mx6/clock.h @@ -72,6 +72,7 @@ int enable_i2c_clk(unsigned char enable, unsigned i2c_num); int enable_spi_clk(unsigned char enable, unsigned spi_num); void enable_ipu_clock(void); int enable_fec_anatop_clock(int fec_id, enum enet_freq freq); +int set_fec_clock(int fec_id, enum enet_freq freq); void enable_enet_clk(unsigned char enable); int enable_lcdif_clock(u32 base_addr, bool enable); void enable_qspi_clk(int qspi_num); diff --git a/arch/arm/mach-imx/mx6/clock.c b/arch/arm/mach-imx/mx6/clock.c index 366a4e3c6b..8a4fb23090 100644 --- a/arch/arm/mach-imx/mx6/clock.c +++ b/arch/arm/mach-imx/mx6/clock.c @@ -902,6 +902,17 @@ void enable_qspi_clk(int qspi_num) #endif
#ifdef CONFIG_FEC_MXC +static void select_fec_clock_source(int fec_id)
How is the fec_id() used in here ?
I guess that you refer to "int fec_id."
Yes, how is it used ? I guess it is not ... so why is this parameter even here ?
Shouldn't this be part of enable_fec_anatop_clock() ?
The enable_fec_anatop_clock() function is used on several board files
- for example:
http://git.denx.de/?p=u-boot.git;a=blob;f=board/dhelectronics/dh_imx6/dh_imx...
And changing it could break some boards.
The select_fec_clock_source() shall be used to replace several: clrsetbits_le32(&iomuxc_regs->gpr[1], 0x1 << 21, 0x1 << 21);
clauses for imx6 variants.
Moreover, I'd pass 'fec_id' parameter anyway - it may be needed by other imx6 variants.
It can be added when it is needed.

This patch is necessary to initialize some board/soc specific clocks - like anatop, which is used to clock PHY and FEC block itself.
The initialization is performed with device tree by introducing two new properties - namely; 'fsl,enet-loopback-clk' and 'fsl,enet-freq' which specify the need to select proper enet clock and the clock value itself.
Previously this setup was done in the board_etc_init() function, which has been removed after switching to DM/DTS.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
drivers/net/fec_mxc.c | 25 +++++++++++++++++++++++++ drivers/net/fec_mxc.h | 2 ++ 2 files changed, 27 insertions(+)
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index 99c5c649a0..728d6c9456 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -1297,6 +1297,23 @@ static void fec_gpio_reset(struct fec_priv *priv) } } #endif +/* + * This function is mostly intended for some soc/board specific + * clock initialization (like anatop clock on iMX6) done + * previously in board_eth_init() + */ +__weak int set_fec_clock(int fec_id, enum enet_freq freq); + +static int fec_clk_init(struct udevice *dev) +{ + struct fec_priv *priv = dev_get_priv(dev); + int ret = 0; + + if (priv->enet_loopback_clk) + ret = set_fec_clock(dev->seq, priv->freq); + + return ret; +}
static int fecmxc_probe(struct udevice *dev) { @@ -1321,6 +1338,10 @@ static int fecmxc_probe(struct udevice *dev) priv->clk_rate = clk_get_rate(&priv->ipg_clk); }
+ ret = fec_clk_init(dev); + if (ret) + return ret; + ret = fec_alloc_descs(priv); if (ret) return ret; @@ -1455,6 +1476,10 @@ static int fecmxc_ofdata_to_platdata(struct udevice *dev) } #endif
+ priv->enet_loopback_clk = dev_read_bool(dev, "fsl,enet-loopback-clk"); + if (priv->enet_loopback_clk) + dev_read_u32(dev, "fsl,enet-freq", &priv->freq); + return 0; }
diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h index e9a661f0a1..666b34304c 100644 --- a/drivers/net/fec_mxc.h +++ b/drivers/net/fec_mxc.h @@ -261,6 +261,8 @@ struct fec_priv { #endif #ifdef CONFIG_DM_ETH u32 interface; + bool enet_loopback_clk; /* anatop reference clk via PAD loopback */ + enum enet_freq freq; #endif struct clk ipg_clk; u32 clk_rate;

On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This patch is necessary to initialize some board/soc specific clocks - like anatop, which is used to clock PHY and FEC block itself.
The initialization is performed with device tree by introducing two new properties - namely; 'fsl,enet-loopback-clk' and 'fsl,enet-freq' which specify the need to select proper enet clock and the clock value itself.
Previously this setup was done in the board_etc_init() function, which has been removed after switching to DM/DTS.
Signed-off-by: Lukasz Majewski lukma@denx.de
drivers/net/fec_mxc.c | 25 +++++++++++++++++++++++++ drivers/net/fec_mxc.h | 2 ++ 2 files changed, 27 insertions(+)
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index 99c5c649a0..728d6c9456 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -1297,6 +1297,23 @@ static void fec_gpio_reset(struct fec_priv *priv) } } #endif +/*
- This function is mostly intended for some soc/board specific
- clock initialization (like anatop clock on iMX6) done
- previously in board_eth_init()
- */
+__weak int set_fec_clock(int fec_id, enum enet_freq freq);
+static int fec_clk_init(struct udevice *dev) +{
- struct fec_priv *priv = dev_get_priv(dev);
- int ret = 0;
- if (priv->enet_loopback_clk)
ret = set_fec_clock(dev->seq, priv->freq);
- return ret;
+}
static int fecmxc_probe(struct udevice *dev) { @@ -1321,6 +1338,10 @@ static int fecmxc_probe(struct udevice *dev) priv->clk_rate = clk_get_rate(&priv->ipg_clk); }
- ret = fec_clk_init(dev);
- if (ret)
return ret;
- ret = fec_alloc_descs(priv); if (ret) return ret;
@@ -1455,6 +1476,10 @@ static int fecmxc_ofdata_to_platdata(struct udevice *dev) } #endif
- priv->enet_loopback_clk = dev_read_bool(dev, "fsl,enet-loopback-clk");
- if (priv->enet_loopback_clk)
dev_read_u32(dev, "fsl,enet-freq", &priv->freq);
dev_read_u32() returns error value, so what happens if this fails ?
- return 0;
}
diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h index e9a661f0a1..666b34304c 100644 --- a/drivers/net/fec_mxc.h +++ b/drivers/net/fec_mxc.h @@ -261,6 +261,8 @@ struct fec_priv { #endif #ifdef CONFIG_DM_ETH u32 interface;
- bool enet_loopback_clk; /* anatop reference clk via PAD loopback */
- enum enet_freq freq;
#endif struct clk ipg_clk; u32 clk_rate;

Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This patch is necessary to initialize some board/soc specific clocks - like anatop, which is used to clock PHY and FEC block itself.
The initialization is performed with device tree by introducing two new properties - namely; 'fsl,enet-loopback-clk' and 'fsl,enet-freq' which specify the need to select proper enet clock and the clock value itself.
Previously this setup was done in the board_etc_init() function, which has been removed after switching to DM/DTS.
Signed-off-by: Lukasz Majewski lukma@denx.de
drivers/net/fec_mxc.c | 25 +++++++++++++++++++++++++ drivers/net/fec_mxc.h | 2 ++ 2 files changed, 27 insertions(+)
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index 99c5c649a0..728d6c9456 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -1297,6 +1297,23 @@ static void fec_gpio_reset(struct fec_priv *priv) } } #endif +/*
- This function is mostly intended for some soc/board specific
- clock initialization (like anatop clock on iMX6) done
- previously in board_eth_init()
- */
+__weak int set_fec_clock(int fec_id, enum enet_freq freq);
+static int fec_clk_init(struct udevice *dev) +{
- struct fec_priv *priv = dev_get_priv(dev);
- int ret = 0;
- if (priv->enet_loopback_clk)
ret = set_fec_clock(dev->seq, priv->freq);
- return ret;
+}
static int fecmxc_probe(struct udevice *dev) { @@ -1321,6 +1338,10 @@ static int fecmxc_probe(struct udevice *dev) priv->clk_rate = clk_get_rate(&priv->ipg_clk); }
- ret = fec_clk_init(dev);
- if (ret)
return ret;
- ret = fec_alloc_descs(priv); if (ret) return ret;
@@ -1455,6 +1476,10 @@ static int fecmxc_ofdata_to_platdata(struct udevice *dev) } #endif
- priv->enet_loopback_clk = dev_read_bool(dev,
"fsl,enet-loopback-clk");
- if (priv->enet_loopback_clk)
dev_read_u32(dev, "fsl,enet-freq", &priv->freq);
dev_read_u32() returns error value, so what happens if this fails ?
Good point - I will add the necessary check.
- return 0;
}
diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h index e9a661f0a1..666b34304c 100644 --- a/drivers/net/fec_mxc.h +++ b/drivers/net/fec_mxc.h @@ -261,6 +261,8 @@ struct fec_priv { #endif #ifdef CONFIG_DM_ETH u32 interface;
- bool enet_loopback_clk; /* anatop reference clk via PAD
loopback */
- enum enet_freq freq;
#endif struct clk ipg_clk; u32 clk_rate;
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

With the current code, it is not possible to assign different than default numbers for mmc controllers.
Several in-tree boards depend on the pre-dm setup, corresponding to following aliases:
mmc0 = &usdhc2; --> fsl,usdhc-index = <1> mmc1 = &usdhc4; --> fsl,usdhc-index = <3>
Without this patch we are either forced to use default aliasing - like:
mmc0 = &usdhc1; mmc1 = &usdhc2; mmc2 = &usdhc3; mmc3 = &usdhc4;
to have the proper clocks setup for the controller. However, such setup is not acceptable for some legacy scripts / code.
With this patch - by introducing 'fsl,usdhc-index' - one can configure (get) clock rate corresponding to used controller.
Moreover, as this code is used in the SPL before relocation (and to save space we strip the SPL DTS from clocks and its names) adding separate properties seems to be the best approach here. One also avoids adding clocks DM code to SPL.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
drivers/mmc/fsl_esdhc.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 3cdfa7f5a6..49a6834a98 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -1401,6 +1401,7 @@ static int fsl_esdhc_probe(struct udevice *dev) fdt_addr_t addr; unsigned int val; struct mmc *mmc; + int usdhc_idx; int ret;
addr = dev_read_addr(dev); @@ -1513,7 +1514,21 @@ static int fsl_esdhc_probe(struct udevice *dev)
priv->sdhc_clk = clk_get_rate(&priv->per_clk); } else { - priv->sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK + dev->seq); + /* + * Check for 'fsl,index' DTS property - as one may want to have + * following mmc setup: + * mmc0 = &usdhc2; --> fsl,index = <1> + * mmc1 = &usdhc4; --> fsl,index = <3> + * + * So we do have dev->seq = {0, 1}, which in the below code + * doesn't correspond to correct USDHC clocks. + * + * For that reason a new "fsl,index" property has been + * introduced. + */ + usdhc_idx = dev_read_u32_default(dev, "fsl,usdhc-index", + dev->seq); + priv->sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK + usdhc_idx); if (priv->sdhc_clk <= 0) { dev_err(dev, "Unable to get clk for %s\n", dev->name); return -EINVAL;

On 1/2/19 12:37 AM, Lukasz Majewski wrote:
With the current code, it is not possible to assign different than default numbers for mmc controllers.
Several in-tree boards depend on the pre-dm setup, corresponding to following aliases:
mmc0 = &usdhc2; --> fsl,usdhc-index = <1> mmc1 = &usdhc4; --> fsl,usdhc-index = <3>
Without this patch we are either forced to use default aliasing - like:
mmc0 = &usdhc1; mmc1 = &usdhc2; mmc2 = &usdhc3; mmc3 = &usdhc4;
to have the proper clocks setup for the controller. However, such setup is not acceptable for some legacy scripts / code.
With this patch - by introducing 'fsl,usdhc-index' - one can configure (get) clock rate corresponding to used controller.
Moreover, as this code is used in the SPL before relocation (and to save space we strip the SPL DTS from clocks and its names) adding separate properties seems to be the best approach here. One also avoids adding clocks DM code to SPL.
Signed-off-by: Lukasz Majewski lukma@denx.de
drivers/mmc/fsl_esdhc.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 3cdfa7f5a6..49a6834a98 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -1401,6 +1401,7 @@ static int fsl_esdhc_probe(struct udevice *dev) fdt_addr_t addr; unsigned int val; struct mmc *mmc;
int usdhc_idx; int ret;
addr = dev_read_addr(dev);
@@ -1513,7 +1514,21 @@ static int fsl_esdhc_probe(struct udevice *dev)
priv->sdhc_clk = clk_get_rate(&priv->per_clk);
} else {
priv->sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK + dev->seq);
/*
* Check for 'fsl,index' DTS property - as one may want to have
* following mmc setup:
NAK, DT is a hardware description. This is encoding a policy, which should not be in DT.
This looks like some reimplementation of SEQ_ALIAS stuff.
* mmc0 = &usdhc2; --> fsl,index = <1>
* mmc1 = &usdhc4; --> fsl,index = <3>
*
* So we do have dev->seq = {0, 1}, which in the below code
* doesn't correspond to correct USDHC clocks.
*
* For that reason a new "fsl,index" property has been
* introduced.
*/
usdhc_idx = dev_read_u32_default(dev, "fsl,usdhc-index",
dev->seq);
if (priv->sdhc_clk <= 0) { dev_err(dev, "Unable to get clk for %s\n", dev->name); return -EINVAL;priv->sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK + usdhc_idx);

On Wed, 2 Jan 2019 02:18:58 +0100 Marek Vasut marex@denx.de wrote:
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
With the current code, it is not possible to assign different than default numbers for mmc controllers.
Several in-tree boards depend on the pre-dm setup, corresponding to following aliases:
mmc0 = &usdhc2; --> fsl,usdhc-index = <1> mmc1 = &usdhc4; --> fsl,usdhc-index = <3>
Without this patch we are either forced to use default aliasing - like:
mmc0 = &usdhc1; mmc1 = &usdhc2; mmc2 = &usdhc3; mmc3 = &usdhc4;
to have the proper clocks setup for the controller. However, such setup is not acceptable for some legacy scripts / code.
With this patch - by introducing 'fsl,usdhc-index' - one can configure (get) clock rate corresponding to used controller.
Moreover, as this code is used in the SPL before relocation (and to save space we strip the SPL DTS from clocks and its names) adding separate properties seems to be the best approach here. One also avoids adding clocks DM code to SPL.
Signed-off-by: Lukasz Majewski lukma@denx.de
drivers/mmc/fsl_esdhc.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 3cdfa7f5a6..49a6834a98 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -1401,6 +1401,7 @@ static int fsl_esdhc_probe(struct udevice *dev) fdt_addr_t addr; unsigned int val; struct mmc *mmc;
int usdhc_idx; int ret;
addr = dev_read_addr(dev);
@@ -1513,7 +1514,21 @@ static int fsl_esdhc_probe(struct udevice *dev) priv->sdhc_clk = clk_get_rate(&priv->per_clk); } else {
priv->sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK +
dev->seq);
/*
* Check for 'fsl,index' DTS property - as one may
want to have
* following mmc setup:
NAK, DT is a hardware description. This is encoding a policy, which should not be in DT.
Please look a few lines up in this file:
mmc0 = &usdhc1; mmc1 = &usdhc2; mmc2 = &usdhc3; mmc3 = &usdhc4;
The fsl_esdhc.c has hardcoded ordering for eMMC devices when setting/getting clock.
If you change aliases on your dts (mmc0 -> usdhc2, etc). Then with a bit of luck your second controller will be initialized with first's one clock value :-). This of course works by chance with default ROM setup.
The problem is that many boards have different mmc ordering (starting with mmc0, which in above scheme is usdhc2 controller. Also mmc1 is the usdhc4 to which eMMC is connected in many boards). Of course this could be changed, but please consider a lot of legacy code pilling up on the customer's side.
The clock index @ (drivers/mmc/fsl_esdhc.c - L1517): mxc_get_clock(MXC_ESDHC_CLK + dev->seq);
Here the dev->seq is 0,1 (with SEQ_ALIAS), which will provide clock values from usdhc1 and usdhc2. However, we use usdhc4 (eMMC) and usdhc2 (SD).
This looks like some reimplementation of SEQ_ALIAS stuff.
No, this is a fix for hardcoded (assumed) clock setup in this driver.
The other option is to provide/port clock stuff from linux (and implement CLK_DM in u-boot at least for this part). However, this will not fix the problem described above (for other boards which use the "legacy" approach).
* mmc0 = &usdhc2; --> fsl,index = <1>
* mmc1 = &usdhc4; --> fsl,index = <3>
*
* So we do have dev->seq = {0, 1}, which in the
below code
* doesn't correspond to correct USDHC clocks.
*
* For that reason a new "fsl,index" property has
been
* introduced.
*/
usdhc_idx = dev_read_u32_default(dev,
"fsl,usdhc-index",
dev->seq);
priv->sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK +
usdhc_idx); if (priv->sdhc_clk <= 0) { dev_err(dev, "Unable to get clk for %s\n", dev->name); return -EINVAL;
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On 1/2/19 11:31 AM, Lukasz Majewski wrote:
On Wed, 2 Jan 2019 02:18:58 +0100 Marek Vasut marex@denx.de wrote:
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
With the current code, it is not possible to assign different than default numbers for mmc controllers.
Several in-tree boards depend on the pre-dm setup, corresponding to following aliases:
mmc0 = &usdhc2; --> fsl,usdhc-index = <1> mmc1 = &usdhc4; --> fsl,usdhc-index = <3>
Without this patch we are either forced to use default aliasing - like:
mmc0 = &usdhc1; mmc1 = &usdhc2; mmc2 = &usdhc3; mmc3 = &usdhc4;
to have the proper clocks setup for the controller. However, such setup is not acceptable for some legacy scripts / code.
With this patch - by introducing 'fsl,usdhc-index' - one can configure (get) clock rate corresponding to used controller.
Moreover, as this code is used in the SPL before relocation (and to save space we strip the SPL DTS from clocks and its names) adding separate properties seems to be the best approach here. One also avoids adding clocks DM code to SPL.
Signed-off-by: Lukasz Majewski lukma@denx.de
drivers/mmc/fsl_esdhc.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 3cdfa7f5a6..49a6834a98 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -1401,6 +1401,7 @@ static int fsl_esdhc_probe(struct udevice *dev) fdt_addr_t addr; unsigned int val; struct mmc *mmc;
int usdhc_idx; int ret;
addr = dev_read_addr(dev);
@@ -1513,7 +1514,21 @@ static int fsl_esdhc_probe(struct udevice *dev) priv->sdhc_clk = clk_get_rate(&priv->per_clk); } else {
priv->sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK +
dev->seq);
/*
* Check for 'fsl,index' DTS property - as one may
want to have
* following mmc setup:
NAK, DT is a hardware description. This is encoding a policy, which should not be in DT.
Please look a few lines up in this file:
mmc0 = &usdhc1; mmc1 = &usdhc2; mmc2 = &usdhc3; mmc3 = &usdhc4;
The fsl_esdhc.c has hardcoded ordering for eMMC devices when setting/getting clock.
If you change aliases on your dts (mmc0 -> usdhc2, etc). Then with a bit of luck your second controller will be initialized with first's one clock value :-). This of course works by chance with default ROM setup.
The problem is that many boards have different mmc ordering (starting with mmc0, which in above scheme is usdhc2 controller. Also mmc1 is the usdhc4 to which eMMC is connected in many boards). Of course this could be changed, but please consider a lot of legacy code pilling up on the customer's side.
The clock index @ (drivers/mmc/fsl_esdhc.c - L1517): mxc_get_clock(MXC_ESDHC_CLK + dev->seq);
Here the dev->seq is 0,1 (with SEQ_ALIAS), which will provide clock values from usdhc1 and usdhc2. However, we use usdhc4 (eMMC) and usdhc2 (SD).
This looks like some reimplementation of SEQ_ALIAS stuff.
No, this is a fix for hardcoded (assumed) clock setup in this driver.
The other option is to provide/port clock stuff from linux (and implement CLK_DM in u-boot at least for this part). However, this will not fix the problem described above (for other boards which use the "legacy" approach).
Well fix the clock code then ? All the other subsystems use the SEQ_ALIAS and DT /aliases node to define ordering, I don't see why FSL SDHC driver should get some special hacky treatment which will only make it difficult to maintain in the future .

This commit defines the TPC70 imx6q board with device tree description.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
arch/arm/dts/imx6q-kp.dts | 227 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 227 insertions(+) create mode 100644 arch/arm/dts/imx6q-kp.dts
diff --git a/arch/arm/dts/imx6q-kp.dts b/arch/arm/dts/imx6q-kp.dts new file mode 100644 index 0000000000..1be8e2e9d5 --- /dev/null +++ b/arch/arm/dts/imx6q-kp.dts @@ -0,0 +1,227 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2018 + * Lukasz Majewski, DENX Software Engineering, lukma@denx.de + * + * SPDX-License-Identifier: GPL-2.0+ or X11 + */ + +/dts-v1/; +#include <dt-bindings/gpio/gpio.h> +#include "imx6q.dtsi" + +/ { + model = "K+P iMX6Q"; + compatible = "kp,imx6-kp", "fsl,imx6"; + + aliases { + mmc0 = &usdhc2; + mmc1 = &usdhc4; + usb1 = &usbh1; + }; + + chosen { + stdout-path = &uart1; + }; + + leds { + compatible = "gpio-leds"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_leds>; + + green { + label = "green"; + gpios = <&gpio3 23 GPIO_ACTIVE_HIGH>; + linux,default-trigger = "gpio"; + default-state = "off"; + }; + + red { + label = "red"; + gpios = <&gpio3 16 GPIO_ACTIVE_HIGH>; + linux,default-trigger = "gpio"; + default-state = "off"; + }; + }; + + memory@10000000 { + reg = <0x10000000 0x40000000>; + }; + + reg_usb_h1_vbus: regulator-usb_h1_vbus { + compatible = "regulator-fixed"; + regulator-name = "usb_h1_vbus"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + gpio = <&gpio3 31 GPIO_ACTIVE_HIGH>; + enable-active-high; + }; +}; + +&fec { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_enet>; + phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>; + phy-reset-duration = <10>; + phy-mode = "rgmii"; + fsl,magic-packet; + fsl,enet-loopback-clk; /* anatop reference clk via PAD loopback */ + fsl,enet-freq = <1>; /* ENET_25MHZ = 0, ENET_50MHZ = 1 */ + /* ENET_100MHZ = 2, ENET_125MHZ = 3 */ + status = "okay"; +}; + +&i2c1 { + clock-frequency = <400000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c1>; + status = "okay"; +}; + +&i2c2 { + clock-frequency = <400000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c2>; + status = "okay"; +}; + +&iomuxc { + pinctrl_enet: enetgrp { + fsl,pins = < + MX6QDL_PAD_ENET_MDIO__ENET_MDIO 0x1b0b0 + MX6QDL_PAD_ENET_MDC__ENET_MDC 0x1b0b0 + MX6QDL_PAD_RGMII_TXC__RGMII_TXC 0x1b0b0 + MX6QDL_PAD_RGMII_TD0__RGMII_TD0 0x1b0b0 + MX6QDL_PAD_RGMII_TD1__RGMII_TD1 0x1b0b0 + MX6QDL_PAD_RGMII_TD2__RGMII_TD2 0x1b0b0 + MX6QDL_PAD_RGMII_TD3__RGMII_TD3 0x1b0b0 + MX6QDL_PAD_RGMII_TX_CTL__RGMII_TX_CTL 0x1b0b0 + MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK 0x1b0b0 + MX6QDL_PAD_RGMII_RXC__RGMII_RXC 0x1b0b0 + MX6QDL_PAD_RGMII_RD0__RGMII_RD0 0x1b0b0 + MX6QDL_PAD_RGMII_RD1__RGMII_RD1 0x1b0b0 + MX6QDL_PAD_RGMII_RD2__RGMII_RD2 0x1b0b0 + MX6QDL_PAD_RGMII_RD3__RGMII_RD3 0x1b0b0 + MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL 0x1b0b0 + MX6QDL_PAD_GPIO_16__ENET_REF_CLK 0x4001b0a8 + MX6QDL_PAD_ENET_CRS_DV__GPIO1_IO25 0x1b0b0 + >; + }; + + pinctrl_leds: gpioledsgrp { + fsl,pins = < + MX6QDL_PAD_EIM_D23__GPIO3_IO23 0x4001b0b0 + MX6QDL_PAD_EIM_D16__GPIO3_IO16 0x4001b0b0 + >; + }; + + pinctrl_i2c1: i2c1grp { + fsl,pins = < + MX6QDL_PAD_CSI0_DAT8__I2C1_SDA 0x4001b8b1 + MX6QDL_PAD_CSI0_DAT9__I2C1_SCL 0x4001b8b1 + >; + }; + + pinctrl_i2c2: i2c2grp { + fsl,pins = < + MX6QDL_PAD_KEY_COL3__I2C2_SCL 0x4001b8b1 + MX6QDL_PAD_KEY_ROW3__I2C2_SDA 0x4001b8b1 + >; + }; + + pinctrl_uart1: uart1grp { + fsl,pins = < + MX6QDL_PAD_SD3_DAT7__UART1_TX_DATA 0x1b0b1 + MX6QDL_PAD_SD3_DAT6__UART1_RX_DATA 0x1b0b1 + >; + u-boot,dm-pre-reloc; + }; + + pinctrl_uart2: uart2grp { + fsl,pins = < + MX6QDL_PAD_EIM_D26__UART2_TX_DATA 0x1b0b1 + MX6QDL_PAD_EIM_D27__UART2_RX_DATA 0x1b0b1 + MX6QDL_PAD_EIM_D28__UART2_CTS_B 0x1b0b1 + MX6QDL_PAD_EIM_D29__UART2_RTS_B 0x1b0b1 + >; + }; + + pinctrl_usbh1: usbh1grp { + fsl,pins = < + MX6QDL_PAD_EIM_D31__GPIO3_IO31 0x1b0b1 + MX6QDL_PAD_ENET_RX_ER__USB_OTG_ID 0x17059 + >; + }; + + pinctrl_usdhc2: usdhc2grp { + fsl,pins = < + MX6QDL_PAD_SD2_CMD__SD2_CMD 0x17019 + MX6QDL_PAD_SD2_CLK__SD2_CLK 0x10019 + MX6QDL_PAD_SD2_DAT0__SD2_DATA0 0x17019 + MX6QDL_PAD_SD2_DAT1__SD2_DATA1 0x17019 + MX6QDL_PAD_SD2_DAT2__SD2_DATA2 0x17019 + MX6QDL_PAD_SD2_DAT3__SD2_DATA3 0x17019 + MX6QDL_PAD_NANDF_CS3__GPIO6_IO16 0x20000 + MX6QDL_PAD_GPIO_4__GPIO1_IO04 0x20000 + >; + u-boot,dm-pre-reloc; + }; + + pinctrl_usdhc4: usdhc4grp { + fsl,pins = < + MX6QDL_PAD_SD4_CMD__SD4_CMD 0x17019 + MX6QDL_PAD_SD4_CLK__SD4_CLK 0x10019 + MX6QDL_PAD_SD4_DAT0__SD4_DATA0 0x17019 + MX6QDL_PAD_SD4_DAT1__SD4_DATA1 0x17019 + MX6QDL_PAD_SD4_DAT2__SD4_DATA2 0x17019 + MX6QDL_PAD_SD4_DAT3__SD4_DATA3 0x17019 + MX6QDL_PAD_SD4_DAT4__SD4_DATA4 0x17019 + MX6QDL_PAD_SD4_DAT5__SD4_DATA5 0x17019 + MX6QDL_PAD_SD4_DAT6__SD4_DATA6 0x17019 + MX6QDL_PAD_SD4_DAT7__SD4_DATA7 0x17019 + >; + u-boot,dm-pre-reloc; + }; +}; + +&uart1 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_uart1>; + u-boot,dm-pre-reloc; + status = "okay"; +}; + +&uart2 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_uart2>; + uart-has-rtscts; +}; + +&usbh1 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_usbh1>; + vbus-supply = <®_usb_h1_vbus>; + status = "okay"; +}; + +&usdhc2 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_usdhc2>; + bus-width = <4>; + cd-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>; + u-boot,dm-pre-reloc; + fsl,usdhc-index = <1>; + status = "okay"; +}; + +&usdhc4 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_usdhc4>; + bus-width = <8>; + non-removable; + no-1-8-v; + keep-power-in-suspend; + u-boot,dm-pre-reloc; + fsl,usdhc-index = <3>; + status = "okay"; +};

On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This commit defines the TPC70 imx6q board with device tree description.
Signed-off-by: Lukasz Majewski lukma@denx.de
Is this pulled from Linux ?

Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This commit defines the TPC70 imx6q board with device tree description.
Signed-off-by: Lukasz Majewski lukma@denx.de
Is this pulled from Linux ?
Yes, it is - the DTS code corresponds to v4.20
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On 1/2/19 9:49 AM, Lukasz Majewski wrote:
Hi Marek,
On 1/2/19 12:37 AM, Lukasz Majewski wrote:
This commit defines the TPC70 imx6q board with device tree description.
Signed-off-by: Lukasz Majewski lukma@denx.de
Is this pulled from Linux ?
Yes, it is - the DTS code corresponds to v4.20
This should be in the commit message, including the commit hash of the commit from which this was taken.

This commit moves the TPC70 to use device model and device tree description in both SPL and u-boot proper.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
arch/arm/mach-imx/mx6/Kconfig | 10 ++ board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c | 174 ------------------------------ board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c | 133 +---------------------- configs/kp_imx6q_tpc_defconfig | 31 +++++- include/configs/kp_imx6q_tpc.h | 21 ---- 5 files changed, 39 insertions(+), 330 deletions(-)
diff --git a/arch/arm/mach-imx/mx6/Kconfig b/arch/arm/mach-imx/mx6/Kconfig index 06c25bae36..b9915e2197 100644 --- a/arch/arm/mach-imx/mx6/Kconfig +++ b/arch/arm/mach-imx/mx6/Kconfig @@ -446,9 +446,19 @@ config TARGET_KP_IMX6Q_TPC select BOARD_EARLY_INIT_F select BOARD_LATE_INIT select DM + select SPL_DM if SPL select DM_THERMAL + select DM_MMC + select DM_ETH + select DM_REGULATOR + select SPL_DM_REGULATOR if SPL + select DM_SERIAL + select DM_I2C + select DM_GPIO + select DM_USB select MX6QDL select SUPPORT_SPL + select SPL_SEPARATE_BSS if SPL imply CMD_DM imply CMD_SPL
diff --git a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c index f0c97ba368..5344ddd560 100644 --- a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c +++ b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c @@ -9,64 +9,17 @@ #include <asm/arch/clock.h> #include <asm/arch/crm_regs.h> #include <asm/arch/imx-regs.h> -#include <asm/arch/iomux.h> #include <asm/arch/mx6-pins.h> #include <asm/arch/sys_proto.h> -#include <asm/gpio.h> -#include <asm/io.h> #include <asm/mach-imx/boot_mode.h> -#include <asm/mach-imx/iomux-v3.h> -#include <asm/mach-imx/mxc_i2c.h> #include <errno.h> -#include <fsl_esdhc.h> -#include <fuse.h> -#include <i2c.h> #include <miiphy.h> -#include <mmc.h> -#include <net.h> -#include <netdev.h> #include <usb.h> #include <usb/ehci-ci.h> #include <led.h>
DECLARE_GLOBAL_DATA_PTR;
-#define ENET_PAD_CTRL \ - (PAD_CTL_PUS_100K_UP | PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm | \ - PAD_CTL_HYS) - -#define I2C_PAD_CTRL \ - (PAD_CTL_PUS_100K_UP | PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm | \ - PAD_CTL_HYS | PAD_CTL_ODE | PAD_CTL_SRE_FAST) - -#define PC MUX_PAD_CTRL(I2C_PAD_CTRL) - -static struct i2c_pads_info kp_imx6q_tpc_i2c_pad_info0 = { - .scl = { - .i2c_mode = MX6Q_PAD_CSI0_DAT9__I2C1_SCL | PC, - .gpio_mode = MX6Q_PAD_CSI0_DAT9__GPIO5_IO27 | PC, - .gp = IMX_GPIO_NR(5, 27) - }, - .sda = { - .i2c_mode = MX6Q_PAD_CSI0_DAT8__I2C1_SDA | PC, - .gpio_mode = MX6Q_PAD_CSI0_DAT8__GPIO5_IO26 | PC, - .gp = IMX_GPIO_NR(5, 26) - } -}; - -static struct i2c_pads_info kp_imx6q_tpc_i2c_pad_info1 = { - .scl = { - .i2c_mode = MX6Q_PAD_KEY_COL3__I2C2_SCL | PC, - .gpio_mode = MX6Q_PAD_KEY_COL3__GPIO4_IO12 | PC, - .gp = IMX_GPIO_NR(4, 12) - }, - .sda = { - .i2c_mode = MX6Q_PAD_KEY_ROW3__I2C2_SDA | PC, - .gpio_mode = MX6Q_PAD_KEY_ROW3__GPIO4_IO13 | PC, - .gp = IMX_GPIO_NR(4, 13) - } -}; - int dram_init(void) { gd->ram_size = imx_ddr_size(); @@ -82,58 +35,6 @@ int overwrite_console(void) return 1; }
-#ifdef CONFIG_FEC_MXC -static iomux_v3_cfg_t const enet_pads[] = { - IOMUX_PADS(PAD_ENET_MDIO__ENET_MDIO | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_ENET_MDC__ENET_MDC | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_TXC__RGMII_TXC | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_TD0__RGMII_TD0 | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_TD1__RGMII_TD1 | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_TD2__RGMII_TD2 | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_TD3__RGMII_TD3 | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_TX_CTL__RGMII_TX_CTL | - MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_ENET_REF_CLK__ENET_TX_CLK | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_RXC__RGMII_RXC | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_RD0__RGMII_RD0 | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_RD1__RGMII_RD1 | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_RD2__RGMII_RD2 | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_RD3__RGMII_RD3 | MUX_PAD_CTRL(ENET_PAD_CTRL)), - IOMUX_PADS(PAD_RGMII_RX_CTL__RGMII_RX_CTL | - MUX_PAD_CTRL(ENET_PAD_CTRL)), - /* AR8031 PHY Reset */ - IOMUX_PADS(PAD_ENET_CRS_DV__GPIO1_IO25 | MUX_PAD_CTRL(NO_PAD_CTRL)), -}; - -static void eth_phy_reset(void) -{ - /* Reset AR8031 PHY */ - gpio_direction_output(IMX_GPIO_NR(1, 25), 0); - mdelay(10); - gpio_set_value(IMX_GPIO_NR(1, 25), 1); - udelay(100); -} - -static int setup_fec_clock(void) -{ - struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR; - - /* set gpr1[21] to select anatop clock */ - clrsetbits_le32(&iomuxc_regs->gpr[1], IOMUXC_GPR1_ENET_CLK_SEL_MASK, - IOMUXC_GPR1_ENET_CLK_SEL_MASK); - - return enable_fec_anatop_clock(0, ENET_50MHZ); -} - -int board_eth_init(bd_t *bis) -{ - SETUP_IOMUX_PADS(enet_pads); - setup_fec_clock(); - eth_phy_reset(); - - return cpu_eth_init(bis); -} - static int ar8031_phy_fixup(struct phy_device *phydev) { unsigned short val; @@ -166,54 +67,6 @@ int board_phy_config(struct phy_device *phydev)
return 0; } -#endif - -#ifdef CONFIG_FSL_ESDHC - -#define USDHC2_CD_GPIO IMX_GPIO_NR(1, 4) -static struct fsl_esdhc_cfg usdhc_cfg[] = { - { USDHC2_BASE_ADDR }, - { USDHC4_BASE_ADDR }, -}; - -int board_mmc_getcd(struct mmc *mmc) -{ - struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv; - - switch (cfg->esdhc_base) { - case USDHC2_BASE_ADDR: - return !gpio_get_value(USDHC2_CD_GPIO); - case USDHC4_BASE_ADDR: - return 1; /* eMMC/uSDHC4 is always present */ - } - - return 0; -} - -int board_mmc_init(bd_t *bis) -{ - int i, ret; - - /* - * According to the board_mmc_init() the following map is done: - * (U-Boot device node) (Physical Port) - * mmc0 micro SD - * mmc2 eMMC - */ - gpio_direction_input(USDHC2_CD_GPIO); - - usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK); - usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK); - - for (i = 0; i < CONFIG_SYS_FSL_USDHC_NUM; i++) { - ret = fsl_esdhc_initialize(bis, &usdhc_cfg[i]); - if (ret) - return ret; - } - - return 0; -} -#endif
#ifdef CONFIG_USB_EHCI_MX6 static void setup_usb(void) @@ -224,30 +77,6 @@ static void setup_usb(void) */ imx_iomux_set_gpr_register(1, 13, 1, 0); } - -int board_usb_phy_mode(int port) -{ - if (port == 1) - return USB_INIT_HOST; - else - return USB_INIT_DEVICE; -} - -int board_ehci_power(int port, int on) -{ - switch (port) { - case 0: - break; - case 1: - gpio_direction_output(IMX_GPIO_NR(3, 31), !!on); - break; - default: - printf("MXC USB port %d not yet supported\n", port); - return -EINVAL; - } - - return 0; -} #endif
int board_early_init_f(void) @@ -269,9 +98,6 @@ int board_init(void) /* Enable eim_slow clocks */ setbits_le32(&mxc_ccm->CCGR6, 0x1 << MXC_CCM_CCGR6_EMI_SLOW_OFFSET);
- setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, &kp_imx6q_tpc_i2c_pad_info0); - setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, &kp_imx6q_tpc_i2c_pad_info1); - return 0; }
diff --git a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c index 07414431f3..25a5e4b9ba 100644 --- a/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c +++ b/board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c @@ -9,30 +9,12 @@ #include <asm/arch/clock.h> #include <asm/arch/crm_regs.h> #include <asm/arch/imx-regs.h> -#include <asm/arch/iomux.h> #include <asm/arch/mx6-ddr.h> -#include <asm/arch/mx6-pins.h> #include <asm/arch/sys_proto.h> -#include <asm/gpio.h> -#include <asm/mach-imx/boot_mode.h> -#include <asm/mach-imx/iomux-v3.h> -#include <asm/mach-imx/mxc_i2c.h> #include <asm/io.h> #include <errno.h> -#include <fuse.h> -#include <fsl_esdhc.h> -#include <i2c.h> -#include <mmc.h> #include <spl.h>
-#define UART_PAD_CTRL \ - (PAD_CTL_PUS_100K_UP | PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm | \ - PAD_CTL_SRE_FAST | PAD_CTL_HYS) - -#define USDHC_PAD_CTRL \ - (PAD_CTL_PUS_47K_UP | PAD_CTL_SPEED_LOW | PAD_CTL_DSE_80ohm | \ - PAD_CTL_SRE_FAST | PAD_CTL_HYS) - DECLARE_GLOBAL_DATA_PTR;
static void ccgr_init(void) @@ -48,60 +30,6 @@ static void ccgr_init(void) writel(0x000003FF, &ccm->CCGR6); }
-/* onboard microSD */ -static iomux_v3_cfg_t const usdhc2_pads[] = { - IOMUX_PADS(PAD_SD2_DAT0__SD2_DATA0 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD2_DAT1__SD2_DATA1 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD2_DAT2__SD2_DATA2 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD2_DAT3__SD2_DATA3 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD2_CLK__SD2_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD2_CMD__SD2_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_NANDF_CS3__GPIO6_IO16 | MUX_PAD_CTRL(NO_PAD_CTRL)), -}; - -/* eMMC */ -static iomux_v3_cfg_t const usdhc4_pads[] = { - IOMUX_PADS(PAD_SD4_DAT0__SD4_DATA0 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD4_DAT1__SD4_DATA1 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD4_DAT2__SD4_DATA2 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD4_DAT3__SD4_DATA3 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD4_DAT4__SD4_DATA4 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD4_DAT5__SD4_DATA5 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD4_DAT6__SD4_DATA6 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD4_DAT7__SD4_DATA7 | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD4_CLK__SD4_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL)), - IOMUX_PADS(PAD_SD4_CMD__SD4_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL)), -}; - -/* SD */ -static void setup_iomux_sd(void) -{ - SETUP_IOMUX_PADS(usdhc2_pads); - SETUP_IOMUX_PADS(usdhc4_pads); -} - -/* UART */ -static iomux_v3_cfg_t const uart1_pads[] = { - IOMUX_PADS(PAD_SD3_DAT7__UART1_TX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL)), - IOMUX_PADS(PAD_SD3_DAT6__UART1_RX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL)), -}; - -static void setup_iomux_uart(void) -{ - SETUP_IOMUX_PADS(uart1_pads); -} - -/* USB */ -static iomux_v3_cfg_t const usb_pads[] = { - IOMUX_PADS(PAD_GPIO_1__USB_OTG_ID | MUX_PAD_CTRL(NO_PAD_CTRL)), - IOMUX_PADS(PAD_EIM_D31__GPIO3_IO31 | MUX_PAD_CTRL(NO_PAD_CTRL)), -}; - -static void setup_iomux_usb(void) -{ - SETUP_IOMUX_PADS(usb_pads); -} - /* DDR3 */ static const struct mx6dq_iomux_ddr_regs mx6_ddr_ioregs = { .dram_sdclk_0 = 0x00000030, @@ -255,59 +183,6 @@ static void spl_dram_init(void) #endif }
-struct fsl_esdhc_cfg usdhc_cfg[] = { - {USDHC2_BASE_ADDR}, - {USDHC4_BASE_ADDR}, -}; - -#define USDHC2_CD_GPIO IMX_GPIO_NR(1, 4) -int board_mmc_getcd(struct mmc *mmc) -{ - struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv; - int ret = 0; - - switch (cfg->esdhc_base) { - case USDHC2_BASE_ADDR: - ret = !gpio_get_value(USDHC2_CD_GPIO); - break; - case USDHC4_BASE_ADDR: - ret = 1; /* eMMC/uSDHC4 is always present */ - break; - } - - return ret; -} - -int board_mmc_init(bd_t *bd) -{ - struct src *psrc = (struct src *)SRC_BASE_ADDR; - unsigned int reg = readl(&psrc->sbmr1) >> 11; - /* - * Upon reading BOOT_CFG register the following map is done: - * Bit 11 and 12 of BOOT_CFG register can determine the current - * mmc port - * 0x1 SD1 - * 0x3 SD4 - */ - - switch (reg & 0x3) { - case 0x1: - SETUP_IOMUX_PADS(usdhc2_pads); - usdhc_cfg[0].esdhc_base = USDHC2_BASE_ADDR; - usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK); - gd->arch.sdhc_clk = usdhc_cfg[0].sdhc_clk; - break; - case 0x3: - SETUP_IOMUX_PADS(usdhc4_pads); - usdhc_cfg[0].esdhc_base = USDHC4_BASE_ADDR; - usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK); - gd->arch.sdhc_clk = usdhc_cfg[0].sdhc_clk; - break; - } - - return fsl_esdhc_initialize(bd, &usdhc_cfg[0]); -} - void board_boot_order(u32 *spl_boot_list) { u32 boot_device = spl_boot_device(); @@ -339,9 +214,8 @@ void board_init_f(ulong dummy) /* setup GP timer */ timer_init();
- setup_iomux_sd(); - setup_iomux_uart(); - setup_iomux_usb(); + /* Early - pre reloc - driver model setup */ + spl_early_init();
/* UART clocks enabled and gd valid - init serial console */ preloader_console_init(); @@ -351,7 +225,4 @@ void board_init_f(ulong dummy)
/* Clear the BSS. */ memset(__bss_start, 0, __bss_end - __bss_start); - - /* load/boot image from boot device */ - board_init_r(NULL, 0); } diff --git a/configs/kp_imx6q_tpc_defconfig b/configs/kp_imx6q_tpc_defconfig index 5ebbe1dc7c..c302749815 100644 --- a/configs/kp_imx6q_tpc_defconfig +++ b/configs/kp_imx6q_tpc_defconfig @@ -4,19 +4,24 @@ CONFIG_SYS_TEXT_BASE=0x17800000 CONFIG_SPL_GPIO_SUPPORT=y CONFIG_SPL_LIBCOMMON_SUPPORT=y CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_SYS_MALLOC_F_LEN=0x2200 CONFIG_MX6_DDRCAL=y CONFIG_TARGET_KP_IMX6Q_TPC=y CONFIG_SPL_MMC_SUPPORT=y CONFIG_SPL_SERIAL_SUPPORT=y CONFIG_SPL=y CONFIG_DISTRO_DEFAULTS=y -CONFIG_NR_DRAM_BANKS=1 +CONFIG_TPL_SYS_MALLOC_F_LEN=0x400 CONFIG_FIT=y CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=arch/arm/mach-imx/spl_sd.cfg" +CONFIG_SD_BOOT=y CONFIG_BOOTDELAY=3 # CONFIG_USE_BOOTCOMMAND is not set CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE=y CONFIG_SPL_RAW_IMAGE_SUPPORT=y +# CONFIG_TPL_BANNER_PRINT is not set +CONFIG_SPL_PAYLOAD="u-boot.img" +CONFIG_SPL_POWER_SUPPORT=y CONFIG_SPL_WATCHDOG_SUPPORT=y CONFIG_AUTOBOOT_KEYED=y CONFIG_AUTOBOOT_STOP_STR="." @@ -26,19 +31,37 @@ CONFIG_AUTOBOOT_STOP_STR="." CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y +# CONFIG_CMD_PINMUX is not set CONFIG_CMD_USB=y CONFIG_CMD_CACHE=y CONFIG_CMD_TIME=y CONFIG_CMD_EXT4_WRITE=y # CONFIG_ISO_PARTITION is not set # CONFIG_EFI_PARTITION is not set +CONFIG_OF_CONTROL=y +CONFIG_SPL_OF_CONTROL=y +CONFIG_DEFAULT_DEVICE_TREE="imx6q-kp" +CONFIG_OF_SPL_REMOVE_PROPS="clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents interrupts clocks dmas dma-names" CONFIG_ENV_IS_IN_MMC=y +# CONFIG_BLOCK_CACHE is not set +CONFIG_I2C_SET_DEFAULT_BUS_NUM=y +CONFIG_SYS_I2C_MXC=y +CONFIG_SYS_I2C_MXC_I2C1=y +CONFIG_SYS_I2C_MXC_I2C2=y +CONFIG_LED=y +CONFIG_LED_GPIO=y CONFIG_PHYLIB=y CONFIG_PHY_ATHEROS=y CONFIG_FEC_MXC=y CONFIG_MII=y +CONFIG_PINCTRL=y +CONFIG_SPL_PINCTRL=y +CONFIG_PINCTRL_IMX6=y +CONFIG_DM_REGULATOR_FIXED=y +CONFIG_SPL_DM_REGULATOR_FIXED=y +# CONFIG_REQUIRE_SERIAL_CONSOLE is not set +# CONFIG_TPL_SERIAL_PRESENT is not set +CONFIG_MXC_UART=y CONFIG_IMX_THERMAL=y CONFIG_USB=y -CONFIG_USB_STORAGE=y -CONFIG_IMX_WATCHDOG=y -CONFIG_OF_LIBFDT=y +# CONFIG_SPL_DM_USB is not set diff --git a/include/configs/kp_imx6q_tpc.h b/include/configs/kp_imx6q_tpc.h index f26b18442b..feed9af5f7 100644 --- a/include/configs/kp_imx6q_tpc.h +++ b/include/configs/kp_imx6q_tpc.h @@ -25,10 +25,6 @@ #define CONFIG_SYS_MALLOC_LEN (4 * SZ_1M)
/* FEC ethernet */ -#define IMX_FEC_BASE ENET_BASE_ADDR -#define CONFIG_FEC_XCV_TYPE RGMII -#define CONFIG_ETHPRIME "FEC" -#define CONFIG_FEC_MXC_PHYADDR 0 #define CONFIG_ARP_TIMEOUT 200UL
/* Fuses */ @@ -36,27 +32,10 @@ #define CONFIG_MXC_OCOTP #endif
-/* I2C Configs */ -#define CONFIG_SYS_I2C -#define CONFIG_SYS_I2C_MXC -#define CONFIG_SYS_I2C_MXC_I2C1 /* enable I2C bus 1 */ -#define CONFIG_SYS_I2C_MXC_I2C2 /* enable I2C bus 2 */ -#define CONFIG_SYS_I2C_SPEED 100000 - -/* MMC Configs */ #define CONFIG_FSL_ESDHC -#define CONFIG_FSL_USDHC -#define CONFIG_SYS_FSL_ESDHC_ADDR 0 -#define CONFIG_SYS_FSL_USDHC_NUM 2 #define CONFIG_SYS_MMC_ENV_DEV 1 /* 0 = SDHC2, 1 = SDHC4 (eMMC) */ #define CONFIG_SUPPORT_EMMC_BOOT
-/* UART */ -#define CONFIG_MXC_UART -#define CONFIG_MXC_UART_BASE UART1_BASE -#define CONFIG_CONS_INDEX 1 -#define CONFIG_BAUDRATE 115200 - /* USB Configs */ #ifdef CONFIG_CMD_USB #define CONFIG_EHCI_HCD_INIT_AFTER_RESET

On Tue, 1 Jan 2019 at 16:38, Lukasz Majewski lukma@denx.de wrote:
This commit moves the TPC70 to use device model and device tree description in both SPL and u-boot proper.
Signed-off-by: Lukasz Majewski lukma@denx.de
arch/arm/mach-imx/mx6/Kconfig | 10 ++ board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c | 174 ------------------------------ board/k+p/kp_imx6q_tpc/kp_imx6q_tpc_spl.c | 133 +---------------------- configs/kp_imx6q_tpc_defconfig | 31 +++++- include/configs/kp_imx6q_tpc.h | 21 ---- 5 files changed, 39 insertions(+), 330 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
participants (3)
-
Lukasz Majewski
-
Marek Vasut
-
Simon Glass