[U-Boot] [v3] armv8: ls1012a: enable/disable eSDHC1 through hwconfig for new RDB

[Y.b. Lu]

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.lu at nxp.com>; u-boot at 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