Re: [U-Boot] commit 83e359adf9f578a58f20daf2e4425a754defac7b breaks mmc
On 09/09/2014 12:27 PM, Felipe Balbi wrote: Hi, On Tue, Sep 09, 2014 at 12:09:02PM -0500, Felipe Balbi wrote: Hi, commit 83e359a (am335x_evm: Enable CONFIG_SPL_ENV_SUPPORT on EMMC_BOOT) breaks MMC on some boards. I believe it only breaks if EMMC has no partition on it, so that would point out to a bug on spl env support itself and this has only exposed it. Here's a bisection log anyway: git bisect start # good: [524123a70761110c5cf3ccc5f52f6d4da071b959] Prepare v2014.07 git bisect good 524123a70761110c5cf3ccc5f52f6d4da071b959 # bad: [0b703dbcee7103f07804d0a4328d1593355c4324] patman: Fix detection of git version git bisect bad 0b703dbcee7103f07804d0a4328d1593355c4324 # good: [63b85adcecdd019f049cbbebf10119cea45d3645] imx: ventana: set dynamic env var for flash layout git bisect good 63b85adcecdd019f049cbbebf10119cea45d3645 # good: [63b85adcecdd019f049cbbebf10119cea45d3645] imx: ventana: set dynamic env var for flash layout git bisect good 63b85adcecdd019f049cbbebf10119cea45d3645 # good: [04b43f32731c1171877541050bb3f2bfeb100e3d] tools/genboardscfg.py: ignore defconfigs starting with a dot git bisect good 04b43f32731c1171877541050bb3f2bfeb100e3d # bad: [6defdc0b5552ab1af4a66a8abac8196cbb6b9e15] Merge branch 'master' of git://git.denx.de/u-boot-ti git bisect bad 6defdc0b5552ab1af4a66a8abac8196cbb6b9e15 # good: [6af857c50df4e62ec08e51ad73c96f63f1480386] Merge branch 'master' of http://git.denx.de/u-boot-sunxi git bisect good 6af857c50df4e62ec08e51ad73c96f63f1480386 # good: [7f14fb20f895016fb38d30ce71aeb4d441b5bcb8] Merge branch 'zynq' of git://www.denx.de/git/u-boot-microblaze git bisect good 7f14fb20f895016fb38d30ce71aeb4d441b5bcb8 # bad: [61f66fd5a81b97478e9d14326c1059baa6626680] keystone2: use EFUSE_BOOTROM information to configure PLLs git bisect bad 61f66fd5a81b97478e9d14326c1059baa6626680 # bad: [fea9543f1bd1d068a372ef378f624941c25989a8] board/ti/am335x: update configs for parallel NAND git bisect bad fea9543f1bd1d068a372ef378f624941c25989a8 # good: [e017fd61c5a89e32db682d94d8d669df1709edbb] tseries: Set CONFIG_ENV_IS_NOWHERE for SPL+NAND git bisect good e017fd61c5a89e32db682d94d8d669df1709edbb # bad: [83e359adf9f578a58f20daf2e4425a754defac7b] am335x_evm: Enable CONFIG_SPL_ENV_SUPPORT on EMMC_BOOT git bisect bad 83e359adf9f578a58f20daf2e4425a754defac7b # good: [00e385325fce36fa13d48091d73b1b3428394b6b] common/Makefile: Consolidate SPL ENV options, correct inclusion git bisect good 00e385325fce36fa13d48091d73b1b3428394b6b # first bad commit: [83e359adf9f578a58f20daf2e4425a754defac7b] am335x_evm: Enable CONFIG_SPL_ENV_SUPPORT on EMMC_BOOT Revert that commit also makes it work, btw. On a further dig into this, it looks like when enabling CONFIG_SPL_ENV_SUPPORT, somehow MMC numbering changes and u-boot ends up trying to read environment from a non-formatted EMMC. forgot to mention, this is all on BBB and affects only am335x-based boards, of course. I believe this is the problem that https://patchwork.ozlabs.org/patch/385354/ is intended to fix. See also discussion at http://www.mail-archive.com/u-boot@lists.denx.de/msg146396.html Peter ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] uboot env in mmc partition
On 09/03/2014 07:54 AM, Naitik Amin wrote: Hi Hannes, So I did as you had indicated and made some progress, so now, my #defines look as below. #define CONFIG_SYS_MMC_ENV_DEV 0 /* device 0 */ #define CONFIG_ENV_OFFSET 0x2190 I calculated the env offset from the block number it was on based on the first block number of the partition. Doing this on uboot startup, I dont get the warning saying, using default environment. Neither it complained abt crc error. So it liked it seems. But what it did is it trashed the partition which had my dtb and zImage. Its a completely different partition. So after that, the bootup gets halted. Any ideas ? What may have trashed the partition ? Look at: http://www.mail-archive.com/u-boot@lists.denx.de/msg146396.html and see if that's relevant, specifically the second patch which restores the mmc device offset calculations to the values that correspond to the partition number that's represented in the device structure. Peter From: Hannes Petermaier hannes.peterma...@br-automation.com To: Naitik Amin naitik.a...@ametek.com, Cc: u-boot@lists.denx.de, u-boot-boun...@lists.denx.de Date: 09/03/2014 01:13 AM Subject:Re: [U-Boot] uboot env in mmc partition Hi Hannes, Hi, Yes, its an eMMC. If I read your response correctly, #define CONFIG_SYS_MMC_ENV_PART Can only have 2 values, 1 2 is that correct ? yes - i think so. please have a look to env_mmc.c, lines 65 to 120 may be the most interesting for you. There are only low-level operations performed, nobody doesn't care about any filesystem or filesystems-partitiontable at this time. Pls also keep in mind that from linux, it works fine, with fw_env.config set to /dev/mmcblk0p4 0 0x1 i think this environment is its own instance and has no interaction with u-boot`s one, is that so? In fact this environment is within User-Area of eMMC and at the place were part #4 starts. best regards, hannes From: Hannes Petermaier hannes.peterma...@br-automation.com To: Naitik Amin naitik.a...@ametek.com, Cc: u-boot@lists.denx.de, u-boot-boun...@lists.denx.de Date: 09/01/2014 12:54 AM Subject:Re: [U-Boot] uboot env in mmc partition HI there, Hi Naitik, I recently made changes to my system, where I created a new partition on my mmc. (mmcblk0p4) do you use MMC or eMMC ? I guess eMMC. Then i dd'd a uboot env image into this partition, updated the fw_env.config to point to /dev/mmcblk0p4. At this point, my fw_printenv and fw_setenv work good. So as a next step, I am tried to modify uboot to make it point to my env image in my new partition. I made below changes to my config header and rebuilt the uboot. On doing printenv from uboot, I dont see the same env that I pushed it from linux, infact I see it as its defined in the config header. Can some one help ? /* environment setting for MMC */ #ifdef CONFIG_ENV_IS_IN_MMC #define CONFIG_SYS_MMC_ENV_DEV 0 /* device 0 */ #define CONFIG_SYS_MMC_ENV_PART 4 #define CONFIG_ENV_OFFSET 0 /* just after the MBR */ #endif I understand this #defines as following: CONFIG_SYS_MMC_ENV_DEV says: use mmc-device #0 CONFIG_SYS_MMC_ENV_PART says: use partition #4 i know that u-boot is using the boot-partitions to store the environment, an emmc only has 2 partitions. So #4 is a an illegal paramater CONFIG_ENV_OFFSET is an absolute address within the device/partition so it is not just after the MBR, instead the environment is instead the MBR best regards, Hannes ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] uboot env in mmc partition
On 09/03/2014 09:14 AM, Hannes Petermaier wrote: On 09/03/2014 07:54 AM, Naitik Amin wrote: Hi Hannes, So I did as you had indicated and made some progress, so now, my #defines look as below. #define CONFIG_SYS_MMC_ENV_DEV 0 /* device 0 */ #define CONFIG_ENV_OFFSET 0x2190 I calculated the env offset from the block number it was on based on the first block number of the partition. Doing this on uboot startup, I dont get the warning saying, using default environment. Neither it complained abt crc error. So it liked it seems. But what it did is it trashed the partition which had my dtb and zImage. Its a completely different partition. So after that, the bootup gets halted. Any ideas ? What may have trashed the partition ? Look at: http://www.mail-archive.com/u-boot@lists.denx.de/msg146396.html and see if that's relevant, specifically the second patch which restores the mmc device offset calculations to the values that correspond to the partition number that's represented in the device structure. Peter Hi Peter, i don't think that your patch has influence on this issue, due to no partition switching is done anymore. That may be the case: I missed most of the previous discussion of this issue. All I know is that if mmc-part_num is 0 when the MMC environment code is entered, then when it leaves the capacity and lba fields of the device are no longer correct. So it'd seemed plausible that if mmc_switch_part were invoked somewhere when looking for an environment partition it could explain an anomaly with subsequent use of the device. Further i would like to check your patch series, i am not sure if there is everything ok. I will do so tommorow - i am right in thinking that you are using MMC not eMMC ? There is eMMC on the BeagleBone I'm using, but it's been zeroed and is not used. The problem arises in SPL mode on a uSD card with two partitions neither of which is an environment. For full context see the meta-ti email thread referenced in the cover letter. Peter ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] mmc: restore capacity when switching to partition 0
On 09/03/2014 10:48 AM, Stephen Warren wrote: On 09/02/2014 05:31 PM, Peter A. Bigot wrote: The capacity and lba for an MMC device with part_num 0 reflects the whole device. When mmc_switch_part() successfully switches to a partition, the capacity is changed to that partition. As partition 0 does not physically exist, attempts to switch back to the whole device will indicate an error, but the capacity setting for the whole device must still be restored to match the partition. diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c @@ -594,10 +594,15 @@ int mmc_switch_part(int dev_num, unsigned int part_num) ret = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_PART_CONF, (mmc-part_config ~PART_ACCESS_MASK) | (part_num PART_ACCESS_MASK)); -if (ret) -return ret; -return mmc_set_capacity(mmc, part_num); +/* + * Set the capacity if the switch succeeded or was intended + * to return to representing the raw device. + */ +if ((ret == 0) || ((ret == -ENODEV) (part_num == 0))) +ret = mmc_set_capacity(mmc, part_num); + +return ret; } I think this wouldn't be needed without patch 1/2, since without that patch, no partition switching should ever happen if HW partitions don't exist, and hence this patch shouldn't be required. Not so. In SPL mode, the mmc device passed in to the environment code is set up for partition 0. In the failure case, u-boot is configured to expect an environment in partition 2, and so invokes mmc_switch_part to go to partition 2 to see if it's a valid partition. In my case that fails because the partition size is zero, but regardless the mmc_switch_part back to mmc-part_num fails because the mmc_switch() call rejects the attempt with an error. Without this second patch, you end up with mmc-part_num left at zero but the capacity/lba fields configured for partition 2 which does not exist and has size zero, and SPL is unable to locate u-boot.img to continue. Please review the details in the meta-ti discussion. I'll respond to the comments on patch 1 separately. Peter ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] env_mmc: remove condition on call to mmc_switch_part
On 09/03/2014 10:46 AM, Stephen Warren wrote: On 09/02/2014 05:31 PM, Peter A. Bigot wrote: Though it might be expected to do so, mmc_switch_part() does not change the part_num field of the device on which the partition has been changed. As such, checking to see whether the partition is already the target partition will fail to correctly restore the original configuration in cases like env_mmc which rely on this behavior to avoid having to preserve the pre-switch partition number outside the device structure. diff --git a/common/env_mmc.c b/common/env_mmc.c -if (part != mmc-part_num) { -ret = mmc_switch_part(dev, part); -if (ret) -puts(MMC partition switch failed\n); -} +ret = mmc_switch_part(dev, part); +if (ret) +puts(MMC partition switch failed\n); I'm not sure this is correct, but it might be. I believe the if() is present to avoid any attempt to call mmc_switch_part() on a device that doesn't have HW partitions (in which case, both part and mmc-part_num should already be 0), since such an attempt might print an error message. That could be true. The patch that added the feature didn't provide that information. In my case, the device does have HW partitions. If you don't observe any error message printed after this change, then perhaps this patch is fine. It does work in my environment, but would not retain the behavior you describe. The existing code is still wrong, but the error is elsewhere: I'll provide a new patch to supersede this one. Peter ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2] env_mmc: correct fini partition to match init partition
The code to set the MMC partition uses an weak function to obtain the correct partition number. Use that instead of the compile-time default when deciding whether it needs to switch back. Signed-off-by: Peter A. Bigot p...@pabigot.com --- V2: * Preserve desired behavior of avoiding diagnostic when no HW partition supported * Supersedes https://patchwork.ozlabs.org/patch/385355/ common/env_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/env_mmc.c b/common/env_mmc.c index a7621a8..14648e3 100644 --- a/common/env_mmc.c +++ b/common/env_mmc.c @@ -113,7 +113,7 @@ static void fini_mmc_for_env(struct mmc *mmc) #ifdef CONFIG_SPL_BUILD dev = 0; #endif - if (CONFIG_SYS_MMC_ENV_PART != mmc-part_num) + if (mmc_get_env_part(mmc) != mmc-part_num) mmc_switch_part(dev, mmc-part_num); #endif } -- 1.8.5.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] mmc: restore capacity when switching to partition 0
On 09/03/2014 11:05 AM, Stephen Warren wrote: On 09/03/2014 09:59 AM, Peter A. Bigot wrote: On 09/03/2014 10:48 AM, Stephen Warren wrote: On 09/02/2014 05:31 PM, Peter A. Bigot wrote: The capacity and lba for an MMC device with part_num 0 reflects the whole device. When mmc_switch_part() successfully switches to a partition, the capacity is changed to that partition. As partition 0 does not physically exist, attempts to switch back to the whole device will indicate an error, but the capacity setting for the whole device must still be restored to match the partition. diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c @@ -594,10 +594,15 @@ int mmc_switch_part(int dev_num, unsigned int part_num) ret = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_PART_CONF, (mmc-part_config ~PART_ACCESS_MASK) | (part_num PART_ACCESS_MASK)); -if (ret) -return ret; -return mmc_set_capacity(mmc, part_num); +/* + * Set the capacity if the switch succeeded or was intended + * to return to representing the raw device. + */ +if ((ret == 0) || ((ret == -ENODEV) (part_num == 0))) +ret = mmc_set_capacity(mmc, part_num); + +return ret; } I think this wouldn't be needed without patch 1/2, since without that patch, no partition switching should ever happen if HW partitions don't exist, and hence this patch shouldn't be required. Not so. In SPL mode, the mmc device passed in to the environment code is set up for partition 0. In the failure case, u-boot is configured to expect an What failure case? environment in partition 2, and so invokes mmc_switch_part to go to partition 2 to see if it's a valid partition. In my case that fails because the partition size is zero, but regardless the mmc_switch_part back to mmc-part_num fails because the mmc_switch() call rejects the attempt with an error. Isn't that where the bug should be fixed then; why doesn't mmc_switch() work as desired? If mmc_switch() isn't intended to work on devices without HW partitions, then why is it being called at all in any case (normal or failure case)? I have no idea. I also wonder why, if your board configuration is set up to assume an eMMC device with HW partitions, you're using a device without eMMC HW partitions; it seems like either the board configuration or your HW configuration is incorrect (or at least don't match), so if you have problems, it's not surprising, and not something that should be fixed. This is a beaglebone (black). It has an eMMC, and an SD card. It can boot from either, and will fall back to the SD card if the eMMC is uninitialized or under other magic conditions. If you believe the beaglebone u-boot configuration is wrong for how the beaglebone is intended to be used, I'll refer you to the TI folks to sort it out. Without this second patch, you end up with mmc-part_num left at zero but the capacity/lba fields configured for partition 2 which does not exist and has size zero, and SPL is unable to locate u-boot.img to continue. Please review the details in the meta-ti discussion. I have no idea what that is. The link in the cover letter I provided with the patches, in hopes it would answer questions about why I was doing this. It all starts here: http://www.mail-archive.com/meta-ti@yoctoproject.org/msg04276.html Tom Rini: OK, so I've provided the patch upstream as you requested. I'm not going to continue to fight to get it incorporated. I believe you understand that there's a problem, and it's on hardware you're probably paid to support, unlike me. Y'all can figure out whether and how you want to resolve it. Peter ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3] env_mmc: correct fini partition to match init partition
The code to set the MMC partition uses an weak function to obtain the correct partition number. Use that instead of the compile-time default when deciding whether it needs to switch back. Fixes: 6e7b7df4df43574 (env_mmc: support env partition setup in runtime) Signed-off-by: Peter A. Bigot p...@pabigot.com --- V3: * Add Fixes line as requested V2: * Preserve desired behavior of avoiding diagnostic when no HW partition supported * Supersedes https://patchwork.ozlabs.org/patch/385355/ common/env_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/env_mmc.c b/common/env_mmc.c index a7621a8..14648e3 100644 --- a/common/env_mmc.c +++ b/common/env_mmc.c @@ -113,7 +113,7 @@ static void fini_mmc_for_env(struct mmc *mmc) #ifdef CONFIG_SPL_BUILD dev = 0; #endif - if (CONFIG_SYS_MMC_ENV_PART != mmc-part_num) + if (mmc_get_env_part(mmc) != mmc-part_num) mmc_switch_part(dev, mmc-part_num); #endif } -- 1.8.5.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] env_mmc: correct fini partition to match init partition
On 09/03/2014 11:52 AM, Stephen Warren wrote: On 09/03/2014 10:32 AM, Peter A. Bigot wrote: The code to set the MMC partition uses an weak function to obtain the correct partition number. Use that instead of the compile-time default when deciding whether it needs to switch back. Yes, this clearly fixes a bug. Can you also please add a Fixes: tag that refers to the commit which introduced the problem (i.e. which updated mmc_set_env_part() to call mmc_get_env_part(), but forgot to update fini_mmc_for_env() to match. Done. If this tag is important enough to ask people to add it and resubmit their patches with no other changes, it should probably be described at http://www.denx.de/wiki/view/U-Boot/Patches#Review_Process_Git_Tags and suggested in the section on general patch submission rules, so the poor contributor might have a chance of being able to avoid the rework. Peter ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] env_mmc: correct fini partition to match init partition
On 09/03/2014 12:46 PM, Stephen Warren wrote: On 09/03/2014 11:30 AM, Peter A. Bigot wrote: On 09/03/2014 11:52 AM, Stephen Warren wrote: On 09/03/2014 10:32 AM, Peter A. Bigot wrote: The code to set the MMC partition uses an weak function to obtain the correct partition number. Use that instead of the compile-time default when deciding whether it needs to switch back. Yes, this clearly fixes a bug. Can you also please add a Fixes: tag that refers to the commit which introduced the problem (i.e. which updated mmc_set_env_part() to call mmc_get_env_part(), but forgot to update fini_mmc_for_env() to match. Done. If this tag is important enough to ask people to add it and resubmit their patches with no other changes, it should probably be described at http://www.denx.de/wiki/view/U-Boot/Patches#Review_Process_Git_Tags and suggested in the section on general patch submission rules, so the poor contributor might have a chance of being able to avoid the rework. I'd expect that if the only issue was a patch was a missing fixes line, the person applying the patch could manually edit it in when applying the patch, so all the contributor would have to do is reply to the email with the desired content. Still, different committers have different levels of tolerance for this, so YMMV! Ah; I'd forgotten that patchwork collects that sort of thing. The need to provide the tag should still be described in the patch process wiki pages, but replying would have been an easier solution if I'd known that doing so would have satisfied your request to add it. Peter ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 0/2] fix issue with mmc partition management
This series aims at addressing an issue discovered with SPL mode when the MMC device being used lacks an environment partition. http://www.mail-archive.com/meta-ti@yoctoproject.org/msg04320.html includes details on the original failure with this diagnosis: This is a bug in handling mmc_switch_part: what's happening is that the code reconfigures the mmc device to look at the partition on which the environment is to be found, but fails to restore it to reflect the state of the whole device. I.e., the mmc capacity and lba are zero in my case (I have no partition 2 on the uSD card), but mmc_switch_part() returns -ENODEV on the attempt to switch back in fini_mmc_for_env() without also resetting the capacity to what the rest of the system expects. The first fixes a mistaken assumption about how mmc_switch_part() behaves. (Personally, I think mmc_switch_part() should have recorded the new partition in the device structure, but that's an behavioral change that I'm not going to introduce.) The second fixes the underlying problem, which was the failure to restore the capacity configuration to the whole device after interacting with the environment. FWIW: The second patch is relevant to 2014.07; the first is not. Peter A. Bigot (2): env_mmc: remove condition on call to mmc_switch_part mmc: restore capacity when switching to partition 0 common/env_mmc.c | 11 --- drivers/mmc/mmc.c | 11 --- 2 files changed, 12 insertions(+), 10 deletions(-) -- 1.8.5.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/2] mmc: restore capacity when switching to partition 0
The capacity and lba for an MMC device with part_num 0 reflects the whole device. When mmc_switch_part() successfully switches to a partition, the capacity is changed to that partition. As partition 0 does not physically exist, attempts to switch back to the whole device will indicate an error, but the capacity setting for the whole device must still be restored to match the partition. Signed-off-by: Peter A. Bigot p...@pabigot.com --- drivers/mmc/mmc.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index a26f3ce..fa04a3f 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -594,10 +594,15 @@ int mmc_switch_part(int dev_num, unsigned int part_num) ret = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_PART_CONF, (mmc-part_config ~PART_ACCESS_MASK) | (part_num PART_ACCESS_MASK)); - if (ret) - return ret; - return mmc_set_capacity(mmc, part_num); + /* +* Set the capacity if the switch succeeded or was intended +* to return to representing the raw device. +*/ + if ((ret == 0) || ((ret == -ENODEV) (part_num == 0))) + ret = mmc_set_capacity(mmc, part_num); + + return ret; } int mmc_getcd(struct mmc *mmc) -- 1.8.5.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/2] env_mmc: remove condition on call to mmc_switch_part
Though it might be expected to do so, mmc_switch_part() does not change the part_num field of the device on which the partition has been changed. As such, checking to see whether the partition is already the target partition will fail to correctly restore the original configuration in cases like env_mmc which rely on this behavior to avoid having to preserve the pre-switch partition number outside the device structure. Signed-off-by: Peter A. Bigot p...@pabigot.com --- common/env_mmc.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/common/env_mmc.c b/common/env_mmc.c index a7621a8..9556296 100644 --- a/common/env_mmc.c +++ b/common/env_mmc.c @@ -78,11 +78,9 @@ static int mmc_set_env_part(struct mmc *mmc) dev = 0; #endif - if (part != mmc-part_num) { - ret = mmc_switch_part(dev, part); - if (ret) - puts(MMC partition switch failed\n); - } + ret = mmc_switch_part(dev, part); + if (ret) + puts(MMC partition switch failed\n); return ret; } @@ -113,8 +111,7 @@ static void fini_mmc_for_env(struct mmc *mmc) #ifdef CONFIG_SPL_BUILD dev = 0; #endif - if (CONFIG_SYS_MMC_ENV_PART != mmc-part_num) - mmc_switch_part(dev, mmc-part_num); + mmc_switch_part(dev, mmc-part_num); #endif } -- 1.8.5.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] ARM: omap3: Implement dpll5 (HSUSB clk) workaround for OMAP36xx/AM/DM37xx according to errata sprz318e.
On 08/16/2013 08:38 AM, Tom Rini wrote: On Wed, Aug 14, 2013 at 09:53:16PM -0500, Peter A. Bigot wrote: On 07/09/2013 02:43 AM, Naumann Andreas wrote: In chapter 'Advisory 2.1 USB Host Clock Drift Causes USB Spec Non-compliance in Certain Configurations' of the TI Errata it is recommended to use certain div/mult values for the DPLL5 clock setup. So far u-boot used the old 34xx values, so I added the errata recommended values specificly for 36xx init only. Also, the FSEL registers exist no longer, so removed them from init. Tested this on a AM3703 board with 19.2MHz oscillator, which previously couldnt lock the dpll5 (kernel complained). As a consequence the EHCI USB port wasnt usable in U-Boot and kernel. With this patch, kernel panics disappear and USB working fine in u-boot and kernel. Signed-off-by: Andreas Naumann anaum...@ultratronik.de While this patch works with Linux that has been patched for this erratum, it will cause problems with some unpatched versions of Linux. Right. So Linux also needs to be patched for the erratum. Yes. My point was that if you update u-boot alone, then try to use it to boot an unpatched Linux that assumes CM_CLKSEL5_PLL has its power-on value, USB will not work. I think it's dangerous to assume that the mixture of an unpatched Linux with a patched u-boot will never occur, and the cause of the failure that results is pretty subtle. So whatever gets merged would be safer if it restored the default setting of CM_CLKSEL5_PLL prior to handing off control to Linux. Peter ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] ARM: omap3: Implement dpll5 (HSUSB clk) workaround for OMAP36xx/AM/DM37xx according to errata sprz318e.
On 07/09/2013 02:43 AM, Naumann Andreas wrote: In chapter 'Advisory 2.1 USB Host Clock Drift Causes USB Spec Non-compliance in Certain Configurations' of the TI Errata it is recommended to use certain div/mult values for the DPLL5 clock setup. So far u-boot used the old 34xx values, so I added the errata recommended values specificly for 36xx init only. Also, the FSEL registers exist no longer, so removed them from init. Tested this on a AM3703 board with 19.2MHz oscillator, which previously couldnt lock the dpll5 (kernel complained). As a consequence the EHCI USB port wasnt usable in U-Boot and kernel. With this patch, kernel panics disappear and USB working fine in u-boot and kernel. Signed-off-by: Andreas Naumann anaum...@ultratronik.de While this patch works with Linux that has been patched for this erratum, it will cause problems with some unpatched versions of Linux. In particular, this patch sets CM_CLKSEL4_PLL to generate (nearly) 960MHz, and CM_CLKSEL5_PLL to divide by 8 to produce the required 120MHz, as recommended by sprz318e advisory 2.1. Version 3.5 of Linux, and possibly others, configure CM_CLKSEL4_PLL (named dpll5_ck) to generate 120MHz and leaves CM_CLKSEL5_PLL unmodified since its clock (named dpll5_m2_ck) does not support set rate. If u-boot has configured a divisor of 8, the result is that the actual clock speed is 15MHz and USB does not work. Not sure how this ought to be resolved; in my case I'm going to skip the u-boot patch and just use the Linux patch. Peter --- arch/arm/cpu/armv7/omap3/clock.c | 20 +++- arch/arm/cpu/armv7/omap3/lowlevel_init.S | 18 ++ arch/arm/include/asm/arch-omap3/clocks_omap3.h | 22 ++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/arch/arm/cpu/armv7/omap3/clock.c b/arch/arm/cpu/armv7/omap3/clock.c index 81cc859..68833ba 100644 --- a/arch/arm/cpu/armv7/omap3/clock.c +++ b/arch/arm/cpu/armv7/omap3/clock.c @@ -491,6 +491,24 @@ static void dpll4_init_36xx(u32 sil_index, u32 clk_index) wait_on_value(ST_PERIPH_CLK, 2, prcm_base-idlest_ckgen, LDELAY); } +static void dpll5_init_36xx(u32 sil_index, u32 clk_index) +{ + struct prcm *prcm_base = (struct prcm *)PRCM_BASE; + dpll_param *ptr = (dpll_param *) get_36x_per2_dpll_param(); + + /* Moving it to the right sysclk base */ + ptr = ptr + clk_index; + + /* PER2 DPLL (DPLL5) */ + sr32(prcm_base-clken2_pll, 0, 3, PLL_STOP); + wait_on_value(1, 0, prcm_base-idlest2_ckgen, LDELAY); + sr32(prcm_base-clksel5_pll, 0, 5, ptr-m2); /* set M2 (usbtll_fck) */ + sr32(prcm_base-clksel4_pll, 8, 11, ptr-m); /* set m (11-bit multiplier) */ + sr32(prcm_base-clksel4_pll, 0, 7, ptr-n); /* set n (7-bit divider)*/ + sr32(prcm_base-clken2_pll, 0, 3, PLL_LOCK); /* lock mode */ + wait_on_value(1, 1, prcm_base-idlest2_ckgen, LDELAY); +} + static void mpu_init_36xx(u32 sil_index, u32 clk_index) { struct prcm *prcm_base = (struct prcm *)PRCM_BASE; @@ -595,7 +613,7 @@ void prcm_init(void) dpll3_init_36xx(0, clk_index); dpll4_init_36xx(0, clk_index); - dpll5_init_34xx(0, clk_index); + dpll5_init_36xx(0, clk_index); iva_init_36xx(0, clk_index); mpu_init_36xx(0, clk_index); diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index eacfef8..66a1b48 100644 --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S @@ -480,6 +480,19 @@ per_36x_dpll_param: .word 26000,432, 12, 9, 16, 9, 4, 3, 1 .word 38400,360, 15, 9, 16, 5, 4, 3, 1 +per2_36x_dpll_param: +/* 12MHz */ +.word PER2_36XX_M_12, PER2_36XX_N_12, 0, PER2_36XX_M2_12 +/* 13MHz */ +.word PER2_36XX_M_13, PER2_36XX_N_13, 0, PER2_36XX_M2_13 +/* 19.2MHz */ +.word PER2_36XX_M_19P2, PER2_36XX_N_19P2, 0, PER2_36XX_M2_19P2 +/* 26MHz */ +.word PER2_36XX_M_26, PER2_36XX_N_26, 0, PER2_36XX_M2_26 +/* 38.4MHz */ +.word PER2_36XX_M_38P4, PER2_36XX_N_38P4, 0, PER2_36XX_M2_38P4 + + ENTRY(get_36x_mpu_dpll_param) adr r0, mpu_36x_dpll_param mov pc, lr @@ -499,3 +512,8 @@ ENTRY(get_36x_per_dpll_param) adr r0, per_36x_dpll_param mov pc, lr ENDPROC(get_36x_per_dpll_param) + +ENTRY(get_36x_per2_dpll_param) + adr r0, per2_36x_dpll_param + mov pc, lr +ENDPROC(get_36x_per2_dpll_param) diff --git a/arch/arm/include/asm/arch-omap3/clocks_omap3.h b/arch/arm/include/asm/arch-omap3/clocks_omap3.h index 5925ac4..59e61e8 100644 --- a/arch/arm/include/asm/arch-omap3/clocks_omap3.h +++ b/arch/arm/include/asm/arch-omap3/clocks_omap3.h @@ -336,4 +336,26 @@ #define PER_36XX_FSEL_38P40x07 #define PER_36XX_M2_38P4 0x09 +/* 36XX PER2 DPLL */ + +#define PER2_36XX_M_12 0x50 +#define