On Fri, May 20, 2011 at 2:24 AM, Nicolas Pitre <nico@xxxxxxxxxxx> wrote: > On Thu, 19 May 2011, zhangfei gao wrote: > >> On Wed, May 18, 2011 at 4:38 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote: >> > On Tuesday 17 May 2011, zhangfei gao wrote: >> >> >> The code handle several register difference are located at >> >> >> arch/arm/mach-mmp/mmp2.c is for mmp2 >> >> >> arch/arm/mach-mmp/pxa910.c is for pxa910 serious, >> >> >> arch/arm/mach-mmp/mmp3.c is for mmp3 serious, considering there may >> >> >> still registers changing. >> >> >> >> >> >> The board difference are directly put in board.c >> >> >> For example ttc_dkb do not use wp pin, so get_ro is provided. >> >> > >> >> > embedding the code in these chip files stops code sharing. For example, >> >> > mmp3 has same controller as mmp2. >> >> >> >> If register are totally same, they can share. >> >> If there is minor difference with chip upgrading, they can put in >> >> different file. >> > >> > When we talked about similar problems in drivers during the Linaro Developer >> > Summit, the broad consensus was to move code from arch/arm into individual >> > device drivers, and I think this should be done for this driver too. >> > >> > My recommendation here would be to have no callbacks in the platform >> > data at all, but instead have the code for all variants in the >> > sdhci-pxa driver itself. You can still have a structure to describe >> > the differences using function pointers, but it's better if that is >> > part of the driver itself. In the platform data, provide a way >> > to identify the variant of the controller (e.g. using an enum) >> > and point to the base addresses as required. >> > >> > Arnd >> > >> Hi, Arnd >> >> Thanks for your suggestion. >> >> To make sdhci-pxa easy to maintain, we still prefer put platform >> specific difference under platform code. > > That's fine to split out platform differences. However please put those > differences, maybe in different files, but alongside the driver itself > in drivers/mmc/host/sdhci-pxa-pxa910.c, sdhci-pxa-mmp2.c, etc. We don't > want to see driver specific stuff hidden in arch/arm/mach-pxa/ or the > like. > >> More and more workaround comes when more and more platform need to >> support, making the driver bigger and bigger and not easy to maintain. >> Nobody remember what's the workaround purpose several years later. > > You really really should spend more time documenting those workarounds > in the source directly in that case. Anyone not familiar with the > issues should be able to understand why a given workaround is there, > whether it is today or in several years. And this is why we want driver > stuff go in the driver directory where more people specifically > interested in MMC/SD are looking. Thanks for explanation, will update accordingly. > > > Nicolas > -- 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
