Re: [U-Boot] [PATCH 1/3] ARM: imx: Rename imx6_is_bmode_from_gpr9 to imx6_is_bmode_from_gpr10
Hi Claudius,
On Wed, 2019-10-16 at 15:27 +0200, Claudius Heine wrote:
The only register used in that function is gpr10, which is used to store the flag. So naming it after this makes sense.
I think the old name had a reason: The _value_ for bmode is stored in GPR9 if this function returns true. But I agree that the name is a bit confusing.
Maybe it would make more sense to call it imx6_is_bmode_from_gpr, without any specific register name and add a comment documenting that if bit (GPR10 & IMX6_SRC_GPR10_BMODE) is set, the bmode value is stored in GPR9?
Signed-off-by: Claudius Heine ch@denx.de
arch/arm/include/asm/mach-imx/sys_proto.h | 2 +- arch/arm/mach-imx/init.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h index 4925dd7894..96530e563e 100644 --- a/arch/arm/include/asm/mach-imx/sys_proto.h +++ b/arch/arm/include/asm/mach-imx/sys_proto.h @@ -89,7 +89,7 @@ enum imx6_bmode { IMX6_BMODE_NAND_MAX = 0xf, };
-static inline u8 imx6_is_bmode_from_gpr9(void) +static inline u8 imx6_is_bmode_from_gpr10(void) { return readl(&src_base->gpr10) & IMX6_SRC_GPR10_BMODE; } diff --git a/arch/arm/mach-imx/init.c b/arch/arm/mach-imx/init.c index b8d8d12372..b2e72d0dbd 100644 --- a/arch/arm/mach-imx/init.c +++ b/arch/arm/mach-imx/init.c @@ -118,7 +118,7 @@ void boot_mode_apply(unsigned cfg_val) #if defined(CONFIG_MX6) u32 imx6_src_get_boot_mode(void) {
- if (imx6_is_bmode_from_gpr9())
- if (imx6_is_bmode_from_gpr10()) return readl(&src_base->gpr9); else return readl(&src_base->sbmr1);
Hi Harald,
On 18/10/2019 11.29, Harald Seiler wrote:
Hi Claudius,
On Wed, 2019-10-16 at 15:27 +0200, Claudius Heine wrote:
The only register used in that function is gpr10, which is used to store the flag. So naming it after this makes sense.
I think the old name had a reason: The _value_ for bmode is stored in GPR9 if this function returns true. But I agree that the name is a bit confusing.
Hmm.. imx6_is_bmode_from_gprX is only used once and where it is used direct register access happens anyway, so maybe we could just remove it and do it directly in boot_mode_apply.
I will do that in v3 if its ok.
regards, Claudius
Maybe it would make more sense to call it imx6_is_bmode_from_gpr, without any specific register name and add a comment documenting that if bit (GPR10 & IMX6_SRC_GPR10_BMODE) is set, the bmode value is stored in GPR9?
Signed-off-by: Claudius Heine ch@denx.de
arch/arm/include/asm/mach-imx/sys_proto.h | 2 +- arch/arm/mach-imx/init.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h index 4925dd7894..96530e563e 100644 --- a/arch/arm/include/asm/mach-imx/sys_proto.h +++ b/arch/arm/include/asm/mach-imx/sys_proto.h @@ -89,7 +89,7 @@ enum imx6_bmode { IMX6_BMODE_NAND_MAX = 0xf, };
-static inline u8 imx6_is_bmode_from_gpr9(void) +static inline u8 imx6_is_bmode_from_gpr10(void) { return readl(&src_base->gpr10) & IMX6_SRC_GPR10_BMODE; } diff --git a/arch/arm/mach-imx/init.c b/arch/arm/mach-imx/init.c index b8d8d12372..b2e72d0dbd 100644 --- a/arch/arm/mach-imx/init.c +++ b/arch/arm/mach-imx/init.c @@ -118,7 +118,7 @@ void boot_mode_apply(unsigned cfg_val) #if defined(CONFIG_MX6) u32 imx6_src_get_boot_mode(void) {
- if (imx6_is_bmode_from_gpr9())
- if (imx6_is_bmode_from_gpr10()) return readl(&src_base->gpr9); else return readl(&src_base->sbmr1);
participants (2)
-
Claudius Heine -
Harald Seiler