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