* [U-Boot] [PATCH 2/3] ARM: imx: Fix bmode detection from grp10
[not found] ` <20191016132753.1639-2-ch@denx.de>
@ 2019-10-16 13:31 ` Fabio Estevam
0 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2019-10-16 13:31 UTC (permalink / raw)
To: u-boot
Hi Claudius,
On Wed, Oct 16, 2019 at 10:28 AM Claudius Heine <ch@denx.de> wrote:
>
> imx6_is_bmode_from_gpr10 always returns false, because
> IMX6_SRC_GPR10_BMODE is 1<<28 and gets casted to u8 on return.
>
> Instead cast them to a boolean value before return of the function.
>
> Signed-off-by: Claudius Heine <ch@denx.de>
> ---
> arch/arm/include/asm/mach-imx/sys_proto.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h
> index 96530e563e..b6105d0798 100644
> --- a/arch/arm/include/asm/mach-imx/sys_proto.h
> +++ b/arch/arm/include/asm/mach-imx/sys_proto.h
> @@ -91,7 +91,7 @@ enum imx6_bmode {
>
> static inline u8 imx6_is_bmode_from_gpr10(void)
Would it make more sense to convert imx6_is_bmode_from_gpr10() to
return bool instead of u8?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2 1/3] ARM: imx: Rename imx6_is_bmode_from_gpr9 to imx6_is_bmode_from_gpr10
[not found] ` <20191017085145.8158-1-ch@denx.de>
@ 2019-10-17 12:19 ` Fabio Estevam
[not found] ` <20191017085145.8158-2-ch@denx.de>
1 sibling, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2019-10-17 12:19 UTC (permalink / raw)
To: u-boot
On Thu, Oct 17, 2019 at 5:52 AM Claudius Heine <ch@denx.de> wrote:
>
> The only register used in that function is gpr10, which is used to store
> the flag. So naming it after this makes sense.
>
> Signed-off-by: Claudius Heine <ch@denx.de>
Reviewed-by: Fabio Estevam <festevam@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 3/3] ARM: imx: Use IMX6_SRC_GPR10_BMODE instead of magic number
[not found] ` <20191016132753.1639-3-ch@denx.de>
@ 2019-10-17 12:20 ` Fabio Estevam
0 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2019-10-17 12:20 UTC (permalink / raw)
To: u-boot
On Wed, Oct 16, 2019 at 10:28 AM Claudius Heine <ch@denx.de> wrote:
>
> Signed-off-by: Claudius Heine <ch@denx.de>
Reviewed-by: Fabio Estevam <festevam@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2 2/3] ARM: imx: Fix bmode detection from grp10
[not found] ` <20191017085145.8158-2-ch@denx.de>
@ 2019-10-17 12:20 ` Fabio Estevam
0 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2019-10-17 12:20 UTC (permalink / raw)
To: u-boot
On Thu, Oct 17, 2019 at 5:52 AM Claudius Heine <ch@denx.de> wrote:
>
> imx6_is_bmode_from_gpr10 always returns false, because
> IMX6_SRC_GPR10_BMODE is 1<<28 and gets casted to u8 on return.
>
> This changes the return type to bool in order to cast correctly.
>
> Signed-off-by: Claudius Heine <ch@denx.de>
Reviewed-by: Fabio Estevam <festevam@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/3] ARM: imx: Rename imx6_is_bmode_from_gpr9 to imx6_is_bmode_from_gpr10
[not found] <20191016132753.1639-1-ch@denx.de>
` (2 preceding siblings ...)
[not found] ` <20191016132753.1639-3-ch@denx.de>
@ 2019-10-18 9:29 ` Harald Seiler
2019-10-21 8:45 ` Claudius Heine
3 siblings, 1 reply; 6+ messages in thread
From: Harald Seiler @ 2019-10-18 9:29 UTC (permalink / raw)
To: u-boot
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);
--
Harald
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: hws at denx.de
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/3] ARM: imx: Rename imx6_is_bmode_from_gpr9 to imx6_is_bmode_from_gpr10
2019-10-18 9:29 ` [U-Boot] [PATCH 1/3] ARM: imx: Rename imx6_is_bmode_from_gpr9 to imx6_is_bmode_from_gpr10 Harald Seiler
@ 2019-10-21 8:45 ` Claudius Heine
0 siblings, 0 replies; 6+ messages in thread
From: Claudius Heine @ 2019-10-21 8:45 UTC (permalink / raw)
To: u-boot
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);
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch at denx.de
PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
Keyserver: hkp://pool.sks-keyservers.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191021/523d00a0/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-21 8:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20191016132753.1639-1-ch@denx.de>
[not found] ` <20191016132753.1639-2-ch@denx.de>
2019-10-16 13:31 ` [U-Boot] [PATCH 2/3] ARM: imx: Fix bmode detection from grp10 Fabio Estevam
[not found] ` <20191017085145.8158-1-ch@denx.de>
2019-10-17 12:19 ` [U-Boot] [PATCH v2 1/3] ARM: imx: Rename imx6_is_bmode_from_gpr9 to imx6_is_bmode_from_gpr10 Fabio Estevam
[not found] ` <20191017085145.8158-2-ch@denx.de>
2019-10-17 12:20 ` [U-Boot] [PATCH v2 2/3] ARM: imx: Fix bmode detection from grp10 Fabio Estevam
[not found] ` <20191016132753.1639-3-ch@denx.de>
2019-10-17 12:20 ` [U-Boot] [PATCH 3/3] ARM: imx: Use IMX6_SRC_GPR10_BMODE instead of magic number Fabio Estevam
2019-10-18 9:29 ` [U-Boot] [PATCH 1/3] ARM: imx: Rename imx6_is_bmode_from_gpr9 to imx6_is_bmode_from_gpr10 Harald Seiler
2019-10-21 8:45 ` Claudius Heine
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.