[U-Boot] [PATCH 1/2] FSL/eSDHC: enable the peripheral clock to detect the card

From: Jerry Huang Chang-Ming.Huang@freescale.com
According to the card detection of p1/p2 paltform RM, we should set SYSCTL[PEREN] to enable the clock. Otherwise, after booting the u-boot, and then inserting the SD card, the SD card can't be detected.
Signed-off-by: Jerry Huang Chang-Ming.Huang@freescale.com CC: Andy Fleming afleming@gmail.com --- drivers/mmc/fsl_esdhc.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index a2f35e3..1682a79 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -491,6 +491,8 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg) /* First reset the eSDHC controller */ esdhc_reset(regs);
+ esdhc_write32(®s->sysctl, SYSCTL_PEREN); + mmc->priv = cfg; mmc->send_cmd = esdhc_send_cmd; mmc->set_ios = esdhc_set_ios;

From: Jerry Huang Chang-Ming.Huang@freescale.com
When first inserting the SD card to slot, the command "mmcinfo" can display the card information correctly. But, then removing the SD card or inserting another SD card to slot, the command "mmcinfo" can't display the information correctly.
Therefore remove this member 'has_init' from 'structure mmc', and add the codes to check the mmc_init, only when mmc_init return the right value, driver will print the information.
Below is the error log SD card removed: => mmcinfo MMC: no card present Device: FSL_SDHC Manufacturer ID: 3 OEM: 5344 Name: SD02G Tran Speed: 25000000 Rd Block Len: 512 SD version 2.0 High Capacity: No Capacity: 1.8 GiB Bus Width: 4-bit
Signed-off-by: Jerry Huang Chang-Ming.Huang@freescale.com CC: Andy Fleming afleming@gmail.com --- common/cmd_mmc.c | 7 ++----- drivers/mmc/mmc.c | 9 +-------- include/mmc.h | 1 - 3 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 8f13c22..382ea4e 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -131,9 +131,8 @@ int do_mmcinfo (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) mmc = find_mmc_device(curr_device);
if (mmc) { - mmc_init(mmc); - - print_mmcinfo(mmc); + if (!mmc_init(mmc)) + print_mmcinfo(mmc); return 0; } else { printf("no mmc device at slot %x\n", curr_device); @@ -172,8 +171,6 @@ int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 1; }
- mmc->has_init = 0; - if (mmc_init(mmc)) return 1; else diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 9055b01..ad0ebc3 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -1235,14 +1235,10 @@ int mmc_init(struct mmc *mmc) int err;
if (mmc_getcd(mmc) == 0) { - mmc->has_init = 0; printf("MMC: no card present\n"); return NO_CARD_ERR; }
- if (mmc->has_init) - return 0; - err = mmc->init(mmc);
if (err) @@ -1277,10 +1273,7 @@ int mmc_init(struct mmc *mmc) }
err = mmc_startup(mmc); - if (err) - mmc->has_init = 0; - else - mmc->has_init = 1; + return err; }
diff --git a/include/mmc.h b/include/mmc.h index 8744604..8e37504 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -277,7 +277,6 @@ struct mmc { void *priv; uint voltages; uint version; - uint has_init; uint f_min; uint f_max; int high_capacity;

Hi, Do you have any comment about this patch? It can be merge into the next version?
Best Regards Jerry Huang
-----Original Message----- From: Huang Changming-R66093 Sent: Tuesday, March 27, 2012 6:25 PM To: u-boot@lists.denx.de Cc: Huang Changming-R66093; Andy Fleming Subject: [PATCH 2/2] SDHC/MMC: remove the member has_init
From: Jerry Huang Chang-Ming.Huang@freescale.com
When first inserting the SD card to slot, the command "mmcinfo" can display the card information correctly. But, then removing the SD card or inserting another SD card to slot, the command "mmcinfo" can't display the information correctly.
Therefore remove this member 'has_init' from 'structure mmc', and add the codes to check the mmc_init, only when mmc_init return the right value, driver will print the information.
Below is the error log SD card removed: => mmcinfo MMC: no card present Device: FSL_SDHC Manufacturer ID: 3 OEM: 5344 Name: SD02G Tran Speed: 25000000 Rd Block Len: 512 SD version 2.0 High Capacity: No Capacity: 1.8 GiB Bus Width: 4-bit
Signed-off-by: Jerry Huang Chang-Ming.Huang@freescale.com CC: Andy Fleming afleming@gmail.com
common/cmd_mmc.c | 7 ++----- drivers/mmc/mmc.c | 9 +-------- include/mmc.h | 1 - 3 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 8f13c22..382ea4e 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -131,9 +131,8 @@ int do_mmcinfo (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) mmc = find_mmc_device(curr_device);
if (mmc) {
mmc_init(mmc);
print_mmcinfo(mmc);
if (!mmc_init(mmc))
return 0; } else { printf("no mmc device at slot %x\n", curr_device);print_mmcinfo(mmc);
@@ -172,8 +171,6 @@ int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 1; }
mmc->has_init = 0;
- if (mmc_init(mmc)) return 1; else
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 9055b01..ad0ebc3 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -1235,14 +1235,10 @@ int mmc_init(struct mmc *mmc) int err;
if (mmc_getcd(mmc) == 0) {
mmc->has_init = 0;
printf("MMC: no card present\n"); return NO_CARD_ERR; }
if (mmc->has_init)
return 0;
err = mmc->init(mmc);
if (err)
@@ -1277,10 +1273,7 @@ int mmc_init(struct mmc *mmc) }
err = mmc_startup(mmc);
- if (err)
mmc->has_init = 0;
- else
mmc->has_init = 1;
- return err;
}
diff --git a/include/mmc.h b/include/mmc.h index 8744604..8e37504 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -277,7 +277,6 @@ struct mmc { void *priv; uint voltages; uint version;
- uint has_init; uint f_min; uint f_max; int high_capacity;
-- 1.7.5.4

I am highly inclined to apply this patch, as it was originally the intent that the init function get called every time. If it doesn't, it's impossible to deal with cards being inserted and removed after U-Boot comes up.
However, I've seen some recent chatter on the list where it sounds like people are relying on this functionality, now. So we need some discussion. Assuming I don't apply this patch, how do we deal with the problem of cards being removed or inserted after U-Boot boots?
On Tue, Mar 27, 2012 at 5:25 AM, Chang-Ming.Huang@freescale.com wrote:
From: Jerry Huang Chang-Ming.Huang@freescale.com
When first inserting the SD card to slot, the command "mmcinfo" can display the card information correctly. But, then removing the SD card or inserting another SD card to slot, the command "mmcinfo" can't display the information correctly.
Therefore remove this member 'has_init' from 'structure mmc', and add the codes to check the mmc_init, only when mmc_init return the right value, driver will print the information.
Below is the error log SD card removed: => mmcinfo MMC: no card present Device: FSL_SDHC Manufacturer ID: 3 OEM: 5344 Name: SD02G Tran Speed: 25000000 Rd Block Len: 512 SD version 2.0 High Capacity: No Capacity: 1.8 GiB Bus Width: 4-bit
Signed-off-by: Jerry Huang Chang-Ming.Huang@freescale.com CC: Andy Fleming afleming@gmail.com
common/cmd_mmc.c | 7 ++----- drivers/mmc/mmc.c | 9 +-------- include/mmc.h | 1 - 3 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 8f13c22..382ea4e 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -131,9 +131,8 @@ int do_mmcinfo (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) mmc = find_mmc_device(curr_device);
if (mmc) {
- mmc_init(mmc);
- print_mmcinfo(mmc);
- if (!mmc_init(mmc))
- print_mmcinfo(mmc);
return 0; } else { printf("no mmc device at slot %x\n", curr_device); @@ -172,8 +171,6 @@ int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 1; }
- mmc->has_init = 0;
if (mmc_init(mmc)) return 1; else diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 9055b01..ad0ebc3 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -1235,14 +1235,10 @@ int mmc_init(struct mmc *mmc) int err;
if (mmc_getcd(mmc) == 0) {
- mmc->has_init = 0;
printf("MMC: no card present\n"); return NO_CARD_ERR; }
- if (mmc->has_init)
- return 0;
err = mmc->init(mmc);
if (err) @@ -1277,10 +1273,7 @@ int mmc_init(struct mmc *mmc) }
err = mmc_startup(mmc);
- if (err)
- mmc->has_init = 0;
- else
- mmc->has_init = 1;
return err; }
diff --git a/include/mmc.h b/include/mmc.h index 8744604..8e37504 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -277,7 +277,6 @@ struct mmc { void *priv; uint voltages; uint version;
- uint has_init;
uint f_min; uint f_max; int high_capacity; -- 1.7.5.4

If some peoples rely on this function, then we need to remove the below codes from 'mmc_init': if (mmc->has_init) return 0;
when we use command "mmcinfo" every time, the driver must initialize the SD card again, instead of assuming the card has been initialized.
Best Regards Jerry Huang
-----Original Message----- From: Andy Fleming [mailto:afleming@gmail.com] Sent: Wednesday, May 09, 2012 6:17 AM To: Huang Changming-R66093 Cc: u-boot@lists.denx.de Subject: Re: [PATCH 2/2] SDHC/MMC: remove the member has_init
I am highly inclined to apply this patch, as it was originally the intent that the init function get called every time. If it doesn't, it's impossible to deal with cards being inserted and removed after U-Boot comes up.
However, I've seen some recent chatter on the list where it sounds like people are relying on this functionality, now. So we need some discussion. Assuming I don't apply this patch, how do we deal with the problem of cards being removed or inserted after U-Boot boots?
On Tue, Mar 27, 2012 at 5:25 AM, Chang-Ming.Huang@freescale.com wrote:
From: Jerry Huang Chang-Ming.Huang@freescale.com
When first inserting the SD card to slot, the command "mmcinfo" can display the card information correctly. But, then removing the SD card or inserting another SD card to slot, the command "mmcinfo" can't display the information correctly.
Therefore remove this member 'has_init' from 'structure mmc', and add the codes to check the mmc_init, only when mmc_init return the right value, driver will print the information.
Below is the error log SD card removed: => mmcinfo MMC: no card present Device: FSL_SDHC Manufacturer ID: 3 OEM: 5344 Name: SD02G Tran Speed: 25000000 Rd Block Len: 512 SD version 2.0 High Capacity: No Capacity: 1.8 GiB Bus Width: 4-bit
Signed-off-by: Jerry Huang Chang-Ming.Huang@freescale.com CC: Andy Fleming afleming@gmail.com
common/cmd_mmc.c | 7 ++----- drivers/mmc/mmc.c | 9 +-------- include/mmc.h | 1 - 3 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 8f13c22..382ea4e 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -131,9 +131,8 @@ int do_mmcinfo (cmd_tbl_t *cmdtp, int flag, int
argc, char * const argv[])
mmc = find_mmc_device(curr_device);
if (mmc) {
- mmc_init(mmc);
- print_mmcinfo(mmc);
- if (!mmc_init(mmc))
- print_mmcinfo(mmc);
return 0; } else { printf("no mmc device at slot %x\n", curr_device); @@ -172,8 +171,6 @@ int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
return 1; }
- mmc->has_init = 0;
if (mmc_init(mmc)) return 1; else diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 9055b01..ad0ebc3 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -1235,14 +1235,10 @@ int mmc_init(struct mmc *mmc) int err;
if (mmc_getcd(mmc) == 0) {
- mmc->has_init = 0;
printf("MMC: no card present\n"); return NO_CARD_ERR; }
- if (mmc->has_init)
- return 0;
err = mmc->init(mmc);
if (err) @@ -1277,10 +1273,7 @@ int mmc_init(struct mmc *mmc) }
err = mmc_startup(mmc);
- if (err)
- mmc->has_init = 0;
- else
- mmc->has_init = 1;
return err; }
diff --git a/include/mmc.h b/include/mmc.h index 8744604..8e37504 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -277,7 +277,6 @@ struct mmc { void *priv; uint voltages; uint version;
- uint has_init;
uint f_min; uint f_max; int high_capacity; -- 1.7.5.4

On 27.03.2012 12:25, Chang-Ming.Huang@freescale.com wrote:
From: Jerry Huang Chang-Ming.Huang@freescale.com
According to the card detection of p1/p2 paltform RM,
typo => platform
we should set SYSCTL[PEREN] to enable the clock. Otherwise, after booting the u-boot, and then inserting the SD card, the SD card can't be detected.
Signed-off-by: Jerry Huang Chang-Ming.Huang@freescale.com CC: Andy Fleming afleming@gmail.com
drivers/mmc/fsl_esdhc.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index a2f35e3..1682a79 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -491,6 +491,8 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg) /* First reset the eSDHC controller */ esdhc_reset(regs);
- esdhc_write32(®s->sysctl, SYSCTL_PEREN);
If I read the iMX6 manual correctly, the SYSCTL_PEREN isn't implemented there. So this should be a NOP for iMX6?
Best regards
Dirk

On 27/03/2012 13:01, Dirk Behme wrote:
On 27.03.2012 12:25, Chang-Ming.Huang@freescale.com wrote:
From: Jerry Huang Chang-Ming.Huang@freescale.com
According to the card detection of p1/p2 paltform RM,
typo => platform
we should set SYSCTL[PEREN] to enable the clock. Otherwise, after booting the u-boot, and then inserting the SD card, the SD card can't be detected.
Signed-off-by: Jerry Huang Chang-Ming.Huang@freescale.com CC: Andy Fleming afleming@gmail.com
drivers/mmc/fsl_esdhc.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index a2f35e3..1682a79 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -491,6 +491,8 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg) /* First reset the eSDHC controller */ esdhc_reset(regs);
- esdhc_write32(®s->sysctl, SYSCTL_PEREN);
If I read the iMX6 manual correctly, the SYSCTL_PEREN isn't implemented there.
In the copy I could put my hands, SYSCTL_PEREN is implemented for i.MX6. However, I do not know if this has an influence. ESDHC should be clocked with SDCLKEN, that is enabled by default on i.MX (both MX5 and MX6).
So this should be a NOP for iMX6?
However, if it is not implemented and it is marked as reserved, we should not set it.
Best regards, Stefano Babic

Thanks Jerry Huang
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: Wednesday, March 28, 2012 1:17 AM To: Dirk Behme Cc: Huang Changming-R66093; u-boot@lists.denx.de; Andy Fleming Subject: Re: [U-Boot] [PATCH 1/2] FSL/eSDHC: enable the peripheral clock to detect the card
On 27/03/2012 13:01, Dirk Behme wrote:
On 27.03.2012 12:25, Chang-Ming.Huang@freescale.com wrote:
From: Jerry Huang Chang-Ming.Huang@freescale.com
According to the card detection of p1/p2 paltform RM,
typo => platform
we should set SYSCTL[PEREN] to enable the clock. Otherwise, after booting the u-boot, and then inserting the SD card, the SD card can't be detected.
Signed-off-by: Jerry Huang Chang-Ming.Huang@freescale.com CC: Andy Fleming afleming@gmail.com
drivers/mmc/fsl_esdhc.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index a2f35e3..1682a79 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -491,6 +491,8 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg) /* First reset the eSDHC controller */ esdhc_reset(regs);
- esdhc_write32(®s->sysctl, SYSCTL_PEREN);
If I read the iMX6 manual correctly, the SYSCTL_PEREN isn't implemented there.
In the copy I could put my hands, SYSCTL_PEREN is implemented for i.MX6. However, I do not know if this has an influence. ESDHC should be clocked with SDCLKEN, that is enabled by default on i.MX (both MX5 and MX6).
So this should be a NOP for iMX6?
However, if it is not implemented and it is marked as reserved, we should not set it.
I.MX6/5 reuse the fsl_esdhc.c driver, and he don't care these reserved field. Otherwise he should change this driver. E.g: In function esdhc_init: esdhc_write32(®s->sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN); in function set_sysctl: clk = SYSCTL_PEREN | SYSCTL_CKEN; esdhc_setbits32(®s->sysctl, clk);
so I think my code has no any problem.

I.MX6/5 reuse the fsl_esdhc.c driver, and he don't care these reserved field. Otherwise he should change this driver. E.g: In function esdhc_init: esdhc_write32(®s->sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN); in function set_sysctl: clk = SYSCTL_PEREN | SYSCTL_CKEN; esdhc_setbits32(®s->sysctl, clk);
so I think my code has no any problem.
ok, fine.
Best regards, Stefano Babic

On 27/03/2012 12:25, Chang-Ming.Huang@freescale.com wrote:
From: Jerry Huang Chang-Ming.Huang@freescale.com
According to the card detection of p1/p2 paltform RM, we should set SYSCTL[PEREN] to enable the clock. Otherwise, after booting the u-boot, and then inserting the SD card, the SD card can't be detected.
Signed-off-by: Jerry Huang Chang-Ming.Huang@freescale.com CC: Andy Fleming afleming@gmail.com
Hi Jerry,
drivers/mmc/fsl_esdhc.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index a2f35e3..1682a79 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -491,6 +491,8 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg) /* First reset the eSDHC controller */ esdhc_reset(regs);
- esdhc_write32(®s->sysctl, SYSCTL_PEREN);
You add this setup to all architectures: PQIII, i.MX5, i.MX6. Is it what you really want ?
Best regards, Stefano Babic

Thanks Jerry Huang
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: Wednesday, March 28, 2012 1:13 AM To: Huang Changming-R66093 Cc: u-boot@lists.denx.de; Andy Fleming Subject: Re: [U-Boot] [PATCH 1/2] FSL/eSDHC: enable the peripheral clock to detect the card
On 27/03/2012 12:25, Chang-Ming.Huang@freescale.com wrote:
From: Jerry Huang Chang-Ming.Huang@freescale.com
According to the card detection of p1/p2 paltform RM, we should set SYSCTL[PEREN] to enable the clock. Otherwise, after booting the u-boot, and then inserting the SD card, the SD card can't be detected.
Signed-off-by: Jerry Huang Chang-Ming.Huang@freescale.com CC: Andy Fleming afleming@gmail.com
Hi Jerry,
drivers/mmc/fsl_esdhc.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index a2f35e3..1682a79 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -491,6 +491,8 @@ int fsl_esdhc_initialize(bd_t *bis, struct
fsl_esdhc_cfg *cfg)
/* First reset the eSDHC controller */ esdhc_reset(regs);
- esdhc_write32(®s->sysctl, SYSCTL_PEREN);
You add this setup to all architectures: PQIII, i.MX5, i.MX6. Is it what you really want ?
In fact, this field is set in function 'set_sysctl', too. In order to detect the card, I just reset it in initialize stage.
For p1/p2, the field SYSCTL[PEREN](bit29) is needed to enable the peripheral clock before Detecting the SD card. For p3041, this field is reserved, I think I.MX5 and I.MX6 has the same setting. And I have tested this field on p3041, accessing this field has no any impact for SD controller.
In fact, you can find out, the driver fsl_esdh.c is first wrote for p1/p2 (has PEREN/KCKEN/IPGEN, bit29/30/31), i.MXx/PQIII just reuses it, and some registers have difference setting, such as SYSCRL[29, 30, 31] for p1/p2, they are reserved for p3041.
participants (5)
-
Andy Fleming
-
Chang-Ming.Huang@freescale.com
-
Dirk Behme
-
Huang Changming-R66093
-
Stefano Babic