On 23 December 2015 at 12:13, Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx> wrote: > On Tue, Dec 22, 2015 at 09:30:00AM +0100, Ulf Hansson wrote: >> This quirk seems a bit strange. It looks like a generic problem being >> solved by the wrong approach. Although, my knowledge of sdhci HW is >> limited so I might be wrong. >> >> Why doesn't sdhci *always* reset the related registers when the >> command or data transfer gets *completed*? Instead as currently, >> delaying that until the *next* request is started and via using >> SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD? > > That would be additional overhead. The real problem is that the tuning Well, yes but I wonder if it isn't negligible. Of course, if it's more suitable to deal with clearing the register *before* starting a new request, that should work fine as well. Anyway, my point is that I find it odd to have a quirk for something that should be common. > paths compete with the normal request paths over how to set the registers. > This competition needs to be killed, and a saner structure for dealing > with requests and tuning commands needs to be created. > > This is something I'm working on as I get time: I'm restructuring > sdhci_send_command() with the aim of getting to the point where the > tuning code is _not_ writing to any registers at all, and that's all > handled by code common to both paths. Sounds great! > >> Sdhci's sdhci_execute_tuning() function must be converted to use >> mmc_send_tuning(). > > That will make the request path more complex, because we then need to > detect the tuning commands and have special handling for them. With > SDHCI, they are not a standard data transfer command, which is what > mmc_send_tuning() wants. If the regular request path isn't used, the core isn't in control of the request. I don't like that, as it becomes an exception of how requests are managed. It also means the host driver will not benefit from the common error handling paths in the core. > > This means we end up with more complexity in what is supposed to be a > fast path, which means more room for bugs to creep in. I assume you refer to that the sdhci's host_ops->request() callback would need to check for the tuning commands and thus treat these as special cases. I agree, we should avoid these kind of checks in host drivers. Typically we can address this by adding a new MMC_CAP* and/or a new host_ops callback. In this case I think a new optional host_ops callback, which mmc_send_tuning() would invoke, should to the trick for sdhci. What do you think? 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
