> Subject: Re: [PATCH 09/22] imx8m: update imx-regs for i.MX8MM > > On 09.08.19 06:15, Peng Fan wrote: > > i.MX8MM has similar architecture with i.MX8MQ, but it has totally > > different PLL design and some register layout change. > > > > Note: Some registers in this file are not updated because not used now. > > I think this is a bad idea. There seem to be many differences in the register > addresses between i.MX8M and i.MX8MM and you are leaving the values for > i.MX8M to be set also when i.MX8MM is used. > > This might not hurt now when those things are not used. But as soon as > someone (like me) uses some more peripherals, they will run into serious > problems because certain register base addresses are wrong. > > You should at least disable the unused macros for i.MX8MM, so we will get a > build error instead of using wrong address values without noticing. Or even > better go through the values and fix them for i.MX8MM. > Maybe it would be even cleaner to add a separate file for i.MX8MM if they > differ too much.
The final goal should be to drop the regs file, since we are moving to use DM/OF. Let me think about the issues you raised. Thanks, Peng. > > > > > Signed-off-by: Peng Fan <peng....@nxp.com> > > --- > > arch/arm/include/asm/arch-imx8m/imx-regs.h | 75 > ++++++++++++++++++++++++++++-- > > 1 file changed, 71 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm/include/asm/arch-imx8m/imx-regs.h > b/arch/arm/include/asm/arch-imx8m/imx-regs.h > > index 68666a535b..a5be2e85da 100644 > > --- a/arch/arm/include/asm/arch-imx8m/imx-regs.h > > +++ b/arch/arm/include/asm/arch-imx8m/imx-regs.h > > @@ -10,8 +10,8 @@ > > > > #include <asm/mach-imx/regs-lcdif.h> > > > > -#define ROM_VERSION_A0 0x800 > > -#define ROM_VERSION_B0 0x83C > > +#define ROM_VERSION_A0 IS_ENABLED(CONFIG_IMX8MQ) ? > 0x800 : 0x800 > > +#define ROM_VERSION_B0 IS_ENABLED(CONFIG_IMX8MQ) ? > 0x83C : 0x800 > > > > #define M4_BOOTROM_BASE_ADDR 0x007E0000 > > > > @@ -93,7 +93,11 @@ > > #define SEMAPHOR_HS_BASE_ADDR 0x30AC0000 > > #define USDHC1_BASE_ADDR 0x30B40000 > > #define USDHC2_BASE_ADDR 0x30B50000 > > +#ifdef CONFIG_IMX8MM > > +#define USDHC3_BASE_ADDR 0x30B60000 > > +#else > > #define MIPI_CS2_BASE_ADDR 0x30B60000 > > +#endif > > #define MIPI_CSI_PHY2_BASE_ADDR 0x30B70000 > > #define CSI2_BASE_ADDR 0x30B80000 > > #define QSPI0_BASE_ADDR 0x30BB0000 > > @@ -120,7 +124,8 @@ > > #define USB1_PHY_BASE_ADDR 0x381F0000 > > #define USB2_PHY_BASE_ADDR 0x382F0000 > > > > -#define MXS_LCDIF_BASE LCDIF_BASE_ADDR > > +#define MXS_LCDIF_BASE is_enable(CONFIG_IMX8MQ) ? \ > > + 0x30320000 : 0x32e00000 > > > > #define SRC_IPS_BASE_ADDR 0x30390000 > > #define SRC_DDRC_RCR_ADDR 0x30391000 > > @@ -149,6 +154,9 @@ > > #define SRC_DDR1_RCR_CORE_RESET_N_MASK BIT(1) > > #define SRC_DDR1_RCR_PRESET_N_MASK BIT(0) > > > > +#define IOMUXC_GPR_GPR1_GPR_ENET1_TX_CLK_SEL_MASK 0x2000u > > +#define IOMUXC_GPR_GPR1_GPR_ENET1_TX_CLK_SEL_SHIFT 13 > > + > > struct iomuxc_gpr_base_regs { > > u32 gpr[47]; > > }; > > @@ -205,6 +213,7 @@ struct fuse_bank1_regs { > > u32 rsvd3[3]; > > }; > > > > +#ifdef CONFIG_IMX8MQ > > struct anamix_pll { > > u32 audio_pll1_cfg0; > > u32 audio_pll1_cfg1; > > @@ -239,6 +248,60 @@ struct anamix_pll { > > u32 frac_pllout_div_cfg; > > u32 sscg_pllout_div_cfg; > > }; > > +#else > > +struct anamix_pll { > > + u32 audio_pll1_gnrl_ctl; > > + u32 audio_pll1_fdiv_ctl0; > > + u32 audio_pll1_fdiv_ctl1; > > + u32 audio_pll1_sscg_ctl; > > + u32 audio_pll1_mnit_ctl; > > + u32 audio_pll2_gnrl_ctl; > > + u32 audio_pll2_fdiv_ctl0; > > + u32 audio_pll2_fdiv_ctl1; > > + u32 audio_pll2_sscg_ctl; > > + u32 audio_pll2_mnit_ctl; > > + u32 video_pll1_gnrl_ctl; > > + u32 video_pll1_fdiv_ctl0; > > + u32 video_pll1_fdiv_ctl1; > > + u32 video_pll1_sscg_ctl; > > + u32 video_pll1_mnit_ctl; > > + u32 reserved[5]; > > + u32 dram_pll_gnrl_ctl; > > + u32 dram_pll_fdiv_ctl0; > > + u32 dram_pll_fdiv_ctl1; > > + u32 dram_pll_sscg_ctl; > > + u32 dram_pll_mnit_ctl; > > + u32 gpu_pll_gnrl_ctl; > > + u32 gpu_pll_div_ctl; > > + u32 gpu_pll_locked_ctl1; > > + u32 gpu_pll_mnit_ctl; > > + u32 vpu_pll_gnrl_ctl; > > + u32 vpu_pll_div_ctl; > > + u32 vpu_pll_locked_ctl1; > > + u32 vpu_pll_mnit_ctl; > > + u32 arm_pll_gnrl_ctl; > > + u32 arm_pll_div_ctl; > > + u32 arm_pll_locked_ctl1; > > + u32 arm_pll_mnit_ctl; > > + u32 sys_pll1_gnrl_ctl; > > + u32 sys_pll1_div_ctl; > > + u32 sys_pll1_locked_ctl1; > > + u32 reserved2[24]; > > + u32 sys_pll1_mnit_ctl; > > + u32 sys_pll2_gnrl_ctl; > > + u32 sys_pll2_div_ctl; > > + u32 sys_pll2_locked_ctl1; > > + u32 sys_pll2_mnit_ctl; > > + u32 sys_pll3_gnrl_ctl; > > + u32 sys_pll3_div_ctl; > > + u32 sys_pll3_locked_ctl1; > > + u32 sys_pll3_mnit_ctl; > > + u32 anamix_misc_ctl; > > + u32 anamix_clk_mnit_ctl; > > + u32 reserved3[437]; > > + u32 digprog; > > +}; > > +#endif > > > > struct fuse_bank9_regs { > > u32 mac_addr0; > > @@ -258,11 +321,13 @@ struct src { > > u32 usbophy2_rcr; > > u32 mipiphy_rcr; > > u32 pciephy_rcr; > > + /* Exits on i.MX8MQ */ > > u32 hdmi_rcr; > > u32 disp_rcr; > > u32 reserved2[2]; > > u32 gpu_rcr; > > u32 vpu_rcr; > > + /* The following four exits on i.MX8MQ */ > > u32 pcie2_rcr; > > u32 mipiphy1_rcr; > > u32 mipiphy2_rcr; > > @@ -285,6 +350,7 @@ struct src { > > u32 gpr10; > > u32 reserved5[985]; > > u32 ddr1_rcr; > > + /* Exist on i.MX8MQ */ > > u32 ddr2_rcr; > > }; > > > > @@ -459,7 +525,8 @@ struct bootrom_sw_info { > > u32 reserved_3[3]; > > }; > > > > -#define ROM_SW_INFO_ADDR_B0 0x00000968 > > +#define ROM_SW_INFO_ADDR_B0 (IS_ENABLED(CONFIG_IMX8MQ) ? > 0x00000968 :\ > > + 0x000009e8) > > #define ROM_SW_INFO_ADDR_A0 0x000009e8 > > > > #define ROM_SW_INFO_ADDR is_soc_rev(CHIP_REV_1_0) ? \ > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot