| Message ID | 20171127083903.13123-1-yangbo.lu@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | York Sun |
| Headers | show |
| Series | [U-Boot,v2] armv8: ls1012a: enable/disable eSDHC1 through hwconfig for RDB | expand |
On 11/27/2017 12:58 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 > eSDHC1 manually. What do you mean "no longer reliable"? This should be addressed by fixing the board. York
Hi York, I copied hardware team Kinjalk's explain here. "Enabling SDHC2 on ‘00’ is not correct on revision D and later boards as the sd wifi is not on there on these revs. The IO expander was designed to override the dip switch values. So, the DIP switch values are driven through low strength drivers. The I2C reads of DIP switch settings may not be correct and reliable as the noise margin is very low. If the IO expander is driving CFG than the situation is different and reads will always be reliable." It seemed ls1012ardb RevD reworked the SDHC2 circuit. The SDIO wifi device was removed and I2C reading of DIP switch setting was not reliable. This changes is causing many kernel error messages on RevD boards. "mmc1: Controller never released inhibit bit(s)." It's safe to disable SDHC2 in default and enable it manually when hardware configuration(DIP switch) is correct for SDHC2 since software doesn’t have way to check. Do you think this should be fixed by hardware? If so, I will try to suggest that. Thanks a lot. Best regards, Yangbo Lu > -----Original Message----- > From: York Sun > Sent: 2017年11月30日 3:45 > To: Y.b. Lu <yangbo.lu@nxp.com>; u-boot@lists.denx.de > Subject: Re: [v2] armv8: ls1012a: enable/disable eSDHC1 through hwconfig for > RDB > > On 11/27/2017 12:58 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 > > eSDHC1 manually. > > What do you mean "no longer reliable"? This should be addressed by fixing the > board. > > York
On 11/29/2017 08:44 PM, Y.b. Lu wrote: > Hi York, > > I copied hardware team Kinjalk's explain here. > > "Enabling SDHC2 on ‘00’ is not correct on revision D and later boards as the sd wifi is not on there on these revs. > The IO expander was designed to override the dip switch values. So, the DIP switch values are driven through low strength drivers. The I2C reads of DIP switch settings may not be correct and reliable as the noise margin is very low. If the IO expander is driving CFG than the situation is different and reads will always be reliable." > > It seemed ls1012ardb RevD reworked the SDHC2 circuit. The SDIO wifi device was removed and I2C reading of DIP switch setting was not reliable. > This changes is causing many kernel error messages on RevD boards. > "mmc1: Controller never released inhibit bit(s)." > It's safe to disable SDHC2 in default and enable it manually when hardware configuration(DIP switch) is correct for SDHC2 since software doesn’t have way to check. > > Do you think this should be fixed by hardware? If so, I will try to suggest that. Yangbo, If the hardware can be fixed to operate the same way, please push for a fix. If rev D board (and later) removes the feature to read DIP switches, and you can make the code backward-compatible, then we can accept this change. Your change is to use hwconfig to determine if eSDHC1 is available. This will cause all previous boards losing the ability to automatically select eSDHC1. Is that acceptable to existing users, and clearly documented? York
Hi York, Thank you for your suggestion. I sent out v3 patch which is backward-compatible. Please check. Thanks a lot. Best regards, Yangbo Lu > -----Original Message----- > From: York Sun > Sent: 2017年12月1日 1:59 > To: Y.b. Lu <yangbo.lu@nxp.com>; u-boot@lists.denx.de > Cc: Xiaobo Xie <xiaobo.xie@nxp.com> > Subject: Re: [v2] armv8: ls1012a: enable/disable eSDHC1 through hwconfig for > RDB > > On 11/29/2017 08:44 PM, Y.b. Lu wrote: > > Hi York, > > > > I copied hardware team Kinjalk's explain here. > > > > "Enabling SDHC2 on ‘00’ is not correct on revision D and later boards as the > sd wifi is not on there on these revs. > > The IO expander was designed to override the dip switch values. So, the DIP > switch values are driven through low strength drivers. The I2C reads of DIP > switch settings may not be correct and reliable as the noise margin is very low. > If the IO expander is driving CFG than the situation is different and reads will > always be reliable." > > > > It seemed ls1012ardb RevD reworked the SDHC2 circuit. The SDIO wifi device > was removed and I2C reading of DIP switch setting was not reliable. > > This changes is causing many kernel error messages on RevD boards. > > "mmc1: Controller never released inhibit bit(s)." > > It's safe to disable SDHC2 in default and enable it manually when hardware > configuration(DIP switch) is correct for SDHC2 since software doesn’t have > way to check. > > > > Do you think this should be fixed by hardware? If so, I will try to suggest that. > > Yangbo, > > If the hardware can be fixed to operate the same way, please push for a fix. If > rev D board (and later) removes the feature to read DIP switches, and you can > make the code backward-compatible, then we can accept this change. > > Your change is to use hwconfig to determine if eSDHC1 is available. This will > cause all previous boards losing the ability to automatically select eSDHC1. Is > that acceptable to existing users, and clearly documented? > > York
diff --git a/board/freescale/ls1012ardb/ls1012ardb.c b/board/freescale/ls1012ardb/ls1012ardb.c index c6c1c71202..3e2b9c27eb 100644 --- a/board/freescale/ls1012ardb/ls1012ardb.c +++ b/board/freescale/ls1012ardb/ls1012ardb.c @@ -132,39 +132,15 @@ 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 io = 0; - u8 mux_sdhc2; - do_fixup_by_path(blob, esdhc0_path, "status", "okay", - sizeof("okay"), 1); - - 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) { - 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) + if (hwconfig("esdhc1")) 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; }
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 eSDHC1 manually. Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> --- Changes for v2: - Just used hwconfig() instead of getenv() and hwconfig_f(). --- board/freescale/ls1012ardb/ls1012ardb.c | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-)