Re: [PATCH 0/5] make sdhci device drivers self registered

From: Arnd Bergmann
Date: Mon Mar 21 2011 - 08:40:08 EST


On Monday 21 March 2011, Shawn Guo wrote:
> This patch set is to take sdhci device driver specific things out
> from sdhci-pltfm.c and make them self registered. Here are the
> differences it makes.
>
> * Get the sdhci device driver follow the Linux trend that driver
> take the registration by its own
> * sdhci-pltfm.c becomes significantly simpler and only has common
> support functions there
> * All sdhci device specific stuff are going back its own driver
> * The dt and non-dt support share the same pair of .probe and
> .remove hooks.
>
> The first one patch adds common support function into sdhci-pltfm.c
> when changing sdhci-esdhc-imx driver, while the last one cleans up
> sdhci-pltfm.c when changing sdhci-tegra driver.
>
> Only the patch of sdhci-esdhc-imx are tested on hardware, and others
> are just build tested. And it is based on the tree below.
>
> git://git.secretlab.ca/git/linux-2.6.git devicetree/test
>
> Comments are welcomed and appreciated.

Hi Shawn,

The changes you make look very nice, great work there!

I think the split of the series into five patches is not ideal
because you have more interdependencies than necessary, and it's
not clear that the series can be safely bisected, especially
with the way you do all the Kconfig/Makefile updates at the
end.

A nicer way to partition the series would be

1. Add sdhci_pltfm_init helper
2-5. Convert each of the users, including the respective
Kconfig/Makefile changes
6. remove obsolete code from sdhci-pltfm.c

As I said, that's just a detail and the end result is the same.
If you don't get any other feedback and Chris is happy with the
changes, I wouldn't expect you to reshuffle all the patches just
for this.

Acked-by: Arnd Bergmann <arnd@xxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/