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/ |
Attachment:
signature.asc
Description: Digital signature
