Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[zhangfei gao <zhangfei.gao@gmail.com>]

On Mon, May 16, 2011 at 9:51 AM, Philip Rakity <prakity@marvell.com> wrote:
>
> On May 15, 2011, at 11:26 PM, zhangfei gao wrote:
>
>> On Sat, May 14, 2011 at 1:01 PM, Philip Rakity <prakity@marvell.com> wrote:
>>>
>>> On May 13, 2011, at 10:11 PM, zhangfei gao wrote:
>>>
>>>> On Fri, May 13, 2011 at 9:47 PM, Chris Ball <cjb@laptop.org> 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.

>
>
>
>>
>>>
>>> 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@laptop.org>   <http://printf.net/>
>>>>> One Laptop Per Child
>>>>>
>>>
>>>
>
>