From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pali =?utf-8?B?Um9ow6Fy?= Date: Wed, 1 Jul 2020 10:32:09 +0200 Subject: Bisected: omap_hsmmc 3.3V IO voltage incompatible with N900 (Was: Re: Bisected: mmc cause reboot loops on N900) In-Reply-To: <20200612130306.5qjfonf2kfkaervv@pali> References: <20200425104217.pzp5zkhhft5a2he5@pali> <20200425115045.w45tb62cnwtq5umk@pali> <20200425212615.4wzs32p6pgvivsq6@pali> <20200425222007.5iasodfuu3jsjktt@pali> <20200425222932.rauirshrgi6qgdta@pali> <5ec5f853-ea6d-221a-5fe6-7e0aefff29c0@ti.com> <20200507151938.jq53mkum7cjldy42@pali> <20200526174954.u6xxdkvesy5kaerq@pali> <20200612130306.5qjfonf2kfkaervv@pali> Message-ID: <20200701083209.pzjr7edgvanfwkz2@pali> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Friday 12 June 2020 15:03:06 Pali Roh?r wrote: > On Tuesday 26 May 2020 19:49:54 Pali Roh?r wrote: > > On Thursday 07 May 2020 17:19:38 Pali Roh?r wrote: > > > On Thursday 07 May 2020 19:10:14 Faiz Abbas wrote: > > > > On 26/04/20 3:59 am, Pali Roh?r wrote: > > > > > On Sunday 26 April 2020 00:20:07 Pali Roh?r wrote: > > > > >> On Saturday 25 April 2020 23:26:15 Pali Roh?r wrote: > > > > >>> Now I tried git bisect and here is problematic commit which caused whole > > > > >>> reboot loop: > > > > >>> > > > > >>> 04a2ea248f58b3b6216d0cd0a6b8698df8b14355 is the first bad commit > > > > >>> commit 04a2ea248f58b3b6216d0cd0a6b8698df8b14355 > > > > >>> Author: Jean-Jacques Hiblot > > > > >>> Date: Thu Sep 21 16:30:08 2017 +0200 > > > > >>> > > > > >>> mmc: disable UHS modes if Vcc cannot be switched on and off > > > > >>> > > > > >>> If a power cycle cannot be done on Vcc, it is safer not to try the UHS > > > > >>> modes because we wouldn't be able to recover from an error occurring > > > > >>> during the UHS initialization. > > > > >>> > > > > >>> Signed-off-by: Jean-Jacques Hiblot > > > > >>> > > > > >>> :040000 040000 04de51428c8311a4b2fb3ad876ac3f6071ab57ee ea7a7959a4bd591c92a2c3d413d5643a8457d2ff M drivers > > > > >>> :040000 040000 03f639baf2a2f55003cb750981fd8accc5b4a993 fbcb9607d37959f0b5240f5d727133f58cc35379 M include > > > > >>> > > > > >>> It changes only core mmc code, nothing platform / board specific. > > > > >>> U-Boot compiled from commit before above has fully working eMMC access > > > > >>> on real Nokia N900. I can read files on FAT eMMC partition without any > > > > >>> problem. > > > > >>> > > > > >>> I'm not sure what is happening here, but it looks like that omap hs mmc > > > > >>> driver used on Nokia N900 is maybe not correctly hooked for UHS support. > > > > >>> > > > > >>> The most suspicious it that this problem cannot be reproduced in qemu > > > > >>> n900 emulator. It happens only on real N900 hw. > > > > >> > > > > >> I took main change from above commit, reverted it on master and U-Boot > > > > >> stopped crashing! No reboot loop anymore. Here is change which fixes > > > > >> reboot loop on Nokia N900: > > > > >> > > > > >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > > > > >> index 523c055967..d07c7745da 100644 > > > > >> --- a/drivers/mmc/mmc.c > > > > >> +++ b/drivers/mmc/mmc.c > > > > >> @@ -2786,18 +2786,7 @@ int mmc_get_op_cond(struct mmc *mmc) > > > > >> MMC_QUIRK_RETRY_APP_CMD; > > > > >> #endif > > > > >> > > > > >> - err = mmc_power_cycle(mmc); > > > > >> - if (err) { > > > > >> - /* > > > > >> - * if power cycling is not supported, we should not try > > > > >> - * to use the UHS modes, because we wouldn't be able to > > > > >> - * recover from an error during the UHS initialization. > > > > >> - */ > > > > >> - pr_debug("Unable to do a full power cycle. Disabling the UHS modes for safety\n"); > > > > >> - uhs_en = false; > > > > >> - mmc->host_caps &= ~UHS_CAPS; > > > > >> - err = mmc_power_on(mmc); > > > > >> - } > > > > >> + err = mmc_power_on(mmc); > > > > >> if (err) > > > > >> return err; > > > > >> > > > > >> > > > > >> Jean, do you have any idea what is happening in above code? It looks > > > > >> like something is broken in power cycle / power on sequence if it cause > > > > >> "data abort" and reboot of board. > > > > >> > > > > >> > > > > >> But even with above my change eMMC on N900 cannot be initialized... > > > > >> I'm running second git bisect with applying above change on every > > > > >> commit. It is in progress now. > > > > > > > > > > And second bisect finished. Result is: > > > > > > > > > > d2c05f50e12f87128597a28146de7092aaa847c3 is the first bad commit > > > > > commit d2c05f50e12f87128597a28146de7092aaa847c3 > > > > > Author: Faiz Abbas > > > > > Date: Fri Apr 5 14:18:46 2019 +0530 > > > > > > > > > > mmc: omap_hsmmc: Set 3.3V for IO voltage > > > > > > > > > > Pbias voltage should match the IO voltage set for the SD card. With the > > > > > latest pbias change to 3.3V, update the capabilities and IO voltages > > > > > settings to 3.3V. > > > > > > > > > > Signed-off-by: Faiz Abbas > > > > > > > > > > :040000 040000 819148217b64a6beaf8247464de6b4384d4581a4 e4fd9288ddb794f33596339813d5386f3bed8fd7 M drivers > > > > > > > > > > I revered above commit on top of master and eMMC on Nokia N900 finally > > > > > started working again! > > > > > > > > > > Faiz, what is the reason for above commit? It basically changes in omap > > > > > hs mmc driver IO voltage from 3.0V to 3.3V. And seems this change is > > > > > incompatible with Nokia N900. Are there any problems with 3.0V on some > > > > > omap3 boards? If not I would vote for revering that commit. Or at least > > > > > making IO voltage configurable. > > > > > > > > > > > > > For the power_cycle patch, it looks like there is a null pointer dereference > > > > that might be causing the reboot loops (probably because your platform doesn't > > > > have DM_MMC enabled) Can you add a few more prints to see where the data abort > > > > comes from exactly? > > > > > > Hello Faiz! > > > > > > Sorry, but this is problematic. Because of reboot loops I cannot read > > > what exactly is put on the display and reboot cause clearing display. > > > > > > Month ago I was able to detect that problem is somewhere in function > > > mmc_set_ios() from mmc.c file. I put long debug line prior and also > > > after mmc_set_ios() call. And it was printed only prior. Not after. > > > So I think NULL pointer dereference is in mmc_set_ios() function. > > > > > > > For the second patch IO voltage patch, what exactly do you see without reverting it? > > > > Are you able to reach prompt but mmc commands fail? > > > > > > Yes, after reverting 04a2ea248f58b3b6216d0cd0a6b8698df8b14355 there is > > > no reboot loop anymore. Just mmc commands fail and eMMC is not detected > > > at all. > > > > > > > Its possible that your platform requires a pbias of 3.0V to work. > > > > Hello Faiz! > > > > Now I figured out what is the root cause of second 3.0V vs 3.3V problem. > > > > In commit d2c05f50e12f87128597a28146de7092aaa847c3 you forgot to replace > > one usage of 3.0V by 3.3V. Below is patch which changes also this last > > one usage. Applying it has same effect on Nokia N900 as reverting > > that problematic commit d2c05f50e12f87128597a28146de7092aaa847c3: > > > > diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c > > index 8636cd713a..dc26e54795 100644 > > --- a/drivers/mmc/omap_hsmmc.c > > +++ b/drivers/mmc/omap_hsmmc.c > > @@ -840,7 +840,7 @@ static int omap_hsmmc_init_setup(struct mmc *mmc) > > omap_hsmmc_conf_bus_power(mmc, (reg_val & VS33_3V3SUP) ? > > MMC_SIGNAL_VOLTAGE_330 : MMC_SIGNAL_VOLTAGE_180); > > #else > > - writel(DTW_1_BITMODE | SDBP_PWROFF | SDVS_3V0, &mmc_base->hctl); > > + writel(DTW_1_BITMODE | SDBP_PWROFF | SDVS_3V3, &mmc_base->hctl); > > writel(readl(&mmc_base->capa) | VS33_3V3SUP | VS18_1V8SUP, > > &mmc_base->capa); > > #endif > > > > So eMMC on real N900 is working fine either with 3.0V or 3.3V. But 3.3V > > needs to be configured on all places. > > Hello! Could you please take a look at this issue and my above fix? Ping. Any comment for above issue or my fix? > > First problem with reboot loop and crash in mmc_set_ios() still remains. > > Reverting 04a2ea248f58b3b6216d0cd0a6b8698df8b14355 fixes it.