On Sun, Jul 13, 2014 at 01:21:07PM +0800, Vincent Yang wrote: > @@ -1763,6 +1763,11 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host, > ctrl |= SDHCI_CTRL_VDD_180; > sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); > > + /* Some controller need to do more when switching */ > + if ((host->quirks2 & SDHCI_QUIRK2_VOLTAGE_SWITCH) && > + host->ops->voltage_switch) > + host->ops->voltage_switch(host); > + Why do you heed SDHCI_QUIRK2_VOLTAGE_SWITCH? Isn't populating ops->voltage_switch enough? to indicate that something needs to be done? It would also be better to turn sdhci.c into a library, and have the platform driver call the appropriate functions in sdhci rather than having sdhci be a core driver with loads of quirks. This is what I've done in my previous series where I changed stuff such as the set_bus_width(), set_uhs_signaling() and similar callbacks. So, it probably makes more sense to split sdhci_do_start_signal_voltage_switch() into a load of smaller library functions which drivers can call in an appropriate sequence, rather than having a quirk hook. The problem with quirk hooks is that what is right for one device is not right for another device - eventually you end up with lots of quirk callbacks scattered on every alternate line. That doesn't scale. Experienced kernel programmers know this and this is why words like "framework" fill those who have encountered this problem with dread. sdhci.c is a prime example of this kind of design mistake. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. -- 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
