| Message ID | 1395646781-24371-1-git-send-email-andrew_gabbasov@mentor.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pantelis Antoniou |
| Headers | show |
Hi Andrew, In general looks good; only a small nag. On Mar 24, 2014, at 9:39 AM, Andrew Gabbasov wrote: > MMC switch command for unsupported feature (e.g. bus width) sets a switch > error bit in card status. This bit should be checked, and, if it's set, > no access with new controller settings should be performed. > > Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com> > --- > drivers/mmc/mmc.c | 4 +++- > include/mmc.h | 2 ++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index 8ab0bc9..16105bc 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -155,6 +155,8 @@ int mmc_send_status(struct mmc *mmc, int timeout) > #endif > return TIMEOUT; > } > + if (cmd.response[0] & MMC_STATUS_SWITCH_ERROR) > + return SWITCH_ERR; > > return 0; > } > @@ -505,7 +507,7 @@ static int mmc_change_freq(struct mmc *mmc) > err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); > > if (err) > - return err; > + return ((err == SWITCH_ERR) ? 0 : err); > Change to: return err == SWITCH_ERR ? 0 : err; No need for parentheses. > /* Now check to see that it worked */ > err = mmc_send_ext_csd(mmc, ext_csd); > diff --git a/include/mmc.h b/include/mmc.h > index e95a237..d5833f8 100644 > --- a/include/mmc.h > +++ b/include/mmc.h > @@ -53,6 +53,7 @@ > #define COMM_ERR -18 /* Communications Error */ > #define TIMEOUT -19 > #define IN_PROGRESS -20 /* operation is in progress */ > +#define SWITCH_ERR -21 /* Card reports failure to switch mode */ > > #define MMC_CMD_GO_IDLE_STATE 0 > #define MMC_CMD_SEND_OP_COND 1 > @@ -108,6 +109,7 @@ > #define SECURE_ERASE 0x80000000 > > #define MMC_STATUS_MASK (~0x0206BF7F) > +#define MMC_STATUS_SWITCH_ERROR (1 << 7) > #define MMC_STATUS_RDY_FOR_DATA (1 << 8) > #define MMC_STATUS_CURR_STATE (0xf << 9) > #define MMC_STATUS_ERROR (1 << 19) > -- > 1.7.10.4 Regards -- Pantelis
Hi Pantelis, > From: Pantelis Antoniou [panto@antoniou-consulting.com] > Sent: Wednesday, April 02, 2014 14:14 > To: Gabbasov, Andrew > Cc: u-boot@lists.denx.de > Subject: Re: [U-Boot] [PATCH] mmc: Handle switch error status bit in MMC card status > > Hi Andrew, > > In general looks good; only a small nag. > > On Mar 24, 2014, at 9:39 AM, Andrew Gabbasov wrote: > > > MMC switch command for unsupported feature (e.g. bus width) sets a switch > > error bit in card status. This bit should be checked, and, if it's set, > > no access with new controller settings should be performed. > > > > Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com> > > --- > > drivers/mmc/mmc.c | 4 +++- > > include/mmc.h | 2 ++ > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > > index 8ab0bc9..16105bc 100644 > > --- a/drivers/mmc/mmc.c > > +++ b/drivers/mmc/mmc.c > > @@ -155,6 +155,8 @@ int mmc_send_status(struct mmc *mmc, int timeout) > > #endif > > return TIMEOUT; > > } > > + if (cmd.response[0] & MMC_STATUS_SWITCH_ERROR) > > + return SWITCH_ERR; > > > > return 0; > > } > > @@ -505,7 +507,7 @@ static int mmc_change_freq(struct mmc *mmc) > > err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); > > > > if (err) > > - return err; > > + return ((err == SWITCH_ERR) ? 0 : err); > > > > Change to: > > return err == SWITCH_ERR ? 0 : err; > > No need for parentheses. Thank you for the review. I'm sending a v2 patch with his change and also re-based to current master head. Thanks! Best regards, Andrew > > > /* Now check to see that it worked */ > > err = mmc_send_ext_csd(mmc, ext_csd); > > diff --git a/include/mmc.h b/include/mmc.h > > index e95a237..d5833f8 100644 > > --- a/include/mmc.h > > +++ b/include/mmc.h > > @@ -53,6 +53,7 @@ > > #define COMM_ERR -18 /* Communications Error */ > > #define TIMEOUT -19 > > #define IN_PROGRESS -20 /* operation is in progress */ > > +#define SWITCH_ERR -21 /* Card reports failure to switch mode */ > > > > #define MMC_CMD_GO_IDLE_STATE 0 > > #define MMC_CMD_SEND_OP_COND 1 > > @@ -108,6 +109,7 @@ > > #define SECURE_ERASE 0x80000000 > > > > #define MMC_STATUS_MASK (~0x0206BF7F) > > +#define MMC_STATUS_SWITCH_ERROR (1 << 7) > > #define MMC_STATUS_RDY_FOR_DATA (1 << 8) > > #define MMC_STATUS_CURR_STATE (0xf << 9) > > #define MMC_STATUS_ERROR (1 << 19) > > -- > > 1.7.10.4 > > Regards > > -- Pantelis >
Hi Pantelis,
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 8ab0bc9..16105bc 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -155,6 +155,8 @@ int mmc_send_status(struct mmc *mmc, int timeout) #endif return TIMEOUT; } + if (cmd.response[0] & MMC_STATUS_SWITCH_ERROR) + return SWITCH_ERR; return 0; } @@ -505,7 +507,7 @@ static int mmc_change_freq(struct mmc *mmc) err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1); if (err) - return err; + return ((err == SWITCH_ERR) ? 0 : err); /* Now check to see that it worked */ err = mmc_send_ext_csd(mmc, ext_csd); diff --git a/include/mmc.h b/include/mmc.h index e95a237..d5833f8 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -53,6 +53,7 @@ #define COMM_ERR -18 /* Communications Error */ #define TIMEOUT -19 #define IN_PROGRESS -20 /* operation is in progress */ +#define SWITCH_ERR -21 /* Card reports failure to switch mode */ #define MMC_CMD_GO_IDLE_STATE 0 #define MMC_CMD_SEND_OP_COND 1 @@ -108,6 +109,7 @@ #define SECURE_ERASE 0x80000000 #define MMC_STATUS_MASK (~0x0206BF7F) +#define MMC_STATUS_SWITCH_ERROR (1 << 7) #define MMC_STATUS_RDY_FOR_DATA (1 << 8) #define MMC_STATUS_CURR_STATE (0xf << 9) #define MMC_STATUS_ERROR (1 << 19)
MMC switch command for unsupported feature (e.g. bus width) sets a switch error bit in card status. This bit should be checked, and, if it's set, no access with new controller settings should be performed. Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com> --- drivers/mmc/mmc.c | 4 +++- include/mmc.h | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-)