Hi On Mon, Sep 6, 2010 at 8:00 PM, Wolfram Sang <w.sang@xxxxxxxxxxxxxx> wrote: > 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? In our case we only want to use the min_clock at probe time without NONSTANDARD_CLOCK. and no need to NONSTANDARD_CLOCK at set_clock since we need other parts also at sdhci.c > > 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). Okay it looks different from Samsung Spec. WIDE8[5]: Extended Data Transfer Width (It is for MMC 8-bit card). 1: 8-bit operatoin 0: Bit width is designated by the bit 1(Data Transfer Width) So no need to set the BIT 1 and BIT5 simultaneously. I also wonder other Specs how described it. > > 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. Good, I think it's possible. I'll try and send a patch. > > Kind regards, > > Wolfram > > -- > Pengutronix e.K. | Wolfram Sang | > Industrial Linux Solutions | http://www.pengutronix.de/ | > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.10 (GNU/Linux) > > iEYEARECAAYFAkyEyesACgkQD27XaX1/VRsm/QCePflpNSE0iQQWSUjf/PY723qK > ZbEAoLxAIS7jIJY/lUQpQvbkJMN6ve19 > =98xD > -----END PGP SIGNATURE----- > > -- 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
