On 12/06/2017 02:19 AM, Yangbo Lu wrote: > For LS1012ARDB RevD and later versions, the I2C reading for DIP > switch setting had been no longer reliable since the board was > reworked. This patch is to add hwconfig support to enable/disable
I think this message is not accurate. How about saying "I2C reading for DIP switch setting is not reliable for LS1012ARDB rev D and later revisions."? > eSDHC1 manually for these boards. Also let kernel decide status > of esdhc0. I see you dropped fixing up esdhc0. What do you mean "let kernel decide"? How? > > Signed-off-by: Yangbo Lu <yangbo...@nxp.com> > --- > Changes for v2: > - Just used hwconfig() instead of getenv() and hwconfig_f(). > Changes for v3: > - only applied hwconfig support for RevD and later versions. > --- > board/freescale/ls1012ardb/ls1012ardb.c | 51 > +++++++++++++++++++++------------ > include/configs/ls1012ardb.h | 4 +++ > 2 files changed, 36 insertions(+), 19 deletions(-) > > diff --git a/board/freescale/ls1012ardb/ls1012ardb.c > b/board/freescale/ls1012ardb/ls1012ardb.c > index c6c1c71202..b73c7d6639 100644 > --- a/board/freescale/ls1012ardb/ls1012ardb.c > +++ b/board/freescale/ls1012ardb/ls1012ardb.c > @@ -132,39 +132,52 @@ int board_init(void) > > int esdhc_status_fixup(void *blob, const char *compat) > { > - char esdhc0_path[] = "/soc/esdhc@1560000"; > char esdhc1_path[] = "/soc/esdhc@1580000"; > + u8 in1; I think you can reuse "io". Not a big deal though. > u8 io = 0; > u8 mux_sdhc2; > - > - do_fixup_by_path(blob, esdhc0_path, "status", "okay", > - sizeof("okay"), 1); > + bool sdhc2_en = false; > > i2c_set_bus_num(0); > > - /* > - * The I2C IO-expander for mux select is used to control the muxing > - * of various onboard interfaces. > - * > - * IO1[3:2] indicates SDHC2 interface demultiplexer select lines. > - * 00 - SDIO wifi > - * 01 - GPIO (to Arduino) > - * 10 - eMMC Memory > - * 11 - SPI > - */ > - if (i2c_read(I2C_MUX_IO1_ADDR, 0, 1, &io, 1) < 0) { > + if (i2c_read(I2C_MUX_IO1_ADDR, 1, 1, &in1, 1) < 0) { It would be helpful to use a named macro instead of "1" here as the register address. It is not obvious that you are reading a revision number. You can also put a comment. You said I2C reading the DIP is not reliable. Is this reading reliable? > printf("Error reading i2c boot information!\n"); > - return 0; /* Don't want to hang() on this error */ > + return 0; What does I2C failure mean here? Returning 0 will cause the code keep going in fdt_fixup_esdhc(). > + } > + > + if ((in1 & (~__SW_REV_MASK)) <= __SW_REV_D) { > + if (hwconfig("esdhc1")) Please double check. I remember in the past if hwconfig is not enabled, any check returns true. > + sdhc2_en = true; > + } else { > + /* > + * The I2C IO-expander for mux select is used to control > + * the muxing of various onboard interfaces. > + * > + * IO1[3:2] indicates SDHC2 interface demultiplexer > + * select lines. > + * 00 - SDIO wifi > + * 01 - GPIO (to Arduino) > + * 10 - eMMC Memory > + * 11 - SPI > + */ > + if (i2c_read(I2C_MUX_IO1_ADDR, 0, 1, &io, 1) < 0) { > + printf("Error reading i2c boot information!\n"); > + return 0; /* Don't want to hang() on this error */ > + } > + > + mux_sdhc2 = (io & 0x0c) >> 2; > + /* Enable SDHC2 only when use SDIO wifi and eMMC */ > + if (mux_sdhc2 == 2 || mux_sdhc2 == 0) > + sdhc2_en = true; > } > > - mux_sdhc2 = (io & 0x0c) >> 2; > - /* Enable SDHC2 only when use SDIO wifi and eMMC */ > - if (mux_sdhc2 == 2 || mux_sdhc2 == 0) > + if (sdhc2_en) > do_fixup_by_path(blob, esdhc1_path, "status", "okay", > sizeof("okay"), 1); > else > do_fixup_by_path(blob, esdhc1_path, "status", "disabled", > sizeof("disabled"), 1); > + > return 0; > } > > diff --git a/include/configs/ls1012ardb.h b/include/configs/ls1012ardb.h > index 794117062f..d5384e6027 100644 > --- a/include/configs/ls1012ardb.h > +++ b/include/configs/ls1012ardb.h > @@ -32,6 +32,10 @@ > #define __SW_REV_MASK 0x07 > #define __SW_REV_A 0xF8 > #define __SW_REV_B 0xF0 > +#define __SW_REV_C 0xE8 > +#define __SW_REV_C1 0xE0 > +#define __SW_REV_C2 0xD8 > +#define __SW_REV_D 0xD0 I don't have strong opinion about these macros. I would not use underscore myself. I guess I missed them at the first place. York _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot