- Drastically decreased cc-list. On 29 January 2015 at 01:55, Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > Ulf, > > On Tue, Jan 27, 2015 at 7:18 AM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: >>> I asked Addy to post upstream against mmc_send_tuning(), but I guess >>> he didn't (he posted against Alex's NAKed patch instead). >>> >>> ...when I talked to him about it, Addy was asserting that when tuning >>> fails it is important (at least on dw_mmc on rk3288) that we wait for >>> the card to stop being busy and that the way to detect was using >>> mmc_send_status(). >> >> So, could that be due to the internal logic of the error handling in >> dw_mmc driver? Or you think this is a generic issue? >> >> According to the specifications (eMMC and SD) both states that the >> tuning command has an R1 response. So, there shouldn't be any busy >> signalling involved - at least according to spec. > > I did a bit of digging into this issue myself. What I found was that > a "response CRC" and "end of transfer". This was why I posted up > <https://patchwork.kernel.org/patch/5623071/>. From that patch: > >> Specifically it looks like in certain error conditions (I saw this >> with Response CRC errors) that data keeps showing up in the FIFO even >> after the error is reported and the CD (command done) bit is set. If >> we don't wait for this data to finish transferring then it confuses >> the next transaction. In the specific failure case I ran into I found >> that I could monitor the data_state_mc_busy bit and wait for it to >> clear, but in other failure cases this bit was stuck at busy when we >> saw an error. Hence a generic big delay seems like the only option. I haven't queued that patch, I was waiting for an ack from Seungwon or Jaehoon. Do you think it could solve this issue, we could give it a try!? > > ...Addy instead fixed the problem using mmc_send_status() to try to > detect when the transfer was all done and it apparently worked, but it > seemed odd to me. My MMC "expertise" pretty much ends with looking > for simple logic errors in the MMC driver, so my hope was that one of > you guys would know this better... > > >>> That would mean that against upstream you'd need to change >>> mmc_send_tuning() to take in the card as well (or move the "host->card >>> = card" assignment to before UHS init, which seems less desirable?) I get your point now. Changing mmc_send_tuning() to take "card" will work due to $subject patch changed the ->execute_tuning() callbacks to take "card" as well. >>> >>> What do you think about that? Is there a better solution? >> >> Why do we need to change mmc_send_tuning()? I thought the issue was >> that mmc_send_status(), which currently takes "card" as a parameter. > > Well, if mmc_send_tuning() needed to call mmc_send_status() then > mmc_send_tuning() would need the card parameter, right? Correct, got it now. :-) I didn't understand that you wanted mmc_send_tuning() to invoke mmc_send_status() while it got some errors. From Addy's patch2, mmc_send_status() is invoked from the host driver. Anyway, I think we should follow your suggestion to change the behaviour of mmc_send_tuning(). Though, I think it should use bus_ops->alive() callback instead (and that callback then also need to change to take "card" as a parameter), since that would be generic and the cover the SDIO case as well. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
