On Tue, Sep 21, 2010 at 6:11 PM, Wolfram Sang <w.sang@xxxxxxxxxxxxxx> wrote: >> >> >> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c >> >> >> new file mode 100644 >> >> >> index 0000000..3e091c1 >> >> >> --- /dev/null >> >> >> +++ b/drivers/mmc/host/sdhci-pxa.c >> >> >> @@ -0,0 +1,253 @@ >> >> >> +/* linux/drivers/mmc/host/sdhci-pxa.c >> >> >> + * >> >> >> + * Copyright 2010 Marvell >> >> >> + *   ÂZhangfei Gao <zhangfei.gao@xxxxxxxxxxx> >> >> >> + * >> >> >> + * This program is free software; you can redistribute it and/or modify >> >> >> + * it under the terms of the GNU General Public License version 2 as >> >> >> + * published by the Free Software Foundation. >> >> >> + */ >> >> >> + >> >> >> +/* Supports: >> >> >> + * SDHCI support for MMP2/PXA910/PXA168 >> >> >> + * >> >> >> + * Based on sdhci-platfm.c >> >> >> + */ >> >> > >> >> > Why is it only "based on" and not directly using it? >> >> >> >> Thanks for your suggestion. >> >> This is the first stage, already implement sd card function, we will >> >> have some patches later to enhance the driver. >> >> We have to access many private registers to enable specific control. >> >> Besides some sdio device need some specific control, like marvell 8787 >> >> host sleep, it is much reasonable to implement inside the driver. >> > >> > Hmm, which hooks are missing for you to implement this as an extension >> > of the pltfm-driver? Maybe it makes sense to simply add this hook? >> >> We refered several mmc host drivers, such as sdhci-s3c.c, >> sdhci-spear.c, and we want to align with others, which also meet pxa >> requirement and low risk to transfer from the existing driver. > > I see. It is true that the pltfm-part is not widely used up to now. The > question is if it might be a good idea to change this? There have been > voices asking to merge the s3c-driver into the pltfm one. If you look at > sdhci-cns3xxx.c, it looks clearer and reduces code duplication. The > current state of the pltfm-driver might not cover all cases (yet), true, > but those should be at least identified IMHO. This will help us for > similar decisions in the future. > >> We have three or four device in one soc with the same driver, for >> example one for sd, one for wifi, one for emmc, etc. >> Each device have specific clock provider, which should be opened at >> start, and closed when no operation to same power. >> Specific quirk is needed for different device, which should be >> transfered from platform data, for example on-chip wifi alwayes stay >> on chip and emmc may require sd_clk free running to init. >> The max_speed also may be different for board issue though controller >> support max is 50M, which could get from capability. > > I guess to fully understand all constraints, one must really working > with your platform, what I don't do. I agree that a nicely working > driver is better than no driver; however, I fear once a driver hit the > mainline being non-pltfm, it will hardly be converted later, even if it > was considered to be worthwhile. So this is why I ask initially if it > couldn't be done. > My two cents, considering the quirks and several other things that sdhci-pltfm.c is missing, e.g. suspend/resume and clk handling. I'd prefer a diverge at this time, until we figure out what is common to all sdhci platform bus based driver, and get those common part back into sdhci-pltfm.c > Chris, do you see a rule of thumb here? Or what are your preferences? > > 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) > > iEYEARECAAYFAkyYhMQACgkQD27XaX1/VRsK1wCfaGVLkGpVEsrn+OQrpPBVHa54 > yDAAniBRPOamvs7wU8krhKMjjWqMxn9q > =zQvu > -----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
