| Message ID | 20200425222007.5iasodfuu3jsjktt@pali |
|---|---|
| State | New |
| Headers | show |
| Series | Bisected: mmc cause reboot loops on N900 (Was: Re: U-Boot is broken on real N900 HW) | expand |
Adding also Faiz to loop... On Sunday 26 April 2020 00:20:07 Pali Roh?r wrote: > On Saturday 25 April 2020 23:26:15 Pali Roh?r wrote: > > Adding Jean to the loop. Could you please look at this problem? Your > > commit (described below) is causing reboot loop on Nokia N900 hardware. > > > > On Saturday 25 April 2020 13:50:45 Pali Roh?r wrote: > > > On Saturday 25 April 2020 06:36:58 Adam Ford wrote: > > > > On Sat, Apr 25, 2020 at 5:42 AM Pali Roh?r <pali at kernel.org> wrote: > > > > > > > > > > On Thursday 02 April 2020 20:42:31 Pali Roh?r wrote: > > > > > > On Wednesday 01 April 2020 12:32:29 Merlijn Wajer wrote: > > ... > > > > > > > U-Boot 2020.04-rc4-00033-g7dbafe0634-dirty (Apr 01 2020 - 12:15:47 +0200) > > > > > > > > > > > > > > OMAP3530-HS ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 MHz > > > > > > > Nokia RX-51 + LPDDR/OneNAND > > > > > > > I2C: ready > > > > > > > DRAM: 256 MiB > > > > > > > NAND: 0 Bytes > > ... > > > > > > > MMC: OMAP SD/MMC: 0, OMAP SD/MMC: 1 > > ... > > > > > > > OMAP die ID: 031400240000000004036ac10b01100f > > > > > > > OMAP3530-HS ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 MHz > > > > > > > > > > > > > > > > > > > And then U-Boot freeze, right? > > > > > > > > > > > > Any idea how to debug this issue? > > > > > > > > > > > > On my N900 I'm getting "data abort" error on display and then instant > > > > > > reboot. > > > > > > > > > > It looks like that omap hs mmc code cause that freeze/reboot on real HW. > > > > > Was there some significat change to OMAP3 or omap hs mmc? > > > > > > > > I booted by board from MMC as shown above. I also use the pinctrl > > > > features from the device tree to mux the pins in U-Boot (not SPL), so > > > > my SPL only does manual muxing the essential components it needs > > > > during the SPL phase, and lets U-Boot do the rest. I only mention > > > > the pinmux because of message regarding pads/pull-ups. > > > > > > > > adam > > > > > > I debugged this problem more. I disabled all preboot commands to get > > > clean U-Boot setup. And it worked. > > > > > > After I called any "mmc" command (e.g. "mmc info") I got that instant > > > board reboot. Preboot commands for n900 try to read some files from mmc, > > > so this is reason why it get into reboot loop. > > > > > > Is there any reason why "mmc info" command can cause "data abort" and > > > instant reboot of board? > > > > > > And do you know what is needed for proper initialization of omap mmc > > > controller for omap3 board? Because it looks like something fundamental > > > is missing. > > > > > > Currently there are just calls for "omap_mmc_init()" functions and > > > "twl4030_power_mmc_init()" functions. See: > > > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/board/nokia/rx51/rx51.c > > > > 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 <jjhiblot at ti.com> > > 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 <jjhiblot at ti.com> > > > > :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 <faiz_abbas at ti.com> 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 <faiz_abbas at ti.com> :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.
Pavel suggested to add Tomi into the loop as Jean is not with TI anymore. Tomi, could you please look at this mmc related problem? See details below. On Sunday 26 April 2020 00:20:07 Pali Roh?r wrote: > On Saturday 25 April 2020 23:26:15 Pali Roh?r wrote: > > Adding Jean to the loop. Could you please look at this problem? Your > > commit (described below) is causing reboot loop on Nokia N900 hardware. > > > > On Saturday 25 April 2020 13:50:45 Pali Roh?r wrote: > > > On Saturday 25 April 2020 06:36:58 Adam Ford wrote: > > > > On Sat, Apr 25, 2020 at 5:42 AM Pali Roh?r <pali at kernel.org> wrote: > > > > > > > > > > On Thursday 02 April 2020 20:42:31 Pali Roh?r wrote: > > > > > > On Wednesday 01 April 2020 12:32:29 Merlijn Wajer wrote: > > ... > > > > > > > U-Boot 2020.04-rc4-00033-g7dbafe0634-dirty (Apr 01 2020 - 12:15:47 +0200) > > > > > > > > > > > > > > OMAP3530-HS ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 MHz > > > > > > > Nokia RX-51 + LPDDR/OneNAND > > > > > > > I2C: ready > > > > > > > DRAM: 256 MiB > > > > > > > NAND: 0 Bytes > > ... > > > > > > > MMC: OMAP SD/MMC: 0, OMAP SD/MMC: 1 > > ... > > > > > > > OMAP die ID: 031400240000000004036ac10b01100f > > > > > > > OMAP3530-HS ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 MHz > > > > > > > > > > > > > > > > > > > And then U-Boot freeze, right? > > > > > > > > > > > > Any idea how to debug this issue? > > > > > > > > > > > > On my N900 I'm getting "data abort" error on display and then instant > > > > > > reboot. > > > > > > > > > > It looks like that omap hs mmc code cause that freeze/reboot on real HW. > > > > > Was there some significat change to OMAP3 or omap hs mmc? > > > > > > > > I booted by board from MMC as shown above. I also use the pinctrl > > > > features from the device tree to mux the pins in U-Boot (not SPL), so > > > > my SPL only does manual muxing the essential components it needs > > > > during the SPL phase, and lets U-Boot do the rest. I only mention > > > > the pinmux because of message regarding pads/pull-ups. > > > > > > > > adam > > > > > > I debugged this problem more. I disabled all preboot commands to get > > > clean U-Boot setup. And it worked. > > > > > > After I called any "mmc" command (e.g. "mmc info") I got that instant > > > board reboot. Preboot commands for n900 try to read some files from mmc, > > > so this is reason why it get into reboot loop. > > > > > > Is there any reason why "mmc info" command can cause "data abort" and > > > instant reboot of board? > > > > > > And do you know what is needed for proper initialization of omap mmc > > > controller for omap3 board? Because it looks like something fundamental > > > is missing. > > > > > > Currently there are just calls for "omap_mmc_init()" functions and > > > "twl4030_power_mmc_init()" functions. See: > > > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/board/nokia/rx51/rx51.c > > > > 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 <jjhiblot at ti.com> > > 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 <jjhiblot at ti.com> > > > > :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.
Pali, On 26/04/20 3:59 am, Pali Roh?r wrote: > Adding also Faiz to loop... > > On Sunday 26 April 2020 00:20:07 Pali Roh?r wrote: >> On Saturday 25 April 2020 23:26:15 Pali Roh?r wrote: >>> Adding Jean to the loop. Could you please look at this problem? Your >>> commit (described below) is causing reboot loop on Nokia N900 hardware. >>> >>> On Saturday 25 April 2020 13:50:45 Pali Roh?r wrote: >>>> On Saturday 25 April 2020 06:36:58 Adam Ford wrote: >>>>> On Sat, Apr 25, 2020 at 5:42 AM Pali Roh?r <pali at kernel.org> wrote: >>>>>> >>>>>> On Thursday 02 April 2020 20:42:31 Pali Roh?r wrote: >>>>>>> On Wednesday 01 April 2020 12:32:29 Merlijn Wajer wrote: >>> ... >>>>>>>> U-Boot 2020.04-rc4-00033-g7dbafe0634-dirty (Apr 01 2020 - 12:15:47 +0200) >>>>>>>> >>>>>>>> OMAP3530-HS ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 MHz >>>>>>>> Nokia RX-51 + LPDDR/OneNAND >>>>>>>> I2C: ready >>>>>>>> DRAM: 256 MiB >>>>>>>> NAND: 0 Bytes >>> ... >>>>>>>> MMC: OMAP SD/MMC: 0, OMAP SD/MMC: 1 >>> ... >>>>>>>> OMAP die ID: 031400240000000004036ac10b01100f >>>>>>>> OMAP3530-HS ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 MHz >>>>>>>> >>>>>>> >>>>>>> And then U-Boot freeze, right? >>>>>>> >>>>>>> Any idea how to debug this issue? >>>>>>> >>>>>>> On my N900 I'm getting "data abort" error on display and then instant >>>>>>> reboot. >>>>>> >>>>>> It looks like that omap hs mmc code cause that freeze/reboot on real HW. >>>>>> Was there some significat change to OMAP3 or omap hs mmc? >>>>> >>>>> I booted by board from MMC as shown above. I also use the pinctrl >>>>> features from the device tree to mux the pins in U-Boot (not SPL), so >>>>> my SPL only does manual muxing the essential components it needs >>>>> during the SPL phase, and lets U-Boot do the rest. I only mention >>>>> the pinmux because of message regarding pads/pull-ups. >>>>> >>>>> adam >>>> >>>> I debugged this problem more. I disabled all preboot commands to get >>>> clean U-Boot setup. And it worked. >>>> >>>> After I called any "mmc" command (e.g. "mmc info") I got that instant >>>> board reboot. Preboot commands for n900 try to read some files from mmc, >>>> so this is reason why it get into reboot loop. >>>> >>>> Is there any reason why "mmc info" command can cause "data abort" and >>>> instant reboot of board? >>>> >>>> And do you know what is needed for proper initialization of omap mmc >>>> controller for omap3 board? Because it looks like something fundamental >>>> is missing. >>>> >>>> Currently there are just calls for "omap_mmc_init()" functions and >>>> "twl4030_power_mmc_init()" functions. See: >>>> https://gitlab.denx.de/u-boot/u-boot/-/blob/master/board/nokia/rx51/rx51.c >>> >>> 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 <jjhiblot at ti.com> >>> 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 <jjhiblot at ti.com> >>> >>> :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 <faiz_abbas at ti.com> > 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 <faiz_abbas at ti.com> > > :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? 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? Its possible that your platform requires a pbias of 3.0V to work. PS: In the next version of these patches, please follow file prefixes for each commit instead of one blanket NOKIA RX-511: prefix. Thanks, Faiz
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 <jjhiblot at ti.com> > >>> 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 <jjhiblot at ti.com> > >>> > >>> :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 <faiz_abbas at ti.com> > > 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 <faiz_abbas at ti.com> > > > > :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. > > PS: In the next version of these patches, please follow file prefixes for each commit > instead of one blanket NOKIA RX-511: prefix. File prefix is nokia rx-51. So which prefix would you want to see? But I think I would not send new version of these patches as Tom already wrote that is fine with them and I guess he would take them (or maybe already merged?).
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 <jjhiblot at ti.com> > > > >>> 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 <jjhiblot at ti.com> > > > >>> > > > >>> :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 <faiz_abbas at ti.com> > > > > 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 <faiz_abbas at ti.com> > > > > > > > > :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? > First problem with reboot loop and crash in mmc_set_ios() still remains. > Reverting 04a2ea248f58b3b6216d0cd0a6b8698df8b14355 fixes it.
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 <jjhiblot at ti.com> > > > > >>> 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 <jjhiblot at ti.com> > > > > >>> > > > > >>> :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 <faiz_abbas at ti.com> > > > > > 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 <faiz_abbas at ti.com> > > > > > > > > > > :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.
Hi Pali, On 01/07/20 2:02 pm, Pali Roh?r wrote: > 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: >>>>>>>> ... >>> >>> 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? > Sorry I missed this earlier. The fix makes sense to me. I can give my reviewed by to the your fix once you send the patch. Thanks, Faiz
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;