Hi,
BTW, Are there reason to omit the sdhci-s3c.c? Maybe Mr. Jung will handle it.
Thank you, Kyungmin Park
On Tue, Apr 19, 2011 at 7:20 PM, Wolfram Sang w.sang@pengutronix.de wrote:
Hi Shawn,
On Fri, Mar 25, 2011 at 04:48:46PM +0800, Shawn Guo wrote:
Here are what the patch set does.
- Remove .probe and .remove hooks from sdhci-pltfm.c and make it be
a pure common helper function providers.
- Add .probe and .remove hooks for sdhci pltfm drivers sdhci-cns3xxx,
sdhci-dove, sdhci-tegra, and sdhci-esdhc-imx to make them self registered with calling helper functions created above.
- Migrate the use of sdhci_of_host and sdhci_of_data to
sdhci_pltfm_host and sdhci_pltfm_data, so that OF version host and data structure works can be saved, and pltfm version works for both cases.
- Add OF common helper stuff into sdhci-pltfm.c, and make OF version
sdhci drivers sdhci-of-esdhc and sdhci-of-hlwd become self registered as well, so that sdhci-of-core.c and sdhci-of.h can be removed.
- Consolidate the OF and pltfm esdhc drivers into one with sharing
the same pair of .probe and .remove hooks. As a result, sdhci-esdhc-imx.c and sdhci-of-esdhc.c go away, while sdhci-esdhc.c comes in and works for both MPCxxx and i.MX.
- Eliminate include/linux/mmc/sdhci-pltfm.h with moving stuff into
drivers/mmc/host/sdhci-pltfm.h.
And the benefits we gain from the changes are:
- Get the sdhci device driver follow the Linux trend that driver
makes the registration by its own.
- sdhci-pltfm.c becomes simple and clean as it only has common helper
stuff there now.
- All sdhci device specific things are going back its own driver.
- The dt and non-dt drivers are consolidated to use the same pair of
.probe and .remove hooks.
- SDHCI driver for Freescale eSDHC controller found on both MPCxxx
and i.MX platforms is consolidated to use the same one .probe function.
The patch set works against the tree below, and was only tested on i.mx51 babbage board, all other targets were build tested.
git://git.secretlab.ca/git/linux-2.6.git devicetree/test
Comments are welcomed and appreciated.
First of all, thanks _a lot_ for doing this! Many people have promised to do such an approach, but you finally made it!
Sorry for the long delay in reviewing it, I didn't want to rush a review so I needed some time for it which I didn't have much in the last weeks. Let's hope my battery will have enough power on my flight back to Germany ;) So, this is for now a purely "visual" review. I hope I can check V2 on real hardware then.
The approach seems sensible, so have a look at my (mostly minor) comments inside the patches. However, there is one bigger piece missing. You converted all the drivers which had a seperate source-file and hooked into sdhci-pltfm.c. However, those are only those users which need additional code to work around the quirks. There are also users which can take the plain pltfm-driver with a properly set platform_data (check the thread "[PATCH] mmc: add SDHCI driver for STM platforms (V2)" for an example). Those have to be converted, too. Now the discussion could be if every of those users gets its own pltfm-<something>.c or if we create something similat to sdhci-pltfm-generic, which can also be setup with platform_data like the old driver (/me likes the latter a bit more. If we don't change the name of the driver (not talking about the sourcefile) and keep it "sdhci-pltfm", then you wouldn't need to change all those users if you ensured it behaves the same.
Also, I think the next version of this series should have all makers of a sdhci-pltfm user CCed so we give them a chance to report breakage. Or donate acks or tested-by.
Regards,
Wolfram
-- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux)
iEYEARECAAYFAk2tYe8ACgkQD27XaX1/VRsCRgCgkp+BmJJsLsKllKL0OI/BnO9F PRkAn1BpnXBv1exnkJFlqFAPe5O2Yt8w =GSqA -----END PGP SIGNATURE-----
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev