Sent out v4 patch-set. Please help to review. Thanks a lot :) > -----Original Message----- > From: York Sun > Sent: 2017年12月8日 1:34 > To: Y.b. Lu <yangbo...@nxp.com>; u-boot@lists.denx.de > Subject: Re: [v3] armv8: ls1012a: enable/disable eSDHC1 through hwconfig for > new RDB > > On 12/07/2017 12:10 AM, Y.b. Lu wrote: > > <snip> > > > > >> > >>> 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(). > > > > [Y.b. Lu] I don’t know the possibility of I2C failure. If the failure > > happens, just > print error message, leave 'status' as it is, and return 0 because the other > fix-ups in fdt_fixup_esdhc() are still needed like clock-frequency. > > You can move on if it makes sense to do so. > > > > >> > >>> + } > >>> + > >>> + 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. > > > > [Y.b. Lu] You're right. It's -ENOSYS. But what should I do here ? > > It depends on which one is safe. If you want to presume esdhc1 is available, > you can keep current code. If you'd rather not to enable it, add a check of > #ifdef. > > > > >> > >>> + 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. > > > > [Y.b. Lu] It's confusing why define macro like this... > > And the __SW_BOOT_EMU is wrong, it should be 0x02, not 0x10. > > Add another patch before this one to clean them up, please. > > York
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot