Hi Jagan,
Love this patch! This was long overdue.
On 01/27/2017 07:12 AM, Jagan Teki wrote:
> Use meaningful meacros IMX6_BMODE_*, instead of numerical
> number in boot mode detection code.
s/meacros/macros/
>
> Cc: Stefano Babic <sba...@denx.de>
> Cc: Tim Harvey <thar...@gateworks.com>
> Signed-off-by: Jagan Teki <ja...@openedev.com>
> ---
> arch/arm/imx-common/spl.c | 39
> ++++++++++++++++++-----------
> arch/arm/include/asm/imx-common/sys_proto.h | 34 +++++++++++++++++++++++++
> 2 files changed, 58 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/imx-common/spl.c b/arch/arm/imx-common/spl.c
> index fc3704b..38789b2 100644
> --- a/arch/arm/imx-common/spl.c
> +++ b/arch/arm/imx-common/spl.c
> @@ -30,39 +30,48 @@ u32 spl_boot_device(void)
> if ((((bmode >> 24) & 0x03) == 0x01) || /* Serial Downloader */
> (imx6_is_bmode_from_gpr9() && (reg == 1)))
> return BOOT_DEVICE_UART;
> +
> /* BOOT_CFG1[7:4] - see IMX6DQRM Table 8-8 */
> - switch ((reg & 0x000000FF) >> 4) {
> + switch ((reg & IMX6_BMODE_MASK) >> IMX6_BMODE_SHIFT) {
> /* EIM: See 8.5.1, Table 8-9 */
> - case 0x0:
> + case IMX6_BMODE_EMI:
> /* BOOT_CFG1[3]: NOR/OneNAND Selection */
> - if ((reg & 0x00000008) >> 3)
> + switch ((reg & IMX6_BMODE_EMI_MASK) >> IMX6_BMODE_EMI_SHIFT) {
> + case IMX6_BMODE_ONENAND:
> return BOOT_DEVICE_ONENAND;
> - else
> + case IMX6_BMODE_NOR:
> return BOOT_DEVICE_NOR;
> - break;
> + }
> /* SATA: See 8.5.4, Table 8-20 */
> - case 0x2:
> + case IMX6_BMODE_SATA:
> return BOOT_DEVICE_SATA;
> /* Serial ROM: See 8.5.5.1, Table 8-22 */
> - case 0x3:
> + case IMX6_BMODE_SERIAL:
> /* BOOT_CFG4[2:0] */
> - switch ((reg & 0x07000000) >> 24) {
> - case 0x0 ... 0x4:
> + switch ((reg & IMX6_BMODE_SERIAL_MASK) >>
> + IMX6_BMODE_SERIAL_SHIFT) {
> + case IMX6_BMODE_ECSPI1:
> + case IMX6_BMODE_ECSPI2:
> + case IMX6_BMODE_ECSPI3:
> + case IMX6_BMODE_ECSPI4:
> + case IMX6_BMODE_ECSPI5:
> return BOOT_DEVICE_SPI;
> - case 0x5 ... 0x7:
> + case IMX6_BMODE_I2C1:
> + case IMX6_BMODE_I2C2:
> + case IMX6_BMODE_I2C3:
> return BOOT_DEVICE_I2C;
> }
> break;
> /* SD/eSD: 8.5.3, Table 8-15 */
> - case 0x4:
> - case 0x5:
> + case IMX6_BMODE_SD:
> + case IMX6_BMODE_ESD:
> return BOOT_DEVICE_MMC1;
> /* MMC/eMMC: 8.5.3 */
> - case 0x6:
> - case 0x7:
> + case IMX6_BMODE_MMC:
> + case IMX6_BMODE_EMMC:
> return BOOT_DEVICE_MMC1;
> /* NAND Flash: 8.5.2, Table 8-10 */
> - case 0x8:
> + case IMX6_BMODE_NAND:
> return BOOT_DEVICE_NAND;
> }
> return BOOT_DEVICE_NONE;
> diff --git a/arch/arm/include/asm/imx-common/sys_proto.h
> b/arch/arm/include/asm/imx-common/sys_proto.h
> index 99e3869..d0cf3f1 100644
> --- a/arch/arm/include/asm/imx-common/sys_proto.h
> +++ b/arch/arm/include/asm/imx-common/sys_proto.h
> @@ -42,6 +42,40 @@
> #ifdef CONFIG_MX6
> #define IMX6_SRC_GPR10_BMODE BIT(28)
>
> +#define IMX6_BMODE_MASK GENMASK(7, 0)
This is a number (4), not a mask:
> +#define IMX6_BMODE_SHIFT BIT(2)
> +#define IMX6_BMODE_EMI_MASK BIT(3)
Ditto (number, not mask):
> +#define IMX6_BMODE_EMI_SHIFT GENMASK(1, 0)
Since there's also a "serial download mode", I'd prefer that these
say "SERIAL_NOR" to avoid confusion.
> +#define IMX6_BMODE_SERIAL_MASK GENMASK(26, 24)
> +#define IMX6_BMODE_SERIAL_SHIFT GENMASK(4, 3)
> +
I'd prefer macros for these because they'd show the
values directly, making a comparison with the RM
easier.
> +enum imx6_bmode_serial {
> + IMX6_BMODE_ECSPI1,
> + IMX6_BMODE_ECSPI2,
> + IMX6_BMODE_ECSPI3,
> + IMX6_BMODE_ECSPI4,
> + IMX6_BMODE_ECSPI5,
> + IMX6_BMODE_I2C1,
> + IMX6_BMODE_I2C2,
> + IMX6_BMODE_I2C3,
> +};
> +
> +enum imx6_bmode_emi {
> + IMX6_BMODE_ONENAND,
> + IMX6_BMODE_NOR,
> +};
> +
> +enum imx6_bmode {
> + IMX6_BMODE_EMI,
Especially when doing things like this:
> + IMX6_BMODE_SATA = 0x2,
> + IMX6_BMODE_SERIAL,
> + IMX6_BMODE_SD,
> + IMX6_BMODE_ESD,
> + IMX6_BMODE_MMC,
> + IMX6_BMODE_EMMC,
> + IMX6_BMODE_NAND,
> +};
> +
> static inline u8 imx6_is_bmode_from_gpr9(void)
> {
> struct src *psrc = (struct src *)SRC_BASE_ADDR;
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
