Hi, while working on the esdhc-controller for the imx-platform, I noticed the recently comitted changes for Samsung controllers and have some remarks (sorry for being late): 1) Checking conditions for of host->ops->get_min_clock The original commit e9510176ff728135383f0cdfc9c90cfe57f9e162 (sdhci: be more strict with get_min_clock() usage) states why the additional checks were added. Under this light, it could be argued that commit cfd1f82f20e0c557a061189f7d8c30d623fbe313 (sdhci: remove useless set_clock() check) could be reverted and this commit ce5f036bbbfc6c21d7b55b8fdaa2e2bd56392d94 (sdhci-s3c: add support for the non standard minimal clock value) could also be reverted if the samsung platform driver just uses SDHCI_QUIRK_NONSTANDARD_CLOCK without setting ops->set_clock? 2) 8-Bit data transfer support The comitted version ae6d6c92212e94b12ab9365c23fb73acc2c3c2e7 (sdhci: 8-bit data transfer width support) looks different from another RFC posted in February: http://www.mail-archive.com/linux-mmc@xxxxxxxxxxxxxxx/msg01250.html As those two already differ, I think it might be wiser to move 8-bit-mode-handling to the platform-specific code? Even the documented features of a SDHC differ across implementations, I fear side-effects when using this kind of undocumented feature (official spec says "reserved" when describing this bit). 3) NO_HI_SPD Commit 5193250168ccdf87364e35a11965336dc088578c (sdhci: add no hi-speed bit quirk support) adds a quirk which can be avoided by using io-accessors like in sdhci-of-esdhc.c. Maybe we can even get rid of more, older quirks this way to save precious quirk flags. Have to check that later. I hope my comments are applicable; because there is no freely available datasheet, I can't verify all of my assumptions. Looking forward to comments. Kind regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature
