| Message ID | 20210123123741.3224-2-stefan_b@posteo.net |
|---|---|
| State | Accepted |
| Commit | 8e2b0af7216d78b60fccb46a107a4a047938aea9 |
| Delegated to: | Peng Fan |
| Headers | show |
| Series | mmc: fix response timeout after switch command | expand |
Hi, On 1/23/21 9:37 PM, Stefan Bosch wrote: > After issuing the switch command: Wait until 'current state' of the card > status becomes 'tran'. This prevents from response timeout at the next > command because of 'current state' = 'data'.> > Signed-off-by: Stefan Bosch <stefan_b@posteo.net> > --- > > drivers/mmc/mmc.c | 3 ++- > include/mmc.h | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index a47700e313..8ccd2058a9 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -823,7 +823,8 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value, > value); > return -EIO; > } > - if (!ret && (status & MMC_STATUS_RDY_FOR_DATA)) > + if (!ret && (status & MMC_STATUS_RDY_FOR_DATA) && > + (status & MMC_STATUS_CURR_STATE) == MMC_STATE_TRANS) AFAIK, MMC_STATUS_RDY_FOR_DATA bit means the signaling about "buffer is empty". Even though its bit was set, does it need to check TRANS state? Best Regards, Jaehoon Chung > return 0; > udelay(100); > } while (get_timer(start) < timeout_ms); > diff --git a/include/mmc.h b/include/mmc.h > index 1d377e0281..18402494c6 100644 > --- a/include/mmc.h > +++ b/include/mmc.h > @@ -178,6 +178,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx) > #define MMC_STATUS_ERROR (1 << 19) > > #define MMC_STATE_PRG (7 << 9) > +#define MMC_STATE_TRANS (4 << 9) > > #define MMC_VDD_165_195 0x00000080 /* VDD voltage 1.65 - 1.95 */ > #define MMC_VDD_20_21 0x00000100 /* VDD voltage 2.0 ~ 2.1 */ >
Hi, On 27.01.21 00:38, Jaehoon Chung wrote: > Hi, > > On 1/23/21 9:37 PM, Stefan Bosch wrote: >> After issuing the switch command: Wait until 'current state' of the card >> status becomes 'tran'. This prevents from response timeout at the next >> command because of 'current state' = 'data'.> >> Signed-off-by: Stefan Bosch <stefan_b@posteo.net> >> --- >> >> drivers/mmc/mmc.c | 3 ++- >> include/mmc.h | 1 + >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c >> index a47700e313..8ccd2058a9 100644 >> --- a/drivers/mmc/mmc.c >> +++ b/drivers/mmc/mmc.c >> @@ -823,7 +823,8 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value, >> value); >> return -EIO; >> } >> - if (!ret && (status & MMC_STATUS_RDY_FOR_DATA)) >> + if (!ret && (status & MMC_STATUS_RDY_FOR_DATA) && >> + (status & MMC_STATUS_CURR_STATE) == MMC_STATE_TRANS) > > AFAIK, MMC_STATUS_RDY_FOR_DATA bit means the signaling about "buffer is empty". > Even though its bit was set, does it need to check TRANS state? > > Best Regards, > Jaehoon Chung > After issuing command 'MMC_CMD_SWITCH' = CMD6 the state transits to 'DATA'. But in DATA state it is not possible to issue a (new) CMD except CMD12 (operation complete) and CMD7 (select/deselect card). See appropriate State Diagram, e.g. "Figure 4-3: SD Memory Card State Diagram (data transfer mode)" in "SD Specifications Part 1 - Physical Layer - Simplified Specification - Version 2.00". Status 'Ready For Data' is in my opinion only relevant for CMD12 and CMD7. Regards Stefan Bosch >> return 0; >> udelay(100); >> } while (get_timer(start) < timeout_ms); >> diff --git a/include/mmc.h b/include/mmc.h >> index 1d377e0281..18402494c6 100644 >> --- a/include/mmc.h >> +++ b/include/mmc.h >> @@ -178,6 +178,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx) >> #define MMC_STATUS_ERROR (1 << 19) >> >> #define MMC_STATE_PRG (7 << 9) >> +#define MMC_STATE_TRANS (4 << 9) >> >> #define MMC_VDD_165_195 0x00000080 /* VDD voltage 1.65 - 1.95 */ >> #define MMC_VDD_20_21 0x00000100 /* VDD voltage 2.0 ~ 2.1 */ >> >
Hi Stefan, On 1/29/21 4:23 AM, Stefan Bosch wrote: > Hi, > > On 27.01.21 00:38, Jaehoon Chung wrote: >> Hi, >> >> On 1/23/21 9:37 PM, Stefan Bosch wrote: >>> After issuing the switch command: Wait until 'current state' of the card >>> status becomes 'tran'. This prevents from response timeout at the next >>> command because of 'current state' = 'data'.> >>> Signed-off-by: Stefan Bosch <stefan_b@posteo.net> >>> --- >>> >>> drivers/mmc/mmc.c | 3 ++- >>> include/mmc.h | 1 + >>> 2 files changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c >>> index a47700e313..8ccd2058a9 100644 >>> --- a/drivers/mmc/mmc.c >>> +++ b/drivers/mmc/mmc.c >>> @@ -823,7 +823,8 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value, >>> value); >>> return -EIO; >>> } >>> - if (!ret && (status & MMC_STATUS_RDY_FOR_DATA)) >>> + if (!ret && (status & MMC_STATUS_RDY_FOR_DATA) && >>> + (status & MMC_STATUS_CURR_STATE) == MMC_STATE_TRANS) >> >> AFAIK, MMC_STATUS_RDY_FOR_DATA bit means the signaling about "buffer is empty". >> Even though its bit was set, does it need to check TRANS state? >> >> Best Regards, >> Jaehoon Chung >> > > After issuing command 'MMC_CMD_SWITCH' = CMD6 the state transits to 'DATA'. But in DATA state it is not possible to issue a (new) CMD except CMD12 (operation complete) and CMD7 (select/deselect card). See appropriate State Diagram, e.g. "Figure 4-3: SD Memory Card State Diagram (data transfer mode)" in "SD Specifications Part 1 - Physical Layer - Simplified Specification - Version 2.00". Status 'Ready For Data' is in my opinion only relevant for CMD12 and CMD7. Thanks for explanation. I have checked Spec about SD and eMMC. It makes sense. Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> Best Regards, Jaehoon Chung > > Regards > Stefan Bosch > >>> return 0; >>> udelay(100); >>> } while (get_timer(start) < timeout_ms); >>> diff --git a/include/mmc.h b/include/mmc.h >>> index 1d377e0281..18402494c6 100644 >>> --- a/include/mmc.h >>> +++ b/include/mmc.h >>> @@ -178,6 +178,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx) >>> #define MMC_STATUS_ERROR (1 << 19) >>> #define MMC_STATE_PRG (7 << 9) >>> +#define MMC_STATE_TRANS (4 << 9) >>> #define MMC_VDD_165_195 0x00000080 /* VDD voltage 1.65 - 1.95 */ >>> #define MMC_VDD_20_21 0x00000100 /* VDD voltage 2.0 ~ 2.1 */ >>> >> >
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index a47700e313..8ccd2058a9 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -823,7 +823,8 @@ static int __mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value, value); return -EIO; } - if (!ret && (status & MMC_STATUS_RDY_FOR_DATA)) + if (!ret && (status & MMC_STATUS_RDY_FOR_DATA) && + (status & MMC_STATUS_CURR_STATE) == MMC_STATE_TRANS) return 0; udelay(100); } while (get_timer(start) < timeout_ms); diff --git a/include/mmc.h b/include/mmc.h index 1d377e0281..18402494c6 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -178,6 +178,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx) #define MMC_STATUS_ERROR (1 << 19) #define MMC_STATE_PRG (7 << 9) +#define MMC_STATE_TRANS (4 << 9) #define MMC_VDD_165_195 0x00000080 /* VDD voltage 1.65 - 1.95 */ #define MMC_VDD_20_21 0x00000100 /* VDD voltage 2.0 ~ 2.1 */
After issuing the switch command: Wait until 'current state' of the card status becomes 'tran'. This prevents from response timeout at the next command because of 'current state' = 'data'. Signed-off-by: Stefan Bosch <stefan_b@posteo.net> --- drivers/mmc/mmc.c | 3 ++- include/mmc.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)