Re: [U-Boot] commit 83e359adf9f578a58f20daf2e4425a754defac7b breaks mmc

2014-09-10 Thread Peter A. Bigot

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

2014-09-03 Thread Peter A. Bigot

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

2014-09-03 Thread Peter A. Bigot

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

2014-09-03 Thread Peter A. Bigot

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

2014-09-03 Thread Peter A. Bigot

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

2014-09-03 Thread Peter A. Bigot
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

2014-09-03 Thread Peter A. Bigot

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

2014-09-03 Thread Peter A. Bigot
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

2014-09-03 Thread Peter A. Bigot

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

2014-09-03 Thread Peter A. Bigot

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

2014-09-02 Thread Peter A. Bigot
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

2014-09-02 Thread Peter A. Bigot
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

2014-09-02 Thread Peter A. Bigot
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.

2013-08-16 Thread Peter A. Bigot

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.

2013-08-14 Thread Peter A. Bigot

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