On May 16, 2011, at 7:02 PM, zhangfei gao wrote: > On Mon, May 16, 2011 at 9:51 AM, Philip Rakity <prakity@xxxxxxxxxxx> wrote: >> >> On May 15, 2011, at 11:26 PM, zhangfei gao wrote: >> >>> On Sat, May 14, 2011 at 1:01 PM, Philip Rakity <prakity@xxxxxxxxxxx> wrote: >>>> >>>> On May 13, 2011, at 10:11 PM, zhangfei gao wrote: >>>> >>>>> On Fri, May 13, 2011 at 9:47 PM, Chris Ball <cjb@xxxxxxxxxx> wrote: >>>>>> Hi, >>>>>> >>>>>> On Thu, May 12 2011, Philip Rakity wrote: >>>>>>> All other platform specific code is in the host/ directory. >>>>>>> >>>>>>> This moves it to arch/arm >>>>>>> >>>>>>> If that is the direction the group wants to go in --> then the patch >>>>>>> is fine provided the mmc group can review the patches. Otherwise they >>>>>>> are handled by the arm maintainer. >>>>>> >>>>>> Thanks. Wolfram, do you have any ideas on what the best design is for >>>>>> these SoC-specific changes to sdhci-pxa? >>>>>> >>>>>> - Chris. >>>>> >>>>> The code in arch/arm is >>>>> 1. Accessing private register, take pxa910 and mmp2 we want to support >>>>> as example, there are several private registers differece, though they >>>>> are same ip, with same issues and quirk. >>>>> 2. Handle platform difference, for example, mmp2 used in two different >>>>> platform, one use wp pin, the other does not. >>>> >>>> The situation is a little more complicated. >>> >>> The interface is used for long time among mmp2, pxa910 and mmp3. >>> Also could be used for new controller with minor register difference >>> but same ip, without adding new specific driver. >> >> applies to both approaches. The mmp2 specific code can be applied to other marvell >> platforms that share the same controller. Just change Kconfig and the Makefile to use >> the mmp2 code. The name of the file is not the important thing. It is what it does. >> >> The pxa168 and pxa910 code are a little less sharable due to io accessors and some >> other quirks. >> >> For other following the discussion we are probably talking about 100 liens of code. >> >> The philosophical location of where the code belongs for host specific drivers is >> to me more important. >> >> Is it in arch/arm and not directly visible to the mmc list or is it like all other platform drivers >> in drivers/mmc/host. > > 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. > >> >> >> >>> >>>> >>>> pxa168 and pxa910 share (almost) the same IP The are both based on SD 2.0 >>>> controller spec with extensions. The pxa910 controller has fixes to the >>>> pxa168 controller. They share the same private registers that allow support >>>> for clock gating and timing adjustments. >>>> >>>> mmp2 is based on SD 3.0 spec. The private register space is different. >>>> >>>> mmc/host/Kconfig takes no account of these differences. mmp2 and pxa168/910 >>>> cannot co-exist. What is currently submitted does not work. One cannot >>>> compile mmp2 and pxa910 nor would they work if one could. >>>> >>>> Mark Brown and I submitted patches to fix this. We added code to the host/ directory >>>> that took into account these differences. It provided a common interface layer >>>> that then used platform specific code in the host/ directory to handle the different >>>> behavior. >>>> >>>> Arng Bergmann provided advice and reviewed the patches to allow explicit selection of the >>>> pxa familty controller and board. Based on this comments we submitted a patch >>>> to allow selection if the appropriate SoC. >>>> >>>> These are two approaches. >>>> >>>>> >>>>> Thanks >>>>>> -- >>>>>> Chris Ball <cjb@xxxxxxxxxx> <http://printf.net/> >>>>>> One Laptop Per Child >>>>>> >>>> >>>> >> >> -- 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