Re: [U-Boot] [RFC] mmc: Properly determine maximum supported bus width
On Tue, Nov 27, 2012 at 5:40 PM, Stephen Warren wrote: > On 11/26/2012 04:33 PM, Fleming Andy-AFLEMING wrote: > > Yes, I'm processing my queue today. > > This patch didn't show up in the pull request you just sent. > Yeah, sorry, I don't know how that happened. I'm applying it now. Andy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] mmc: Properly determine maximum supported bus width
On 11/26/2012 04:33 PM, Fleming Andy-AFLEMING wrote: > Yes, I'm processing my queue today. This patch didn't show up in the pull request you just sent. > On Nov 26, 2012, at 17:27, "Stephen Warren" wrote: > >> On 10/31/2012 11:02 PM, Andy Fleming wrote: >>> At some point, a confusion arose about the use of the bit >>> definitions in host_caps for bus widths, and the value >>> in ext_csd. By coincidence, a simple shift could convert >>> between one and the other: >>> >>> MMC_MODE_1BIT = 0, EXT_CSD_BUS_WIDTH_1 = 0 >>> MMC_MODE_4BIT = 0x100, EXT_CSD_BUS_WIDTH_4 = 1 >>> MMC_MODE_8BIT = 0x200, EXT_CSD_BUS_WIDTH_8 = 2 >>> >>> However, as host_caps is a bitmask of supported things, >>> there is not, in fact, a one-to-one correspondence. host_caps >>> is capable of containing MODE_4BIT | MODE_8BIT, so nonsensical >>> things were happening where we would try to set the bus width >>> to 12. >>> >>> The new code clarifies the very different namespaces: >>> >>> host_caps/card_caps = bitmask (MMC_MODE_*) >>> ext CSD fields are just an index (EXT_CSD_BUS_WIDTH_*) >>> mmc->bus_width integer number of bits (1, 4, 8) >>> >>> We create arrays to map between the namespaces, like in Linux. >> >> Andy, is this patch likely to get merged soon to u-boot.git branch >> master? Unfortunately, some Tegra patches that ideally rely on this >> patch being present have already been applied and merged into >> u-boot-arm.git branch master. >> > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] mmc: Properly determine maximum supported bus width
Yes, I'm processing my queue today. On Nov 26, 2012, at 17:27, "Stephen Warren" wrote: > On 10/31/2012 11:02 PM, Andy Fleming wrote: >> At some point, a confusion arose about the use of the bit >> definitions in host_caps for bus widths, and the value >> in ext_csd. By coincidence, a simple shift could convert >> between one and the other: >> >> MMC_MODE_1BIT = 0, EXT_CSD_BUS_WIDTH_1 = 0 >> MMC_MODE_4BIT = 0x100, EXT_CSD_BUS_WIDTH_4 = 1 >> MMC_MODE_8BIT = 0x200, EXT_CSD_BUS_WIDTH_8 = 2 >> >> However, as host_caps is a bitmask of supported things, >> there is not, in fact, a one-to-one correspondence. host_caps >> is capable of containing MODE_4BIT | MODE_8BIT, so nonsensical >> things were happening where we would try to set the bus width >> to 12. >> >> The new code clarifies the very different namespaces: >> >> host_caps/card_caps = bitmask (MMC_MODE_*) >> ext CSD fields are just an index (EXT_CSD_BUS_WIDTH_*) >> mmc->bus_width integer number of bits (1, 4, 8) >> >> We create arrays to map between the namespaces, like in Linux. > > Andy, is this patch likely to get merged soon to u-boot.git branch > master? Unfortunately, some Tegra patches that ideally rely on this > patch being present have already been applied and merged into > u-boot-arm.git branch master. > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] mmc: Properly determine maximum supported bus width
On 10/31/2012 11:02 PM, Andy Fleming wrote: > At some point, a confusion arose about the use of the bit > definitions in host_caps for bus widths, and the value > in ext_csd. By coincidence, a simple shift could convert > between one and the other: > > MMC_MODE_1BIT = 0, EXT_CSD_BUS_WIDTH_1 = 0 > MMC_MODE_4BIT = 0x100, EXT_CSD_BUS_WIDTH_4 = 1 > MMC_MODE_8BIT = 0x200, EXT_CSD_BUS_WIDTH_8 = 2 > > However, as host_caps is a bitmask of supported things, > there is not, in fact, a one-to-one correspondence. host_caps > is capable of containing MODE_4BIT | MODE_8BIT, so nonsensical > things were happening where we would try to set the bus width > to 12. > > The new code clarifies the very different namespaces: > > host_caps/card_caps = bitmask (MMC_MODE_*) > ext CSD fields are just an index (EXT_CSD_BUS_WIDTH_*) > mmc->bus_width integer number of bits (1, 4, 8) > > We create arrays to map between the namespaces, like in Linux. Andy, is this patch likely to get merged soon to u-boot.git branch master? Unfortunately, some Tegra patches that ideally rely on this patch being present have already been applied and merged into u-boot-arm.git branch master. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] mmc: Properly determine maximum supported bus width
On 10/31/2012 11:02 PM, Andy Fleming wrote: > At some point, a confusion arose about the use of the bit > definitions in host_caps for bus widths, and the value > in ext_csd. By coincidence, a simple shift could convert > between one and the other: > > MMC_MODE_1BIT = 0, EXT_CSD_BUS_WIDTH_1 = 0 > MMC_MODE_4BIT = 0x100, EXT_CSD_BUS_WIDTH_4 = 1 > MMC_MODE_8BIT = 0x200, EXT_CSD_BUS_WIDTH_8 = 2 > > However, as host_caps is a bitmask of supported things, > there is not, in fact, a one-to-one correspondence. host_caps > is capable of containing MODE_4BIT | MODE_8BIT, so nonsensical > things were happening where we would try to set the bus width > to 12. > > The new code clarifies the very different namespaces: > > host_caps/card_caps = bitmask (MMC_MODE_*) > ext CSD fields are just an index (EXT_CSD_BUS_WIDTH_*) > mmc->bus_width integer number of bits (1, 4, 8) > > We create arrays to map between the namespaces, like in Linux. (Coupled with a small change to the Tegra driver to correctly set host_caps for 8-/4-bit, which I'll post shortly), Tested-by: Stephen Warren Interestingly, I just yesterday started working on a bug I'd filed months ago to enable 8-bit eMMC support on Tegra, and found the 12-bit issue, and came up with a very similar patch. Oh well! ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] mmc: Properly determine maximum supported bus width
Hi Andy,
It's working fine on my environment.
Tested-by: Jaehoon Chung
Best Regards,
Jaehoon Chung
On 11/01/2012 02:02 PM, Andy Fleming wrote:
> At some point, a confusion arose about the use of the bit
> definitions in host_caps for bus widths, and the value
> in ext_csd. By coincidence, a simple shift could convert
> between one and the other:
>
> MMC_MODE_1BIT = 0, EXT_CSD_BUS_WIDTH_1 = 0
> MMC_MODE_4BIT = 0x100, EXT_CSD_BUS_WIDTH_4 = 1
> MMC_MODE_8BIT = 0x200, EXT_CSD_BUS_WIDTH_8 = 2
>
> However, as host_caps is a bitmask of supported things,
> there is not, in fact, a one-to-one correspondence. host_caps
> is capable of containing MODE_4BIT | MODE_8BIT, so nonsensical
> things were happening where we would try to set the bus width
> to 12.
>
> The new code clarifies the very different namespaces:
>
> host_caps/card_caps = bitmask (MMC_MODE_*)
> ext CSD fields are just an index (EXT_CSD_BUS_WIDTH_*)
> mmc->bus_width integer number of bits (1, 4, 8)
>
> We create arrays to map between the namespaces, like in Linux.
>
> Signed-off-by: Andy Fleming
> ---
>
> This is, I think, the approach we should use to identify the
> proper bus width for an mmc device. Please review and/or test.
>
>
> drivers/mmc/mmc.c | 47 +++
> 1 file changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 5fbf956..c379a74 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -867,7 +867,7 @@ void mmc_set_bus_width(struct mmc *mmc, uint width)
>
> int mmc_startup(struct mmc *mmc)
> {
> - int err, width;
> + int err;
> uint mult, freq;
> u64 cmult, csize, capacity;
> struct mmc_cmd cmd;
> @@ -1086,21 +1086,44 @@ int mmc_startup(struct mmc *mmc)
> else
> mmc->tran_speed = 2500;
> } else {
> - width = ((mmc->host_caps & MMC_MODE_MASK_WIDTH_BITS) >>
> - MMC_MODE_WIDTH_BITS_SHIFT);
> - for (; width >= 0; width--) {
> - /* Set the card to use 4 bit*/
> + int idx;
> +
> + /* An array of possible bus widths in order of preference */
> + static unsigned ext_csd_bits[] = {
> + EXT_CSD_BUS_WIDTH_8,
> + EXT_CSD_BUS_WIDTH_4,
> + EXT_CSD_BUS_WIDTH_1,
> + };
> +
> + /* An array to map CSD bus widths to host cap bits */
> + static unsigned ext_to_hostcaps[] = {
> + [EXT_CSD_BUS_WIDTH_4] = MMC_MODE_4BIT,
> + [EXT_CSD_BUS_WIDTH_8] = MMC_MODE_8BIT,
> + };
> +
> + /* An array to map chosen bus width to an integer */
> + static unsigned widths[] = {
> + 8, 4, 1,
> + };
> +
> + for (idx=0; idx < ARRAY_SIZE(ext_csd_bits); idx++) {
> + unsigned int extw = ext_csd_bits[idx];
> +
> + /*
> + * Check to make sure the controller supports
> + * this bus width, if it's more than 1
> + */
> + if (extw != EXT_CSD_BUS_WIDTH_1 &&
> + !(mmc->host_caps &
> ext_to_hostcaps[extw]))
> + continue;
> +
> err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
> - EXT_CSD_BUS_WIDTH, width);
> + EXT_CSD_BUS_WIDTH, extw);
>
> if (err)
> continue;
>
> - if (!width) {
> - mmc_set_bus_width(mmc, 1);
> - break;
> - } else
> - mmc_set_bus_width(mmc, 4 * width);
> + mmc_set_bus_width(mmc, widths[idx]);
>
> err = mmc_send_ext_csd(mmc, test_csd);
> if (!err && ext_csd[EXT_CSD_PARTITIONING_SUPPORT] \
> @@ -1114,7 +1137,7 @@ int mmc_startup(struct mmc *mmc)
>&& memcmp(&ext_csd[EXT_CSD_SEC_CNT], \
> &test_csd[EXT_CSD_SEC_CNT], 4) == 0) {
>
> - mmc->card_caps |= width;
> + mmc->card_caps |= ext_to_hostcaps[extw];
> break;
> }
> }
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [RFC] mmc: Properly determine maximum supported bus width
At some point, a confusion arose about the use of the bit
definitions in host_caps for bus widths, and the value
in ext_csd. By coincidence, a simple shift could convert
between one and the other:
MMC_MODE_1BIT = 0, EXT_CSD_BUS_WIDTH_1 = 0
MMC_MODE_4BIT = 0x100, EXT_CSD_BUS_WIDTH_4 = 1
MMC_MODE_8BIT = 0x200, EXT_CSD_BUS_WIDTH_8 = 2
However, as host_caps is a bitmask of supported things,
there is not, in fact, a one-to-one correspondence. host_caps
is capable of containing MODE_4BIT | MODE_8BIT, so nonsensical
things were happening where we would try to set the bus width
to 12.
The new code clarifies the very different namespaces:
host_caps/card_caps = bitmask (MMC_MODE_*)
ext CSD fields are just an index (EXT_CSD_BUS_WIDTH_*)
mmc->bus_width integer number of bits (1, 4, 8)
We create arrays to map between the namespaces, like in Linux.
Signed-off-by: Andy Fleming
---
This is, I think, the approach we should use to identify the
proper bus width for an mmc device. Please review and/or test.
drivers/mmc/mmc.c | 47 +++
1 file changed, 35 insertions(+), 12 deletions(-)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 5fbf956..c379a74 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -867,7 +867,7 @@ void mmc_set_bus_width(struct mmc *mmc, uint width)
int mmc_startup(struct mmc *mmc)
{
- int err, width;
+ int err;
uint mult, freq;
u64 cmult, csize, capacity;
struct mmc_cmd cmd;
@@ -1086,21 +1086,44 @@ int mmc_startup(struct mmc *mmc)
else
mmc->tran_speed = 2500;
} else {
- width = ((mmc->host_caps & MMC_MODE_MASK_WIDTH_BITS) >>
-MMC_MODE_WIDTH_BITS_SHIFT);
- for (; width >= 0; width--) {
- /* Set the card to use 4 bit*/
+ int idx;
+
+ /* An array of possible bus widths in order of preference */
+ static unsigned ext_csd_bits[] = {
+ EXT_CSD_BUS_WIDTH_8,
+ EXT_CSD_BUS_WIDTH_4,
+ EXT_CSD_BUS_WIDTH_1,
+ };
+
+ /* An array to map CSD bus widths to host cap bits */
+ static unsigned ext_to_hostcaps[] = {
+ [EXT_CSD_BUS_WIDTH_4] = MMC_MODE_4BIT,
+ [EXT_CSD_BUS_WIDTH_8] = MMC_MODE_8BIT,
+ };
+
+ /* An array to map chosen bus width to an integer */
+ static unsigned widths[] = {
+ 8, 4, 1,
+ };
+
+ for (idx=0; idx < ARRAY_SIZE(ext_csd_bits); idx++) {
+ unsigned int extw = ext_csd_bits[idx];
+
+ /*
+* Check to make sure the controller supports
+* this bus width, if it's more than 1
+*/
+ if (extw != EXT_CSD_BUS_WIDTH_1 &&
+ !(mmc->host_caps &
ext_to_hostcaps[extw]))
+ continue;
+
err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
- EXT_CSD_BUS_WIDTH, width);
+ EXT_CSD_BUS_WIDTH, extw);
if (err)
continue;
- if (!width) {
- mmc_set_bus_width(mmc, 1);
- break;
- } else
- mmc_set_bus_width(mmc, 4 * width);
+ mmc_set_bus_width(mmc, widths[idx]);
err = mmc_send_ext_csd(mmc, test_csd);
if (!err && ext_csd[EXT_CSD_PARTITIONING_SUPPORT] \
@@ -1114,7 +1137,7 @@ int mmc_startup(struct mmc *mmc)
&& memcmp(&ext_csd[EXT_CSD_SEC_CNT], \
&test_csd[EXT_CSD_SEC_CNT], 4) == 0) {
- mmc->card_caps |= width;
+ mmc->card_caps |= ext_to_hostcaps[extw];
break;
}
}
--
1.7.9.7
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
